[clang][driver][AIX] Change linker bcdtor mode to default to mbr#191265
[clang][driver][AIX] Change linker bcdtor mode to default to mbr#191265
Conversation
The bcdtor mode affects how the AIX linker choose to pull in static constructors and destructors (https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command) to the link. The current setting of `all` makes static init in archive members live regardless of if the archive member would be otherwise referenced, causing that whole archive member to become part of the link. This default was initially retained for compatibility purposes with historical compilers on the platform which defaulted to this setting. Unfortunately this greedy pulling in of static init can have unintended consequences for applications, for example for programs linked against parts of compiler-rt which contain optional instrumentation (containing static initializers) which may be unused as these now become live in all programs regardless of use. For that reason and similar reasons, this PR switches the default to `mbr`, which only extracts static init from archive members which would otherwise be referenced. This gives a behaviour very consistent with linkers on other platforms (e.g. Linux). Users requiring the old default behaviour can manually pass `-bcdtors:all` on the link step which will override any default we pass here.
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-backend-powerpc Author: David Tenty (daltenty) ChangesThe bcdtor mode affects how the AIX linker choose to pull in static constructors and destructors (https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command) to the link. The current setting of This default was initially retained for compatibility purposes with historical compilers on the platform which defaulted to this setting. Unfortunately this greedy pulling in of static init can have unintended consequences for applications, for example for programs linked against parts of compiler-rt which contain optional instrumentation (containing static initializers) which may be unused as these now become live in all programs regardless of use. For that reason and similar reasons, this PR switches the default to Users requiring the old default behaviour can manually pass 8 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0f1190510f0f8..9f09a7b4e7ebb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -496,6 +496,11 @@ CUDA Support
AIX Support
^^^^^^^^^^^
+- The driver default for the linker flag `-bcdtors` now defaults to `mbr`
+ (instead of `all`) which only extracts static init from archive members which
+ would otherwise be referenced.
+ (See https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command for details).
+
NetBSD Support
^^^^^^^^^^^^^^
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp
index e7f4792ac5bba..e36910f9aaf46 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -212,7 +212,7 @@ void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// language link invocations. This has to come before AddLinkerInputs as the
// implied option needs to precede any other '-bcdtors' settings or
// '-bnocdtors' that '-Wl' might forward.
- CmdArgs.push_back("-bcdtors:all:0:s");
+ CmdArgs.push_back("-bcdtors:mbr:0:s");
if (Args.hasArg(options::OPT_rpath)) {
for (const auto &bopt : Args.getAllArgValues(options::OPT_b))
diff --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 1dae61d91c4e3..bedd224eeca9c 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -353,9 +353,9 @@
// CHECK-LD32-ARG-ORDER: "-bpT:0x10000000" "-bpD:0x20000000"
// CHECK-LD32-ARG-ORDER: "[[SYSROOT]]/usr/lib{{/|\\\\}}crt0.o"
// CHECK-LD32-ARG-ORDER: "[[SYSROOT]]/usr/lib{{/|\\\\}}crti.o"
-// CHECK-LD32-ARG-ORDER: "-bcdtors:all:0:s"
+// CHECK-LD32-ARG-ORDER: "-bcdtors:mbr:0:s"
// CHECK-LD32-ARG-ORDER: "-bnocdtors"
-// CHECK-LD32-ARG-ORDER-NOT: "-bcdtors:all:0:s"
+// CHECK-LD32-ARG-ORDER-NOT: "-bcdtors:mbr:0:s"
// CHECK-LD32-ARG-ORDER-NOT: "-lc++"
// CHECK-LD32-ARG-ORDER-NOT: "-lc++abi"
// CHECK-LD32-ARG-ORDER: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}aix{{/|\\\\}}libclang_rt.builtins-powerpc.a"
@@ -382,9 +382,9 @@
// CHECK-LD32-CXX-ARG-ORDER: "-bpT:0x10000000" "-bpD:0x20000000"
// CHECK-LD32-CXX-ARG-ORDER: "[[SYSROOT]]/usr/lib{{/|\\\\}}crt0.o"
// CHECK-LD32-CXX-ARG-ORDER: "[[SYSROOT]]/usr/lib{{/|\\\\}}crti.o"
-// CHECK-LD32-CXX-ARG-ORDER: "-bcdtors:all:0:s"
+// CHECK-LD32-CXX-ARG-ORDER: "-bcdtors:mbr:0:s"
// CHECK-LD32-CXX-ARG-ORDER: "-bnocdtors"
-// CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"
+// CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:mbr:0:s"
// CHECK-LD32-CXX-ARG-ORDER: "-lc++"
// CHECK-LD32-CXX-ARG-ORDER: "-lc++abi"
// CHECK-LD32-CXX-ARG-ORDER: "[[RESOURCE_DIR]]{{/|\\\\}}lib{{/|\\\\}}aix{{/|\\\\}}libclang_rt.builtins-powerpc.a"
diff --git a/compiler-rt/lib/profile/InstrProfilingPlatformAIX.c b/compiler-rt/lib/profile/InstrProfilingPlatformAIX.c
index 9cb313bc7a1fc..7141a553b023f 100644
--- a/compiler-rt/lib/profile/InstrProfilingPlatformAIX.c
+++ b/compiler-rt/lib/profile/InstrProfilingPlatformAIX.c
@@ -181,7 +181,7 @@ void __llvm_profile_register_names_function(void *NamesStart,
// reference symbols from the profile library (for example when no files were
// compiled with -fprofile-generate). That's because these symbols are kept
// alive through references in constructor functions that are always live in the
-// default linking model on AIX (-bcdtors:all). The __start_SECNAME and
+// `-bcdtors:all` linking model on AIX. The __start_SECNAME and
// __stop_SECNAME symbols are only resolved by the linker when the SECNAME
// section exists. So for the scenario where the user objects have no such
// section (i.e. when they are compiled with -fno-profile-generate), we always
diff --git a/compiler-rt/test/profile/instrprof-merge-entry-cover.c b/compiler-rt/test/profile/instrprof-merge-entry-cover.c
index a99458898392f..5856773c09f1b 100644
--- a/compiler-rt/test/profile/instrprof-merge-entry-cover.c
+++ b/compiler-rt/test/profile/instrprof-merge-entry-cover.c
@@ -38,6 +38,7 @@ __attribute__((noinline)) void foo(char c) {
__attribute__((noinline)) void bar(int M) { g += M; }
int main(int argc, const char *argv[]) {
+ __llvm_profile_test_initialize();
int i;
if (argc < 4)
return 1;
diff --git a/compiler-rt/test/profile/instrprof-merge.c b/compiler-rt/test/profile/instrprof-merge.c
index 8be1016da873e..a94708919daba 100644
--- a/compiler-rt/test/profile/instrprof-merge.c
+++ b/compiler-rt/test/profile/instrprof-merge.c
@@ -37,6 +37,7 @@ void foo(char c) {
void bar(int M) { g += M; }
int main(int argc, const char *argv[]) {
+ __llvm_profile_test_initialize();
int i;
if (argc < 4)
return 1;
diff --git a/compiler-rt/test/profile/instrprof-write-file-atexit-explicitly.c b/compiler-rt/test/profile/instrprof-write-file-atexit-explicitly.c
index 6923e1d1d55d6..63171cb66ec4c 100644
--- a/compiler-rt/test/profile/instrprof-write-file-atexit-explicitly.c
+++ b/compiler-rt/test/profile/instrprof-write-file-atexit-explicitly.c
@@ -4,10 +4,13 @@
// RUN: llvm-profdata merge -o %t.profdata %t.profraw
// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s
+#include "profile_test.h"
+
int __llvm_profile_runtime = 0;
int __llvm_profile_register_write_file_atexit(void);
void __llvm_profile_set_filename(const char *);
int main(int argc, const char *argv[]) {
+ __llvm_profile_test_initialize();
__llvm_profile_register_write_file_atexit();
// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
if (argc < 2)
diff --git a/compiler-rt/test/profile/profile_test.h b/compiler-rt/test/profile/profile_test.h
index 557db99abb51f..b410606a1f2bf 100644
--- a/compiler-rt/test/profile/profile_test.h
+++ b/compiler-rt/test/profile/profile_test.h
@@ -9,4 +9,14 @@
# define ALIGNED(x) __attribute__((aligned(x)))
#endif
+inline void __llvm_profile_test_initialize() {
+ // This is a no-op on most platforms, but on AIX it forces the linker to
+ // keep the start/stop stub data for the runtime. Normally this data is
+ // referenced by pulling in `__llvm_profile_runtime` from the runtime but
+ // some tests explicitly supress that.
+#ifdef _AIX
+ extern __attribute__((visibility("hidden"))) void *__llvm_profile_keep[];
+ (void)*(void *volatile *)__llvm_profile_keep;
+#endif // _AIX
+}
#endif // PROFILE_TEST_H
|
| // This is a no-op on most platforms, but on AIX it forces the linker to | ||
| // keep the start/stop stub data for the runtime. Normally this data is | ||
| // referenced by pulling in `__llvm_profile_runtime` from the runtime but | ||
| // some tests explicitly supress that. |
There was a problem hiding this comment.
just to give some context about __llvm_profile_keep (no need to take action):
the __llvm_profile_keep references a bunch of stub data variables that are necessary to allow the linker to define the __stop___llvm_prf_vnds and other start/stop symbols, which are live because they are referenced by parts of the runtime that compute the filename. The start/stop symbols are weak declarations, but on AIX an undefined WEAK is an error (just like a regular undefined), so we need those stub data variables and stop the linker from GC'ing them.
There was a problem hiding this comment.
so the usecase of defining __llvm_profile_runtime in the user code to avoid automatic initialization of the runtime, under the -bcdtors:mbr settings, is broken and users would need to create a reference to __llvm_profile_keep. We can add -u__llvm_profile_keep to the AIX link step in the driver, but I'm not sure if it will have some unintended consequences such as keeping any constructors defined in the same file as __llvm_profile_keep (none today). I'm in favor of keeping things as is, until we find the need to fix this.
…twco [AIX] update linker default to bcdtors The bcdtor mode affects how the AIX linker choose to pull in static constructors and destructors (https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command) to the link. The current setting of all makes static init in archive members live regardless of if the archive member would be otherwise referenced, causing that whole archive member to become part of the link. This default was initially retained for compatibility purposes with historical compilers on the platform which defaulted to this setting. Unfortunately this greedy pulling in of static init can have unintended consequences for applications, for example for programs linked against parts of compiler-rt which contain optional instrumentation (containing static initializers) which may be unused as these now become live in all programs regardless of use. For that reason and similar reasons, this PR switches the default to mbr, which only extracts static init from archive members which would otherwise be referenced. This gives a behaviour very consistent with linkers on other platforms (e.g. Linux). Users requiring the old default behaviour can manually pass `-bcdtors:all` on the link step which will override any default we pass here. (Note: this mirrors LLVM change: llvm/llvm-project#191265)
Rollup merge of #155247 - daltenty:daltenty/bcdtors, r=davidtwco [AIX] update linker default to bcdtors The bcdtor mode affects how the AIX linker choose to pull in static constructors and destructors (https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command) to the link. The current setting of all makes static init in archive members live regardless of if the archive member would be otherwise referenced, causing that whole archive member to become part of the link. This default was initially retained for compatibility purposes with historical compilers on the platform which defaulted to this setting. Unfortunately this greedy pulling in of static init can have unintended consequences for applications, for example for programs linked against parts of compiler-rt which contain optional instrumentation (containing static initializers) which may be unused as these now become live in all programs regardless of use. For that reason and similar reasons, this PR switches the default to mbr, which only extracts static init from archive members which would otherwise be referenced. This gives a behaviour very consistent with linkers on other platforms (e.g. Linux). Users requiring the old default behaviour can manually pass `-bcdtors:all` on the link step which will override any default we pass here. (Note: this mirrors LLVM change: llvm/llvm-project#191265)
The bcdtor mode affects how the AIX linker choose to pull in static constructors and destructors (https://www.ibm.com/docs/en/aix/7.2.0?topic=l-ld-command) to the link.
The current setting of
allmakes static init in archive members live regardless of if the archive member would be otherwise referenced, causing that whole archive member to become part of the link.This default was initially retained for compatibility purposes with historical compilers on the platform which defaulted to this setting. Unfortunately this greedy pulling in of static init can have unintended consequences for applications, for example for programs linked against parts of compiler-rt which contain optional instrumentation (containing static initializers) which may be unused as these now become live in all programs regardless of use.
For that reason and similar reasons, this PR switches the default to
mbr, which only extracts static init from archive members which would otherwise be referenced. This gives a behaviour very consistent with linkers on other platforms (e.g. Linux).Users requiring the old default behaviour can manually pass
-bcdtors:allon the link step which will override any default we pass here.