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

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libunwind/src/assembly.h:97
+#define WEAK_ALIAS(name, aliasname)                       \
+  WEAK_SYMBOL(aliasname) SEPARATOR                        \
+  SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
----------------
mstorsjo wrote:
> mstorsjo wrote:
> > Thanks, this looks great overall now! Just one minor mistake.
> > 
> > The mingw version of the `WEAK_ALIAS` macro lacks a definition of `WEAK_SYMBOL()` - to match the C version we probably should just leave it out? With that changed, this compiles and links just fine.
> > 
> > Didn't test the non-mingw codepaths though.
> Sorry, I keep coming up with more things...
> 
> The alias needs to be exported to be visible outside of the dll. (In that case there's no issue with conflicting non-reserved symbols at all.) And it needs a `.globl`.
> 
> The non-mingw version is missing some `#` to make string literals. And that requires a few layers of macro indirection.
> 
> ```
> diff --git a/src/assembly.h b/src/assembly.h
> index 3ff959d..1d47e4d 100644
> --- a/src/assembly.h
> +++ b/src/assembly.h
> @@ -93,13 +93,21 @@
>  
>  #if defined(__MINGW32__)
>  #define WEAK_ALIAS(name, aliasname)                                            \
> +  .globl SYMBOL_NAME(aliasname) SEPARATOR                                      \
> +  EXPORT_SYMBOL(aliasname) SEPARATOR                                           \
>    SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
>  #else
> -#define WEAK_ALIAS(name, aliasname)                                            \
> +
> +#define WEAK_ALIAS3(name, aliasname)                                           \
>    .section .drectve,"yn" SEPARATOR                                             \
> -  .ascii "-alternatename:", SYMBOL_NAME(aliasname), "=",                       \
> -                            SYMBOL_NAME(name), "\0" SEPARATOR                  \
> +  .ascii "-alternatename:", #aliasname, "=", #name, "\0" SEPARATOR             \
>    .text
> +#define WEAK_ALIAS2(name, aliasname)                                           \
> +  WEAK_ALIAS3(name, aliasname)
> +#define WEAK_ALIAS(name, aliasname)                                            \
> +  EXPORT_SYMBOL(aliasname) SEPARATOR                                           \
> +  WEAK_ALIAS2(SYMBOL_NAME(name), SYMBOL_NAME(aliasname))
> +
>  #endif
>  
>  #define NO_EXEC_STACK_DIRECTIVE
> ```
I also forgot to mention, the `EXPORT_SYMBOL` macro needs to add the symbol prefix when building for msvc (or any non-mingw I guess), but not for mingw. But that's a preexisting issue, unrelated to this patch.


Repository:
  rUNW libunwind

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

https://reviews.llvm.org/D59921