[libcxx-commits] [PATCH] D59921: [libunwind] Export the unw_* symbols as weak symbols

Thu Mar 28 01:30:12 PDT 2019

mstorsjo added reviewers: mstorsjo, rnk.
mstorsjo added inline comments.

================
Comment at: libunwind/src/assembly.h:95
+  WEAK_SYMBOL(aliasname)                                                       \
+  SEPARATOR alias<SYMBOL_NAME(aliasname)> = <SYMBOL_NAME(name)>

----------------
What assembler syntax is this `alias<somesymbol> = <othersymbol>`? The LLVM assembler certainly doesn't support it at least. Is it some thing for a microsoft assembler? The rest of this code doesn't assemble with any such tool anyway. Or is it a leftover marker indicating that you meant to do something?

If we go with `/alternatename` via a pragma on the C level, the corresponding thing here would be an alternatename directive in the exact same way as `EXPORT_SYMBOL2` above. If we go with `__attribute__((weak,alias()))`, or `__attribute__((alias()))` the corresponding thing is the same as for ELF above, with or without the `.weak` directive.

================
Comment at: libunwind/src/config.h:77
+#define _LIBUNWIND_WEAK_ALIAS(name, aliasname)                                 \
+  __pragma(comment(linker, "/alternatename:" #aliasname "=" #name))            \
+      _LIBUNWIND_EXPORT extern "C" __typeof(name) aliasname;
----------------
This kind of pragma requires building with `-fms-extensions` for mingw mode (which is the only windows configuration where there's any need to use libunwind at all, as far as I know), which breaks building with gcc (and the directive is only supported by lld, not by ld.bfd). This requires adding `unwind_append_if(LIBUNWIND_COMPILE_FLAGS MINGW -fms-extensions)` to the main `CMakeLists.txt`.

That's not a blocker for this change by any means, but I'll just let it be noted that it's a change from how things were before.

The `__attribute__((weak, alias()))` form does work in both gcc and clang actually though, so that might almost also be an option, but it's a bit messy (both clang and ld.bfd seem to have a bit of gotchas relating to it), so perhaps we shouldn't.

Alternatively, we could also just postpone making them properly weak on mingw (as it's not strictly necessary contrary to e.g. sanitizers), and just do this with a plain `__attribute__((alias(#name)))` as above, without the weak.

Also, I'm pretty sure that `/alternatename` requires using `__USER_LABEL_PREFIX__` here (judging from how it's used in sanitizers).

Repository:
  rUNW libunwind

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

https://reviews.llvm.org/D59921