-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Pack relocations for Android API >= 28 #117624
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
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: AdityaK (hiraditya) ChangesPatch copied from: android/ndk#909 (comment) Full diff: https://github.com/llvm/llvm-project/pull/117624.diff 2 Files Affected:
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)?}}"
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Details in: android/ndk#1196 Fixes: android/ndk#1294
Patch copied from: android/ndk#909 (comment) Fixes: android/ndk#909
breaks this bot https://lab.llvm.org/buildbot/#/builders/186/builds/4581 |
This reverts commit 004e75e.
// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NDK only provides LLD, but one can still use another linker via -fuse-ld, and this breaks that use case. And yes, there are people doing that.
Patch copied from: android/ndk#909 (comment)
Fixes: android/ndk#909