[lld-macho] Fix an assertion failure when -u specifies an undefined section$start symbol

[MaskRay]

Summary

This matches ld64. Also improve the test for -dead_strip.

[int3]

I don't understand this... shouldn't -u symbols also be marked live? MarkLive.cpp does traverse explicitUndefineds after all. Also I patched this diff, added back the assert, and ran the start-end.s test, and the assert wasn't triggered

[MaskRay]

fix test: now it does crash without the patch

[int3]

Gotcha, thanks for fixing the test. I still think the assert is correct... as long as we fix MarkLive to mark all explicitUndefineds:

--- a/lld/MachO/MarkLive.cpp
+++ b/lld/MachO/MarkLive.cpp
@@ -96,8 +96,7 @@ void markLive() {
   }
   // -u symbols
   for (Symbol *sym : config->explicitUndefineds)
-    if (auto *defined = dyn_cast<Defined>(sym))
-      addSym(defined);
+    addSym(sym);
   // local symbols explicitly marked .no_dead_strip
   for (const InputFile *file : inputFiles)
     if (auto *objFile = dyn_cast<ObjFile>(file))

[MaskRay]

properly fix the issue as Jez suggested

[MaskRay]

add missing RUN line

[int3]

Thanks!

[thakis]

LGTM too. Thanks for the patch, and thanks to int3 for the excellent review :)

lld/MachO/MarkLive.cpp

if (auto *defined = dyn_cast<Defined>(sym)) {
       if (externsAreRoots && !defined->privateExtern) {
         addSym(defined);
         continue;
       }
     }
   }
   // -u symbols
   for (Symbol *sym : config->explicitUndefineds)
-    if (auto *defined = dyn_cast<Defined>(sym))
-      addSym(defined);
+    addSym(sym);
   // local symbols explicitly marked .no_dead_strip
   for (const InputFile *file : inputFiles)
     if (auto *objFile = dyn_cast<ObjFile>(file))
       for (Symbol *sym : objFile->symbols)
         if (auto *defined = dyn_cast_or_null<Defined>(sym))
           if (!defined->isExternal() && defined->noDeadStrip)
             addSym(defined);
   if (auto *stubBinder =

lld/test/MachO/start-end.s

 # OTHERENTRY:      [[#%x, OTHERADDR:]] g     F __TEXT,__text _otherfun
 # OTHERENTRY:      LC_MAIN
 # OTHERENTRY-NEXT: cmdsize
 # OTHERENTRY-NEXT: entryoff [[#%d, OTHERADDR - 0x100000000]]
 
 
 ## Test that the link succeeds with dead-stripping enabled too.
 # RUN: %lld -dead_strip -lSystem %t/main.o -o %t/stripped.out
+# RUN: llvm-objdump --macho --syms --section-headers %t/stripped.out > %t-stripped-dump.txt
+# RUN: llvm-objdump --macho -d --no-symbolic-operands --no-show-raw-insn %t/stripped.out >> %t-stripped-dump.txt
+# RUN: FileCheck --check-prefix=STRIP %s < %t-stripped-dump.txt
+
+## -u 'section$start$*' does not cause an undefined symbol error. This matches ld64.
+# RUN: %lld -dead_strip -lSystem %t/main.o -u 'section$start$__FOO$__notexist' -o %t/stripped1.out
+# RUN: llvm-objdump --section-headers %t/stripped1.out | FileCheck --check-prefix=STRIP2 %s
 
 ## (Fun fact: `-e 'section$start$__TEXT$__text -dead_strip` strips
 ## everything in the text section because markLive runs well before
 ## section$start symbols are replaced, so the entry point is just
 ## an undefined symbol that keeps nothing alive, and then later it
 ## sets the entry point to the start of the now-empty text section
 ## and the output program crashes when running. This matches ld64's
 ## behavior.)
 
+# STRIP-LABEL: Sections:
+# STRIP-NEXT:  Idx Name           Size     VMA              Type
+# STRIP-NEXT:  0 __text           {{[0-9a-f]*}} [[#%x, TEXTSTART:]] TEXT
+# STRIP-NEXT:  1 __cstring        00000000      [[#%x, CSTRINGSTART:]] DATA
+# STRIP-NEXT:  2 __data           00000000
+# STRIP-NEXT:  3 __llvm_orderfile 00000000
+# STRIP-NEXT:  4 __mybss          00000000
+# STRIP-NEXT:  5 __bar            00000000
+# STRIP-NEXT:  6 __ever           00000000
+# STRIP-NEXT:  7 __lookup         00000000
+# STRIP-NEXT:  8 symbol           00000000
+# STRIP-NEXT:  9 __quux           00000000
+
+# STRIP-LABEL: SYMBOL TABLE:
+# STRIP-NOT:   section$start$__FOO$__bar
+
+# STRIP-LABEL: _main:
+# STRIP:      [[#%x, PC1:]]:
+# STRIP-SAME: leaq [[#%d, TEXTSTART - PC1 - 7]](%rip), %rax
+# STRIP-NEXT: [[#%x, PC2:]]:
+# STRIP-SAME: leaq [[#%d, CSTRINGSTART - PC2 - 7]](%rip), %rbx
+
+# STRIP2-LABEL: Sections:
+# STRIP2-NEXT:  Idx Name           Size     VMA              Type
+# STRIP2-NEXT:  0 __text           {{[0-9a-f]*}} [[#%x, TEXTSTART:]] TEXT
+# STRIP2-NEXT:  1 __cstring        00000000      [[#%x, CSTRINGSTART:]] DATA
+# STRIP2-NEXT:  2 __data           00000000
+# STRIP2-NEXT:  3 __llvm_orderfile 00000000
+# STRIP2-NEXT:  4 __mybss          00000000
+# STRIP2-NEXT:  5 __bar            00000000
+# STRIP2-NEXT:  6 __notexist       00000000
+# STRIP2-NEXT:  7 __ever           00000000
+# STRIP2-NEXT:  8 __lookup         00000000
+# STRIP2-NEXT:  9 symbol           00000000
+# STRIP2-NEXT:  10 __quux          00000000
+
 # CHECK-LABEL: Sections:
 # CHECK-NEXT:  Idx Name           Size     VMA              Type
 # CHECK:       0 __text           {{[0-9a-f]*}} [[#%x, TEXTSTART:]] TEXT
 # CHECK:       1 __aftertext      {{[0-9a-f]*}} [[#%x, TEXTEND:]]
 # CHECK:       2 __cstring        {{[0-9a-f]*}} [[#%x, CSTRINGSTART:]] DATA
 # CHECK:       3 __aftercstring   {{[0-9a-f]*}} [[#%x, CSTRINGEND:]]
 # CHECK:       4 __data           00000008      [[#%x, DATASTART:]] DATA
 # CHECK:       5 __llvm_orderfile 00000000      [[#%x, LLVMORDERFILESTART:]] DATA