[lld/mac] Correctly set nextdefsym

[thakis]

Summary

In LC_DYSYMTAB, private externs were still emitted as exported symbols instead
of as locals.

Fixes PR50373. See bug for details.

[int3]

lld/MachO/SyntheticSections.cpp

569–575

can we just inline this into a simple check against defined->privateExtern? The assert doesn't seem very useful (we already check it once during InputFile::parseSymbols)

[int3]

lld/MachO/SyntheticSections.cpp

569–575

(perhaps that change would make more sense as part of D102661)

[thakis]

lld/MachO/SyntheticSections.cpp

569–575

Done as part of D102661.

[thakis]

rebase

[int3]

Thanks!

lld/MachO/SyntheticSections.cpp

os << flags << sym.getName() << '\0'
      << static_cast<uint8_t>(BIND_OPCODE_DO_BIND)
      << static_cast<uint8_t>(BIND_OPCODE_DONE);
   return opstreamOffset;
 }
 
 ExportSection::ExportSection()
     : LinkEditSection(segment_names::linkEdit, section_names::export_) {}
 
-static bool shouldExportSymbol(const Defined *defined) {
+static bool shouldExportExternalSymbol(const Defined *defined) {
   if (defined->privateExtern) {
     assert(defined->isExternal() && "invalid input file");
     return false;
   }
   return true;
 }
 
 void ExportSection::finalizeContents() {
   trieBuilder.setImageBase(in.header->addr);
   for (const Symbol *sym : symtab->getSymbols()) {
     if (const auto *defined = dyn_cast<Defined>(sym)) {
-      if (!shouldExportSymbol(defined))
+      if (!shouldExportExternalSymbol(defined))
         continue;
       trieBuilder.addSymbol(*defined);
       hasWeakSymbol = hasWeakSymbol || sym->isWeakDef();
     }
   }
   size = trieBuilder.build();
 }
 
void SymtabSection::finalizeContents() {
   // exist in any object file.
   if (Defined *dyldPrivate = in.stubHelper->dyldPrivate)
     addSymbol(localSymbols, dyldPrivate);
 
   for (Symbol *sym : symtab->getSymbols()) {
     if (auto *defined = dyn_cast<Defined>(sym)) {
       if (!defined->includeInSymtab)
         continue;
-      assert(defined->isExternal());
+      assert(defined->isExternal() && "non-external symbol in symtab");
+      if (!shouldExportExternalSymbol(defined))
+        addSymbol(localSymbols, defined);
+      else
         addSymbol(externalSymbols, defined);
     } else if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
       if (dysym->isReferenced())
         addSymbol(undefinedSymbols, sym);
     }
   }
 
   emitStabs();
   uint32_t symtabIndex = stabs.size();
template <class LP> void SymtabSectionImpl<LP>::writeTo(uint8_t *buf) const {
   }
 
   for (const SymtabEntry &entry : concat<const SymtabEntry>(
            localSymbols, externalSymbols, undefinedSymbols)) {
     nList->n_strx = entry.strx;
     // TODO populate n_desc with more flags
     if (auto *defined = dyn_cast<Defined>(entry.sym)) {
       uint8_t scope = 0;
-      if (!shouldExportSymbol(defined)) {
+      if (defined->isExternal()) {
+        if (!shouldExportExternalSymbol(defined)) {
           // Private external -- dylib scoped symbol.
           // Promote to non-external at link time.
           scope = N_PEXT;
-      } else if (defined->isExternal()) {
+        } else {
           // Normal global symbol.
           scope = N_EXT;
+        }
       } else {
         // TU-local symbol from localSymbols.
         scope = 0;
       }
 
       if (defined->isAbsolute()) {
         nList->n_type = scope | N_ABS;
         nList->n_sect = NO_SECT;

lld/test/MachO/symtab.s

 # REQUIRES: x86
 # RUN: rm -rf %t && split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test2.s -o %t/test2.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
 # RUN: %lld -dylib %t/libfoo.o -o %t/libfoo.dylib
-# RUN: %lld -lSystem %t/test.o %t/libfoo.dylib -o %t/test
+# RUN: %lld -lSystem %t/test.o %t/test2.o %t/libfoo.dylib -o %t/test
 
 # RUN: llvm-readobj --syms --macho-dysymtab %t/test | FileCheck %s
 # CHECK:      Symbols [
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: _local
 # CHECK-NEXT:     Type: Section (0xE)
 # CHECK-NEXT:     Section: __data
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: __dyld_private
 # CHECK-NEXT:     Type: Section (0xE)
 # CHECK-NEXT:     Section: __data
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _private_external
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _private_external_weak
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: _main
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Section (0xE)
 # CHECK-NEXT:     Section: __text (0x1)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}

 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Section (0xE)
 # CHECK-NEXT:     Section: __data
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
-# CHECK-NEXT:  Symbol {
-# CHECK-NEXT:    Name: __mh_execute_header (81)
-# CHECK-NEXT:    Extern
-# CHECK-NEXT:    Type: Section (0xE)
-# CHECK-NEXT:    Section: __text (0x1)
-# CHECK-NEXT:    RefType: UndefinedNonLazy (0x0)
-# CHECK-NEXT:    Flags [ (0x10)
-# CHECK-NEXT:      ReferencedDynamically (0x10)
-# CHECK-NEXT:    ]
-# CHECK-NEXT:    Value: 0x100000000
-# CHECK-NEXT:  }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: __mh_execute_header
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x10)
+# CHECK-NEXT:       ReferencedDynamically (0x10)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x100000000
+# CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: dyld_stub_binder
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Undef (0x0)
 # CHECK-NEXT:     Section:  (0x0)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x100)
 # CHECK-NEXT:       SymbolResolver (0x100)

 # CHECK-NEXT:     Flags [ (0x200)
 # CHECK-NEXT:       AltEntry (0x200)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x0
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 # CHECK-NEXT: Dysymtab {
 # CHECK-NEXT:   ilocalsym: 0
-# CHECK-NEXT:   nlocalsym: 2
-# CHECK-NEXT:   iextdefsym: 2
+# CHECK-NEXT:   nlocalsym: 4
+# CHECK-NEXT:   iextdefsym: 4
 # CHECK-NEXT:   nextdefsym: 4
-# CHECK-NEXT:   iundefsym: 6
+# CHECK-NEXT:   iundefsym: 8
 # CHECK-NEXT:   nundefsym: 2
 
 ## Verify that the first entry in the StringTable is a space, and that
 ## unreferenced symbols aren't emitted.
 # RUN: obj2yaml %t/test | FileCheck %s --check-prefix=YAML
 # YAML:      StringTable:
 # YAML-NEXT: ' '
 # YAML-NOT: _unreferenced
 
 #--- libfoo.s
 .globl _dynamic
 _dynamic:
 
 #--- test.s
-.globl _main, _external, _external_weak, _unreferenced
+.globl _main, _external, _private_external, _external_weak, _private_external_weak, _unreferenced
 
 .data
 _external:
-  .space 0
+  .space 1
 _local:
-  .space 0
+  .space 1
 
 .text
 .weak_definition _external_weak
 _external_weak:
-  .space 0
+  .space 1
+
+.private_extern _private_external
+_private_external:
+  .space 1
+
+.weak_definition _private_external_weak
+.private_extern _private_external_weak
+_private_external_weak:
+  .space 1
 
 _main:
+  callq _private_external
   callq _dynamic
   mov $0, %rax
   ret
+
+#--- test2.s
+## These are both already in test.s and should make it into the symbol table
+## just once.
+.globl _external_weak, _private_external_weak
+.text
+.weak_definition _external_weak
+_external_weak:
+  .space 1
+.weak_definition _private_external_weak
+.private_extern _private_external_weak
+_private_external_weak:
+  .space 1