[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 13 17:09:53 PST 2018


LGTM

James Henderson via Phabricator <reviews at reviews.llvm.org> writes:

> jhenderson updated this revision to Diff 133634.
> jhenderson added a comment.
>
> Clang-format
>
>
> 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 %t1.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symbol-ordering-file-warnings1.s -o %t2.o
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symbol-ordering-file-warnings2.s -o %t3.o
> +# RUN: ld.lld -shared %t2.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 %t1.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 %t1.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 %t1.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 %t1.o %t3.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 %t1.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 %t1.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 %t1.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 %t1.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 they are present in the kept instance.
> +# RUN: echo "comdat" > %t-order-comdat.txt
> +# RUN: ld.lld %t1.o %t2.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 %t1.o %t2.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 %t1.o %t2.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 %t3.o
> +# RUN: ld.lld %t1.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 %t1.o %t2.o %t3.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 %t1.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 %t1.o %t3.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: {{.*}}.txt: duplicate ordered symbol: _start
> +# WARN-NOT:    warning:
> +# ABSOLUTE:    warning: {{.*}}1.o: unable to order absolute symbol: absolute
> +# WARN-NOT:    warning:
> +# DISCARD:     warning: {{.*}}1.o: unable to order discarded symbol: discard
> +# WARN-NOT:    warning:
> +# GC:          warning: {{.*}}1.o: unable to order discarded symbol: gc
> +# WARN-NOT:    warning:
> +# SHARED:      warning: {{.*}}1.o: unable to order shared symbol: shared
> +# WARN-NOT:    warning:
> +# UNDEFINED:   warning: {{.*}}3.o: unable to order undefined symbol: undefined
> +# WARN-NOT:    warning:
> +# MISSING:     warning: symbol ordering file: no such symbol: missing
> +# MISSING2:    warning: symbol ordering file: no such symbol: missing_sym
> +# COMDAT:      warning: {{.*}}1.o: unable to order discarded symbol: comdat
> +# COMDAT-NEXT: warning: {{.*}}2.o: unable to order discarded symbol: comdat
> +# MULTI:       warning: {{.*}}2.o: unable to order absolute symbol: multi
> +# MULTI-NEXT:  warning: {{.*}}3.o: unable to order undefined symbol: multi
> +# 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
> @@ -1027,24 +1027,56 @@
>    if (Config->SymbolOrderingFile.empty())
>      return SectionOrder;
>  
> +  struct SymbolOrderEntry {
> +    int Priority;
> +    bool Present;
> +  };
> +
>    // 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;
> +  DenseMap<StringRef, SymbolOrderEntry> SymbolOrder;
>    int Priority = -Config->SymbolOrderingFile.size();
>    for (StringRef S : Config->SymbolOrderingFile)
> -    SymbolOrder.insert({S, Priority++});
> +    SymbolOrder.insert({S, {Priority++, false}});
>  
>    // Build a map from sections to their priorities.
>    for (InputFile *File : ObjectFiles) {
>      for (Symbol *Sym : File->getSymbols()) {
>        auto *D = dyn_cast<Defined>(Sym);
> +      auto It = SymbolOrder.find(Sym->getName());
> +      if (It == SymbolOrder.end())
> +        continue;
> +      SymbolOrderEntry &Ent = It->second;
> +      Ent.Present = true;
> +
> +      if (Config->WarnSymbolOrdering) {
> +        if (Sym->isUndefined())
> +          warn(File->getName() +
> +               ": unable to order undefined symbol: " + Sym->getName());
> +        else if (Sym->isShared())
> +          warn(File->getName() +
> +               ": unable to order shared symbol: " + Sym->getName());
> +        else if (D && !D->Section)
> +          warn(File->getName() +
> +               ": unable to order absolute symbol: " + Sym->getName());
> +        else if (D && !D->Section->Live)
> +          warn(File->getName() +
> +               ": unable to order discarded symbol: " + Sym->getName());
> +      }
>        if (!D || !D->Section)
>          continue;
> +
>        int &Priority = SectionOrder[D->Section];
> -      Priority = std::min(Priority, SymbolOrder.lookup(D->getName()));
> +      Priority = std::min(Priority, Ent.Priority);
>      }
>    }
> +
> +  if (Config->WarnSymbolOrdering)
> +    for (auto OrderEntry : SymbolOrder)
> +      if (!OrderEntry.second.Present)
> +        warn("symbol ordering file: no such symbol: " + OrderEntry.first);
> +
>    return SectionOrder;
>  }
>  
> Index: ELF/Options.td
> ===================================================================
> --- ELF/Options.td
> +++ ELF/Options.td
> @@ -324,6 +324,10 @@
>      "Warn about duplicate common symbols",
>      "Do not warn about duplicate common symbols">;
>  
> +defm warn_symbol_ordering : B<"warn-symbol-ordering",
> +    "Warn about problems with the symbol ordering file",
> +    "Do not 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
> @@ -45,6 +45,7 @@
>  #include "lld/Common/TargetOptionsCommandFlags.h"
>  #include "lld/Common/Threads.h"
>  #include "lld/Common/Version.h"
> +#include "llvm/ADT/SetVector.h"
>  #include "llvm/ADT/StringExtras.h"
>  #include "llvm/ADT/StringSwitch.h"
>  #include "llvm/Support/CommandLine.h"
> @@ -589,6 +590,16 @@
>    return V;
>  }
>  
> +// Parse the symbol ordering file and warn for any duplicate entries.
> +static std::vector<StringRef> getSymbolOrderingFile(MemoryBufferRef MB) {
> +  SetVector<StringRef> Names;
> +  for (StringRef S : args::getLines(MB))
> +    if (!Names.insert(S) && Config->WarnSymbolOrdering)
> +      warn(MB.getBufferIdentifier() + ": duplicate ordered symbol: " + S);
> +
> +  return Names.takeVector();
> +}
> +
>  // Initializes Config members by the command line options.
>  void LinkerDriver::readConfigs(opt::InputArgList &Args) {
>    Config->AllowMultipleDefinition =
> @@ -677,6 +688,8 @@
>    Config->Verbose = Args.hasArg(OPT_verbose);
>    errorHandler().Verbose = Config->Verbose;
>    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);
>    Config->ZCombreloc = !hasZOption(Args, "nocombreloc");
>    Config->ZExecstack = hasZOption(Args, "execstack");
>    Config->ZNocopyreloc = hasZOption(Args, "nocopyreloc");
> @@ -767,7 +780,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(*Buffer);
>  
>    // 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
> @@ -153,6 +153,7 @@
>    bool Verbose;
>    bool WarnCommon;
>    bool WarnMissingEntry;
> +  bool WarnSymbolOrdering;
>    bool WriteAddends;
>    bool ZCombreloc;
>    bool ZExecstack;


More information about the llvm-commits mailing list