[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