[PATCH] D142888: [compiler-rt] Fix build on ARM

Mon Jan 30 05:51:36 PST 2023

DavidSpickett added a comment.

I would split this into 2 commits, where one is the triple fix. You can copy the description as well. There is some thinking behind that change that should be preserved in it's own commit.

Then this can be "Fix building GWPASAN on ARM". Also include in the commit message the cmake command that was broken. Just because it's useful to see exactly what config was fixed, when looking back. They can get pretty specific, and it lets people compare their usage against a known good command.

Last time we fixed something like this was https://reviews.llvm.org/D131250 for scudo. I'm a bit confused to not see that change in this file too. Diana guessed at the time the cause was passing `unwindlib=none` and now we're adding it back to fix a similar issue.

The option `COMPILER_RT_USE_LLVM_UNWINDER` also adds `--unwindlib=None`, does that have the same effect as this change? I'm trying to understand if whatever unwind lib you get without this change, should be providing `__aeabi_unwind_cpp_pr*` or not. And exactly which unwind lib we attempt to use in each scenario.

================
Comment at: compiler-rt/lib/scudo/standalone/CMakeLists.txt:41
 append_list_if(COMPILER_RT_HAS_NOSTDLIBXX_FLAG -nostdlib++ SCUDO_LINK_FLAGS)
-append_list_if(CXX_SUPPORTS_UNWINDLIB_NONE_FLAG --unwindlib=none SCUDO_LINK_FLAGS)
+# On ARM, we must link against default unwind lib when GWPAsan is used,
+# otherwise we get undefined references to __aeabi_unwind_cpp_pr* symbols.
----------------
Can you include the why here as well.

"This is because on ARM unwinding does/does not whatever..."

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142888/new/

https://reviews.llvm.org/D142888