[lld] 9f2272f - [lld-macho] Allow dead_strip to work with exported private extern symbols

[Vincent Lee via llvm-commits]

Author: Vincent Lee
Date: 2022-04-22T18:45:27-07:00
New Revision: 9f2272ff51b16ffa9c1056fc707daf088ccca784

URL: https://github.com/llvm/llvm-project/commit/9f2272ff51b16ffa9c1056fc707daf088ccca784
DIFF: https://github.com/llvm/llvm-project/commit/9f2272ff51b16ffa9c1056fc707daf088ccca784.diff

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

It seems like we are overly asserting when running `-dead_strip` with
exported symbols. ld64 treats exported private extern symbols as a liveness
root. Loosen the assert to match ld64's behavior.

Reviewed By: #lld-macho, int3

Differential Revision: https://reviews.llvm.org/D124143

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lld/MachO/MarkLive.cpp b/lld/MachO/MarkLive.cpp
index e9b0fd4f7b1a2..9cae677ee03ac 100644
--- a/lld/MachO/MarkLive.cpp
+++ b/lld/MachO/MarkLive.cpp
@@ -228,11 +228,10 @@ void markLive() {
       // -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;
       }

diff  --git a/lld/test/MachO/export-options.s b/lld/test/MachO/export-options.s
index 55e189edbb1e5..654834ecba121 100644
--- a/lld/test/MachO/export-options.s
+++ b/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