[lld-macho] Fix symbol relocs handling for compact unwind's functionAddress

[int3]

Summary

Clang seems to emit all functionAddress relocs as section relocs, but
ld -r can turn those relocs into symbol ones. It turns out that we
weren't handling that case correctly when the symbol was a weak def
whose definition did not prevail.

[int3]

simplify test

[int3]

elaborate on comment

[oontvoo]

LG

lld/MachO/InputFiles.cpp

930

sorry, not 100% related to this, but if the same input file is repeated on the command line, does the pointer equality hold true? or do they count as separate files?

lld/test/MachO/weak-definition-gc.s

262

at this point is it worth implementing -r yet?
these yaml files are kind of annoying

473–481

do we need all these?

[int3]

lld/MachO/InputFiles.cpp

930

separate files (there's a dup symbol test along those lines)

lld/test/MachO/weak-definition-gc.s

262

I mean, feel free to attempt it :P it's not a priority for me

473–481

looks like the empty strings can be deleted yeah. the other two are symbol names

[int3]

update

[int3]

lld/test/MachO/weak-definition-gc.s

262

also, it's not clear to me that implementing -r would remove the need for these YAML files, unless we want to reproduce all the quirks of ld64. We might very well find it easier to not convert section to symbol relocations in our own implementation.

[int3]

did you forget to stamp this? :)

[oontvoo]

👍

lld/MachO/InputFiles.cpp

for (auto it = isec->relocs.begin(); it != isec->relocs.end();) {
       Reloc &r = *it;
       // We only wish to handle the relocation for CUE::functionAddress.
       if (r.offset != 0) {
         ++it;
         continue;
       }
       uint64_t add = r.addend;
       if (auto *sym = cast_or_null<Defined>(r.referent.dyn_cast<Symbol *>())) {
+        // Check whether the symbol defined in this file is the prevailing one.
+        // Skip if it is e.g. a weak def that didn't prevail.
+        if (sym->getFile() != this) {
+          ++it;
+          continue;
+        }
         add += sym->value;
         referentIsec = cast<ConcatInputSection>(sym->isec);
       } else {
         referentIsec =
             cast<ConcatInputSection>(r.referent.dyn_cast<InputSection *>());
       }
       if (referentIsec->getSegName() != segment_names::text)
         error("compact unwind references address in " + toString(referentIsec) +

lld/test/MachO/weak-definition-gc.s

 ## We can even strip weak symbols without subsections_via_symbols as long
 ## as none of the weak symbols in a section are needed.
 # RUN: %lld -dylib -o %t/out.dylib %t/weak-nosub.o %t/weak-nosub.o
 # RUN: llvm-otool -jtV %t/out.dylib | FileCheck --check-prefix=SUB %s
 
 ## Test that omitted weak symbols don't add entries to the compact unwind table.
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-lsda.s -o %t/weak-sub-lsda.o
 # RUN: %lld -dylib -lc++ -o %t/out.dylib %t/weak-sub-lsda.o %t/weak-sub-lsda.o
-# RUN: llvm-objdump --macho --unwind-info --syms %t/out.dylib | FileCheck %s --check-prefix=UNWIND -D#%x,BASE=0
-
-# UNWIND:      SYMBOL TABLE:
-# UNWIND-DAG:  [[#%x,FOO:]]       w  F __TEXT,__text _foo
-# UNWIND-NOT:                          __TEXT,__text _foo
-
-# UNWIND:      Contents of __unwind_info section:
-# UNWIND:        LSDA descriptors:
-# UNWIND:           [0]: function offset=0x[[#%.8x,FOO-BASE]]
-# UNWIND-NOT:       [1]:
-# UNWIND:        Second level indices:
-# UNWIND-DAG:      [0]: function offset=0x[[#%.8x,FOO-BASE]]
-# UNWIND-NOT:      [1]:
+# RUN: llvm-objdump --macho --unwind-info --syms %t/out.dylib | FileCheck %s --check-prefix=ONE-UNWIND
+# RUN: %lld -dylib -lc++ -o %t/out.dylib %t/weak-sub.o %t/weak-sub-lsda.o
+# RUN: llvm-objdump --macho --unwind-info --syms %t/out.dylib | FileCheck %s --check-prefix=NO-UNWIND
+# RUN: yaml2obj %t/weak-sub-lsda-r.yaml > %t/weak-sub-lsda-r.o
+# RUN: %lld -dylib -lc++ -o %t/out.dylib %t/weak-sub.o %t/weak-sub-lsda-r.o
+# RUN: llvm-objdump --macho --unwind-info --syms %t/out.dylib | FileCheck %s --check-prefix=NO-UNWIND
+
+# ONE-UNWIND:      SYMBOL TABLE:
+# ONE-UNWIND-DAG:  [[#%x,FOO:]]       w  F __TEXT,__text _foo
+# ONE-UNWIND-NOT:                          __TEXT,__text _foo
+
+# ONE-UNWIND:      Contents of __unwind_info section:
+# ONE-UNWIND:        LSDA descriptors:
+# ONE-UNWIND:           [0]: function offset=0x[[#%.8x,FOO]]
+# ONE-UNWIND-NOT:       [1]:
+# ONE-UNWIND:        Second level indices:
+# ONE-UNWIND-DAG:      [0]: function offset=0x[[#%.8x,FOO]]
+# ONE-UNWIND-NOT:      [1]:
+
+# NO-UNWIND:      SYMBOL TABLE:
+# NO-UNWIND-DAG:  [[#%x,FOO:]]       w  F __TEXT,__text _foo
+# NO-UNWIND-NOT:                          __TEXT,__text _foo
+# NO-UNWIND-NOT:  Contents of __unwind_info section:
 
 ## Test interaction with .alt_entry
 ## FIXME: ld64 manages to strip both one copy of _foo and _bar each.
 ##        We only manage this if we're lucky and the object files are in
 ##        the right order. We're happy to not crash at link time for now.
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-alt.s -o %t/weak-sub-alt.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/weak-sub-alt2.s -o %t/weak-sub-alt2.o
 # RUN: %lld -dylib -o %t/out.dylib %t/weak-sub-alt.o %t/weak-sub-alt2.o

 #--- strong-def.s
 .globl _foo, _bar
 
 _foo:
   .4byte 0x33333333
   retq
 
 .subsections_via_symbols
+
+#--- weak-sub-lsda-r.yaml
+## This was generated from compiling weak-sub-lsda.s above at rev a2404f11c77e
+## and then running it through `ld -r`. This converts a number of unwind-related
+## relocations from section- to symbol-based ones.
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x1000007
+  cpusubtype:      0x3
+  filetype:        0x1
+  ncmds:           2
+  sizeofcmds:      464
+  flags:           0x2000
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         392
+    segname:         ''
+    vmaddr:          0
+    vmsize:          152
+    fileoff:         528
+    filesize:        152
+    maxprot:         7
+    initprot:        7
+    nsects:          3
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0
+        size:            6
+        offset:          0x210
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         554889E55DC3
+      - sectname:        __gcc_except_tab
+        segname:         __TEXT
+        addr:            0x6
+        size:            32
+        offset:          0x216
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x0
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         '0000000000000000000000000000000000000000000000000000000000000000'
+      - sectname:        __eh_frame
+        segname:         __TEXT
+        addr:            0x28
+        size:            80
+        offset:          0x238
+        align:           3
+        reloff:          0x2A8
+        nreloc:          7
+        flags:           0x0
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         1C00000000000000017A504C5200017810079B0400000010100C0708900100002C00000004000000F8FFFFFFFFFFFFFF060000000000000008E7FFFFFFFFFFFFFF410E800111067A430D060000000000
+        relocations:
+          - address:         0x13
+            symbolnum:       5
+            pcrel:           true
+            length:          2
+            extern:          true
+            type:            4
+            scattered:       false
+            value:           0
+          - address:         0x24
+            symbolnum:       2
+            pcrel:           false
+            length:          2
+            extern:          true
+            type:            5
+            scattered:       false
+            value:           0
+          - address:         0x24
+            symbolnum:       3
+            pcrel:           false
+            length:          2
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x28
+            symbolnum:       3
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            5
+            scattered:       false
+            value:           0
+          - address:         0x28
+            symbolnum:       4
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x39
+            symbolnum:       3
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            5
+            scattered:       false
+            value:           0
+          - address:         0x39
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+      - sectname:        __compact_unwind
+        segname:         __LD
+        addr:            0x78
+        size:            32
+        offset:          0x288
+        align:           3
+        reloff:          0x2E0
+        nreloc:          4
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         '0000000000000000060000000000004100000000000000000000000000000000'
+        relocations:
+          - address:         0x0
+            symbolnum:       4
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x18
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x10
+            symbolnum:       5
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x18
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          768
+    nsyms:           6
+    stroff:          864
+    strsize:         64
+LinkEditData:
+  NameList:
+    - n_strx:          29
+      n_type:          0xE
+      n_sect:          2
+      n_desc:          0
+      n_value:         6
+    - n_strx:          34
+      n_type:          0xE
+      n_sect:          2
+      n_desc:          0
+      n_value:         22
+    - n_strx:          39
+      n_type:          0xE
+      n_sect:          3
+      n_desc:          0
+      n_value:         40
+    - n_strx:          49
+      n_type:          0xE
+      n_sect:          3
+      n_desc:          0
+      n_value:         72
+    - n_strx:          2
+      n_type:          0xF
+      n_sect:          1
+      n_desc:          128
+      n_value:         0
+    - n_strx:          7
+      n_type:          0x1
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ' '
+    - _foo
+    - ___gxx_personality_v0
+    - l001
+    - l002
+    - EH_Frame1
+    - func.eh
+    - ''
+    - ''
+    - ''
+    - ''
+    - ''
+    - ''
+    - ''
+...