lld + ThinLTO + fprofile-generate causes duplicate symbol errors

The duplicate symbol errors are for __llvm_profile_filename and __llvm_profile_raw_version. IIUC, these are supposed to be weak symbols but Thin LTO seems to break this in some way. This does’t happen with gold or no LTO or full LTO.

$ cat > a.c

extern int foo();

int main() {
return foo();
}

$ cat > b.c

int foo() {
return 0;
}

$ clang a.c -fprofile-generate -flto=thin -c
$ clang b.c -fprofile-generate -flto=thin -c
$ clang a.o b.o -fprofile-generate -flto=thin -fuse-ld=lld
ld.lld: error: duplicate symbol: __llvm_profile_filename

defined at a.c
lto.tmp:(__llvm_profile_filename)
defined at b.c
lto.tmp:(.rodata.__llvm_profile_filename+0x0)

ld.lld: error: duplicate symbol: __llvm_profile_raw_version

defined at a.c
lto.tmp:(__llvm_profile_raw_version)
defined at b.c
lto.tmp:(.rodata.__llvm_profile_raw_version+0x0)
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:10: recipe for target ‘a.out’ failed
make: *** [a.out] Error 1

Hi Pirama,

I can’t reproduce with either lld or gold, using a compiler built from head. What version is your clang?

Thanks,
Teresa

Hi Pirama,

I can’t reproduce with either lld or gold, using a compiler built from head. What version is your clang?

(and your lld)

Hi Teresa,

Thanks for looking into this. I hadn’t initially tried ToT, but it reproduces in ToT as well when I tried.

$ ./clang --version

clang version 7.0.0 (trunk 331879) (llvm/trunk 331888)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /ssd2/pirama/llvm-upstream/llvm-release/bin/.
$ ./ld.lld --version
LLD 7.0.0 ([https://git.llvm.org/git/lld.git](https://git.llvm.org/git/lld.git) dc4977e104c7f33758ac6119d1528aafcd8a94dc) (compatible with GNU linkers)

LLD revision is r331862. To add, I had initially tried it on r328903, which also reproduced the issue.

Sorry, operator error. I can reproduce now. Interestingly, this does not reproduce using gold, and they utilize the same underlying LTO API. Let me dig a little using save-temps and see where they diverge.

Teresa

Adding Peter to comment on the linker resolution issue.

From adding save-temps, it looks like lld and gold are giving different resolutions to the symbols, which is presumably creating this issue:

(first file is with lld and second is with gold)

$ diff a.out.resolution.txt gold/
4c4
< -r=a.o,__llvm_profile_raw_version,plx

The problem is that ThinLTO is not dropping the non-prevailing definitions, and they end up being emitted into the object file for b.o.

$ …/ra/bin/llvm-dis -o - b.o0.0.preopt.bc | grep __llvm_prof
$__llvm_profile_raw_version = comdat any
$__llvm_profile_filename = comdat any
@__llvm_profile_raw_version = constant i64 72057594037927940, comdat
@_llvm_profile_filename = constant [19 x i8] c"default%m.profraw\00", comdat

lld ignores comdats in LTO object files because it expects all comdats to have already been resolved. So we see this error.

We are supposed to drop non-prevailing symbols here, but for some reason we do it only for weak symbols. That’s not correct in ELF where comdat symbols can be both strong and non-prevailing.
http://llvm-cs.pcc.me.uk/lib/LTO/LTO.cpp#290

This patch fixes the reproducer but it leads to other test failures that would need to be looked at.

diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 7e8b9b3c6390…ee11d07d6b8e 100644
— a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -287,7 +287,7 @@ static void thinLTOResolveWeakForLinkerGUID(
recordNewLinkage) {
for (auto &S : GVSummaryList) {
GlobalValue::LinkageTypes OriginalLinkage = S->linkage();

  • if (!GlobalValue::isWeakForLinker(OriginalLinkage))
  • if (GlobalValue::isLocalLinkage(OriginalLinkage))
    continue;
    // We need to emit only one of these. The prevailing module will keep it,
    // but turned into a weak, while the others will drop it when possible.
    diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
    index 246d75caefa2…61790c9fc435 100644
    — a/llvm/lib/Transforms/IPO/FunctionImport.cpp
    +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
    @@ -765,7 +765,7 @@ void llvm::thinLTOResolveWeakForLinkerModule(
    return;
    }
  • if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
  • if (GlobalValue::isLocalLinkage(GV.getLinkage()))
    return;
    // Check for a non-prevailing def that has interposable linkage
    // (e.g. non-odr weak or linkonce). In that case we can’t simply

Peter

Thanks Peter, your patch does fix the reproducer. I filed https://bugs.llvm.org/show_bug.cgi?id=37422 to track this bug. I have no clue on how to resolve the tests - whether further cleanup is required in the code or in the tests. But if Teresa or you cannot get to it, I can, with some help, take a crack at fixing the tests.

I added some updates to the bug. Let’s move the rest of the discussion there.

Teresa