[PATCH] D124143: [lld-macho] Allow dead_strip to work with exported private extern symbols

[Vincent Lee via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f2272ff51b1: [lld-macho] Allow dead_strip to work with exported private extern symbols (authored by thevinster).

Changed prior to commit:
  https://reviews.llvm.org/D124143?vs=424597&id=424678#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124143

Files:
  lld/MachO/MarkLive.cpp
  lld/test/MachO/export-options.s


Index: lld/test/MachO/export-options.s
===================================================================
--- lld/test/MachO/export-options.s
+++ lld/test/MachO/export-options.s
@@ -133,7 +133,7 @@
 # RUN: llvm-nm -g %t/exp-autohide.dylib | FileCheck %s --check-prefix=EXP-AUTOHIDE
 
 # RUN: not %lld -dylib -exported_symbol "_foo" %t/autohide-private-extern.o \
-# RUN: -o /dev/null  2>&1 | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
+# RUN:   -o /dev/null  2>&1 | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
 
 # RUN: not %lld -dylib -exported_symbol "_foo" %t/autohide.o \
 # RUN:   %t/glob-private-extern.o -o /dev/null 2>&1 | \
@@ -143,8 +143,24 @@
 # RUN:   %t/weak-private-extern.o -o /dev/null 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
 
+## Test that exported hidden symbols are still treated as a liveness root.
+## This previously used to crash when enabling -dead_strip since it's unconventional
+## to add treat private extern symbols as a liveness root.
+# RUN: %no-fatal-warnings-lld -dylib -exported_symbol "_foo" %t/autohide-private-extern.o \
+# RUN:   -dead_strip -o %t/exported-hidden
+# RUN: llvm-nm -m %t/exported-hidden | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE-DEAD-STRIP
+
+# RUN: %no-fatal-warnings-lld -dylib -exported_symbol "_foo" %t/autohide.o \
+# RUN:   -dead_strip %t/glob-private-extern.o -o %t/exported-hidden
+# RUN: llvm-nm -m %t/exported-hidden | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE-DEAD-STRIP
+
+# RUN: %no-fatal-warnings-lld -dylib -exported_symbol "_foo" %t/autohide.o \
+# RUN:   -dead_strip %t/weak-private-extern.o -o %t/exported-hidden
+# RUN: llvm-nm -m %t/exported-hidden | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE-DEAD-STRIP
+
 # EXP-AUTOHIDE: T _foo
 # AUTOHIDE-PRIVATE: error: cannot export hidden symbol _foo
+# AUTOHIDE-PRIVATE-DEAD-STRIP: (__TEXT,__text) non-external (was a private external) _foo
 
 #--- default.s
 
Index: lld/MachO/MarkLive.cpp
===================================================================
--- lld/MachO/MarkLive.cpp
+++ lld/MachO/MarkLive.cpp
@@ -228,11 +228,10 @@
       // -exported_symbol(s_list)
       if (!config->exportedSymbols.empty() &&
           config->exportedSymbols.match(defined->getName())) {
-        // FIXME: Instead of doing this here, maybe the Driver code doing
-        // the matching should add them to explicitUndefineds? Then the
-        // explicitUndefineds code below would handle this automatically.
-        assert(!defined->privateExtern &&
-               "should have been rejected by driver");
+        // NOTE: Even though exporting private externs is an ill-defined
+        // operation, we are purposely not checking for privateExtern in
+        // order to follow ld64's behavior of treating all exported private
+        // extern symbols as live, irrespective of whether they are autohide.
         marker->addSym(defined);
         continue;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124143.424678.patch
Type: text/x-patch
Size: 2923 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220423/a5c11da6/attachment.bin>