Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LLD emits corrupt version for weak symbol when using version script #49259

Closed
aaronpuchert opened this issue Apr 10, 2021 · 14 comments
Closed

LLD emits corrupt version for weak symbol when using version script #49259

aaronpuchert opened this issue Apr 10, 2021 · 14 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla lld:ELF

Comments

@aaronpuchert
Copy link
Member

aaronpuchert commented Apr 10, 2021

Bugzilla Link 49915
Resolution FIXED
Resolved on May 12, 2021 09:40
Version unspecified
OS Linux
Blocks #48661
CC @h0tc0d3,@MaskRay,@nikic,@smithp35,@tstellar
Fixed by commit(s) 1c00530 6912082

Extended Description

When linking against a lld-linked libLLVM.so with ld.bfd I got the following error:

/usr/bin/ld: /usr/lib64/libLLVM.so: __morestack: invalid needed version 2
/usr/bin/ld: /usr/lib64/libLLVM.so: error adding symbols: bad value
clang-12.0: error: linker command failed with exit code 1 (use -v to see invocation)

Indeed:

readelf --wide --dyn-syms /usr/lib64/libLLVM.so.12 | grep morestack
332: 0000000000000000 0 NOTYPE WEAK DEFAULT UND __morestack@@

This is coming from llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp. I've managed to build a small reproducer that should work with 12.0.0rc5:

cat morestack.cpp
extern "C" attribute((weak)) void __morestack();

unsigned long f() {
return (unsigned long)&__morestack;
}

cat script.map
LLVM_13 { global: *; };
clang -c -fPIC -g morestack.cpp
clang -fuse-ld=lld -shared -o test.so -Wl,--version-script,script.map morestack.o
readelf --wide --dyn-syms test.so | grep __morestack
5: 0000000000000000 0 NOTYPE WEAK DEFAULT UND __morestack@@

Without -fuse-ld=lld, i.e. with ld.bfd I get

 2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __morestack

Now lld itself seems to fine with the corrupt version, so maybe binutils are just overly sensitive? I don't know.

@aaronpuchert
Copy link
Member Author

aaronpuchert commented Apr 10, 2021

assigned to @MaskRay

@aaronpuchert
Copy link
Member Author

aaronpuchert commented Apr 12, 2021

Just checked with llvm-readelf, which sees

 5: 0000000000000000     0 NOTYPE  WEAK   DEFAULT   UND __morestack@LLVM_13

So who is right? But even if llvm-readelf is right, this is obviously not what we want. Generally undefined symbols shouldn't get LLVM_13, but I can't find any syntax to exclude them in the version script.

If you say this behavior is intentional, I guess we need to change the LLVM build.

@MaskRay
Copy link
Member

MaskRay commented Apr 16, 2021

Created https://reviews.llvm.org/D100624

llvm-readelf -V should report in such a case.

@aaronpuchert
Copy link
Member Author

aaronpuchert commented Apr 16, 2021

Created https://reviews.llvm.org/D100624
Thanks! I can confirm that it fixes the issue for the reproducer. Both readelf's read it as

 5: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __morestack

afterwards.

llvm-readelf -V should report in such a case.
It looks pretty inconspicuous to me, but maybe I'm mixing something up:

$ llvm-readelf -V test.so
Version symbols section '.gnu.version' contains 7 entries:
Addr: 0000000000000330 Offset: 0x000330 Link: 2 (.dynsym)
000: 0 (local) 1 (global) 1 (global) 1 (global)
004: 3 (GLIBC_2.2.5) 2 (LLVM_13) 2 (LLVM_13)

Version definition section '.gnu.version_d' contains 2 entries:
Addr: 0000000000000340 Offset: 0x000340 Link: 8 (.dynstr)
0x0000: Rev: 1 Flags: BASE Index: 1 Cnt: 1 Name: test.so
0x001c: Rev: 1 Flags: none Index: 2 Cnt: 1 Name: LLVM_13

Version needs section '.gnu.version_r' contains 1 entries:
Addr: 0000000000000378 Offset: 0x000378 Link: 8 (.dynstr)
0x0000: Version: 1 File: libc.so.6 Cnt: 1
0x0010: Name: GLIBC_2.2.5 Flags: none Version: 3

@MaskRay
Copy link
Member

MaskRay commented Apr 20, 2021

llvm-readelf -V should report in such a case.

I have figured out the GNU readelf behavior. It maintains max_vd_ndx for the max version id for a definition while iterating over the dynamic symbol table. When it sees the incorrect __morestack@LLVM_13, max_vd_ndx is 0, so it reports an error. When it sees _Z1fv@@LLVM_13, it will update max_vd_ndx but that is too late.

I don't think the constraint is necessary so llvm-readelf probably should stick with the current behavior.

@MaskRay
Copy link
Member

MaskRay commented Apr 20, 2021

Fixed by 1c00530 (target: 13.0.0)

@nikic
Copy link
Contributor

nikic commented Apr 20, 2021

It would be nice to pick this up for LLVM 12.0.1.

@MaskRay
Copy link
Member

MaskRay commented Apr 24, 2021

It would be nice to pick this up for LLVM 12.0.1.

Sounds good!

@h0tc0d3
Copy link

h0tc0d3 commented May 11, 2021

I have same error libLLVM-12.so: __morestack in the LLVM 12

/usr/bin/ld: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../lib64/libLLVM-12.so: __morestack: invalid needed version 2
/usr/bin/ld: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../lib64/libLLVM-12.so: error adding symbols: bad value error

It interferes build MESA 21.1.0 and RUST.

This patch 1c00530
not helped for me.

I found solution how to fixit - build LLVM with export LLVM_IAS=1
My LLVM build script: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=llvm12-git

@aaronpuchert
Copy link
Member Author

aaronpuchert commented May 11, 2021

This patch
https://github.com/llvm/llvm-project/commit/
1c00530
not helped for me.
Then it's probably a different bug, because the reproducer in comment 0 was fixed by that commit.

I found solution how to fixit - build LLVM with export LLVM_IAS=1
Interesting that symbol versions are affected by the choice of assembler, especially since the build script doesn't seem to enable LTO. Can you observe a (relevant) difference in the object file for llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp between LLVM_IAS=1 and without it?

The linker using an assembler would surprise me a bit, but I don't know a lot about this.

@tstellar
Copy link
Collaborator

tstellar commented May 12, 2021

I

This patch
https://github.com/llvm/llvm-project/commit/
1c00530
not helped for me.
Then it's probably a different bug, because the reproducer in comment 0 was
fixed by that commit.

Can someone open a new bug for this?

@tstellar
Copy link
Collaborator

tstellar commented May 12, 2021

Merged: 6912082

@h0tc0d3
Copy link

h0tc0d3 commented May 12, 2021

Interesting that symbol versions are affected by the choice of assembler,
especially since the build script doesn't seem to enable LTO. Can you
observe a (relevant) difference in the object file for
llvm/lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp between
LLVM_IAS=1 and without it?

I accidentally found a solution to the problem. Initially, I always bild LLVM with LLVM_IAS=1, but when I removed this line I got this errors in MESA. Only removal -DLLVM_USE_LINKER=lld helped. Then I decided to return LLVM_IAS=1
and then the problem was solved. During the couple of hours that I rebuilt LLVM, there were no new commits. I try reproduce and send objdumps.

@h0tc0d3
Copy link

h0tc0d3 commented May 12, 2021

Today I double-checked, the error did not repeat itself. The files are identical. It’s strange. Yesterday, I applied a patch and cleaned up the build folder every time, and the patch didn't help. Today this patch is already in the llvm 12 branch. The mistake did not repeat itself today.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla lld:ELF
Projects
None yet
Development

No branches or pull requests

5 participants