[PATCH] D41987: [ELF] - Define linkerscript symbols early.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 29 09:16:13 PST 2018


LGTM.

Thanks!

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 131750.
> grimar edited the summary of this revision.
> grimar added a comment.
>
> https://reviews.llvm.org/rL323633 was landed and this one is unblocked now. Rebased.
>
>
> https://reviews.llvm.org/D41987
>
> Files:
>   ELF/Driver.cpp
>   ELF/LTO.cpp
>   ELF/LinkerScript.cpp
>   ELF/LinkerScript.h
>   test/ELF/linkerscript/symbols-synthetic.s
>   test/ELF/lto/defsym.ll
>   test/ELF/lto/linker-script-symbols-assign.ll
>   test/ELF/lto/linker-script-symbols-ipo.ll
>
> Index: test/ELF/lto/linker-script-symbols-ipo.ll
> ===================================================================
> --- test/ELF/lto/linker-script-symbols-ipo.ll
> +++ test/ELF/lto/linker-script-symbols-ipo.ll
> @@ -16,9 +16,9 @@
>  ; RUN: llvm-objdump -d %t4 | FileCheck %s --check-prefix=NOIPO
>  ; NOIPO:      Disassembly of section .text:
>  ; NOIPO:      foo:
> -; NOIPO-NEXT:   201010: {{.*}} movl $2, %eax
> +; NOIPO-NEXT:   {{.*}} movl $2, %eax
>  ; NOIPO:      _start:
> -; NOIPO-NEXT:   201020: {{.*}} jmp -21 <foo>
> +; NOIPO-NEXT:   {{.*}} jmp -21 <foo>
>  
>  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>  target triple = "x86_64-unknown-linux-gnu"
> Index: test/ELF/lto/linker-script-symbols-assign.ll
> ===================================================================
> --- test/ELF/lto/linker-script-symbols-assign.ll
> +++ test/ELF/lto/linker-script-symbols-assign.ll
> @@ -6,16 +6,7 @@
>  ; RUN: llvm-readobj -symbols %t2.lto.o | FileCheck %s
>  
>  ; CHECK-NOT: bar
> -; CHECK:     Symbol {
> -; CHECK:         Name: foo
> -; CHECK-NEXT:    Value: 0x0
> -; CHECK-NEXT:    Size: 4
> -; CHECK-NEXT:    Binding: Weak
> -; CHECK-NEXT:    Type: Object
> -; CHECK-NEXT:    Other: 0
> -; CHECK-NEXT:    Section: .bss.foo
> -; CHECK-NEXT:  }
> -; CHECK-NEXT:]
> +; CHECK-NOT: foo
>  
>  ; RUN: llvm-readobj -symbols %t2 | FileCheck %s --check-prefix=VAL
>  ; VAL:       Symbol {
> Index: test/ELF/lto/defsym.ll
> ===================================================================
> --- test/ELF/lto/defsym.ll
> +++ test/ELF/lto/defsym.ll
> @@ -2,14 +2,18 @@
>  ; LTO
>  ; RUN: llvm-as %s -o %t.o
>  ; RUN: llvm-as %S/Inputs/defsym-bar.ll -o %t1.o
> -; RUN: ld.lld %t.o %t1.o -shared -o %t.so -defsym=bar2=bar3
> +; RUN: ld.lld %t.o %t1.o -shared -o %t.so -defsym=bar2=bar3 -save-temps
> +; RUN: llvm-readelf -t %t.so.lto.o | FileCheck --check-prefix=OBJ %s
>  ; RUN: llvm-objdump -d %t.so | FileCheck %s
>  
>  ; ThinLTO
>  ; RUN: opt -module-summary %s -o %t.o
>  ; RUN: opt -module-summary %S/Inputs/defsym-bar.ll -o %t1.o
> -; RUN: ld.lld %t.o %t1.o -shared -o %t.so -defsym=bar2=bar3
> -; RUN: llvm-objdump -d %t.so | FileCheck %s --check-prefix=THIN
> +; RUN: ld.lld %t.o %t1.o -shared -o %t2.so -defsym=bar2=bar3 -save-temps
> +; RUN: llvm-readelf -t %t2.so1.lto.o | FileCheck --check-prefix=OBJ %s
> +; RUN: llvm-objdump -d %t2.so | FileCheck %s --check-prefix=THIN
> +
> +; OBJ:  UND bar2
>  
>  ; Call to bar2() should not be inlined and should be routed to bar3()
>  ; Symbol bar3 should not be eliminated
> Index: test/ELF/linkerscript/symbols-synthetic.s
> ===================================================================
> --- test/ELF/linkerscript/symbols-synthetic.s
> +++ test/ELF/linkerscript/symbols-synthetic.s
> @@ -61,14 +61,14 @@
>  # SIMPLE-NEXT: 0000000000000120         .foo    00000000 _begin_sec
>  # SIMPLE-NEXT: 0000000000000128         *ABS*   00000000 _end_sec_abs
>  # SIMPLE-NEXT: 0000000000001048         .text   00000000 _start
> +# SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_3
>  # SIMPLE-NEXT: 0000000000000120         .foo    00000000 begin_foo
>  # SIMPLE-NEXT: 0000000000000128         .foo    00000000 end_foo
>  # SIMPLE-NEXT: 0000000000000008         *ABS*   00000000 size_foo_1
>  # SIMPLE-NEXT: 0000000000000008         *ABS*   00000000 size_foo_1_abs
>  # SIMPLE-NEXT: 0000000000001000         .foo    00000000 begin_bar
>  # SIMPLE-NEXT: 0000000000001004         .foo    00000000 end_bar
>  # SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_2
> -# SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_3
>  # SIMPLE-NEXT: 0000000000001004         .eh_frame_hdr     00000000 __eh_frame_hdr_start
>  # SIMPLE-NEXT: 0000000000001010         *ABS*             00000000 __eh_frame_hdr_start2
>  # SIMPLE-NEXT: 0000000000001018         .eh_frame_hdr     00000000 __eh_frame_hdr_end
> Index: ELF/LinkerScript.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -268,6 +268,7 @@
>    void assignAddresses();
>    void allocateHeaders(std::vector<PhdrEntry *> &Phdrs);
>    void processSectionCommands();
> +  void declareSymbols();
>  
>    // SECTIONS command list.
>    std::vector<BaseCommand *> SectionCommands;
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -114,16 +114,28 @@
>      Ctx->OutSec->Size = Dot - Ctx->OutSec->Addr;
>  }
>  
> -// This function is called from processSectionCommands,
> -// while we are fixing the output section layout.
> -void LinkerScript::addSymbol(SymbolAssignment *Cmd) {
> +// Used for handling linker symbol assignments, for both finalizing
> +// their values and doing early declarations. Returns true if symbol
> +// should be defined from linker script.
> +static bool shouldDefineSym(SymbolAssignment *Cmd) {
>    if (Cmd->Name == ".")
> -    return;
> +    return false;
>  
> -  // If a symbol was in PROVIDE(), we need to define it only when
> -  // it is a referenced undefined symbol.
> +  if (!Cmd->Provide)
> +    return true;
> +
> +  // If a symbol was in PROVIDE(), we need to define it only
> +  // when it is a referenced undefined symbol.
>    Symbol *B = Symtab->find(Cmd->Name);
> -  if (Cmd->Provide && (!B || B->isDefined()))
> +  if (!B || B->isDefined())
> +    return false;
> +  return true;
> +}
> +
> +// This function is called from processSectionCommands,
> +// while we are fixing the output section layout.
> +void LinkerScript::addSymbol(SymbolAssignment *Cmd) {
> +  if (!shouldDefineSym(Cmd))
>      return;
>  
>    // Define a symbol.
> @@ -153,6 +165,30 @@
>    Cmd->Sym = cast<Defined>(Sym);
>  }
>  
> +// Symbols defined in script should not be inlined by LTO. At the same time
> +// we don't know their final values until late stages of link. Here we scan
> +// over symbol assignment commands and create placeholder symbols if needed.
> +void LinkerScript::declareSymbols() {
> +  assert(!Ctx);
> +  for (BaseCommand *Base : SectionCommands) {
> +    auto *Cmd = dyn_cast<SymbolAssignment>(Base);
> +    if (!Cmd || !shouldDefineSym(Cmd))
> +      continue;
> +
> +    // We can't calculate final value right now.
> +    Symbol *Sym;
> +    uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
> +    std::tie(Sym, std::ignore) =
> +        Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
> +                       /*CanOmitFromDynSym*/ false,
> +                       /*File*/ nullptr);
> +    replaceSymbol<Defined>(Sym, nullptr, Cmd->Name, STB_GLOBAL, Visibility,
> +                           STT_NOTYPE, 0, 0, nullptr);
> +    Cmd->Sym = cast<Defined>(Sym);
> +    Cmd->Provide = false;
> +  }
> +}
> +
>  // This function is called from assignAddresses, while we are
>  // fixing the output section addresses. This function is supposed
>  // to set the final value for a given symbol assignment.
> Index: ELF/LTO.cpp
> ===================================================================
> --- ELF/LTO.cpp
> +++ ELF/LTO.cpp
> @@ -129,11 +129,6 @@
>    std::vector<Symbol *> Syms = F.getSymbols();
>    std::vector<lto::SymbolResolution> Resols(Syms.size());
>  
> -  DenseSet<StringRef> ScriptSymbols;
> -  for (BaseCommand *Base : Script->SectionCommands)
> -    if (auto *Cmd = dyn_cast<SymbolAssignment>(Base))
> -      ScriptSymbols.insert(Cmd->Name);
> -
>    bool IsExecutable = !Config->Shared && !Config->Relocatable;
>  
>    // Provide a resolution to the LTO API for each symbol.
> @@ -164,12 +159,10 @@
>      if (R.Prevailing)
>        undefine(Sym);
>  
> -    // We tell LTO to not apply interprocedural optimization for following
> -    // symbols because otherwise LTO would inline them while their values are
> -    // still not final:
> -    // 1) Aliased (with --defsym) or wrapped (with --wrap) symbols.
> -    // 2) Symbols redefined in linker script.
> -    R.LinkerRedefined = !Sym->CanInline || ScriptSymbols.count(Sym->getName());
> +    // We tell LTO to not apply interprocedural optimization for wrapped
> +    // (with --wrap) symbols because otherwise LTO would inline them while
> +    // their values are still not final.
> +    R.LinkerRedefined = !Sym->CanInline;
>    }
>    checkError(LTOObj->add(std::move(F.Obj), Resols));
>  }
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -1063,6 +1063,10 @@
>    if (!Config->Relocatable)
>      addReservedSymbols();
>  
> +  // We want to declare linker script's symbols early,
> +  // so that we can version them.
> +  Script->declareSymbols();
> +
>    // Apply version scripts.
>    Symtab->scanVersionScript();
>  


More information about the llvm-commits mailing list