Skip to content
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

Pack relocations for Android API >= 28 #117624

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Pack relocations for Android API >= 28 #117624

merged 2 commits into from
Dec 4, 2024

Conversation

hiraditya
Copy link
Collaborator

Patch copied from: android/ndk#909 (comment)
Fixes: android/ndk#909

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Nov 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: AdityaK (hiraditya)

Changes

Patch copied from: android/ndk#909 (comment)
Fixes: android/ndk#909


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Linux.cpp (+18)
  • (modified) clang/test/Driver/linux-ld.c (+30)
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index d1cb625613415b..83f4df19d43fd4 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -256,6 +256,24 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
       ExtraOpts.push_back("-z");
       ExtraOpts.push_back("max-page-size=16384");
     }
+    if (Triple.isAndroidVersionLT(29)) {
+      // https://github.com/android/ndk/issues/1196
+      // The unwinder used by the crash handler on versions of Android prior to
+      // API 29 did not correctly handle binaries built with rosegment, which is
+      // enabled by default for LLD. Android only supports LLD, so it's not an
+      // issue that this flag is not accepted by other linkers.
+      ExtraOpts.push_back("--no-rosegment");
+    }
+    if (!Triple.isAndroidVersionLT(28)) {
+      // Android supports relr packing starting with API 28 and had its own flavor
+      // (--pack-dyn-relocs=android) starting in API 23.
+      // TODO: It's possible to use both with --pack-dyn-relocs=android+relr,
+      // but we need to gather some data on the impact of that form before we
+      // can know if it's a good default.
+      // On the other hand, relr should always be an improvement.
+      ExtraOpts.push_back("--use-android-relr-tags");
+      ExtraOpts.push_back("--pack-dyn-relocs=relr");
+    }
   }
 
   if (GCCInstallation.getParentLibPath().contains("opt/rh/"))
diff --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index 28fb075a80dbbc..4d641c8f1b46e9 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -940,6 +940,36 @@
 // CHECK-ANDROID-HASH-STYLE-M: "{{.*}}ld{{(.exe)?}}"
 // CHECK-ANDROID-HASH-STYLE-M: "--hash-style=gnu"
 
+// Check that we pass --no-rosegment for pre-29 Android versions and do not for
+// 29+.
+// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN:     --target=armv7-linux-android28 \
+// RUN:   | FileCheck --check-prefix=CHECK-ANDROID-ROSEGMENT-28 %s
+// CHECK-ANDROID-ROSEGMENT-28: "{{.*}}ld{{(.exe)?}}"
+// CHECK-ANDROID-ROSEGMENT-28: "--no-rosegment"
+//
+// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN:     --target=armv7-linux-android29 \
+// RUN:   | FileCheck --check-prefix=CHECK-ANDROID-ROSEGMENT-29 %s
+// CHECK-ANDROID-ROSEGMENT-29: "{{.*}}ld{{(.exe)?}}"
+// CHECK-ANDROID-ROSEGMENT-29-NOT: "--no-rosegment"
+
+// Check that we pass --pack-dyn-relocs=relr for API 28+ and not before.
+// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN:     --target=armv7-linux-android27 \
+// RUN:   | FileCheck --check-prefix=CHECK-ANDROID-RELR-27 %s
+// CHECK-ANDROID-RELR-27: "{{.*}}ld{{(.exe)?}}"
+// CHECK-ANDROID-RELR-27-NOT: "--pack-dyn-relocs=relr"
+// CHECK-ANDROID-RELR-27-NOT: "--pack-dyn-relocs=android+relr"
+//
+// RUN: %clang %s -### -o %t.o 2>&1 \
+// RUN:     --target=armv7-linux-android28 \
+// RUN:   | FileCheck --check-prefix=CHECK-ANDROID-RELR-28 %s
+// CHECK-ANDROID-RELR-28: "{{.*}}ld{{(.exe)?}}"
+// CHECK-ANDROID-RELR-28: "--use-android-relr-tags"
+// CHECK-ANDROID-RELR-28: "--pack-dyn-relocs=relr"
+// CHECK-ANDROID-RELR-28-NOT: "--pack-dyn-relocs=android+relr"
+
 // RUN: %clang -### %s -no-pie 2>&1 --target=mips64-linux-gnuabin32 \
 // RUN:   | FileCheck --check-prefix=CHECK-MIPS64EL-GNUABIN32 %s
 // CHECK-MIPS64EL-GNUABIN32: "{{.*}}ld{{(.exe)?}}"

Copy link

github-actions bot commented Nov 25, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@hiraditya hiraditya changed the title Pack relocations for Android API >= 28 Pack relocations for Android API > 28 Dec 2, 2024
@hiraditya hiraditya changed the title Pack relocations for Android API > 28 Pack relocations for Android API >= 28 Dec 2, 2024
@hiraditya hiraditya merged commit 004e75e into llvm:main Dec 4, 2024
8 checks passed
@hiraditya hiraditya deleted the relr branch December 4, 2024 21:53
@vitalybuka
Copy link
Collaborator

vitalybuka added a commit that referenced this pull request Dec 5, 2024
broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024
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.

[BUG] Hoist relocation packing flags into the Clang driver
5 participants