[ELF] Move section assignment from initializeSymbols to postParse

[MaskRay]

Summary

https://discourse.llvm.org/t/parallel-input-file-parsing/60164

initializeSymbols currently sets Defined::section and handles non-prevailing
COMDAT groups. Move the code to the parallel postParse to reduce work from the
single-threading code path and make parallel section initialization infeasible.

Postpone reporting duplicate symbol errors so that the messages have the
section information. (Defined::section is assigned in postParse and another
thread may not have the information).

• duplicated-synthetic-sym.s: BinaryFile duplicate definition (very rare) now has no section information
• comdat-binding: %t/w.o %t/g.o leads to an undesired undefined symbol. This is not ideal but we report a diagnostic to inform that this is unsupported. (See release note)
• comdat-discarded-lazy.s: %tdef.o is unextracted. The new behavior (discarded section error) makes more sense

Depends on D120640

[MaskRay]

improve comment

[MaskRay]

Depends on D120626

[peter.smith]

Not supporting comdat-binding: %t/w.o %t/g.o leads to an undesired undefined symbol. This is not ideal but acceptable because valid input cannot form this case. makes me a bit nervous. In a clang/gcc world this is not a problem, but there may be third party compilers that differ. For example arm's legacy compiler armcc from Arm Compiler 5 uses STB_GLOBAL for definitions in COMDAT groups, whereas GCC, at least used to, uses STB_WEAK. It is possible that this is illegal, but I can't find in the spec where it says that.

Perhaps it could be accounted for in resolve as I think we'd be passing a global with the discarded section. Perhaps we could prefer the existing symbol in that case?

lld/ELF/Driver.h

64 ↗

(On Diff #411726)

Could be worth a named struct? Something like DuplicateSymbol. This could be accepted by reportUndefined rather than 4 individual tuple field accesses.

lld/ELF/InputFiles.cpp

1149

nit: !sym.file allows a symbol assignment to redefine a symbol without an error.

1163

If this is happening in parallel, is there a chance of interleaved output? For example a traced symbol in multiple groups? I guess that would need something like driver->duplicates?

lld/ELF/Symbols.cpp

544–553

I'm trying to think how this can happen? Would this be a case where there is a definition of symbol S outside of a group and then an existing weak S from prevailing group and a global S from a discarded group.

May be worth expanding on the case in the comment as it isn't easy (or at least I wasn't able to easily work it out) from the code.

lld/test/ELF/comdat-binding.s

13

I'm not so sure about that. It is certainly an area that is ill-specified. I can't find anything ELF prohibiting it though. Certainly it wouldn't within the same tool-chain I'd expect consistency, but I could see toolchain X using weak symbols in comdat groups and toolchain Y using global symbols unless the C++ ABI for the platform is explicit about it.

I can only find references to weak symbols in groups for http://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-static . The Arm C++ ABI references this https://github.com/ARM-software/abi-aa/blob/main/cppabi32/cppabi32.rst#426elf-binding-of-static-data-guard-variable-symbols but is silent on other cases.

[MaskRay]

address some comments

[MaskRay]

In D120626#3352236, @peter.smith wrote:

Not supporting comdat-binding: %t/w.o %t/g.o leads to an undesired undefined symbol. This is not ideal but acceptable because valid input cannot form this case. makes me a bit nervous. In a clang/gcc world this is not a problem, but there may be third party compilers that differ. For example arm's legacy compiler armcc from Arm Compiler 5 uses STB_GLOBAL for definitions in COMDAT groups, whereas GCC, at least used to, uses STB_WEAK. It is possible that this is illegal, but I can't find in the spec where it says that.

Perhaps it could be accounted for in resolve as I think we'd be passing a global with the discarded section. Perhaps we could prefer the existing symbol in that case?

Avoiding accessing sections[*] is what the patch strives to do: untangle section initialization and symbol initialization/resolution as much as possible.
I have a plan to merge section initialization and initializeLocalSymbols into initSectionsAndLocalSyms, which parallelizes section initialization.
In the long term, this is a necessary step to achieve parallel input file parsing (https://discourse.llvm.org/t/parallel-input-file-parsing/60164) (and perhaps important to keep lld relevant with the emerging of mold.)

Keeping if (sec == &InputSection::discarded) will address the %t/w.o %t/g.o dilemma, but that probably will not fly.
Personally I'd like to say such a dilemma is unsupported. If we have to, it doesn't seem like there is a good way supporting it...

lld/ELF/InputFiles.cpp

1163

Yes. If the determinism is desired, elf::printTraceSymbol( needs to some refactoring: the message call needs to be replaced with something like driver->duplicates.

Placing duplicates in LinkerDriver is a bit arbitrary. I wonder whether we should use ctx->duplicates.

lld/ELF/Symbols.cpp

544–553

Updated the comment (the original one was a bit wrong). It's related to test/ELF/comdat-linkonce.s

[peter.smith]

In D120626#3353151, @MaskRay wrote:

In D120626#3352236, @peter.smith wrote:

Not supporting comdat-binding: %t/w.o %t/g.o leads to an undesired undefined symbol. This is not ideal but acceptable because valid input cannot form this case. makes me a bit nervous. In a clang/gcc world this is not a problem, but there may be third party compilers that differ. For example arm's legacy compiler armcc from Arm Compiler 5 uses STB_GLOBAL for definitions in COMDAT groups, whereas GCC, at least used to, uses STB_WEAK. It is possible that this is illegal, but I can't find in the spec where it says that.

Perhaps it could be accounted for in resolve as I think we'd be passing a global with the discarded section. Perhaps we could prefer the existing symbol in that case?

Avoiding accessing sections[*] is what the patch strives to do: untangle section initialization and symbol initialization/resolution as much as possible.
I have a plan to merge section initialization and initializeLocalSymbols into initSectionsAndLocalSyms, which parallelizes section initialization.
In the long term, this is a necessary step to achieve parallel input file parsing (https://discourse.llvm.org/t/parallel-input-file-parsing/60164) (and perhaps important to keep lld relevant with the emerging of mold.)

Keeping if (sec == &InputSection::discarded) will address the %t/w.o %t/g.o dilemma, but that probably will not fly.
Personally I'd like to say such a dilemma is unsupported. If we have to, it doesn't seem like there is a good way supporting it...

Thanks for the updates so far.

My thoughts so far:

• I agree that mixing symbol bindings in the same link is a niche use case, likely for compilers not derived from LLVM or GCC.
• I expect that parallel reading will be in development for some time. It probably isn't worth working extra hard right now.
• If it isn't possible to cleanly support it, then can we diagnose it with a specific error message? For example LLD does not support COMDAT groups where different groups use different symbol bindings for the same symbol name. is better than an undefined symbol error.
• We'll need to document and release note the restriction.

lld/ELF/InputFiles.cpp

1163

By interleaved output I was thinking of something along the lines of:

Some people, when confronted with a problem, think, “I know, I’ll use threads,” and then two they hav erpoblesms.

from https://nedbatchelder.com/blog/201204/two_problems.html

Perhaps another mutex for the block if it is susceptible. It is possible that the stream is protected with a lock though.

ctx->duplicates is probably more friendly to the people that want to use LLD in the context of a library.

[MaskRay]

I agree that mixing symbol bindings in the same link is a niche use case, likely for compilers not derived from LLVM or GCC.
I expect that parallel reading will be in development for some time. It probably isn't worth working extra hard right now.

This change is just a necessary step to unlock more parallelism (section initialization), unless there is any other approach I haven't thought of.
(Our progress is not particularly rapid, just incrementally making more stuff parallel)

If it isn't possible to cleanly support it, then can we diagnose it with a specific error message? For example LLD does not support COMDAT groups where different groups use different symbol bindings for the same symbol name. is better than an undefined symbol error.
We'll need to document and release note the restriction.

Unfortunately I can't think of a way to detect the case without introducing a new loop.
The best I can think of is to add a boolean variable to InputFile. After parallel section initialization, add some code to postParse to detect the case, then in a new loop, resolve the symbol to the prevailing (even if weak) COMDAT group.
This seems inelegant and a lot of engineering to report a good diagnostic for this case ...

[peter.smith]

Lowest cost option I can think of is to additionally set a flag when InputFiles.cpp:1163:

if (sym.file == this)
  sym.replace(Undefined{this, sym.getName(), sym.binding, sym.stOther,
                        sym.type, secIdx});
continue;

Perhaps a new bit field entry in undefined. At the time of reporting undefined symbols we could check for that flag and report some additional context.
`Symbol <sym> was defined in non-prevailing group. Either the symbol was not present in the prevailing group, or the symbol in the prevailing group had STB_WEAK binding and this symbol had STB_GLOBAL binding. LLD does not support mixing of groups with STB_WEAK and STB_GLOBAL bindings for the same symbol name.'

I don't want to hold this up any more than I have already. If it is going to be too much to support then worth adding something to the documentation, or eventually the release notes stating what we don't support.

[MaskRay]

Add a diagnostic.
Add release note

[MaskRay]

Move duplicates from LinkerDriver to a new struct Ctx

Ctx has short name because we may use it in many functions' signatures in the future.
Using LinkerDriver for global states may look a bit weird

[MaskRay]

lld/ELF/InputFiles.cpp

1163

There is no interleaving. A single-line or multi-line message is emitted as a whole.

lld/Common/ErrorHandler.cpp uses lock_guard for messages and diagnostics.

[peter.smith]

Thanks for the updates. I've set approved as my comments have been addressed.

[MaskRay]

Thanks for accepting the compromise!

[vitalybuka]

Could this be related to the patch https://lab.llvm.org/buildbot/#/builders/37/builds/11582 ?

FYI @pcc

[haowei]

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

[3945/4056] Performing configure step for 'runtimes-i386-unknown-linux-gnu'
-- The C compiler identification is Clang 15.0.0
-- The CXX compiler identification is Clang 15.0.0
-- The ASM compiler identification is Clang with GNU-like command-line
-- Found assembler: /mnt/nvme_sec/SRC/llvm-project/build-repro/./bin/clang
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - failed
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - failed
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Could NOT find ZLIB (missing: ZLIB_LIBRARY) (found version "1.2.11")
-- Found LibXml2: /usr/lib/x86_64-linux-gnu/libxml2.so (found version "2.9.12")
-- Could NOT find ZLIB (missing: ZLIB_LIBRARY) (found version "1.2.11")
-- Performing Test LLVM_RUNTIMES_LINKING_WORKS
-- Performing Test LLVM_RUNTIMES_LINKING_WORKS - Failed
-- Performing Test LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG
-- Performing Test LLVM_RUNTIMES_SUPPORT_UNWINDLIB_NONE_FLAG - Failed
-- Performing Test LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG
-- Performing Test LLVM_RUNTIMES_SUPPORT_NOSTDLIBXX_FLAG - Failed
-- Performing Test LLVM_RUNTIMES_SUPPORT_NOSTDINCXX_FLAG
-- Performing Test LLVM_RUNTIMES_SUPPORT_NOSTDINCXX_FLAG - Failed
CMake Warning (dev) at /usr/share/cmake-3.22/Modules/GNUInstallDirs.cmake:239 (message):
  Unable to determine default CMAKE_INSTALL_LIBDIR directory because no
  target architecture is known.  Please enable at least one language before
  including GNUInstallDirs.
Call Stack (most recent call first):
  /mnt/nvme_sec/SRC/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1 (include)
  CMakeLists.txt:126 (include)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Linker detection: unknown
-- Performing Test CXX_SUPPORTS_CUSTOM_LINKER
-- Performing Test CXX_SUPPORTS_CUSTOM_LINKER - Failed
CMake Error at /mnt/nvme_sec/SRC/llvm-project/llvm/cmake/modules/HandleLLVMOptions.cmake:308 (message):
  Host compiler does not support '-fuse-ld=lld'
Call Stack (most recent call first):
  CMakeLists.txt:127 (include)

Example of the build failure on the bots: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8819676041146779809/+/u/clang/build/stdout
This breakage was masked by another breakage caused by e049a87f04cff8e81b4177097a6b2fdf37c7b148 so it took us a bit while to figure out. We locally verified 0c3156bd43842d8f0ea54bea3fea628e8fee93d2 (1 patch before) will pass the build while c30e6447c0225f675773d07f2b6d4e3c2b962155 (this patch) will fall with the error message above.

Could you take a look? If it takes a bit long time to investigate, could you revert this change first please?

[MaskRay]

In D120626#3383784, @vitalybuka wrote:

Could this be related to the patch https://lab.llvm.org/buildbot/#/builders/37/builds/11582 ?

FYI @pcc

I could verify that clang -fuse-ld=lld -m32 a.c failed to link if I used an older crti.o (glibc<2.32)

usr/lib32/crti.o: definition of __x86.get_pc_thunk.bx
usr/lib32/crti.o: reference to __x86.get_pc_thunk.bx
usr/lib32/crti.o: reference to __x86.get_pc_thunk.bx
ld.lld: error: relocation refers to a symbol in a discarded section: __x86.get_pc_thunk.bx
>>> defined in usr/lib32/crti.o
>>> referenced by usr/lib32/crti.o:(.init+0x5)
>>> referenced by usr/lib32/crti.o:(.fini+0x5)

It was related to the partially supported(worked around) .gnu.linkonce.t.__x86.get_pc_thunk.bx broken by this change.
Should be fixed by 6be457c14dafd634989c2c0b702a9231b438e2c4

In D120626#3383810, @haowei wrote:

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

Thanks for the report, but with Host compiler does not support '-fuse-ld=lld' it might be difficult to know for sure whether the culprit was in an lld change or a brittle configure.
Providing CMakeError.log or CMakeOutput.log with the linker diagnostic could make me more sure that it was related to this lld change.
Anycase, I think the issue was exactly the one for i386 sanitizers and it should have been fixed by the aforementioned commit.

[haowei]

In D120626#3383810, @haowei wrote:

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

Thanks for the report, but with Host compiler does not support '-fuse-ld=lld' it might be difficult to know for sure whether the culprit was in an lld change or a brittle configure.
Providing CMakeError.log or CMakeOutput.log with the linker diagnostic could make me more sure that it was related to this lld change.
Anycase, I think the issue was exactly the one for i386 sanitizers and it should have been fixed by the aforementioned commit.

I looked up my local CMakeError.log file for this patch and it contains:

[2/2] Linking C executable cmTC_5b689
FAILED: cmTC_5b689
: && /mnt/nvme_sec/SRC/llvm-project/build-repro/./bin/clang --target=i386-unknown-linux-gnu --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --target=i386-unknown-linux-gnu -fuse-ld=lld -v CMakeFiles/cmTC_5b689.dir/CMakeCCompilerABI.c.o -o cmTC_5b689   && :
Fuchsia clang version 15.0.0 ([e-mail]:llvm/llvm-project.git c30e6447c0225f675773d07f2b6d4e3c2b962155)
Target: i386-unknown-linux-gnu
Thread model: posix
InstalledDir: /mnt/nvme_sec/SRC/llvm-project/build-repro/./bin
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8
Selected GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Candidate multilib: .;@m32
Selected multilib: .;@m32
 "/mnt/nvme_sec/SRC/llvm-project/build-repro/./bin/ld.lld" --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --build-id --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o cmTC_5b689 /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crt1.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtbegin.o -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8 -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib CMakeFiles/cmTC_5b689.dir/CMakeCCompilerABI.c.o /mnt/nvme_sec/SRC/llvm-project/build-repro/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a -lc /mnt/nvme_sec/SRC/llvm-project/build-repro/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtend.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crtn.o
ld.lld: error: relocation refers to a symbol in a discarded section: __x86.get_pc_thunk.bx
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o:(.init+0x5)
>>> referenced by /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o:(.fini+0x5)
>>> referenced by elf-init.oS:(__libc_csu_init) in archive /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/libc_nonshared.a
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

So it is indeed the same type of error mentioned by @vitalybuka . I tested your fix in 6be457c14dafd634989c2c0b702a9231b438e2c4 in our builder. but I am still seeing the same error: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8819586782274771361/overview . The CMakeError.log contains:

[2/2] Linking C executable cmTC_ace5f
FAILED: cmTC_ace5f
: && /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin/clang --target=i386-unknown-linux-gnu --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --target=i386-unknown-linux-gnu -fuse-ld=lld -v CMakeFiles/cmTC_ace5f.dir/CMakeCCompilerABI.c.o -o cmTC_ace5f   && :
Fuchsia clang version 15.0.0 ([e-mail]:llvm/llvm-project.git 6be457c14dafd634989c2c0b702a9231b438e2c4)
Target: i386-unknown-linux-gnu
Thread model: posix
InstalledDir: /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8
Selected GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Candidate multilib: .;@m32
Selected multilib: .;@m32
 "/mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin/ld.lld" --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --build-id --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o cmTC_ace5f /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crt1.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtbegin.o -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8 -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib CMakeFiles/cmTC_ace5f.dir/CMakeCCompilerABI.c.o /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a -lc /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtend.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crtn.o
ld.lld: error: duplicate symbol: __x86.get_pc_thunk.bx
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/libc_nonshared.a(elf-init.oS)
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Still complaining the same symbol but with a slightly different error: ld.lld: error: duplicate symbol: __x86.get_pc_thunk.bx. compared to earlier ld.lld: error: relocation refers to a symbol in a discarded section: __x86.get_pc_thunk.bx

Could you take a look?

[MaskRay]

In D120626#3384404, @haowei wrote:

In D120626#3383810, @haowei wrote:

We are seeing build failures in cmake config step for the clang runtime builds after this patch. Example of error messages:

Thanks for the report, but with Host compiler does not support '-fuse-ld=lld' it might be difficult to know for sure whether the culprit was in an lld change or a brittle configure.
Providing CMakeError.log or CMakeOutput.log with the linker diagnostic could make me more sure that it was related to this lld change.
Anycase, I think the issue was exactly the one for i386 sanitizers and it should have been fixed by the aforementioned commit.

I looked up my local CMakeError.log file for this patch and it contains:

[2/2] Linking C executable cmTC_5b689
FAILED: cmTC_5b689
: && /mnt/nvme_sec/SRC/llvm-project/build-repro/./bin/clang --target=i386-unknown-linux-gnu --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --target=i386-unknown-linux-gnu -fuse-ld=lld -v CMakeFiles/cmTC_5b689.dir/CMakeCCompilerABI.c.o -o cmTC_5b689   && :
Fuchsia clang version 15.0.0 ([e-mail]:llvm/llvm-project.git c30e6447c0225f675773d07f2b6d4e3c2b962155)
Target: i386-unknown-linux-gnu
Thread model: posix
InstalledDir: /mnt/nvme_sec/SRC/llvm-project/build-repro/./bin
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8
Selected GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Candidate multilib: .;@m32
Selected multilib: .;@m32
 "/mnt/nvme_sec/SRC/llvm-project/build-repro/./bin/ld.lld" --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --build-id --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o cmTC_5b689 /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crt1.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtbegin.o -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8 -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib CMakeFiles/cmTC_5b689.dir/CMakeCCompilerABI.c.o /mnt/nvme_sec/SRC/llvm-project/build-repro/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a -lc /mnt/nvme_sec/SRC/llvm-project/build-repro/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtend.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crtn.o
ld.lld: error: relocation refers to a symbol in a discarded section: __x86.get_pc_thunk.bx
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o:(.init+0x5)
>>> referenced by /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o:(.fini+0x5)
>>> referenced by elf-init.oS:(__libc_csu_init) in archive /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/libc_nonshared.a
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

So it is indeed the same type of error mentioned by @vitalybuka . I tested your fix in 6be457c14dafd634989c2c0b702a9231b438e2c4 in our builder. but I am still seeing the same error: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8819586782274771361/overview . The CMakeError.log contains:

[2/2] Linking C executable cmTC_ace5f
FAILED: cmTC_ace5f
: && /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin/clang --target=i386-unknown-linux-gnu --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --target=i386-unknown-linux-gnu -fuse-ld=lld -v CMakeFiles/cmTC_ace5f.dir/CMakeCCompilerABI.c.o -o cmTC_ace5f   && :
Fuchsia clang version 15.0.0 ([e-mail]:llvm/llvm-project.git 6be457c14dafd634989c2c0b702a9231b438e2c4)
Target: i386-unknown-linux-gnu
Thread model: posix
InstalledDir: /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Found candidate GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8
Selected GCC installation: /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8
Candidate multilib: .;@m32
Selected multilib: .;@m32
 "/mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/./bin/ld.lld" --sysroot=/usr/local/google/home/haowei/SRC/fuchsia-sysroot --build-id --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o cmTC_ace5f /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crt1.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtbegin.o -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8 -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/lib -L/usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib CMakeFiles/cmTC_ace5f.dir/CMakeCCompilerABI.c.o /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a -lc /mnt/nvme_sec/SRC/llvm-project/build-repro-mitigate/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/crtend.o /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crtn.o
ld.lld: error: duplicate symbol: __x86.get_pc_thunk.bx
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/crti.o
>>> defined in /usr/local/google/home/haowei/SRC/fuchsia-sysroot/usr/lib/i386-linux-gnu/libc_nonshared.a(elf-init.oS)
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Still complaining the same symbol but with a slightly different error: ld.lld: error: duplicate symbol: __x86.get_pc_thunk.bx. compared to earlier ld.lld: error: relocation refers to a symbol in a discarded section: __x86.get_pc_thunk.bx

Could you take a look?

I just suppressed the error in c1d4c67718db88be48f451b91f7bddab8fff2424.

Can you provide i386-linux-gnu/crti.o and i386-linux-gnu/libc_nonshared.a(elf-init.oS)?
I need to understand the issue better to craft a better test. I believe the state before this patch worked for such symbols were accidental.

[haowei]

I just suppressed the error in c1d4c67718db88be48f451b91f7bddab8fff2424.

Can you provide i386-linux-gnu/crti.o and i386-linux-gnu/libc_nonshared.a(elf-init.oS)?
I need to understand the issue better to craft a better test. I believe the state before this patch worked for such symbols were accidental.

These files are available in archive https://chrome-infra-packages.appspot.com/p/fuchsia/third_party/sysroot/linux/+/tp7-Zyo4pv2SVEoK_eaU6yuKmyxJWcR54vtJKTWpTIYC under:

./usr/lib/i386-linux-gnu/crti.o
./usr/lib/i386-linux-gnu/libc_nonshared.a

The bots that failed the build used this exact revision.

[haowei]

While c1d4c67718db88be48f451b91f7bddab8fff2424 clears the cmake error. I am now seeing runtimes-i386-unknown-linux-gnu build failures related to the same symbol:

[367/1127] Linking CXX shared library /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so
FAILED: /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so 
: && /b/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=i386-unknown-linux-gnu --sysroot=/b/s/w/ir/x/w/cipd/linux -fPIC --target=i386-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -ffile-prefix-map=/b/s/w/ir/x/w/staging/llvm_build/runtimes/runtimes-i386-unknown-linux-gnu-bins=../staging/llvm_build/runtimes/runtimes-i386-unknown-linux-gnu-bins -ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -Wall -std=c++14 -Wno-unused-parameter -O2 -g -DNDEBUG  -fuse-ld=lld  -Wl,-z,nodelete -fuse-ld=lld -Wl,--color-diagnostics   -nodefaultlibs -Wl,-z,text -nostdlib++ -shared -Wl,-soname,libclang_rt.ubsan_minimal.so -o /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so compiler-rt/lib/ubsan_minimal/CMakeFiles/RTUbsan_minimal.i386.dir/ubsan_minimal_handlers.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib:/b/s/w/ir/x/w/staging/llvm_build/./lib"  -lc  /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a && :
ld.lld: error: relocation R_386_PC32 cannot refer to absolute symbol: __x86.get_pc_thunk.bx
>>> defined in /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o:(.init+0x5)

ld.lld: error: relocation R_386_PC32 cannot refer to absolute symbol: __x86.get_pc_thunk.bx
>>> defined in /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o:(.fini+0x5)
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

The failed build is https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8819579887511161281/overview which was started after your patch was landed.

[MaskRay]

In D120626#3384544, @haowei wrote:

While c1d4c67718db88be48f451b91f7bddab8fff2424 clears the cmake error. I am now seeing runtimes-i386-unknown-linux-gnu build failures related to the same symbol:

[367/1127] Linking CXX shared library /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so
FAILED: /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so 
: && /b/s/w/ir/x/w/staging/llvm_build/./bin/clang++ --target=i386-unknown-linux-gnu --sysroot=/b/s/w/ir/x/w/cipd/linux -fPIC --target=i386-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -ffile-prefix-map=/b/s/w/ir/x/w/staging/llvm_build/runtimes/runtimes-i386-unknown-linux-gnu-bins=../staging/llvm_build/runtimes/runtimes-i386-unknown-linux-gnu-bins -ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes -Wall -std=c++14 -Wno-unused-parameter -O2 -g -DNDEBUG  -fuse-ld=lld  -Wl,-z,nodelete -fuse-ld=lld -Wl,--color-diagnostics   -nodefaultlibs -Wl,-z,text -nostdlib++ -shared -Wl,-soname,libclang_rt.ubsan_minimal.so -o /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.ubsan_minimal.so compiler-rt/lib/ubsan_minimal/CMakeFiles/RTUbsan_minimal.i386.dir/ubsan_minimal_handlers.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib:/b/s/w/ir/x/w/staging/llvm_build/./lib"  -lc  /b/s/w/ir/x/w/staging/llvm_build/lib/clang/15.0.0/lib/i386-unknown-linux-gnu/libclang_rt.builtins.a && :
ld.lld: error: relocation R_386_PC32 cannot refer to absolute symbol: __x86.get_pc_thunk.bx
>>> defined in /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o:(.init+0x5)

ld.lld: error: relocation R_386_PC32 cannot refer to absolute symbol: __x86.get_pc_thunk.bx
>>> defined in /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o
>>> referenced by /b/s/w/ir/x/w/cipd/linux/usr/lib/i386-linux-gnu/crti.o:(.fini+0x5)
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

The failed build is https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8819579887511161281/overview which was started after your patch was landed.

I just reverted the patch in 9b61fff0eb9332553ecdbe00e01e3d08ad392768 since I need to think a bit how to support __x86.get_pc_thunk.bx.
Thanks for the i386 sysroot. It allowed me to improve the test i386-linkonce.s to describe the case we have to work around more precisely.

lld/ELF/Config.h

 #include <memory>
 #include <vector>
 
 namespace lld {
 namespace elf {
 
 class InputFile;
 class InputSectionBase;
+class Symbol;
 
 enum ELFKind : uint8_t {
   ELFNoneKind,
   ELF32LEKind,
   ELF32BEKind,
   ELF64LEKind,
   ELF64BEKind
 };
struct Configuration {
 
   // 4 for ELF32, 8 for ELF64.
   int wordsize;
 };
 
 // The only instance of Configuration struct.
 extern std::unique_ptr<Configuration> config;
 
+struct DuplicateSymbol {
+  const Symbol *sym;
+  const InputFile *file;
+  InputSectionBase *section;
+  uint64_t value;
+};
+
+struct Ctx {
+  // Duplicate symbol candidates.
+  SmallVector<DuplicateSymbol, 0> duplicates;
+  // Symbols in a non-prevailing COMDAT group which should be changed to an
+  // Undefined.
+  SmallVector<std::pair<Symbol *, unsigned>, 0> nonPrevailingSyms;
+};
+
+// The only instance of Ctx struct.
+extern std::unique_ptr<Ctx> ctx;
+
 // The first two elements of versionDefinitions represent VER_NDX_LOCAL and
 // VER_NDX_GLOBAL. This helper returns other elements.
 static inline ArrayRef<VersionDefinition> namedVersionDefs() {
   return llvm::makeArrayRef(config->versionDefinitions).slice(2);
 }
 
 void errorOrWarn(const Twine &msg);
 

lld/ELF/Driver.cpp

 using namespace llvm::ELF;
 using namespace llvm::object;
 using namespace llvm::sys;
 using namespace llvm::support;
 using namespace lld;
 using namespace lld::elf;
 
 std::unique_ptr<Configuration> elf::config;
+std::unique_ptr<Ctx> elf::ctx;
 std::unique_ptr<LinkerDriver> elf::driver;
 
 static void setConfigs(opt::InputArgList &args);
 static void readConfigs(opt::InputArgList &args);
 
 void elf::errorOrWarn(const Twine &msg) {
   if (config->noinhibitExec)
     warn(msg);
ctx->e.cleanupCallback = []() {
 
     SharedFile::vernauxNum = 0;
   };
   ctx->e.logName = args::getFilenameWithoutExe(args[0]);
   ctx->e.errorLimitExceededMsg = "too many errors emitted, stopping now (use "
                                  "--error-limit=0 to see all errors)";
 
   config = std::make_unique<Configuration>();
+  elf::ctx = std::make_unique<Ctx>();
   driver = std::make_unique<LinkerDriver>();
   script = std::make_unique<LinkerScript>();
   symtab = std::make_unique<SymbolTable>();
 
   partitions.clear();
   partitions.emplace_back();
 
   config->progName = args[0];
void LinkerDriver::link(opt::InputArgList &args) {
   // Archive members defining __wrap symbols may be extracted.
   std::vector<WrappedSymbol> wrapped = addWrappedSymbols(args);
 
   // No more lazy bitcode can be extracted at this point. Do post parse work
   // like checking duplicate symbols.
   parallelForEach(objectFiles, initializeLocalSymbols);
   parallelForEach(objectFiles, postParseObjectFile);
   parallelForEach(bitcodeFiles, [](BitcodeFile *file) { file->postParse(); });
+  for (auto &it : ctx->nonPrevailingSyms) {
+    Symbol &sym = *it.first;
+    sym.replace(Undefined{sym.file, sym.getName(), sym.binding, sym.stOther,
+                          sym.type, it.second});
+    cast<Undefined>(sym).nonPrevailing = true;
+  }
+  ctx->nonPrevailingSyms.clear();
+  for (const DuplicateSymbol &d : ctx->duplicates)
+    reportDuplicate(*d.sym, d.file, d.section, d.value);
+  ctx->duplicates.clear();
 
   // Return if there were name resolution errors.
   if (errorCount())
     return;
 
   // We want to declare linker script's symbols early,
   // so that we can version them.
   // They also might be exported if referenced by DSOs.
void LinkerDriver::link(opt::InputArgList &args) {
   if (skipLinkedOutput)
     return;
 
   // compileBitcodeFiles may have produced lto.tmp object files. After this, no
   // more file will be added.
   auto newObjectFiles = makeArrayRef(objectFiles).slice(numObjsBeforeLTO);
   parallelForEach(newObjectFiles, initializeLocalSymbols);
   parallelForEach(newObjectFiles, postParseObjectFile);
+  for (const DuplicateSymbol &d : ctx->duplicates)
+    reportDuplicate(*d.sym, d.file, d.section, d.value);
 
   // Handle --exclude-libs again because lto.tmp may reference additional
   // libcalls symbols defined in an excluded archive. This may override
   // versionId set by scanVersionScript().
   if (args.hasArg(OPT_exclude_libs))
     excludeLibs(args);
 
   // Apply symbol renames for --wrap and combine foo@v1 and foo@@v1.

lld/ELF/InputFiles.cpp

if ((sec.sh_flags & SHF_MERGE) && shouldMerge(sec, name))
     return make<MergeInputSection>(*this, sec, name);
   return make<InputSection>(*this, sec, name);
 }
 
 // Initialize this->Symbols. this->Symbols is a parallel array as
 // its corresponding ELF symbol table.
 template <class ELFT>
 void ObjFile<ELFT>::initializeSymbols(const object::ELFFile<ELFT> &obj) {
-  ArrayRef<InputSectionBase *> sections(this->sections);
   SymbolTable &symtab = *elf::symtab;
 
   ArrayRef<Elf_Sym> eSyms = this->getELFSyms<ELFT>();
   symbols.resize(eSyms.size());
 
   // Some entries have been filled by LazyObjFile.
   for (size_t i = firstGlobal, end = eSyms.size(); i != end; ++i)
     if (!symbols[i])
if (LLVM_UNLIKELY(binding == STB_LOCAL)) {
       continue;
     }
     uint32_t secIdx = eSym.st_shndx;
     if (secIdx == SHN_UNDEF) {
       undefineds.push_back(i);
       continue;
     }
 
-    if (LLVM_UNLIKELY(secIdx == SHN_XINDEX))
-      secIdx = check(getExtendedSymbolTableIndex<ELFT>(eSym, i, shndxTable));
-    else if (secIdx >= SHN_LORESERVE)
-      secIdx = 0;
-    if (LLVM_UNLIKELY(secIdx >= sections.size()))
-      fatal(toString(this) + ": invalid section index: " + Twine(secIdx));
-    InputSectionBase *sec = sections[secIdx];
     uint8_t stOther = eSym.st_other;
     uint8_t type = eSym.getType();
     uint64_t value = eSym.st_value;
     uint64_t size = eSym.st_size;
 
     Symbol *sym = symbols[i];
     sym->isUsedInRegularObj = true;
     if (LLVM_UNLIKELY(eSym.st_shndx == SHN_COMMON)) {
       if (value == 0 || value >= UINT32_MAX)
         fatal(toString(this) + ": common symbol '" + sym->getName() +
               "' has invalid alignment: " + Twine(value));
       hasCommonSyms = true;
       sym->resolve(
           CommonSymbol{this, StringRef(), binding, stOther, type, value, size});
       continue;
     }
 
-    // If a defined symbol is in a discarded section, handle it as if it
-    // were an undefined symbol. Such symbol doesn't comply with the
-    // standard, but in practice, a .eh_frame often directly refer
-    // COMDAT member sections, and if a comdat group is discarded, some
-    // defined symbol in a .eh_frame becomes dangling symbols.
-    if (sec == &InputSection::discarded) {
-      Undefined und{this, StringRef(), binding, stOther, type, secIdx};
-      // !LazyObjFile::lazy indicates that the file containing this object has
-      // not finished processing, i.e. this symbol is a result of a lazy symbol
-      // extract. We should demote the lazy symbol to an Undefined so that any
-      // relocations outside of the group to it will trigger a discarded section
-      // error.
-      if (sym->symbolKind == Symbol::LazyObjectKind && !sym->file->lazy)
-        sym->replace(und);
-      else
-        sym->resolve(und);
-      continue;
-    }
-
-    // Handle global defined symbols.
+    // Handle global defined symbols. Defined::section will be set in postParse.
     if (binding == STB_GLOBAL || binding == STB_WEAK ||
         binding == STB_GNU_UNIQUE) {
-      sym->resolve(
-          Defined{this, StringRef(), binding, stOther, type, value, size, sec});
+      sym->resolve(Defined{this, StringRef(), binding, stOther, type, value,
+                           size, nullptr});
       continue;
     }
 
     fatal(toString(this) + ": unexpected binding: " + Twine((int)binding));
   }
 
   // Undefined symbols (excluding those defined relative to non-prevailing
   // sections) can trigger recursive extract. Process defined symbols first so
else
                                eSym.st_value, eSym.st_size, sec);
     symbols[i]->isUsedInRegularObj = true;
   }
 }
 
 // Called after all ObjFile::parse is called for all ObjFiles. This checks
 // duplicate symbols and may do symbol property merge in the future.
 template <class ELFT> void ObjFile<ELFT>::postParse() {
+  static std::mutex mu;
   ArrayRef<Elf_Sym> eSyms = this->getELFSyms<ELFT>();
   for (size_t i = firstGlobal, end = eSyms.size(); i != end; ++i) {
     const Elf_Sym &eSym = eSyms[i];
-    const Symbol &sym = *symbols[i];
+    Symbol &sym = *symbols[i];
+    uint32_t secIdx = eSym.st_shndx;
 
     // st_value of STT_TLS represents the assigned offset, not the actual
     // address which is used by STT_FUNC and STT_OBJECT. STT_TLS symbols can
     // only be referenced by special TLS relocations. It is usually an error if
     // a STT_TLS symbol is replaced by a non-STT_TLS symbol, vice versa.
     if (LLVM_UNLIKELY(sym.isTls()) && eSym.getType() != STT_TLS &&
         eSym.getType() != STT_NOTYPE)
       errorOrWarn("TLS attribute mismatch: " + toString(sym) + "\n>>> in " +
                   toString(sym.file) + "\n>>> in " + toString(this));
 
-    // !sym.file allows a symbol assignment redefines a symbol without an error.
-    if (sym.file == this || !sym.file || !sym.isDefined() ||
-        eSym.st_shndx == SHN_UNDEF || eSym.st_shndx == SHN_COMMON ||
-        eSym.getBinding() == STB_WEAK)
+    // Handle non-COMMON defined symbol below. !sym.file allows a symbol
+    // assignment to redefine a symbol without an error.
+    if (!sym.file || !sym.isDefined() || secIdx == SHN_UNDEF ||
+        secIdx == SHN_COMMON)
       continue;
-    uint32_t secIdx = eSym.st_shndx;
+
     if (LLVM_UNLIKELY(secIdx == SHN_XINDEX))
-      secIdx = cantFail(getExtendedSymbolTableIndex<ELFT>(eSym, i, shndxTable));
+      secIdx = check(getExtendedSymbolTableIndex<ELFT>(eSym, i, shndxTable));
     else if (secIdx >= SHN_LORESERVE)
       secIdx = 0;
-    if (sections[secIdx] == &InputSection::discarded)
+    if (LLVM_UNLIKELY(secIdx >= sections.size()))
+      fatal(toString(this) + ": invalid section index: " + Twine(secIdx));
+    InputSectionBase *sec = sections[secIdx];
+    if (sec == &InputSection::discarded) {
+      if (sym.traced) {
+        printTraceSymbol(Undefined{this, sym.getName(), sym.binding,
+                                   sym.stOther, sym.type, secIdx},
+                         sym.getName());
+      }
+      if (sym.file == this) {
+        std::lock_guard<std::mutex> lock(mu);
+        ctx->nonPrevailingSyms.emplace_back(&sym, secIdx);
+      }
       continue;
-    // Allow absolute symbols with the same value for GNU ld compatibility.
-    if (!cast<Defined>(sym).section && !sections[secIdx] &&
-        cast<Defined>(sym).value == eSym.st_value)
+    }
+
+    if (sym.file == this) {
+      cast<Defined>(sym).section = sec;
+      continue;
+    }
+
+    if (eSym.getBinding() == STB_WEAK)
       continue;
-    reportDuplicate(sym, this, sections[secIdx], eSym.st_value);
+    std::lock_guard<std::mutex> lock(mu);
+    ctx->duplicates.push_back({&sym, this, sec, eSym.st_value});
   }
 }
 
 // The handling of tentative definitions (COMMON symbols) in archives is murky.
 // A tentative definition will be promoted to a global definition if there are
 // no non-tentative definitions to dominate it. When we hold a tentative
 // definition to a symbol and are inspecting archive members for inclusion
 // there are 2 ways we can proceed:

lld/ELF/Relocations.cpp

static std::string maybeReportDiscarded(Undefined &sym) {
 
   Elf_Shdr_Impl<ELFT> elfSec = objSections[sym.discardedSecIdx - 1];
   if (elfSec.sh_type != SHT_GROUP)
     return msg;
 
   // If the discarded section is a COMDAT.
   StringRef signature = file->getShtGroupSignature(objSections, elfSec);
   if (const InputFile *prevailing =
-          symtab->comdatGroups.lookup(CachedHashStringRef(signature)))
+          symtab->comdatGroups.lookup(CachedHashStringRef(signature))) {
     msg += "\n>>> section group signature: " + signature.str() +
            "\n>>> prevailing definition is in " + toString(prevailing);
+    if (sym.nonPrevailing) {
+      msg += "\n>>> or the symbol in the prevailing group had STB_WEAK "
+             "binding and the symbol in a non-prevailing group had STB_GLOBAL "
+             "binding. Mixing groups with STB_WEAK and STB_GLOBAL binding "
+             "signature is not supported";
+    }
+  }
   return msg;
 }
 
 // Undefined diagnostics are collected in a vector and emitted once all of
 // them are known, so that some postprocessing on the list of undefined symbols
 // can happen before lld emits diagnostics.
 struct UndefinedDiag {
   Undefined *sym;

lld/ELF/Symbols.h

Undefined(InputFile *file, StringRef name, uint8_t binding, uint8_t stOther,
             uint8_t type, uint32_t discardedSecIdx = 0)
       : Symbol(UndefinedKind, file, name, binding, stOther, type),
         discardedSecIdx(discardedSecIdx) {}
 
   static bool classof(const Symbol *s) { return s->kind() == UndefinedKind; }
 
   // The section index if in a discarded section, 0 otherwise.
   uint32_t discardedSecIdx;
+  bool nonPrevailing = false;
 };
 
 class SharedSymbol : public Symbol {
 public:
   static bool classof(const Symbol *s) { return s->kind() == SharedKind; }
 
   SharedSymbol(InputFile &file, StringRef name, uint8_t binding,
                uint8_t stOther, uint8_t type, uint64_t value, uint64_t size,

 }
 
 template <typename... T> Defined *makeDefined(T &&...args) {
   return new (reinterpret_cast<Defined *>(
       getSpecificAllocSingleton<SymbolUnion>().Allocate()))
       Defined(std::forward<T>(args)...);
 }
 
-void reportDuplicate(const Symbol &sym, InputFile *newFile,
+void reportDuplicate(const Symbol &sym, const InputFile *newFile,
                      InputSectionBase *errSec, uint64_t errOffset);
 void maybeWarnUnorderableSymbol(const Symbol *sym);
 bool computeIsPreemptible(const Symbol &sym);
 void reportBackrefs();
 
 } // namespace elf
 } // namespace lld
 
 #endif

lld/ELF/Symbols.cpp

bool Symbol::shouldReplace(const Defined &other) const {
   // some vague linkage data in COMDAT from STB_WEAK to STB_GNU_UNIQUE. Treat
   // STB_GNU_UNIQUE like STB_WEAK so that we prefer the first among all
   // STB_WEAK/STB_GNU_UNIQUE copies. If we prefer an incoming STB_GNU_UNIQUE to
   // an existing STB_WEAK, there may be discarded section errors because the
   // selected copy may be in a non-prevailing COMDAT.
   return !isGlobal() && other.isGlobal();
 }
 
-void elf::reportDuplicate(const Symbol &sym, InputFile *newFile,
+void elf::reportDuplicate(const Symbol &sym, const InputFile *newFile,
                           InputSectionBase *errSec, uint64_t errOffset) {
   if (config->allowMultipleDefinition)
     return;
-  const Defined *d = cast<Defined>(&sym);
+  // A definition in a COMDAT may be reported as duplicate with a definition
+  // relative to .gnu.linkonce.t.__x86.get_pc_thunk.bx.
+  // .gnu.linkonce.t.__x86.get_pc_thunk.bx will be discarded, so there is
+  // actually no duplicate.
+  const Defined *d = dyn_cast<Defined>(&sym);
+  if (!d)
+    return;
+  // Allow absolute symbols with the same value for GNU ld compatibility.
+  if (!d->section && !errSec && errOffset && d->value == errOffset)
+    return;
   if (!d->section || !errSec) {
     error("duplicate symbol: " + toString(sym) + "\n>>> defined in " +
           toString(sym.file) + "\n>>> defined in " + toString(newFile));
     return;
   }
 
   // Construct and print an error message in the form of:
   //

lld/docs/ReleaseNotes.rst

* ``-z pack-relative-relocs`` is now available to support ``DT_RELR`` for glibc 2.36+.
   (`D120701 <https://reviews.llvm.org/D120701>`_)
 
 Breaking changes
 ----------------
 
 * The GNU ld incompatible ``--no-define-common`` has been removed.
 * The obscure ``-dc``/``-dp`` options have been removed.
 * ``-d`` is now ignored.
+* If a prevailing COMDAT group defines STB_WEAK symbol, having a STB_GLOBAL symbol in a non-prevailing group is now rejected with a diagnostic.
+  (`D120626 <https://reviews.llvm.org/D120626>`_)
 
 COFF Improvements
 -----------------
 
 * Added autodetection of MSVC toolchain, a la clang-cl.  Also added /winsysroot
   support for explicit specification of MSVC toolchain location.
   (`D118070 <https://reviews.llvm.org/D118070>`_)
 * ...

lld/test/ELF/comdat-binding.s

 # REQUIRES: x86
 # RUN: rm -rf %t && split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/g.s -o %t/g.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/w.s -o %t/w.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/u.s -o %t/u.o
 # RUN: ld.lld -e 0 %t/w.o %t/u.o -o %t/w
 # RUN: llvm-readelf -s %t/w | FileCheck %s --check-prefix=WEAK
 # RUN: ld.lld -e 0 %t/u.o %t/w.o -o %t/u
 # RUN: llvm-readelf -s %t/u | FileCheck %s --check-prefix=UNIQUE
 
-# RUN: ld.lld -e 0 %t/w.o %t/g.o -o %t/w
-# RUN: llvm-readelf -s %t/w | FileCheck %s --check-prefix=WEAK
+## We prefer STB_GLOBAL definition, then changing it to undefined since it is in
+## in a non-prevailing COMDAT. Ideally this should be defined, but our behavior
+## is fine because valid input cannot form this case.
+# RUN: ld.lld -e 0 %t/w.o %t/g.o -o %t/und --noinhibit-exec 2>&1 | FileCheck %s --check-prefix=WARN
+# RUN: llvm-readelf -s %t/und | FileCheck %s --check-prefix=UND
 
 # WEAK:   NOTYPE WEAK   DEFAULT [[#]] _ZZ1fvE1x
 # UNIQUE: OBJECT UNIQUE DEFAULT [[#]] _ZZ1fvE1x
+# UND:    NOTYPE GLOBAL DEFAULT UND   _ZZ1fvE1x
+
+# WARN: warning: relocation refers to a symbol in a discarded section: f()::x
+# WARN-NEXT: >>> defined in {{.*}}g.o
+# WARN-NEXT: >>> section group signature: _ZZ1fvE1x
+# WARN-NEXT: >>> prevailing definition is in {{.*}}w.o
+# WARN-NEXT: >>> or the symbol in the prevailing group had STB_WEAK binding and the symbol in a non-prevailing group had STB_GLOBAL binding. Mixing groups with STB_WEAK and STB_GLOBAL binding signature is not supported
+# WARN-NEXT: >>> referenced by {{.*}}g.o:(.text+0x3)
 
 #--- g.s
 movq _ZZ1fvE1x@gotpcrel(%rip), %rax
 
 .section .bss._ZZ1fvE1x,"awG",@nobits,_ZZ1fvE1x,comdat
 .globl _ZZ1fvE1x
 _ZZ1fvE1x:
 

lld/test/ELF/comdat-discarded-error.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
 # RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl foo; foo:' |\
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
 # RUN: echo '.weak foo; foo: .section .text.foo,"axG",@progbits,foo,comdat; .globl bar; bar:' |\
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t3.o
 
 # RUN: not ld.lld %t2.o %t3.o %t1.o -o /dev/null 2>&1 | FileCheck %s
 
 # CHECK:      error: relocation refers to a symbol in a discarded section: bar
 # CHECK-NEXT: >>> defined in {{.*}}3.o
 # CHECK-NEXT: >>> section group signature: foo
 # CHECK-NEXT: >>> prevailing definition is in {{.*}}2.o
+# CHECK-NEXT: >>> or the symbol in the prevailing group {{.*}}
 # CHECK-NEXT: >>> referenced by {{.*}}1.o:(.text+0x1)
 
 # CHECK:      error: relocation refers to a discarded section: .text.foo
 # CHECK-NEXT: >>> defined in {{.*}}1.o
 # CHECK-NEXT: >>> section group signature: foo
 # CHECK-NEXT: >>> prevailing definition is in {{.*}}2.o
 # CHECK-NEXT: >>> referenced by {{.*}}1.o:(.data+0x0)
 

lld/test/ELF/comdat-discarded-lazy.s

 
 # AA-NM: aa
 # AA-NM: f2
 
 # AA:      error: relocation refers to a symbol in a discarded section: aa
 # AA-NEXT: >>> defined in {{.*}}aa.o
 # AA-NEXT: >>> section group signature: foo
 # AA-NEXT: >>> prevailing definition is in {{.*}}1.o
+# AA-NEXT: >>> or the symbol in the prevailing group {{.*}}
 # AA-NEXT: >>> referenced by {{.*}}aa.o:(.text+0x1)
 
 ## Test the case when the symbol causing a "discarded section" is ordered
 ## *after* the symbol fetching the lazy object.
 # RUN: echo '.globl f2, zz; .weak foo; foo: f2: call zz; \
 # RUN:   .section .text.foo,"axG",@progbits,foo,comdat; zz:' | \
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %tzz.o
 # RUN: llvm-nm -p %tzz.o | FileCheck --check-prefix=ZZ-NM %s
 # RUN: not ld.lld %t.o --start-lib %t1.o %tzz.o --end-lib -o /dev/null 2>&1 | FileCheck --check-prefix=ZZ %s
 # RUN: rm -f %tzz.a && llvm-ar rc %tzz.a %tzz.o
 # RUN: not ld.lld %t.o --start-lib %t1.o --end-lib %tzz.a -o /dev/null 2>&1 | FileCheck --check-prefix=ZZ %s
 
 # ZZ-NM: f2
 # ZZ-NM: zz
 
 # ZZ:      error: relocation refers to a symbol in a discarded section: zz
 # ZZ-NEXT: >>> defined in {{.*}}zz.o
 # ZZ-NEXT: >>> section group signature: foo
 # ZZ-NEXT: >>> prevailing definition is in {{.*}}1.o
+# ZZ-NEXT: >>> or the symbol in the prevailing group {{.*}}
 # ZZ-NEXT: >>> referenced by {{.*}}zz.o:(.text+0x1)
 
-## Don't error if the symbol which would cause "discarded section"
-## was inserted before %tzz.o
+## The definition in %tdef.o is outside a group. Currently we give an error
+## because %tdef.o is not extracted.
 # RUN: echo '.globl zz; zz:' | llvm-mc -filetype=obj -triple=x86_64 - -o %tdef.o
-# RUN: ld.lld %t.o --start-lib %t1.o %tdef.o %tzz.o --end-lib -o /dev/null
+# RUN: not ld.lld %t.o --start-lib %t1.o %tdef.o %tzz.o --end-lib -o /dev/null 2>&1 | FileCheck --check-prefix=ZZ %s
 # RUN: rm -f %tdef.a && llvm-ar rc %tdef.a %tdef.o
-# RUN: ld.lld %t.o --start-lib %t1.o %tdef.a %tzz.o --end-lib -o /dev/null
+# RUN: not ld.lld %t.o --start-lib %t1.o %tdef.a %tzz.o --end-lib -o /dev/null 2>&1 | FileCheck --check-prefix=ZZ %s
 
 .globl _start
 _start:
   call f1
   call f2

lld/test/ELF/duplicated-synthetic-sym.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: rm -rf %t.dir
 // RUN: mkdir %t.dir
 // RUN: cd %t.dir
 // RUN: echo > file.bin
 
 // RUN: not ld.lld %t.o --format=binary file.bin -o /dev/null 2>&1 | FileCheck %s
 // RUN: not ld.lld %t.o --format binary file.bin -o /dev/null 2>&1 | FileCheck %s
 
 // CHECK:      duplicate symbol: _binary_file_bin_start
-// CHECK-NEXT: defined at {{.*}}.o:(.text+0x0)
-// CHECK-NEXT: defined at file.bin:(.data+0x0)
+// CHECK-NEXT: defined in {{.*}}.o
+// CHECK-NEXT: defined in <internal>
 
 .globl  _binary_file_bin_start
 _binary_file_bin_start:
   .long 0

lld/test/ELF/exclude-discarded-error2.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
-# RUN: echo '.section .foo,"ae"; .weak foo; foo:' | \
+# RUN: echo '.section .foo,"aG",@progbits,zz,comdat; .weak foo; foo:' | \
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t1.o
-# RUN: not ld.lld %t.o %t1.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: ld.lld %t.o %t1.o -o %t
+# RUN: llvm-readelf -s %t | FileCheck %s
 
-# Because foo defined in %t1.o is weak, it does not override global undefined
-# in %t.o
-# CHECK-NOT: discarded section
-# CHECK: undefined symbol: foo
+# CHECK: NOTYPE WEAK DEFAULT UND foo
 
 .globl _start
 _start:
   jmp foo
+
+.section .foo,"aG",@progbits,zz,comdat

lld/test/ELF/lto/comdat-mixed-archive.test

 BCSYM:      U bar
 BCSYM-NEXT: W foo
 
 ;; Check that the symbols are handled in the expected order.
 TRACE:      lib.a(obj.o): lazy definition of foo
 TRACE-NEXT: lib.a(obj.o): lazy definition of bar
 TRACE-NEXT: lib.a(bc.bc): definition of foo
 TRACE-NEXT: lib.a(bc.bc): reference to bar
-TRACE-NEXT: lib.a(obj.o): reference to foo
 TRACE-NEXT: lib.a(obj.o): definition of bar
+TRACE-NEXT: lib.a(obj.o): reference to foo
 TRACE-NEXT: <internal>: reference to foo
 ;; The definition of "foo" is visible outside the LTO result.
 TRACE-NEXT: lto.tmp: definition of foo
 TRACE-NEXT: lto.tmp: reference to bar
 
 ;--- start.s
   .global _start, baz
 _start: