Page MenuHomePhabricator

[ELF] Add --dependency-file option
ClosedPublic

Authored by phosek on Jun 24 2020, 12:31 AM.

Details

Summary

Clang and GCC have a feature (-MD flag) to create a dependency file
in a format that build systems such as Make or Ninja can read, which
specifies all the additional inputs such .h files.

This change introduces the same functionality to lld bringing it to
feature parity with ld and gold which gained this feature recently.
See https://sourceware.org/bugzilla/show_bug.cgi?id=22843 for more
details and discussion.

The implementation corresponds to -MD -MP compiler flag where the
generated dependency file also includes phony targets which works
around the errors where the dependency is removed. This matches the
format used by ld and gold.

Fixes PR42806

Diff Detail

Event Timeline

phosek created this revision.Jun 24 2020, 12:31 AM

This is an updated version of D65430 which matches the feature implemented by ld and gold in https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=f37b21b481a7804a13c5806098c19b6119288ba4. I'm happy to abandon this change in favor of D65430 if @ruiu wants to go ahead with his change.

phosek updated this revision to Diff 272925.Jun 24 2020, 12:35 AM

This is an updated version of D65430 which matches the feature implemented by ld and gold in https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=f37b21b481a7804a13c5806098c19b6119288ba4. I'm happy to abandon this change in favor of D65430 if @ruiu wants to go ahead with his change.

You can just paste https://sourceware.org/bugzilla/show_bug.cgi?id=22843 beside "ld and gold which gained this feature recently" in the description.

For the tests, you can read reproduce* and make a summary.

lld/ELF/Options.td
135

EEq

We don't want to support -dependency-file

phosek updated this revision to Diff 273241.Jun 25 2020, 12:44 AM
phosek marked an inline comment as done.
phosek edited the summary of this revision. (Show Details)
MaskRay added inline comments.Jun 25 2020, 9:42 AM
lld/ELF/Driver.cpp
1614

The backslash escaping rule is complex. Do you have a description how it works? We should have a test as well. If it can cause problems on Windows, add UNSUPPORTED: system-windows

phosek added inline comments.Thu, Jul 16, 7:40 PM
lld/ELF/Driver.cpp
1614

See the comment above this lambda:

We use the same escape rules as Clang/GCC which are accepted by Make/Ninja:
* A space is escaped by a backslash which itself must be escaped.
* A hash sign is escaped by a single backslash.
* $ is escapes as $$.

I can drop this if we decide that we don't want to support escaping, but I wanted to match what the existing consumers expect.

Do you have any other comments? I'd like to see this landed.

MaskRay added inline comments.Fri, Jul 24, 4:58 PM
lld/test/ELF/dependency-file.s
8

Backslashes and # are not tested. If they can be problems on Windows, just add UNSUPPORTED: system-windows

9

Can you use FileCheck -DFILE=%t so that the CHECK lines can test the full paths?

phosek updated this revision to Diff 281297.Tue, Jul 28, 11:02 AM
phosek marked 2 inline comments as done.

Ping

Updated, can you take a look again?

MaskRay accepted this revision.Wed, Jul 29, 2:07 PM

Thanks!

lld/ELF/Driver.cpp
1584

I hope @mcgrathr can spend a bit more time inspecting the wording here as he is the author of the GNU ld and gold patch.

lld/test/ELF/dependency-file.s
4

x86_64-unknown-linux -> x86_64

This revision is now accepted and ready to land.Wed, Jul 29, 2:07 PM
phosek updated this revision to Diff 282015.Thu, Jul 30, 12:30 PM
phosek marked 2 inline comments as done.
phosek added inline comments.
lld/ELF/Driver.cpp
1584

I talked to @mcgrathr offline and he said it looks fine.

This revision was automatically updated to reflect the committed changes.
phosek reopened this revision.EditedSun, Aug 2, 2:02 PM

I'm still debugging the failure which has showed up on some bots:

/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/clang++  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3  -fuse-ld=lld -Wl,--color-diagnostics -Wl,-allow-shlib-undefined    -Wl,-O3 -Wl,--gc-sections unittests/Target/X86/CMakeFiles/X86Tests.dir/MachineSizeOptsTest.cpp.o  -o unittests/Target/X86/X86Tests  lib/libLLVMAnalysis.a  lib/libLLVMCodeGen.a  lib/libLLVMCore.a  lib/libLLVMMC.a  lib/libLLVMMIRParser.a  lib/libLLVMSupport.a  lib/libLLVMTarget.a  lib/libLLVMX86CodeGen.a  lib/libLLVMX86Desc.a  lib/libLLVMX86Info.a  lib/libLLVMSupport.a  -lpthread  lib/libgtest_main.a  lib/libgtest.a  -lpthread  lib/libLLVMAsmParser.a  lib/libLLVMMCDisassembler.a  lib/libLLVMAsmPrinter.a  lib/libLLVMDebugInfoDWARF.a  lib/libLLVMCFGuard.a  lib/libLLVMGlobalISel.a  lib/libLLVMSelectionDAG.a  lib/libLLVMCodeGen.a  lib/libLLVMBitWriter.a  lib/libLLVMScalarOpts.a  lib/libLLVMAggressiveInstCombine.a  lib/libLLVMInstCombine.a  lib/libLLVMTarget.a  lib/libLLVMTransformUtils.a  lib/libLLVMAnalysis.a  lib/libLLVMObject.a  lib/libLLVMBitReader.a  lib/libLLVMMCParser.a  lib/libLLVMMC.a  lib/libLLVMDebugInfoCodeView.a  lib/libLLVMDebugInfoMSF.a  lib/libLLVMTextAPI.a  lib/libLLVMProfileData.a  lib/libLLVMCore.a  lib/libLLVMBinaryFormat.a  lib/libLLVMRemarks.a  lib/libLLVMBitstreamReader.a  lib/libLLVMSupport.a  -lrt  -ldl  -lpthread  -lm  /usr/lib64/libz.so  lib/libLLVMDemangle.a  -lpthread && :
ld.lld: /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/include/llvm/ADT/DenseMap.h:408: void llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::moveFromOldBuckets(BucketT*, BucketT*) [with DerivedT = llvm::DenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::StringRef>, llvm::detail::DenseSetPair<llvm::StringRef> >; KeyT = llvm::StringRef; ValueT = llvm::detail::DenseSetEmpty; KeyInfoT = llvm::DenseMapInfo<llvm::StringRef>; BucketT = llvm::detail::DenseSetPair<llvm::StringRef>]: Assertion `!FoundVal && "Key already in new map?"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld --eh-frame-hdr -m elf64lppc -dynamic-linker /lib64/ld64.so.2 -o unittests/Target/X86/X86Tests /usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crt1.o /usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crti.o /usr/lib/gcc/ppc64le-redhat-linux/8/crtbegin.o -L/usr/lib/gcc/ppc64le-redhat-linux/8 -L/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/ppc64le-redhat-linux/8/../../.. -L/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/../lib -L/lib -L/usr/lib --color-diagnostics -allow-shlib-undefined -O3 --gc-sections unittests/Target/X86/CMakeFiles/X86Tests.dir/MachineSizeOptsTest.cpp.o lib/libLLVMAnalysis.a lib/libLLVMCodeGen.a lib/libLLVMCore.a lib/libLLVMMC.a lib/libLLVMMIRParser.a lib/libLLVMSupport.a lib/libLLVMTarget.a lib/libLLVMX86CodeGen.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMSupport.a -lpthread lib/libgtest_main.a lib/libgtest.a -lpthread lib/libLLVMAsmParser.a lib/libLLVMMCDisassembler.a lib/libLLVMAsmPrinter.a lib/libLLVMDebugInfoDWARF.a lib/libLLVMCFGuard.a lib/libLLVMGlobalISel.a lib/libLLVMSelectionDAG.a lib/libLLVMCodeGen.a lib/libLLVMBitWriter.a lib/libLLVMScalarOpts.a lib/libLLVMAggressiveInstCombine.a lib/libLLVMInstCombine.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMAnalysis.a lib/libLLVMObject.a lib/libLLVMBitReader.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMTextAPI.a lib/libLLVMProfileData.a lib/libLLVMCore.a lib/libLLVMBinaryFormat.a lib/libLLVMRemarks.a lib/libLLVMBitstreamReader.a lib/libLLVMSupport.a -lrt -ldl -lpthread -lm /usr/lib64/libz.so lib/libLLVMDemangle.a -lpthread -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /usr/lib/gcc/ppc64le-redhat-linux/8/crtend.o /usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crtn.o 
 #0 0x000000001042e3b8 llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1042e3b8)
 #1 0x000000001042e4e0 PrintStackTraceSignalHandler(void*) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1042e4e0)
 #2 0x000000001042bbd4 llvm::sys::RunSignalHandlers() (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1042bbd4)
 #3 0x000000001042bdac SignalHandler(int) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1042bdac)
 #4 0x00007fff9ff004d8 (linux-vdso64.so.1+0x4d8)
 #5 0x00007fff9f874308 raise (/lib64/libc.so.6+0x44308)
 #6 0x00007fff9f85438c abort (/lib64/libc.so.6+0x2438c)
 #7 0x00007fff9f8678d0 __assert_fail_base (/lib64/libc.so.6+0x378d0)
 #8 0x00007fff9f867974 __assert_fail (/lib64/libc.so.6+0x37974)
 #9 0x000000001048dc8c llvm::DenseMap<llvm::StringRef, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::StringRef>, llvm::detail::DenseSetPair<llvm::StringRef> >::grow(unsigned int) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1048dc8c)
#10 0x00000000105c360c lld::elf::readFile(llvm::StringRef) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x105c360c)
#11 0x000000001054526c lld::elf::LinkerDriver::addFile(llvm::StringRef, bool) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x1054526c)
#12 0x0000000010545858 lld::elf::LinkerDriver::addLibrary(llvm::StringRef) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x10545858)
#13 0x0000000010545dfc lld::elf::LinkerDriver::createFiles(llvm::opt::InputArgList&) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x10545dfc)
#14 0x0000000010309bc0 lld::elf::LinkerDriver::main(llvm::ArrayRef<char const*>) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x10309bc0)
#15 0x00000000105666a4 lld::elf::link(llvm::ArrayRef<char const*>, bool, llvm::raw_ostream&, llvm::raw_ostream&) (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x105666a4)
#16 0x0000000010306d0c main (/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/ld.lld+0x10306d0c)
#17 0x00007fff9f8549f8 generic_start_main.isra.0 (/lib64/libc.so.6+0x249f8)
#18 0x00007fff9f854be4 __libc_start_main (/lib64/libc.so.6+0x24be4)
clang-12: error: unable to execute command: Aborted (core dumped)
This revision is now accepted and ready to land.Sun, Aug 2, 2:02 PM

CC @nemanjai who can test this patch on ppc64be.

phosek added a comment.Sun, Aug 2, 3:12 PM

CC @nemanjai who can test this patch on ppc64be.

I have also managed to reproduce the failure locally on x86_64 with assertions enabled. I don't understand the failure though. It seems like the insertion into the SetVector is failing because the element is already present (which shouldn't cause a failure), and if I insert a presence check before calling insert, it returns false.

phosek updated this revision to Diff 282743.Mon, Aug 3, 3:26 PM

Switching from llvm::SetVector<StringRef> to llvm::SetVector<llvm::CachedHashString> addresses the issue.

MaskRay accepted this revision.Mon, Aug 3, 4:01 PM

LGTM.

lld/ELF/Options.td
136

I wonder whether <file> will work better than <path> because file conveys more information: a path can be either a file or a directory (if we exclude other possibilities).

phosek updated this revision to Diff 282754.Mon, Aug 3, 4:07 PM
phosek marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.