[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG729899f7b6bf: [libunwind] unw_* alias fixes for ELF and 
Mach-O (authored by rprichard).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -111,7 +111,7 @@
   if (!invoker.export) {
 cflags = [ "-fvisibility=hidden" ]
 cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
   }
   deps = [ "//compiler-rt/lib/builtins" ]
   configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added a comment.

The coff-dwarf.test buildbot failures were fixed by 
https://github.com/llvm/llvm-project/commit/0fd7c31a098efdfaa5a57dbac6e9c0921b00a999
 (on Feb 11).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.

LGTM

I'd also suggest renaming `LIBCXXABI_HERMETIC_STATIC_LIBRARY` and 
`LIBCXX_HERMETIC_STATIC_LIBRARY` to `LIBCXXABI_HIDE_SYMBOLS` and 
`LIBCXX_HIDE_SYMBOLS` respectively in a follow up change to maintain 
consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

My main concern is just that weak alias for libc++abi that never worked for 
Darwin :)

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Dan Albert via Phabricator via cfe-commits]

danalbert added a comment.

It sounds like everyone is happy here but the tools. Could we get a libunwind 
reviewer (preferably @compnerd, since his review is the red one) to LGTM this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D93003#2567088 , @steven_wu wrote:

> In D93003#2566906 , @rprichard wrote:
>
>> Maybe this is blocked on someone from Apple reviewing the Mach-O parts?
>
> This is fine with me for how Apple builds libunwind. I am not sure if the 
> open source libunwind build for MachO do expect different behavior but I 
> don't think there are other reasonable behaviors.
>
> I can give my LGTM if @ldionne has no problem with that.

Oh, I had missed that. I'm not a libunwind owner, but if Steven says it's good 
for Apple, then it is. I trust him a lot more than I trust myself in this area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D93003#2566906 , @rprichard wrote:

> Maybe this is blocked on someone from Apple reviewing the Mach-O parts?

This is fine with me for how Apple builds libunwind. I am not sure if the open 
source libunwind build for MachO do expect different behavior but I don't think 
there are other reasonable behaviors.

I can give my LGTM if @ldionne has no problem with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

rprichard added a comment.

Maybe this is blocked on someone from Apple reviewing the Mach-O parts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

rprichard updated this revision to Diff 323222.
rprichard added a comment.

Rebase and fix merge conflict in gn file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -111,7 +111,7 @@
   if (!invoker.export) {
 cflags = [ "-fvisibility=hidden" ]
 cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
   }
   deps = [ "//compiler-rt/lib/builtins" ]
   configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -160,11 

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Ryan Prichard via Phabricator via cfe-commits]

rprichard updated this revision to Diff 323221.
rprichard edited the summary of this revision.
rprichard added a comment.

Update libunwind/src/BUILD.gn for macro name change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93003

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  libunwind/src/assembly.h
  libunwind/src/config.h
  llvm/utils/gn/secondary/libunwind/src/BUILD.gn

Index: llvm/utils/gn/secondary/libunwind/src/BUILD.gn
===
--- llvm/utils/gn/secondary/libunwind/src/BUILD.gn
+++ llvm/utils/gn/secondary/libunwind/src/BUILD.gn
@@ -98,7 +98,7 @@
 if (libunwind_hermetic_static_library) {
   cflags = [ "-fvisibility=hidden" ]
   cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
-  defines = [ "_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS" ]
+  defines = [ "_LIBUNWIND_HIDE_SYMBOLS" ]
 }
 deps = [ "//compiler-rt/lib/builtins" ]
 configs += [ ":unwind_config" ]
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -52,7 +52,8 @@
   #endif
 #endif
 
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+  // The CMake file passes -fvisibility=hidden to control ELF/Mach-O visibility.
   #define _LIBUNWIND_EXPORT
   #define _LIBUNWIND_HIDDEN
 #else
@@ -70,11 +71,15 @@
 #define SYMBOL_NAME(name) XSTR(__USER_LABEL_PREFIX__) #name
 
 #if defined(__APPLE__)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define _LIBUNWIND_ALIAS_VISIBILITY(name) __asm__(".private_extern " name)
+#else
+#define _LIBUNWIND_ALIAS_VISIBILITY(name)
+#endif
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   __asm__(".globl " SYMBOL_NAME(aliasname));   \
   __asm__(SYMBOL_NAME(aliasname) " = " SYMBOL_NAME(name)); \
-  extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
-  __attribute__((weak_import));
+  _LIBUNWIND_ALIAS_VISIBILITY(SYMBOL_NAME(aliasname));
 #elif defined(__ELF__)
 #define _LIBUNWIND_WEAK_ALIAS(name, aliasname) \
   extern "C" _LIBUNWIND_EXPORT __typeof(name) aliasname\
Index: libunwind/src/assembly.h
===
--- libunwind/src/assembly.h
+++ libunwind/src/assembly.h
@@ -70,12 +70,15 @@
 #if defined(__APPLE__)
 
 #define SYMBOL_IS_FUNC(name)
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .private_extern name
-#define WEAK_SYMBOL(name) .weak_reference name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_ALIAS(name, aliasname)\
   .globl SYMBOL_NAME(aliasname) SEPARATOR  \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 
 #define NO_EXEC_STACK_DIRECTIVE
@@ -87,17 +90,23 @@
 #else
 #define SYMBOL_IS_FUNC(name) .type name,@function
 #endif
-#define EXPORT_SYMBOL(name)
 #define HIDDEN_SYMBOL(name) .hidden name
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
+#define EXPORT_SYMBOL(name) HIDDEN_SYMBOL(name)
+#else
+#define EXPORT_SYMBOL(name)
+#endif
 #define WEAK_SYMBOL(name) .weak name
 
 #if defined(__hexagon__)
-#define WEAK_ALIAS(name, aliasname) \
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+#define WEAK_ALIAS(name, aliasname)\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   .equiv SYMBOL_NAME(aliasname), SYMBOL_NAME(name)
 #else
 #define WEAK_ALIAS(name, aliasname)\
-  WEAK_SYMBOL(aliasname) SEPARATOR \
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  \
+  WEAK_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)
 #endif
 
@@ -119,7 +128,7 @@
   .section .drectve,"yn" SEPARATOR \
   .ascii "-export:", #name, "\0" SEPARATOR \
   .text
-#if defined(_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS)
+#if defined(_LIBUNWIND_HIDE_SYMBOLS)
 #define EXPORT_SYMBOL(name)
 #else
 #define EXPORT_SYMBOL(name) EXPORT_SYMBOL2(name)
Index: libunwind/src/CMakeLists.txt
===
---