[lld] 2e1be96 - Reland "[lld-macho] Fix assertion failure in registerCompactUnwind""

[Vy Nguyen via llvm-commits]

Author: Vy Nguyen
Date: 2021-11-09T11:52:46-05:00
New Revision: 2e1be96df696edc352255779e0498034c35daf57

URL: https://github.com/llvm/llvm-project/commit/2e1be96df696edc352255779e0498034c35daf57
DIFF: https://github.com/llvm/llvm-project/commit/2e1be96df696edc352255779e0498034c35daf57.diff

LOG: Reland "[lld-macho] Fix assertion failure in registerCompactUnwind""

PR/52372

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

New changes:
- use llvm-otool instead of `otool` which doesn't in exist on non-OSX platforms
- add llvm-otool to the set of tools used by test so that the bot will use the <build_dir>/bin/llvm-otool instead of the unqualified `llvm-otool` (which may not exist)
- update tests since the latest (TOT) llvm-otool prints a space between two bytes and the old one doesn't.

Added: 
    

Modified: 
    lld/MachO/SymbolTable.cpp
    lld/test/MachO/weak-definition-gc.s
    lld/test/lit.cfg.py

Removed: 
    


################################################################################
diff  --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 37c3fe0d38c74..df7237468ae3d 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -62,28 +62,32 @@ Defined *SymbolTable::addDefined(StringRef name, InputFile *file,
   if (!wasInserted) {
     if (auto *defined = dyn_cast<Defined>(s)) {
       if (isWeakDef) {
+
+        // See further comment in createDefined() in InputFiles.cpp
         if (defined->isWeakDef()) {
-          // Both old and new symbol weak (e.g. inline function in two TUs):
-          // If one of them isn't private extern, the merged symbol isn't.
           defined->privateExtern &= isPrivateExtern;
           defined->referencedDynamically |= isReferencedDynamically;
           defined->noDeadStrip |= noDeadStrip;
-
-          // FIXME: Handle this for bitcode files.
-          // FIXME: We currently only do this if both symbols are weak.
-          //        We could do this if either is weak (but getting the
-          //        case where !isWeakDef && defined->isWeakDef() right
-          //        requires some care and testing).
-          if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
-            concatIsec->wasCoalesced = true;
         }
-
+        // FIXME: Handle this for bitcode files.
+        if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
+          concatIsec->wasCoalesced = true;
         return defined;
       }
-      if (!defined->isWeakDef())
+
+      if (defined->isWeakDef()) {
+        // FIXME: Handle this for bitcode files.
+        if (auto concatIsec =
+                dyn_cast_or_null<ConcatInputSection>(defined->isec)) {
+          concatIsec->wasCoalesced = true;
+          concatIsec->symbols.erase(llvm::find(concatIsec->symbols, defined));
+        }
+      } else {
         error("duplicate symbol: " + name + "\n>>> defined in " +
               toString(defined->getFile()) + "\n>>> defined in " +
               toString(file));
+      }
+
     } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {
       overridesWeakDef = !isWeakDef && dysym->isWeakDef();
       dysym->unreference();

diff  --git a/lld/test/MachO/weak-definition-gc.s b/lld/test/MachO/weak-definition-gc.s
index 0660ccc524d42..819f33faa4508 100644
--- a/lld/test/MachO/weak-definition-gc.s
+++ b/lld/test/MachO/weak-definition-gc.s
@@ -66,6 +66,25 @@
 # ALIGN-NEXT: {{0*}}[[#ADDR]]      11111111 33333333 22222222 00000000
 # ALIGN-NEXT: {{0*}}[[#ADDR+0x10]] 81818181 81818181 82828282 82828282
 
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-def.s -o %t/weak-def.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/strong-def.s -o %t/strong-def.o
+# RUN: %lld -dylib -lc++ -o %t/weak-strong-mixed.dylib %t/weak-def.o %t/strong-def.o
+# RUN: %lld -dylib -lc++ -o %t/strong-weak-mixed.dylib %t/strong-def.o %t/weak-def.o
+## Check that omitted weak symbols are not adding their section and unwind stuff.
+
+# RUN: llvm-otool -jtV %t/weak-strong-mixed.dylib | FileCheck --check-prefix=MIXED %s
+# RUN: llvm-otool -jtV %t/strong-weak-mixed.dylib | FileCheck --check-prefix=MIXED %s
+# MIXED: (__TEXT,__text) section
+# MIXED-NEXT: _foo:
+# MIXED-NEXT: {{.+}} 	33 33            	xorl	(%rbx), %esi
+# MIXED-NEXT: {{.+}} 	33 33            	xorl	(%rbx), %esi
+# MIXED-NEXT: {{.+}}	c3              	retq
+
+# RUN: llvm-objdump --macho --syms --unwind-info %t/weak-strong-mixed.dylib | FileCheck --check-prefix=MIXED-UNWIND %s
+# RUN: llvm-objdump --macho --syms --unwind-info %t/strong-weak-mixed.dylib | FileCheck --check-prefix=MIXED-UNWIND %s
+# MIXED-UNWIND: g     F __TEXT,__text _foo
+# MIXED-UNWIND-NOT: Contents of __unwind_info section:
+
 #--- weak-sub.s
 .globl _foo, _bar
 .weak_definition _foo, _bar
@@ -195,3 +214,35 @@ _main:
   retq
 
 .subsections_via_symbols
+
+#--- weak-def.s
+.section __TEXT,__text,regular,pure_instructions
+
+.globl _foo
+.weak_definition _foo
+_foo:
+  .cfi_startproc
+  .cfi_personality 155, ___gxx_personality_v0
+  .cfi_lsda 16, Lexception
+  pushq %rbp
+  .cfi_def_cfa_offset 128
+  .cfi_offset %rbp, 48
+  movq %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  popq %rbp
+  retq
+  .cfi_endproc
+
+.section __TEXT,__gcc_except_tab
+Lexception:
+    .space 0x10
+
+.subsections_via_symbols
+#--- strong-def.s
+.globl _foo, _bar
+
+_foo:
+  .4byte 0x33333333
+  retq
+
+.subsections_via_symbols

diff  --git a/lld/test/lit.cfg.py b/lld/test/lit.cfg.py
index d03a383cc8758..236c46187002d 100644
--- a/lld/test/lit.cfg.py
+++ b/lld/test/lit.cfg.py
@@ -38,7 +38,7 @@
 llvm_config.use_lld()
 
 tool_patterns = [
-    'llc', 'llvm-as', 'llvm-mc', 'llvm-nm', 'llvm-objdump', 'llvm-pdbutil',
+    'llc', 'llvm-as', 'llvm-mc', 'llvm-nm', 'llvm-objdump', 'llvm-otool', 'llvm-pdbutil',
     'llvm-dwarfdump', 'llvm-readelf', 'llvm-readobj', 'obj2yaml', 'yaml2obj',
     'opt', 'llvm-dis']