[libunwind] Pass -Wl,--export-dynamic on all supported platforms

[arichardson]

I was trying to run the tests on FreeBSD and noticed that we weren't printing symbol names. It turns out this is because of the missing -Wl,--export-dynamic flag. Instead of hardcoding the name of the flag and only passing it for Linux hosts, use a pre-existing CMake variable instead. I was not aware of this flag, but it appears to have been supported for the past 16 years (with support for more platforms added later): https://gitlab.kitware.com/cmake/cmake/-/commit/66d1930f5674f08e09f455b3f0777f2de3e0717e

[llvmbot]

@llvm/pr-subscribers-libunwind

Changes

I was trying to run the tests on FreeBSD and noticed that we weren't printing symbol names. It turns out this is because of the missing -Wl,--export dynamic flag. Instead of hardcoding the name of the flag and only passing it for Linux hosts, use a pre-existing CMake variable instead. I was not aware of this flag, but it appears to have been supported for the past 16 years (with support for more platforms added later): https://gitlab.kitware.com/cmake/cmake/-/commit/66d1930f5674f08e09f455b3f0777f2de3e0717e

---

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

3 Files Affected:

• (modified) libunwind/test/configs/llvm-libunwind-merged.cfg.in (+4-2)
• (modified) libunwind/test/configs/llvm-libunwind-shared.cfg.in (+4-2)
• (modified) libunwind/test/configs/llvm-libunwind-static.cfg.in (+4-2)

diff --git a/libunwind/test/configs/llvm-libunwind-merged.cfg.in b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
index 10650f7edf66a2f..e6fafd1edd9756f 100644
--- a/libunwind/test/configs/llvm-libunwind-merged.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
@@ -11,8 +11,10 @@ link_flags = []
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
-if '@CMAKE_SYSTEM_NAME@' == 'Linux':
-    link_flags.append('-Wl,--export-dynamic')
+# Add -Wl,--export-dynamic if supported by the linker (this CMake variable will
+# be empty for Apple platforms).
+if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
+    link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')
 
 if '@CMAKE_DL_LIBS@':
     link_flags.append('-l@CMAKE_DL_LIBS@')
diff --git a/libunwind/test/configs/llvm-libunwind-shared.cfg.in b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
index 97185e57234ff7f..f2e98c7a388dd32 100644
--- a/libunwind/test/configs/llvm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
@@ -10,8 +10,10 @@ link_flags = []
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
-if '@CMAKE_SYSTEM_NAME@' == 'Linux':
-    link_flags.append('-Wl,--export-dynamic')
+# Add -Wl,--export-dynamic if supported by the linker (this CMake variable will
+# be empty for Apple platforms).
+if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
+    link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')
 
 if '@CMAKE_DL_LIBS@':
     link_flags.append('-l@CMAKE_DL_LIBS@')
diff --git a/libunwind/test/configs/llvm-libunwind-static.cfg.in b/libunwind/test/configs/llvm-libunwind-static.cfg.in
index fc6a18d057f3884..1fbdec249855d4a 100644
--- a/libunwind/test/configs/llvm-libunwind-static.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-static.cfg.in
@@ -13,8 +13,10 @@ if @LIBUNWIND_ENABLE_THREADS@:
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
-if '@CMAKE_SYSTEM_NAME@' == 'Linux':
-    link_flags.append('-Wl,--export-dynamic')
+# Add -Wl,--export-dynamic if supported by the linker (this CMake variable will
+# be empty for Apple platforms).
+if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
+    link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')
 
 if '@CMAKE_DL_LIBS@':
     link_flags.append('-l@CMAKE_DL_LIBS@')

[MaskRay]

the missing -Wl,--export dynamic flag

typo: missing -

[MaskRay]

for non-ELF platforms.

AIX linker doesn't have this option.

[brad0]

I don't really like the idea of the comment in the round brackets. I think it would be better to just say something like.. On ELF platforms, add -Wl,--export-dynamic if supported by the linker.

[jsonn]

Is this really desirable? It can actually break applications as it changes ELF semantics.

[arichardson]

Is this really desirable? It can actually break applications as it changes ELF semantics.

I agree that it's not generally desirable, but this is only for the libunwind tests (which expect these semantics).

[jsonn]

Can you as a small follow-up just update the comment to include that part? E.g.

# On ELF platforms, link tests with -Wl,--export-dynamic if supported by the linker.

Side note: the much older way for doing this is actually -rdynamic.

[arichardson]

Can you as a small follow-up just update the comment to include that part? E.g.

Will do.

# On ELF platforms, link tests with -Wl,--export-dynamic if supported by the linker.

Side note: the much older way for doing this is actually -rdynamic.

It appears CMake actually still does this in one case:

Modules/Platform/Linux-TinyCC-C.cmake:set(CMAKE_EXE_EXPORTS_C_FLAG "-rdynamic ")