[PATCH] D60750: [libc++][CMake] Clean up logic for choosing which unwinder lib to link with tests

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: libcxx/utils/libcxx/test/target_info.py:245
+        if llvm_unwinder and not libcxx_is_linker_script:
+            # FIXME: Why does llvm unwinder need -ldl?
             flags += ['-lunwind', '-ldl']
----------------
It's needed by [`LocalAddressSpace::findFunctionName`](https://github.com/llvm/llvm-project/blob/master/libunwind/src/AddressSpace.hpp#L594) (and on Darwin also [`_dyld_find_unwind_sections`](https://github.com/llvm/llvm-project/blob/master/libunwind/src/AddressSpace.hpp#L68)). AFAIK this function is only used by `__unw_get_proc_name` which is not needed for C++ exception unwinding (it's not part of the `_Unwind_*` interface), so one possible solution for eliminating that dependency would be to separate the `_Unwind_*` and `unw_*` interfaces into two different libraries which is something I proposed in D59921, but that's likely a non-trivial effort. Alternative would be to avoid `dladdr` and instead read the symbol information directly from the binary file which is what https://www.nongnu.org/libunwind/ does, that's likely more feasible given the current implementation.


================
Comment at: libcxx/utils/libcxx/test/target_info.py:246
+            # FIXME: Why does llvm unwinder need -ldl?
             flags += ['-lunwind', '-ldl']
+        elif not llvm_unwinder:
----------------
Note that libunwind also needs `-lpthread` which is used by https://github.com/llvm/llvm-project/blob/master/libunwind/src/RWMutex.hpp which is used by https://github.com/llvm/llvm-project/blob/master/libunwind/src/UnwindCursor.hpp when DWARF unwinding is being used. It's also used for sjlj unwinding on Darwin in https://github.com/llvm/llvm-project/blob/master/libunwind/src/Unwind-sjlj.c.


================
Comment at: libcxx/utils/libcxx/test/target_info.py:250
+            # unwind implementation.
+            # FIXME: Why is -lgcc needed for the unwinder?
+            flags += ['-lgcc_s', '-lgcc']
----------------
libgcc contains the GCC unwinder.


================
Comment at: libcxx/utils/libcxx/test/target_info.py:251
+            # FIXME: Why is -lgcc needed for the unwinder?
+            flags += ['-lgcc_s', '-lgcc']
+        # If we are using a linker script and llvm_unwinder is eanbled, then we
----------------
`-lgcc` is `libgcc.a`, `-lgcc_s` is `libgcc_s.so.1`, you probably only want one of those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60750