From 1d92aa73803ae51aa92f02b291d4915010ebf0f4 Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Mon, 9 Apr 2018 23:05:48 +0000 Subject: [PATCH] Add --warn-backrefs to maintain compatibility with other linkers I'm proposing a new command line flag, --warn-backrefs in this patch. The flag and the feature proposed below don't exist in GNU linkers nor the current lld. --warn-backrefs is an option to detect reverse or cyclic dependencies between static archives, and it can be used to keep your program compatible with GNU linkers after you switch to lld. I'll explain the feature and why you may find it useful below. lld's symbol resolution semantics is more relaxed than traditional Unix linkers. Therefore, ld.lld foo.a bar.o succeeds even if bar.o contains an undefined symbol that have to be resolved by some object file in foo.a. Traditional Unix linkers don't allow this kind of backward reference, as they visit each file only once from left to right in the command line while resolving all undefined symbol at the moment of visiting. In the above case, since there's no undefined symbol when a linker visits foo.a, no files are pulled out from foo.a, and because the linker forgets about foo.a after visiting, it can't resolve undefined symbols that could have been resolved otherwise. That lld accepts more relaxed form means (besides it makes more sense) that you can accidentally write a command line or a build file that works only with lld, even if you have a plan to distribute it to wider users who may be using GNU linkers. With --check-library-dependency, you can detect a library order that doesn't work with other Unix linkers. The option is also useful to detect cyclic dependencies between static archives. Again, lld accepts ld.lld foo.a bar.a even if foo.a and bar.a depend on each other. With --warn-backrefs it is handled as an error. Here is how the option works. We assign a group ID to each file. A file with a smaller group ID can pull out object files from an archive file with an equal or greater group ID. Otherwise, it is a reverse dependency and an error. A file outside --{start,end}-group gets a fresh ID when instantiated. All files within the same --{start,end}-group get the same group ID. E.g. ld.lld A B --start-group C D --end-group E A and B form group 0, C, D and their member object files form group 1, and E forms group 2. I think that you can see how this group assignment rule simulates the traditional linker's semantics. Differential Revision: https://reviews.llvm.org/D45195 llvm-svn: 329636 --- lld/ELF/Config.h | 1 + lld/ELF/Driver.cpp | 12 ++++++++ lld/ELF/InputFiles.cpp | 21 +++++++++++--- lld/ELF/InputFiles.h | 9 ++++++ lld/ELF/Options.td | 20 ++++++++------ lld/ELF/ScriptParser.cpp | 26 +++++++++++------ lld/ELF/SymbolTable.cpp | 66 ++++++++++++++++++++++++++++++++++++++++++-- lld/test/ELF/warn-backrefs.s | 32 +++++++++++++++++++++ 8 files changed, 164 insertions(+), 23 deletions(-) create mode 100644 lld/test/ELF/warn-backrefs.s diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h index 85794d6..81655ef 100644 --- a/lld/ELF/Config.h +++ b/lld/ELF/Config.h @@ -153,6 +153,7 @@ struct Configuration { bool Target1Rel; bool Trace; bool UndefinedVersion; + bool WarnBackrefs; bool WarnCommon; bool WarnMissingEntry; bool WarnSymbolOrdering; diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index d59b124..539ef97 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -704,6 +704,8 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) { Config->UndefinedVersion = Args.hasFlag(OPT_undefined_version, OPT_no_undefined_version, true); Config->UnresolvedSymbols = getUnresolvedSymbolPolicy(Args); + Config->WarnBackrefs = + Args.hasFlag(OPT_warn_backrefs, OPT_no_warn_backrefs, false); Config->WarnCommon = Args.hasFlag(OPT_warn_common, OPT_no_warn_common, false); Config->WarnSymbolOrdering = Args.hasFlag(OPT_warn_symbol_ordering, OPT_no_warn_symbol_ordering, true); @@ -937,6 +939,16 @@ void LinkerDriver::createFiles(opt::InputArgList &Args) { Files.back()->JustSymbols = true; } break; + case OPT_start_group: + if (InputFile::IsInGroup) + error("nested --start-group"); + InputFile::IsInGroup = true; + break; + case OPT_end_group: + if (!InputFile::IsInGroup) + error("stray --end-group"); + InputFile::IsInGroup = false; + break; case OPT_start_lib: InLib = true; break; diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 6a7350c..27d39c6 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -38,6 +38,8 @@ using namespace llvm::sys::fs; using namespace lld; using namespace lld::elf; +bool InputFile::IsInGroup; +uint32_t InputFile::NextGroupId; std::vector elf::BinaryFiles; std::vector elf::BitcodeFiles; std::vector elf::ObjectFiles; @@ -45,7 +47,13 @@ std::vector elf::SharedFiles; TarWriter *elf::Tar; -InputFile::InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {} +InputFile::InputFile(Kind K, MemoryBufferRef M) + : MB(M), GroupId(NextGroupId), FileKind(K) { + // All files within the same --{start,end}-group get the same group ID. + // Otherwise, a new file will get a new group ID. + if (!IsInGroup) + ++NextGroupId; +} Optional elf::readFile(StringRef Path) { // The --chroot option changes our virtual root directory. @@ -760,8 +768,10 @@ InputFile *ArchiveFile::fetch(const Archive::Symbol &Sym) { if (Tar && C.getParent()->isThin()) Tar->append(relativeToRoot(CHECK(C.getFullName(), this)), MB.getBuffer()); - return createObjectFile(MB, getName(), - C.getParent()->isThin() ? 0 : C.getChildOffset()); + InputFile *File = createObjectFile( + MB, getName(), C.getParent()->isThin() ? 0 : C.getChildOffset()); + File->GroupId = GroupId; + return File; } template @@ -1168,7 +1178,10 @@ InputFile *LazyObjFile::fetch() { MemoryBufferRef MBRef = getBuffer(); if (MBRef.getBuffer().empty()) return nullptr; - return createObjectFile(MBRef, ArchiveName, OffsetInArchive); + + InputFile *File = createObjectFile(MBRef, ArchiveName, OffsetInArchive); + File->GroupId = GroupId; + return File; } template void LazyObjFile::parse() { diff --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h index ab12bb1..6b60ba9 100644 --- a/lld/ELF/InputFiles.h +++ b/lld/ELF/InputFiles.h @@ -112,6 +112,13 @@ public: // True if this is an argument for --just-symbols. Usually false. bool JustSymbols = false; + // GroupId is used for --warn-backrefs which is an optional error + // checking feature. All files within the same --{start,end}-group + // get the same group ID. Otherwise, each file gets a new group + // ID. For more info, see checkDependency() in SymbolTable.cpp. + uint32_t GroupId; + static bool IsInGroup; + protected: InputFile(Kind K, MemoryBufferRef M); std::vector Sections; @@ -119,6 +126,8 @@ protected: private: const Kind FileKind; + + static uint32_t NextGroupId; }; template class ELFFileBase : public InputFile { diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td index 49aba63..03ac284 100644 --- a/lld/ELF/Options.td +++ b/lld/ELF/Options.td @@ -113,6 +113,9 @@ def emit_relocs: F<"emit-relocs">, HelpText<"Generate relocations in output">; def enable_new_dtags: F<"enable-new-dtags">, HelpText<"Enable new dynamic tags">; +def end_group: F<"end-group">, + HelpText<"Ignored for compatibility with GNU unless you pass --warn-backrefs">; + def end_lib: F<"end-lib">, HelpText<"End a grouping of objects that should be treated as if they were together in an archive">; @@ -274,6 +277,9 @@ defm soname: Eq<"soname">, HelpText<"Set DT_SONAME">; defm sort_section: Eq<"sort-section">, HelpText<"Specifies sections sorting rule when linkerscript is used">; +def start_group: F<"start-group">, + HelpText<"Ignored for compatibility with GNU unless you pass --warn-backrefs">; + def start_lib: F<"start-lib">, HelpText<"Start a grouping of objects that should be treated as if they were together in an archive">; @@ -323,6 +329,10 @@ def version: F<"version">, HelpText<"Display the version number and exit">; defm version_script: Eq<"version-script">, HelpText<"Read a version script">; +defm warn_backrefs: B<"warn-backrefs", + "Warn about backward symbol references to fetch archive members", + "Do not warn about backward symbol references to fetch archive members">; + defm warn_common: B<"warn-common", "Warn about duplicate common symbols", "Do not warn about duplicate common symbols">; @@ -357,6 +367,7 @@ def alias_define_common_dp: F<"dp">, Alias; def alias_discard_all_x: Flag<["-"], "x">, Alias; def alias_discard_locals_X: Flag<["-"], "X">, Alias; def alias_emit_relocs: Flag<["-"], "q">, Alias; +def alias_end_group_paren: Flag<["-"], ")">, Alias; def alias_entry_e: JoinedOrSeparate<["-"], "e">, Alias; def alias_export_dynamic_E: Flag<["-"], "E">, Alias; def alias_filter: Separate<["-"], "F">, Alias; @@ -374,6 +385,7 @@ def alias_rpath_R: JoinedOrSeparate<["-"], "R">, Alias; def alias_script_T: JoinedOrSeparate<["-"], "T">, Alias