Skip to content

[Driver] Add -fandroid-pad-segment/-fno-android-pad-segment #77244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jan 7, 2024

-fandroid-pad-segment is an Android-specific opt-in option that
links in crt_pad_segment.o (beside other crt*.o relocatable files).

crt_pad_segment.o contains a note section, which will be included in the
linker-created PT_NOTE segment. This PT_NOTE tell Bionic that: when
create a map for a PT_LOAD segment, extend the end to cover the gap so
that we will have fewer kernel 'struct vm_area_struct' objects when
page_size < MAXPAGESIZE.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=31076

Link: https://r.android.com/2902180

Created using spr 1.3.4
Created using spr 1.3.4
@MaskRay MaskRay marked this pull request as ready for review January 12, 2024 22:48
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 12, 2024
@MaskRay MaskRay requested a review from rprichard January 12, 2024 22:48
@llvmbot
Copy link
Member

llvmbot commented Jan 12, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Fangrui Song (MaskRay)

Changes

-fandroid-pad-segment is an Android-specific opt-in option that
links in crt_pad_segment.o (beside other crt*.o relocatable files).

crt_pad_segment.o contains a note section, which will be included in the
linker-created PT_NOTE segment. This PT_NOTE tell Bionic that: when
create a map for a PT_LOAD segment, extend the end to cover the gap so
that we will have fewer kernel 'struct vm_area_struct' objects when
page_size < MAXPAGESIZE.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=31076

Link: https://r.android.com/2902180


Full diff: https://github.com/llvm/llvm-project/pull/77244.diff

4 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+5)
  • (added) clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/crt_pad_segment.o ()
  • (modified) clang/test/Driver/linux-ld.c (+18-1)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c237d1f6619348..bd897d5b6de31b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6297,6 +6297,11 @@ def fno_sycl : Flag<["-"], "fno-sycl">,
   Visibility<[ClangOption, CLOption]>,
   Group<sycl_Group>, HelpText<"Disables SYCL kernels compilation for device">;
 
+// OS-specific options
+let Flags = [TargetSpecific] in {
+defm android_pad_segment : BooleanFFlag<"android-pad-segment">, Group<f_Group>;
+} // let Flags = [TargetSpecific]
+
 //===----------------------------------------------------------------------===//
 // FLangOption + NoXarchOption
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 771240dac7a83e..5a1d40478295d6 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -508,6 +508,11 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
     // Add crtfastmath.o if available and fast math is enabled.
     ToolChain.addFastMathRuntimeIfAvailable(Args, CmdArgs);
+
+    if (isAndroid && Args.hasFlag(options::OPT_fandroid_pad_segment,
+                                  options::OPT_fno_android_pad_segment, false))
+      CmdArgs.push_back(
+          Args.MakeArgString(ToolChain.GetFilePath("crt_pad_segment.o")));
   }
 
   Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_u});
diff --git a/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/crt_pad_segment.o b/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/crt_pad_segment.o
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index d5cc3103a3a746..dd3f4b6784d487 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -1347,7 +1347,24 @@
 // RUN:     --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:   | FileCheck --check-prefix=CHECK-ANDROID-PTHREAD-LINK %s
 // CHECK-ANDROID-PTHREAD-LINK-NOT: argument unused during compilation: '-pthread'
-//
+
+/// Check -fandroid-pad-segment.
+// RUN: %clang -### %s --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
+// RUN:   --gcc-toolchain="" --sysroot=%S/Inputs/basic_android_tree/sysroot \
+// RUN:   -fandroid-pad-segment 2>&1 | FileCheck --check-prefix=CHECK-ANDROID-PAD-PHDR %s
+// CHECK-ANDROID-PAD-PHDR: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ANDROID-PAD-PHDR: "[[SYSROOT]]/usr/lib/crtbegin_dynamic.o" "[[SYSROOT]]/usr/lib/crt_pad_phdr.o"
+
+// RUN: %clang -### %s --target=aarch64-linux-android -rtlib=platform --unwindlib=platform \
+// RUN:   --gcc-toolchain="" --sysroot=%S/Inputs/basic_android_tree/sysroot \
+// RUN:   -fandroid-pad-segment -fno-android-pad-segment 2>&1 | FileCheck --check-prefix=CHECK-NO-ANDROID-PAD-PHDR %s
+// CHECK-NO-ANDROID-PAD-PHDR: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-NO-ANDROID-PAD-PHDR: "[[SYSROOT]]/usr/lib/crtbegin_dynamic.o"
+// CHECK-NO-ANDROID-PAD-PHDR-NOT: crt_pad_phdr.o"
+
+// RUN: not %clang -### %s --target=aarch64-linux -fandroid-pad-segment 2>&1 | FileCheck --check-prefix=ERR-ANDROID-PAD-EHDR %s
+// ERR-ANDROID-PAD-EHDR: error: unsupported option '-fandroid-pad-segment' for target 'aarch64-linux'
+
 // Check linker invocation on a Debian LoongArch sysroot.
 // RUN: %clang -### %s -no-pie 2>&1 \
 // RUN:     --target=loongarch64-linux-gnu -rtlib=platform --unwindlib=platform \

@MaskRay MaskRay requested a review from enh-google January 12, 2024 22:48
Created using spr 1.3.4
@MaskRay MaskRay merged commit 5133a8f into main Jan 17, 2024
@MaskRay MaskRay deleted the users/MaskRay/spr/driver-add-fandroid-pad-segment-fno-android-pad-segment branch January 17, 2024 22:39
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
-fandroid-pad-segment is an Android-specific opt-in option that
links in crt_pad_segment.o (beside other crt*.o relocatable files).

crt_pad_segment.o contains a note section, which will be included in the
linker-created PT_NOTE segment. This PT_NOTE tell Bionic that: when
create a map for a PT_LOAD segment, extend the end to cover the gap so
that we will have fewer kernel 'struct vm_area_struct' objects when
page_size < MAXPAGESIZE.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=31076

Link: https://r.android.com/2902180
jrguzman-ms pushed a commit to msft-mirror-aosp/platform.build.soong that referenced this pull request Feb 2, 2024
crt_pad_segment adds a NOTE to the ELF which is used by the binoic
loader to determine whether it should pad segments when mapping them
into the virtual address space, such that there are no gaps between
mappings of consecutive segments. This avoids an increase in
unreclaimable kernel slab memory usage for VMAs on devices where the
runtime-page-size > elf-segment-p_align.

Since -fandroid-pad-segment [1] respects -nostdlib used in android
platform builds, soong must link in crt_pad_segment to platform shared
libraries.

For simplicity, link crt_pad_segment everywhere that crtend_so is
applicable, ignoring nocrt property, as there is no other reason
to track these separately.

Example:

❯ readelf -WS /system/lib64/libc++.so [Output simplified]

...
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
...
  [ 2] .note.android.pad_segment NOTE            0000000000000288 000288 000018 00   A  0   0  4
...

[1] llvm/llvm-project#77244

Bug: 316403210
Test: readelf -WS <lib>.so
Change-Id: Icc06611376cfd5ee4de7281b4134f9f8ffe7ca60
Signed-off-by: Kalesh Singh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants