Page MenuHomePhabricator

[CMake] Remove -Wl,-allow-shlib-undefined which was added in rL221530
ClosedPublic

Authored by MaskRay on Aug 29 2020, 12:20 PM.

Details

Summary

In GNU ld, gold and LLD, --no-allow-shlib-undefined is the default when
linking an executable. The option disallows unresolved symbols in shared objects.
(gold and LLD catch fewer cases than GNU ld. See D57385 for details)
See D57569 why it is bad idea to use --allow-shlib-undefined for executables [a].

GNU ld traditionally copied DT_NEEDED entries transitively. This was
deemed not good, so GNU ld 2.22 defaulted to --no-copy-dt-needed-entries.
gold and LLD always behave like --no-copy-dt-needed-entries.
rL221530 added -Wl,-allow-shlib-undefined to make some old releases of GNU ld's
--no-copy-dt-needed-entries to actually work.

Due to [a] and [b], this patch drops -Wl,-allow-shlib-undefined.

[b]: In a -DBUILD_SHARED_LIBS=on build, --as-needed --allow-shlib-undefined
can unexpectedly suppress some .dynsym entries. The issue can cause
mlir-cpu-runner to fail at runtime. Note, on Debian, gcc newer than (gcc-9-20190125-2) enable
--as-needed by default.
See https://sourceware.org/bugzilla/show_bug.cgi?id=26551 for a reduced example.

Diff Detail

Event Timeline

MaskRay created this revision.Aug 29 2020, 12:20 PM
MaskRay requested review of this revision.Aug 29 2020, 12:20 PM
MaskRay edited the summary of this revision. (Show Details)Aug 29 2020, 12:23 PM
mehdi_amini accepted this revision.Aug 29 2020, 1:36 PM

Thanks!

I rather get approval from someone else on this as well though.

It can also be mentioned that Debian (and Ubuntu as a consequence) changed their default for adding automatically --as-needed a few years ago. I don't think it was the case at the time where we added allow-shlib-undefined to LLVM builds. Right now it means that the behaviors depends on the Linux distribution (non-Debian-based would not hit this bug), and if you're using clang or gcc as a driver to invoke the linker... (clang on Debian does not add --as-needed).

This revision is now accepted and ready to land.Aug 29 2020, 1:36 PM
MaskRay edited the summary of this revision. (Show Details)Aug 29 2020, 11:48 PM

@rriddle Does this patch look good to you? :)

Thanks for the patch! I can confirm that it is fixing our build locally (using gcc 7.5.0).

echristo accepted this revision.Sep 1 2020, 7:47 PM
echristo added a subscriber: echristo.

I think this is the right thing to do here. In addition, we should not cherry pick this to any release branch so we have the opportunity to get it more fully evaluated by distros/vendors/etc over the next months.

-eric

Thanks for the patch! I can confirm that it is fixing our build locally (using gcc 7.5.0).

Thanks for testing, and

I think this is the right thing to do here. In addition, we should not cherry pick this to any release branch so we have the opportunity to get it more fully evaluated by distros/vendors/etc over the next months.

thanks for the additional approval power!

(I am pretty confident that nothing will go off. We use -z defs as well, which is a strictly stronger guarantee than --allow-shlib-undefined.)

uabelho added a subscriber: uabelho.Sep 2 2020, 5:36 AM

Hi!

We ran into problems with this commit.

On a RHEL 7 machine, compiling with gcc 7.4 and using libc 2.12, we now get the following:

12:14:08 FAILED: bin/llvm-tblgen
12:14:08 : && /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/bin/g++  -I/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/include  -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -O3  -L/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64 -Wl,-R/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64 -L/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/lib -Wl,-R/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/lib    -Wl,-rpath-link,/repo/bbiswjenk/fem023-eiffel003/workspace/llvm/llvm-master-gcc/llvm/build-all-bbigcc-builtins/./lib  -Wl,-O3 -Wl,--gc-sections utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterInst.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Attributes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CallingConvEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeEmitterGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenDAGPatterns.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenHwModes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenInstruction.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenMapTable.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenSchedule.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenTarget.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherOpt.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcher.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAPacketizerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DirectiveEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/ExegesisEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FastISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FixedLenDecoderEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/GICombinerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InfoByHwMode.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrDocsEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptParserEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptRSTEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PredicateExpander.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PseudoLoweringEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RISCVCompressInstEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterBankEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SDNodeProperties.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SearchableTableEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetFeatureInfo.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/TableGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Types.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86DisassemblerTables.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86EVEX2VEXTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86FoldTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86ModRMFilters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86RecognizableInstr.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/WebAssemblyDisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CTagsEmitter.cpp.o  -o bin/llvm-tblgen  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  lib/libLLVMTableGen.a  -lpthread  lib/libLLVMTableGenGlobalISel.a  lib/libLLVMTableGen.a  lib/libLLVMSupport.a  -lrt  -ldl  -lpthread  -lm  /usr/lib64/libtinfo.so  lib/libLLVMDemangle.a && :
12:14:08 /usr/lib64/libtinfo.so: undefined reference to `__fdelt_chk@GLIBC_2.15'
12:14:08 /usr/lib64/libtinfo.so: undefined reference to `memcpy@GLIBC_2.14'
12:14:08 collect2: error: ld returned 1 exit status

Anything obvious we should do to make this work again?

bjope added a subscriber: bjope.Sep 2 2020, 6:38 AM
dstenb added a subscriber: dstenb.Sep 2 2020, 9:26 AM
MaskRay added a comment.EditedSep 2 2020, 9:50 AM

Hi!

We ran into problems with this commit.

On a RHEL 7 machine, compiling with gcc 7.4 and using libc 2.12, we now get the following:

12:14:08 FAILED: bin/llvm-tblgen
12:14:08 : && /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/bin/g++  -I/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/include  -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -O3  -L/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64 -Wl,-R/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64 -L/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/lib -Wl,-R/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/lib    -Wl,-rpath-link,/repo/bbiswjenk/fem023-eiffel003/workspace/llvm/llvm-master-gcc/llvm/build-all-bbigcc-builtins/./lib  -Wl,-O3 -Wl,--gc-sections utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterInst.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Attributes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CallingConvEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeEmitterGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenDAGPatterns.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenHwModes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenInstruction.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenMapTable.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenSchedule.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenTarget.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherOpt.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcher.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAPacketizerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DirectiveEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/ExegesisEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FastISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FixedLenDecoderEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/GICombinerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InfoByHwMode.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrDocsEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptParserEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptRSTEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PredicateExpander.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PseudoLoweringEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RISCVCompressInstEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterBankEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SDNodeProperties.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SearchableTableEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetFeatureInfo.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/TableGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Types.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86DisassemblerTables.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86EVEX2VEXTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86FoldTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86ModRMFilters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86RecognizableInstr.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/WebAssemblyDisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CTagsEmitter.cpp.o  -o bin/llvm-tblgen  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  lib/libLLVMTableGen.a  -lpthread  lib/libLLVMTableGenGlobalISel.a  lib/libLLVMTableGen.a  lib/libLLVMSupport.a  -lrt  -ldl  -lpthread  -lm  /usr/lib64/libtinfo.so  lib/libLLVMDemangle.a && :
12:14:08 /usr/lib64/libtinfo.so: undefined reference to `__fdelt_chk@GLIBC_2.15'
12:14:08 /usr/lib64/libtinfo.so: undefined reference to `memcpy@GLIBC_2.14'
12:14:08 collect2: error: ld returned 1 exit status

Anything obvious we should do to make this work again?

glibc version? I think the system might be mis-configured: /usr/lib64/libtinfo.so does not match libc.so.6, as evidenced by:

12:14:08 /usr/lib64/libtinfo.so: undefined reference to `__fdelt_chk@GLIBC_2.15'
12:14:08 /usr/lib64/libtinfo.so: undefined reference to `memcpy@GLIBC_2.14'
12:14:08 collect2: error: ld returned 1 exit status

You can create an empty file and run /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/bin/gcc empty.c /usr/lib64/libtinfo.so => it should fail with the same reason.

The glibc version may match the custom gcc but does not match your libtinfo.so. You probably need to install a ncurses matching the glibc version.

bjope added a comment.Sep 2 2020, 10:13 AM

So are we perhaps missing tinfo (and ncurses) in /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64?
I doubt that we should pick up libtinfo from /usr/lib64 here, even if we do not really need to crosscompile llvm-tblgen (tblgen should only execute on the current host anyway).

(BTW, thing do work fine when building with clang and for the current host, so it is something with the crosscompile setup... a bit interesting that it has worked in the past though...)

bjope added a comment.Sep 2 2020, 10:25 AM

Things seem to link again for us if we set LLVM_ENABLE_TERMINFO=OFF (as a workaround to avoid dependency to libtinfo), but I guess we might lose some color output or something by doing that.

So are we perhaps missing tinfo (and ncurses) in /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.7.4.0-3/crosscompiler/lib64?

Yes

I doubt that we should pick up libtinfo from /usr/lib64 here, even if we do not really need to crosscompile llvm-tblgen (tblgen should only execute on the current host anyway).

(BTW, thing do work fine when building with clang and for the current host, so it is something with the crosscompile setup... a bit interesting that it has worked in the past though...)

It worked likely because libtinfo.so does not set DF_BIND_NOW or DF_1_NOW. LD_BIND_NOW=1 linked-exe would likely fail.

Things seem to link again for us if we set LLVM_ENABLE_TERMINFO=OFF (as a workaround to avoid dependency to libtinfo), but I guess we might lose some color output or something by doing that.

LLVMSupport uses terminfo to check whether the terminal supports colors. Yes, -DLLVM_ENABLE_TERMINFO=OFF if terminfo is not available.