[PATCH] D42475: [ELF] Add warnings for various symbols that cannot be ordered
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 6 08:59:24 PST 2018
I think this is OK.
It probably just need to be rebased to use the new option metaclass to
handle --no-foo/--foo.
With support for computing a ordering given a call graph also in review,
we would probably want the corresponding warnings there too.
Since this patch is simpler it should probably land first.
Cheers,
Rafael
James Henderson via Phabricator <reviews at reviews.llvm.org> writes:
> jhenderson updated this revision to Diff 132584.
> jhenderson marked 3 inline comments as done.
> jhenderson added a comment.
>
> Move warning of duplicate entries to earlier, and renamed a few fields and variables.
>
>
> Repository:
> rLLD LLVM Linker
>
> https://reviews.llvm.org/D42475
>
> Files:
> ELF/Config.h
> ELF/Driver.cpp
> ELF/Options.td
> ELF/Writer.cpp
> test/ELF/Inputs/symbol-ordering-file-warnings1.s
> test/ELF/Inputs/symbol-ordering-file-warnings2.s
> test/ELF/symbol-ordering-file-warnings.s
>
> Index: test/ELF/symbol-ordering-file-warnings.s
> ===================================================================
> --- test/ELF/symbol-ordering-file-warnings.s
> +++ test/ELF/symbol-ordering-file-warnings.s
> @@ -0,0 +1,140 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symbol-ordering-file-warnings1.s -o %t1.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symbol-ordering-file-warnings2.s -o %t2.o
> +# RUN: ld.lld -shared %t1.o -o %t.so
> +
> +# Check that a warning is emitted for entries in the file that are not present in any used input.
> +# RUN: echo "missing" > %t-order-missing.txt
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-missing.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,MISSING
> +
> +# Check that the warning can be disabled.
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-missing.txt --unresolved-symbols=ignore-all --no-warn-symbol-ordering 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN --allow-empty
> +
> +# Check that the warning can be re-enabled
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-missing.txt --unresolved-symbols=ignore-all --no-warn-symbol-ordering --warn-symbol-ordering 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,MISSING
> +
> +# Check that a warning is emitted for undefined symbols.
> +# RUN: echo "undefined" > %t-order-undef.txt
> +# RUN: ld.lld %t.o %t2.o -o %t --symbol-ordering-file %t-order-undef.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,UNDEFINED
> +
> +# Check that a warning is emitted for imported shared symbols.
> +# RUN: echo "shared" > %t-order-shared.txt
> +# RUN: ld.lld %t.o %t.so -o %t --symbol-ordering-file %t-order-shared.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,SHARED
> +
> +# Check that a warning is emitted for absolute symbols.
> +# RUN: echo "absolute" > %t-order-absolute.txt
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-absolute.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,ABSOLUTE
> +
> +# Check that a warning is emitted for symbols discarded due to --gc-sections.
> +# RUN: echo "gc" > %t-order-gc.txt
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-gc.txt --gc-sections --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,GC
> +
> +# Check that a warning is emitted for symbols discarded due to a linker script /DISCARD/ section.
> +# RUN: echo "discard" > %t-order-discard.txt
> +# RUN: echo "SECTIONS { /DISCARD/ : { *(.text.discard) } }" > %t.script
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-discard.txt -T %t.script --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,DISCARD
> +
> +# Check that LLD does not warn for discarded COMDAT symbols, if at least instance was kept.
> +# RUN: echo "comdat" > %t-order-comdat.txt
> +# RUN: ld.lld %t.o %t1.o -o %t --symbol-ordering-file %t-order-comdat.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN --allow-empty
> +
> +# Check that if a COMDAT was unused and discarded via --gc-sections, warn for each instance.
> +# RUN: ld.lld %t.o %t1.o -o %t --symbol-ordering-file %t-order-comdat.txt --gc-sections --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,COMDAT
> +
> +# Check that if a weak symbol is not kept, because of an equivalent global symbol, no warning is emitted.
> +# RUN: echo "glob_or_wk" > %t-order-weak.txt
> +# RUN: ld.lld %t.o %t1.o -o %t --symbol-ordering-file %t-order-weak.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN --allow-empty
> +
> +# Check that symbols only in unused archive members result in a warning.
> +# RUN: rm -f %t.a
> +# RUN: llvm-ar rc %t.a %t2.o
> +# RUN: ld.lld %t.o %t.a -o %t --symbol-ordering-file %t-order-missing.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,MISSING --allow-empty
> +
> +# Check that a warning for each same-named symbol with an issue.
> +# RUN: echo "multi" > %t-order-same-name.txt
> +# RUN: ld.lld %t.o %t1.o %t2.o -o %t --symbol-ordering-file %t-order-same-name.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,MULTI
> +
> +# Check that a warning is emitted if the same symbol is mentioned multiple times in the ordering file.
> +# RUN: echo "_start" > %t-order-multiple-same.txt
> +# RUN: echo "_start" >> %t-order-multiple-same.txt
> +# RUN: ld.lld %t.o -o %t --symbol-ordering-file %t-order-multiple-same.txt --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,SAMESYM
> +
> +# Check that all warnings can be emitted from the same input.
> +# RUN: echo "missing_sym" > %t-order-multi.txt
> +# RUN: echo "undefined" >> %t-order-multi.txt
> +# RUN: echo "_start" >> %t-order-multi.txt
> +# RUN: echo "shared" >> %t-order-multi.txt
> +# RUN: echo "absolute" >> %t-order-multi.txt
> +# RUN: echo "gc" >> %t-order-multi.txt
> +# RUN: echo "discard" >> %t-order-multi.txt
> +# RUN: echo "_start" >> %t-order-multi.txt
> +# RUN: ld.lld %t.o %t2.o %t.so -o %t --symbol-ordering-file %t-order-multi.txt --gc-sections -T %t.script --unresolved-symbols=ignore-all 2>&1 | \
> +# RUN: FileCheck %s --check-prefixes=WARN,SAMESYM,ABSOLUTE,SHARED,UNDEFINED,GC,DISCARD,MISSING2
> +
> +# WARN-NOT: warning:
> +# SAMESYM: warning: symbol ordering file: symbol '_start' specified multiple times
> +# WARN-NOT: warning:
> +# ABSOLUTE: warning: symbol ordering file: unable to order absolute symbol 'absolute' from file
> +# WARN-NOT: warning:
> +# DISCARD: warning: symbol ordering file: unable to order discarded symbol 'discard' from file
> +# WARN-NOT: warning:
> +# GC: warning: symbol ordering file: unable to order discarded symbol 'gc' from file
> +# WARN-NOT: warning:
> +# SHARED: warning: symbol ordering file: unable to order shared symbol 'shared' from file
> +# WARN-NOT: warning:
> +# UNDEFINED: warning: symbol ordering file: unable to order undefined symbol 'undefined' from file
> +# WARN-NOT: warning:
> +# MISSING: warning: symbol ordering file: no symbol 'missing' found in input
> +# MISSING2: warning: symbol ordering file: no symbol 'missing_sym' found in input
> +# COMDAT: warning: symbol ordering file: unable to order discarded symbol 'comdat' from file
> +# COMDAT-NEXT: warning: symbol ordering file: unable to order discarded symbol 'comdat' from file
> +# MULTI: warning: symbol ordering file: unable to order absolute symbol 'multi' from file
> +# MULTI-NEXT: warning: symbol ordering file: unable to order undefined symbol 'multi' from file
> +# WARN-NOT: warning:
> +
> +absolute = 0x1234
> +
> +.section .text.gc,"ax", at progbits
> +.global gc
> +gc:
> + nop
> +
> +.section .text.discard,"ax", at progbits
> +.global discard
> +discard:
> + nop
> +
> +.section .text.comdat,"axG", at progbits,comdat,comdat
> +.weak comdat
> +comdat:
> + nop
> +
> +.section .text.glob_or_wk,"ax", at progbits
> +.weak glob_or_wk
> +glob_or_wk:
> + nop
> +
> +.text
> +.global _start
> +_start:
> + movq %rax, absolute
> + callq shared
> +
> +# This is a "good" instance of the symbol
> +multi:
> + nop
> Index: test/ELF/Inputs/symbol-ordering-file-warnings2.s
> ===================================================================
> --- test/ELF/Inputs/symbol-ordering-file-warnings2.s
> +++ test/ELF/Inputs/symbol-ordering-file-warnings2.s
> @@ -0,0 +1,6 @@
> +.text
> +.global missing
> +missing:
> + callq undefined
> + # This is a "bad" (undefined) instance of the symbol
> + callq multi
> Index: test/ELF/Inputs/symbol-ordering-file-warnings1.s
> ===================================================================
> --- test/ELF/Inputs/symbol-ordering-file-warnings1.s
> +++ test/ELF/Inputs/symbol-ordering-file-warnings1.s
> @@ -0,0 +1,19 @@
> +# This is a "bad" (absolute) instance of the symbol
> +multi = 1234
> +
> +.text
> +.global shared
> +.type shared, @function
> +shared:
> + movq %rax, multi
> + ret
> +
> +.section .text.comdat,"axG", at progbits,comdat,comdat
> +.weak comdat
> +comdat:
> + ret
> +
> +.section .text.glob_or_wk,"ax", at progbits
> +.global glob_or_wk
> +glob_or_wk:
> + ret
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1026,24 +1026,45 @@
> if (Config->SymbolOrderingFile.empty())
> return SectionOrder;
>
> - // Build a map from symbols to their priorities. Symbols that didn't
> - // appear in the symbol ordering file have the lowest priority 0.
> - // All explicitly mentioned symbols have negative (higher) priorities.
> - DenseMap<StringRef, int> SymbolOrder;
> - int Priority = -Config->SymbolOrderingFile.size();
> - for (StringRef S : Config->SymbolOrderingFile)
> - SymbolOrder.insert({S, Priority++});
> -
> // Build a map from sections to their priorities.
> for (InputFile *File : ObjectFiles) {
> for (Symbol *Sym : File->getSymbols()) {
> auto *D = dyn_cast<Defined>(Sym);
> + auto OrderEntry = Config->SymbolOrderingFile.find(Sym->getName());
> + if (OrderEntry == Config->SymbolOrderingFile.end())
> + continue;
> + SymbolOrderEntry &EntryValue = OrderEntry->second;
> + EntryValue.Present = true;
> +
> + if (Config->WarnSymbolOrdering) {
> + if (Sym->isUndefined())
> + warn("symbol ordering file: unable to order undefined symbol '" +
> + Sym->getName() + "' from file '" + File->getName() + "'");
> + else if (Sym->isShared())
> + warn("symbol ordering file: unable to order shared symbol '" +
> + Sym->getName() + "' from file '" + File->getName() + "'");
> + else if (D && !D->Section)
> + warn("symbol ordering file: unable to order absolute symbol '" +
> + Sym->getName() + "' from file '" + File->getName() + "'");
> + else if (D && !D->Section->Live)
> + warn("symbol ordering file: unable to order discarded symbol '" +
> + Sym->getName() + "' from file '" + File->getName() + "'");
> + }
> if (!D || !D->Section)
> continue;
> +
> int &Priority = SectionOrder[D->Section];
> - Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
> + Priority = std::min(Priority, EntryValue.Priority);
> }
> }
> +
> + if (Config->WarnSymbolOrdering)
> + for (auto OrderEntry : Config->SymbolOrderingFile) {
> + if (!OrderEntry.second.Present)
> + warn("symbol ordering file: no symbol '" + OrderEntry.first +
> + "' found in input");
> + }
> +
> return SectionOrder;
> }
>
> Index: ELF/Options.td
> ===================================================================
> --- ELF/Options.td
> +++ ELF/Options.td
> @@ -210,6 +210,9 @@
> def no_threads: F<"no-threads">,
> HelpText<"Do not run the linker multi-threaded">;
>
> +def no_warn_symbol_ordering: F<"no-warn-symbol-ordering">,
> + HelpText<"Do not warn about problems with the symbol ordering file">;
> +
> def no_whole_archive: F<"no-whole-archive">,
> HelpText<"Restores the default behavior of loading archive members">;
>
> @@ -329,6 +332,9 @@
> def warn_common: F<"warn-common">,
> HelpText<"Warn about duplicate common symbols">;
>
> +def warn_symbol_ordering: F<"warn-symbol-ordering">,
> + HelpText<"Warn about problems with the symbol ordering file">;
> +
> def warn_unresolved_symbols: F<"warn-unresolved-symbols">,
> HelpText<"Report unresolved symbols as warnings">;
>
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -593,6 +593,27 @@
> return V;
> }
>
> +// Parse the symbol ordering file to determine priorities of different symbol
> +// names. Symbols that didn't appear in the symbol ordering file have the lowest
> +// priority 0. All explicitly mentioned symbols have negative (higher)
> +// priorities. Warnings are emitted for any symbols that appear more than once.
> +static MapVector<StringRef, SymbolOrderEntry>
> +getSymbolOrderingFile(StringRef Filename) {
> + Optional<MemoryBufferRef> MB = readFile(Filename);
> + MapVector<StringRef, SymbolOrderEntry> Ret;
> + if (!MB)
> + return Ret;
> +
> + int Priority = std::numeric_limits<int>::min();
> + for (StringRef S : args::getLines(*MB)) {
> + bool Inserted = Ret.insert({S, {Priority++, false}}).second;
> + if (Config->WarnSymbolOrdering && !Inserted)
> + warn("symbol ordering file: symbol '" + S + "' specified multiple times");
> + }
> +
> + return Ret;
> +}
> +
> // Initializes Config members by the command line options.
> void LinkerDriver::readConfigs(opt::InputArgList &Args) {
> Config->AllowMultipleDefinition =
> @@ -676,6 +697,8 @@
> Config->Verbose = Args.hasArg(OPT_verbose);
> errorHandler().Verbose = Config->Verbose;
> Config->WarnCommon = Args.hasArg(OPT_warn_common);
> + Config->WarnSymbolOrdering =
> + Args.hasFlag(OPT_warn_symbol_ordering, OPT_no_warn_symbol_ordering, true);
> Config->ZCombreloc = !hasZOption(Args, "nocombreloc");
> Config->ZExecstack = hasZOption(Args, "execstack");
> Config->ZNocopyreloc = hasZOption(Args, "nocopyreloc");
> @@ -760,8 +783,7 @@
> }
>
> if (auto *Arg = Args.getLastArg(OPT_symbol_ordering_file))
> - if (Optional<MemoryBufferRef> Buffer = readFile(Arg->getValue()))
> - Config->SymbolOrderingFile = args::getLines(*Buffer);
> + Config->SymbolOrderingFile = getSymbolOrderingFile(Arg->getValue());
>
> // If --retain-symbol-file is used, we'll keep only the symbols listed in
> // the file and discard all others.
> Index: ELF/Config.h
> ===================================================================
> --- ELF/Config.h
> +++ ELF/Config.h
> @@ -70,13 +70,20 @@
> size_t NameOff = 0; // Offset in the string table
> };
>
> +// This struct contains information about entries in the symbol order file.
> +struct SymbolOrderEntry {
> + int Priority;
> + bool Present;
> +};
> +
> // This struct contains the global configuration for the linker.
> // Most fields are direct mapping from the command line options
> // and such fields have the same name as the corresponding options.
> // Most fields are initialized by the driver.
> struct Configuration {
> uint8_t OSABI = 0;
> llvm::CachePruningPolicy ThinLTOCachePolicy;
> + llvm::MapVector<StringRef, SymbolOrderEntry> SymbolOrderingFile;
> llvm::StringMap<uint64_t> SectionStartMap;
> llvm::StringRef Chroot;
> llvm::StringRef DynamicLinker;
> @@ -98,7 +105,6 @@
> std::vector<llvm::StringRef> AuxiliaryList;
> std::vector<llvm::StringRef> FilterList;
> std::vector<llvm::StringRef> SearchPaths;
> - std::vector<llvm::StringRef> SymbolOrderingFile;
> std::vector<llvm::StringRef> Undefined;
> std::vector<SymbolVersion> DynamicList;
> std::vector<SymbolVersion> VersionScriptGlobals;
> @@ -152,6 +158,7 @@
> bool Verbose;
> bool WarnCommon;
> bool WarnMissingEntry;
> + bool WarnSymbolOrdering;
> bool ZCombreloc;
> bool ZExecstack;
> bool ZNocopyreloc;
More information about the llvm-commits
mailing list