set default max-page-size to 4KB in lld for Android Aarch64

[zhizhouy]

Summary

This patch passes an option '-z max-page-size=4096' to lld through clang driver.

This is for Android on Aarch64 target.

The lld default page size is too large for Aarch64, which produces larger .so files and images for arm64 device targets.
In this patch we set default page size to 4KB for Android Aarch64 targets instead.

[ruiu]

+peter.smith

Somewhat tangent to this patch, but is 64KiB a reasonable default for -z max-page-size? I believe that max-page-size is set to 64KiB so that OS/CPU whose minimum page size is 64KiB can load an executable linked with lld, but is there any real target OS/CPU that does not allow 4KiB pages? Or, is there any other reason to use that default value?

[srhines]

Thanks Zhizhou! I'm curious about Peter's answer to the 64KiB default as well, but I think we should move forward with getting this patch submitted.

[ruiu]

I wouldn't rush to submit this now, given that this issue is not new at all. Maybe we can just wait for Peter's response?

[srhines]

In D55029#1312167, @ruiu wrote:

I wouldn't rush to submit this now, given that this issue is not new at all. Maybe we can just wait for Peter's response?

Sure, that's fine too. I misunderstood the "tangent" part of your initial comment to mean that any such change would be done in a separate CL.

[ruiu]

Yeah, I believe this patch is fine to submit, but since Peter is in a different timezone, I wanted give him a chance to review this one.

[peter.smith]

Yes this is fine. The effects are entirely within the Android target.

The change in LLD to a 64k page size was made in D25079. The main reason given was that a sufficient number of linux distributions including Redhat had chosen a 64k page size. GNU ld had changed its default page-size to 64k so that applications and libraries would always be compatible with distributions that had chosen 4k page size but not necessarily vice versa. So it was essentially a safe default.

[zhizhouy]

Thanks for the review and comments!

[ruiu]

LGTM. Please commit.

Peter, I wonder if you are fine with the default 64KiB page size with lld, especially given that lld always round up the text segment size to the maximum page size on disk and fill the padding with trap instructions. On average, that should increase the executable size by 32 KiB compared to other linkers. I don't think that size is necessarily bad, because we are doing that for a security purpose, but I wonder if people are OK with that.

[peter.smith]

In D55029#1313120, @ruiu wrote:

LGTM. Please commit.

Peter, I wonder if you are fine with the default 64KiB page size with lld, especially given that lld always round up the text segment size to the maximum page size on disk and fill the padding with trap instructions. On average, that should increase the executable size by 32 KiB compared to other linkers. I don't think that size is necessarily bad, because we are doing that for a security purpose, but I wonder if people are OK with that.

I think the default is fine at 64KiB . Going back to 4KiB risks breaking programs that currently use default options on platforms that have chosen 64KiB which I think is a step too far. So far the concern about ELF file size has come from Android, where we have a separate target in clang where it is fairly easy to pass a 4KiB page size by default. There is a chance that this may change in the future as more general AArch64 linux platforms start being deployed on devices with limited storage. I guess at that point we could consider implementing common-page-size if it were a problem to pass 4KiB page size.

lib/Driver/ToolChains/Linux.cpp

Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
   }
 
   if (Distro.IsOpenSUSE() || Distro.IsUbuntu() || Distro.IsAlpineLinux() ||
       Triple.isAndroid()) {
     ExtraOpts.push_back("-z");
     ExtraOpts.push_back("relro");
   }
 
+  // The lld default page size is too large for Aarch64, which produces much
+  // larger .so files and images for arm64 device targets. Use 4KB page size
+  // for Android arm64 targets instead.
+  if (Triple.isAArch64() && Triple.isAndroid()) {
+    ExtraOpts.push_back("-z");
+    ExtraOpts.push_back("max-page-size=4096");
+  }
+
   if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
       StringRef::npos)
     // With devtoolset on RHEL, we want to add a bin directory that is relative
     // to the detected gcc install, because if we are using devtoolset gcc then
     // we want to use other tools from devtoolset (e.g. ld) instead of the
     // standard system tools.
     PPaths.push_back(Twine(GCCInstallation.getParentLibPath() +
                      "/../bin").str());

test/Driver/android-aarch64-link.cpp

 // Check that we automatically add relevant linker flags for Android aarch64.
 
 // RUN: %clang -target aarch64-none-linux-android \
 // RUN:   -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=GENERIC-ARM < %t %s
 //
 // RUN: %clang -target aarch64-none-linux-android \
 // RUN:   -mcpu=cortex-a53 -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=CORTEX-A53 < %t %s
 //
 // RUN: %clang -target aarch64-none-linux-android \
 // RUN:   -mcpu=cortex-a57 -### -v %s 2> %t
 // RUN: FileCheck -check-prefix=CORTEX-A57 < %t %s
 //
+// RUN: %clang -target aarch64-none-linux-android \
+// RUN:   -### -v %s 2> %t
+// RUN: FileCheck -check-prefix=MAX-PAGE-SIZE < %t %s
+//
 // GENERIC-ARM: --fix-cortex-a53-843419
 // CORTEX-A53: --fix-cortex-a53-843419
 // CORTEX-A57-NOT: --fix-cortex-a53-843419
+// MAX-PAGE-SIZE: max-page-size=4096