[PATCH] D108016: [lld-macho] Ignore LLVM segments to prevent duplicate syms

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:294
+      } else if (isec->getSegName() == segment_names::llvm) {
+        // Sections within the llvm segment aren't known to induce symbols errors.
+        // Push an empty map to ensure indices line up for the remaining sections.
----------------
I would say that ld64 does not appear to emit contents from sections within the `__LLVM` segment, and that the symbols in those sections point to bitcode metadata rather than being actual symbols. In particular, global symbols within those sections can have the same name without causing duplicate symbol errors.

maybe also add a TODO to figure out if we are supposed to do anything useful with said metadata...


================
Comment at: lld/test/MachO/ignore-symbols.s:6-8
+## _llvm. symbols are not special. LLD would produce duplicate symbol errors
+## if they were not within the LLVM segment.
+# RUN: %lld -dylib %t/foo.o %t/bar.o -o %t/libDuplicate.dylib
----------------
we need to test a few more things:

1. that we discard `__LLVM` sections entirely, just like ld64 (you can use `llvm-objdump --section-headers`)
2. when an `_llvm.foo` symbol is not under an `__LLVM` section, a duplicate symbol error is indeed raised

(test should be retitled too, something like discard-llvm-sections.s)


================
Comment at: lld/test/MachO/ignore-symbols.s:11-17
+.globl _llvm.foo
+.section __LLVM,__bitcode
+_llvm.foo:
+.asciz "test"
+
+#--- bar.s
+.globl _llvm.foo
----------------
oontvoo wrote:
> How does ld64 deal with this, though? (quick glance at the code, I didn't see any special handling for _llvm segments)
> 
> Just concerned that this change is hiding a bug somewhere else
see ld64's LLVMBitcode class :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108016/new/

https://reviews.llvm.org/D108016