As for x86_64, the default image base for AArch64 and i386 should be aligned to a superpage appropriate for the architecture.
On AArch64, this is 2 MiB, on i386 it is 4 MiB.
Paths
| Differential D50297
Align AArch64 and i386 image base to superpage ClosedPublic Authored by dim on Aug 4 2018, 6:03 AM.
Details Summary As for x86_64, the default image base for AArch64 and i386 should be aligned to a superpage appropriate for the architecture. On AArch64, this is 2 MiB, on i386 it is 4 MiB.
Diff Detail
Event TimelineComment Actions Note that we have already committed this in FreeBSD, see https://reviews.freebsd.org/D16385 and https://reviews.freebsd.org/rS337282. Comment Actions I do not know if this change is OK or not, 35> Failing Tests (48): Comment Actions Looking at this a bit more, I see that it is unconditionally setting the DefaultImage base for all platforms, which cannot be right. I didn't notice this when it was committed into FreeBSD, but it doesn't matter there, of course. I'm not sure if there is a good way to override this setting on a per-platform basis. Maybe just if (os == FreeBSD) DefaultImageBase=x? Comment Actions
Is there any reason that this value doesn't work for other operating systems? I guess the new image base values work everywhere. If that's the case, I'd change the default so that we have less moving parts in the linker. Comment Actions
I'm not sure if it will work on all OSes, I can't test them all. In any case, I'll update the test cases first, then update this review. Comment Actions I'll just note that there is an identical default in X86_64.cpp. Also see the last comment by alc here: https://reviews.freebsd.org/D16385 . Comment Actions Please change the patch's title and description if your intent is to affect more than just FreeBSD. dim retitled this revision from On FreeBSD, align AArch64 and i386 image base to superpage to Align AArch64 and i386 image base to superpage.Aug 28 2018, 1:03 PM Comment Actions Oh, I didn't notice that you are trying to make a change for i386, not for x86-64. What is your motivation to use superpages for 32-bit applications? I believe that the applications you want to use superpages are large programs and naturally be 64-bit, so I wonder if this change is actually useful. Comment Actions
I think it is for consistency, and for efficiency in the virtual memory manager, but @markj is probably better at explaining it. :) Comment Actions
All executables benefit from being loaded at a superpage-aligned address, assuming that the RO segment is large enough to fill out at least one superpage. The following segments can then be suitably aligned using -z max-page-size. Further, when an application faults on a shared segment (e.g., the text section of an executable), if the fault region is mapped as a superpage in another process, FreeBSD will attempt to create a new superpage mapping in the faulting application, potentially eliding many faults on 4KB pages. This is all mostly orthogonal to the application-specific uses of superpages and applies to i386 as well as x86-64. Comment Actions I'm fine with this. For lld, we tried to not cargo-cult default image base values, and the current default values are chosen based on the rationale described in Target.h. If some other value is better than the current default, we shouldn't too hesitate to change that. Since this patch changes the default image base for AArch64, I'd wait for Peter Smith's LGTM.
Comment Actions I've added Stephen Hines as a reviewer to see if this affects Android in a significant way. I'd like to run this by a few people in the OS community and also check that are no adverse code/file size increases as these could be significant on low-end mobile devices. Will hopefully get back to you soon. Comment Actions In theory this change shouldn't affect the amount of memory used by an application, since it affects only the virtual address space, no? The only bad scenario I can think about this patch is, if your i386 application barely fits within the 2 GiB/4GiB address space, increasing the base address a few megabyte could cause a link failure. But I don't think that's a realistic scenario. Comment Actions +Ryan in case there is anything here that could affect Bionic loading from these pages. Comment Actions
I believe that is the case, yes. In the past I've worked on a proprietary i386 application that used as much address space as it could get, but this is a very unusual case, which can be addressed on an as-needed basis, and is largely irrelevant today as such applications transitioned to x86_64 years ago. Comment Actions Does FreeBsd use 4Kb granules for the page size or the LLD default of 64Kb. From what I can see from the MMU documentation when 64Kb pages are used the equivalent of the Super Page size is 512MiB and not 2 MiB? If I'm right then aligning to 2MiB is only going to be beneficial when 4Kb pages are used. On the assumption that it doesn't do much harm to people using 64Kb pages I don't have any objections in changing it though. Aligning to 512MiB for 64kb pages sounds a bit extreme though. To summarise if Android/Bionic are happy that this isn't going to be a problem then I'm ok with it.
Comment Actions
Currently we don't have any support for anything other than 4KB pages on aarch64. This change doesn't have any effect on 64KB alignment so I can't see why anyone using 64KB pages would be negatively affected. Comment Actions
AFAIK, this patch isn't a problem for Android. It looks like it could only affect NDK-built non-PIE executables running on 32-bit x86 Jelly Bean or KitKat. It looks like the ImageBase only matters for non-PIE executables, and the Bionic loader rejects those on android-21 (L) and up for security reasons. The oldest Android OS that the current NDK supports is android-16, which allows PIE executables but can also still run non-PIE executables. L is the first 64-bit release, so this patch can't affect arm64 Android. The NDK Clang driver turns PIE on by default for android-16 and up, but someone could turn it off or use a build system that doesn't set the android-16 API level. bfd and gold use a default image base of 0x08048000 on x86 (a bit over 128MB), but AFAICT none of the rationales I found for that make sense today. I experimented with various bases from 0 up to 3GB on android-16 and android-19 emulators, and everything seems to work. This revision is now accepted and ready to land.Sep 19 2018, 8:04 AM Closed by commit rL342746: Align AArch64 and i386 image base to superpage (authored by dim). · Explain WhySep 21 2018, 9:59 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 166495 lld/trunk/ELF/Arch/AArch64.cpp
lld/trunk/ELF/Arch/X86.cpp
lld/trunk/test/ELF/aarch64-abs16.s
lld/trunk/test/ELF/aarch64-abs32.s
lld/trunk/test/ELF/aarch64-call26-thunk.s
lld/trunk/test/ELF/aarch64-condb-reloc.s
lld/trunk/test/ELF/aarch64-copy.s
lld/trunk/test/ELF/aarch64-copy2.s
lld/trunk/test/ELF/aarch64-cortex-a53-843419-large.s
lld/trunk/test/ELF/aarch64-cortex-a53-843419-recognize.s
lld/trunk/test/ELF/aarch64-data-relocs.s
lld/trunk/test/ELF/aarch64-gnu-ifunc-plt.s
lld/trunk/test/ELF/aarch64-gnu-ifunc.s
lld/trunk/test/ELF/aarch64-jump26-thunk.s
lld/trunk/test/ELF/aarch64-lo12-alignment.s
lld/trunk/test/ELF/aarch64-prel16.s
lld/trunk/test/ELF/aarch64-prel32.s
lld/trunk/test/ELF/aarch64-relocs.s
lld/trunk/test/ELF/aarch64-thunk-section-location.s
lld/trunk/test/ELF/aarch64-tls-gdie.s
lld/trunk/test/ELF/aarch64-tls-gdle.s
lld/trunk/test/ELF/aarch64-tls-ie.s
lld/trunk/test/ELF/aarch64-tls-iele.s
lld/trunk/test/ELF/aarch64-tls-le.s
lld/trunk/test/ELF/aarch64-tlsld-ldst.s
lld/trunk/test/ELF/aarch64-tstbr14-reloc.s
lld/trunk/test/ELF/aarch64-undefined-weak.s
lld/trunk/test/ELF/basic-aarch64.s
lld/trunk/test/ELF/basic32.s
lld/trunk/test/ELF/gc-sections-implicit-addend.s
lld/trunk/test/ELF/gnu-ifunc-i386.s
lld/trunk/test/ELF/gnu-ifunc-plt-i386.s
lld/trunk/test/ELF/got-i386.s
lld/trunk/test/ELF/got32-i386.s
lld/trunk/test/ELF/got32x-i386.s
lld/trunk/test/ELF/i386-pc8-pc16-addend.s
lld/trunk/test/ELF/i386-retpoline-nopic.s
lld/trunk/test/ELF/map-file-i686.s
lld/trunk/test/ELF/plt-aarch64.s
lld/trunk/test/ELF/plt-i686.s
lld/trunk/test/ELF/relocation-b-aarch64.test
lld/trunk/test/ELF/relocation-copy-i686.s
lld/trunk/test/ELF/relocation-i686.s
lld/trunk/test/ELF/shared.s
lld/trunk/test/ELF/static-with-export-dynamic.s
lld/trunk/test/ELF/tls-i686.s
lld/trunk/test/ELF/tls-opt-gdiele-i686.s
lld/trunk/test/ELF/tls-opt-i686.s
lld/trunk/test/ELF/tls-opt-iele-i686-nopic.s
lld/trunk/test/ELF/undef-with-plt-addr-i686.s
|