[lld] r306071 - [COFF] Fix SECTION and SECREL relocation handling for absolute symbols

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 22 17:39:42 PDT 2017


I'm getting this warning building lld with this change.

tools/lld/COFF/Writer.cpp:222:13: warning: unused variable 'D'
[-Wunused-variable]
  if (auto *D = dyn_cast<DefinedAbsolute>(this))
            ^

On Thu, Jun 22, 2017 at 4:33 PM, Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Thu Jun 22 18:33:04 2017
> New Revision: 306071
>
> URL: http://llvm.org/viewvc/llvm-project?rev=306071&view=rev
> Log:
> [COFF] Fix SECTION and SECREL relocation handling for absolute symbols
>
> Summary:
> For SECTION relocations against absolute symbols, MSVC emits the largest
> output section index plus one. I've implemented that by threading a
> global variable through DefinedAbsolute that is filled in by the Writer.
> A more library-oriented approach would be to thread the Writer through
> Chunk::writeTo and SectionChunk::applyRel*, but Rui seems to prefer
> doing it this way.
>
> MSVC rejects SECREL relocations against absolute symbols, but only when
> the relocation is in a real output section. When the relocation is in a
> CodeView debug info section destined for the PDB, it seems that this
> relocation error is suppressed, and absolute symbols become zeros in the
> object file. This is easily implemented by checking the input section
> from which we're applying relocations.
>
> This should fix errors about __safe_se_handler_table and
> __guard_fids_table when linking the CRT and generating a PDB.
>
> Reviewers: ruiu
>
> Subscribers: aprantl, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D34541
>
> Added:
>     lld/trunk/test/COFF/pdb-secrel-absolute.yaml
>     lld/trunk/test/COFF/secidx-absolute.s
>     lld/trunk/test/COFF/secrel-absolute.s
> Modified:
>     lld/trunk/COFF/Chunks.cpp
>     lld/trunk/COFF/Symbols.cpp
>     lld/trunk/COFF/Symbols.h
>     lld/trunk/COFF/Writer.cpp
>
> Modified: lld/trunk/COFF/Chunks.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.
> cpp?rev=306071&r1=306070&r2=306071&view=diff
> ============================================================
> ==================
> --- lld/trunk/COFF/Chunks.cpp (original)
> +++ lld/trunk/COFF/Chunks.cpp Thu Jun 22 18:33:04 2017
> @@ -52,6 +52,15 @@ static void add32(uint8_t *P, int32_t V)
>  static void add64(uint8_t *P, int64_t V) { write64le(P, read64le(P) + V);
> }
>  static void or16(uint8_t *P, uint16_t V) { write16le(P, read16le(P) | V);
> }
>
> +static void applySecRel(const SectionChunk *Sec, uint8_t *Off, Defined
> *Sym) {
> +  // Don't apply section relative relocations to absolute symbols in
> codeview
> +  // debug info sections. MSVC does not treat such relocations as fatal
> errors,
> +  // and they can be found in the standard library for linker-provided
> symbols
> +  // like __guard_fids_table and __safe_se_handler_table.
> +  if (!(isa<DefinedAbsolute>(Sym) && Sec->isCodeView()))
> +    add32(Off, Sym->getSecrel());
> +}
> +
>  void SectionChunk::applyRelX64(uint8_t *Off, uint16_t Type, Defined *Sym,
>                                 uint64_t P) const {
>    uint64_t S = Sym->getRVA();
> @@ -66,7 +75,7 @@ void SectionChunk::applyRelX64(uint8_t *
>    case IMAGE_REL_AMD64_REL32_4:  add32(Off, S - P - 8); break;
>    case IMAGE_REL_AMD64_REL32_5:  add32(Off, S - P - 9); break;
>    case IMAGE_REL_AMD64_SECTION:  add16(Off, Sym->getSectionIndex());
> break;
> -  case IMAGE_REL_AMD64_SECREL:   add32(Off, Sym->getSecrel()); break;
> +  case IMAGE_REL_AMD64_SECREL:   applySecRel(this, Off, Sym); break;
>    default:
>      fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
>    }
> @@ -81,7 +90,7 @@ void SectionChunk::applyRelX86(uint8_t *
>    case IMAGE_REL_I386_DIR32NB:  add32(Off, S); break;
>    case IMAGE_REL_I386_REL32:    add32(Off, S - P - 4); break;
>    case IMAGE_REL_I386_SECTION:  add16(Off, Sym->getSectionIndex()); break;
> -  case IMAGE_REL_I386_SECREL:   add32(Off, Sym->getSecrel()); break;
> +  case IMAGE_REL_I386_SECREL:   applySecRel(this, Off, Sym); break;
>    default:
>      fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
>    }
> @@ -141,7 +150,7 @@ void SectionChunk::applyRelARM(uint8_t *
>    case IMAGE_REL_ARM_BRANCH20T: applyBranch20T(Off, S - P - 4); break;
>    case IMAGE_REL_ARM_BRANCH24T: applyBranch24T(Off, S - P - 4); break;
>    case IMAGE_REL_ARM_BLX23T:    applyBranch24T(Off, S - P - 4); break;
> -  case IMAGE_REL_ARM_SECREL:    add32(Off, Sym->getSecrel()); break;
> +  case IMAGE_REL_ARM_SECREL:    applySecRel(this, Off, Sym); break;
>    default:
>      fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
>    }
>
> Modified: lld/trunk/COFF/Symbols.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/
> Symbols.cpp?rev=306071&r1=306070&r2=306071&view=diff
> ============================================================
> ==================
> --- lld/trunk/COFF/Symbols.cpp (original)
> +++ lld/trunk/COFF/Symbols.cpp Thu Jun 22 18:33:04 2017
> @@ -61,6 +61,8 @@ COFFSymbolRef DefinedCOFF::getCOFFSymbol
>    return COFFSymbolRef(reinterpret_cast<const coff_symbol32 *>(Sym));
>  }
>
> +uint16_t DefinedAbsolute::OutputSectionIndex = 0;
> +
>  static Chunk *makeImportThunk(DefinedImportData *S, uint16_t Machine) {
>    if (Machine == AMD64)
>      return make<ImportThunkChunkX64>(S);
>
> Modified: lld/trunk/COFF/Symbols.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/
> Symbols.h?rev=306071&r1=306070&r2=306071&view=diff
> ============================================================
> ==================
> --- lld/trunk/COFF/Symbols.h (original)
> +++ lld/trunk/COFF/Symbols.h Thu Jun 22 18:33:04 2017
> @@ -212,6 +212,11 @@ public:
>    uint64_t getRVA() { return VA - Config->ImageBase; }
>    void setVA(uint64_t V) { VA = V; }
>
> +  // The sentinel absolute symbol section index. Section index relocations
> +  // against absolute symbols resolve to this 16 bit number, and it is the
> +  // largest valid section index plus one. This is written by the Writer.
> +  static uint16_t OutputSectionIndex;
> +
>  private:
>    uint64_t VA;
>  };
>
> Modified: lld/trunk/COFF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Writer.
> cpp?rev=306071&r1=306070&r2=306071&view=diff
> ============================================================
> ==================
> --- lld/trunk/COFF/Writer.cpp (original)
> +++ lld/trunk/COFF/Writer.cpp Thu Jun 22 18:33:04 2017
> @@ -219,6 +219,8 @@ uint64_t Defined::getSecrel() {
>  uint64_t Defined::getSectionIndex() {
>    if (auto *D = dyn_cast<DefinedRegular>(this))
>      return D->getChunk()->getOutputSection()->SectionIndex;
> +  if (auto *D = dyn_cast<DefinedAbsolute>(this))
> +    return DefinedAbsolute::OutputSectionIndex;
>    fatal("SECTION relocation points to a non-regular symbol: " +
>          toString(*this));
>  }
> @@ -775,6 +777,10 @@ void Writer::setSectionPermissions() {
>
>  // Write section contents to a mmap'ed file.
>  void Writer::writeSections() {
> +  // Record the section index that should be used when resolving a section
> +  // relocation against an absolute symbol.
> +  DefinedAbsolute::OutputSectionIndex = OutputSections.size() + 1;
> +
>    uint8_t *Buf = Buffer->getBufferStart();
>    for (OutputSection *Sec : OutputSections) {
>      uint8_t *SecBuf = Buf + Sec->getFileOff();
>
> Added: lld/trunk/test/COFF/pdb-secrel-absolute.yaml
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/
> pdb-secrel-absolute.yaml?rev=306071&view=auto
> ============================================================
> ==================
> --- lld/trunk/test/COFF/pdb-secrel-absolute.yaml (added)
> +++ lld/trunk/test/COFF/pdb-secrel-absolute.yaml Thu Jun 22 18:33:04 2017
> @@ -0,0 +1,84 @@
> +# RUN: yaml2obj %s -o %t.obj
> +# RUN: lld-link -debug -entry:main -out:%t.exe -pdb:%t.pdb %t.obj
> +# RUN: llvm-pdbutil raw -symbols %t.pdb | FileCheck %s
> +
> +# There is an S_GDATA32 symbol record with .secrel32 and .secidx
> relocations in
> +# it in this debug info. This is similar to the relocations in the
> loadcfg.obj
> +# file in the MSVC CRT. We need to make sure that our relocation logic
> matches
> +# MSVC's for these absolute, linker-provided symbols.
> +
> +# CHECK: Mod 0000 |
> +# CHECK-NEXT: - S_GDATA32 [size = 36] `__guard_fids_table`
> +# CHECK-NEXT:     type = 0x0022 (unsigned long), addr = 0003:0000
> +# CHECK-NEXT: Mod 0001 | `* Linker *`:
> +
> +--- !COFF
> +header:
> +  Machine:         IMAGE_FILE_MACHINE_AMD64
> +  Characteristics: [  ]
> +sections:
> +  - Name:            '.debug$S'
> +    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA,
> IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
> +    Alignment:       1
> +    Subsections:
> +      - !Symbols
> +        Records:
> +          - Kind:            S_GDATA32
> +            DataSym:
> +              Type:            34
> +              DisplayName:     __guard_fids_table
> +      - !StringTable
> +        Strings:
> +    Relocations:
> +      - VirtualAddress:  20
> +        SymbolName:      __guard_fids_table
> +        Type:            IMAGE_REL_AMD64_SECREL
> +      - VirtualAddress:  24
> +        SymbolName:      __guard_fids_table
> +        Type:            IMAGE_REL_AMD64_SECTION
> +  - Name:            '.text$mn'
> +    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE,
> IMAGE_SCN_MEM_READ ]
> +    Alignment:       16
> +    SectionData:     488D0500000000C3
> +    Relocations:
> +      - VirtualAddress:  3
> +        SymbolName:      __guard_fids_table
> +        Type:            IMAGE_REL_AMD64_REL32
> +symbols:
> +  - Name:            '.debug$S'
> +    Value:           0
> +    SectionNumber:   1
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_NULL
> +    StorageClass:    IMAGE_SYM_CLASS_STATIC
> +    SectionDefinition:
> +      Length:          372
> +      NumberOfRelocations: 6
> +      NumberOfLinenumbers: 0
> +      CheckSum:        0
> +      Number:          0
> +  - Name:            '.text$mn'
> +    Value:           0
> +    SectionNumber:   2
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_NULL
> +    StorageClass:    IMAGE_SYM_CLASS_STATIC
> +    SectionDefinition:
> +      Length:          8
> +      NumberOfRelocations: 1
> +      NumberOfLinenumbers: 0
> +      CheckSum:        1092178131
> +      Number:          0
> +  - Name:            main
> +    Value:           0
> +    SectionNumber:   2
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
> +    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
> +  - Name:            __guard_fids_table
> +    Value:           0
> +    SectionNumber:   0
> +    SimpleType:      IMAGE_SYM_TYPE_NULL
> +    ComplexType:     IMAGE_SYM_DTYPE_NULL
> +    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
> +...
>
> Added: lld/trunk/test/COFF/secidx-absolute.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/
> secidx-absolute.s?rev=306071&view=auto
> ============================================================
> ==================
> --- lld/trunk/test/COFF/secidx-absolute.s (added)
> +++ lld/trunk/test/COFF/secidx-absolute.s Thu Jun 22 18:33:04 2017
> @@ -0,0 +1,33 @@
> +# RUN: llvm-mc %s -filetype=obj -triple=x86_64-windows-msvc -o %t.obj
> +# RUN: lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe
> +# RUN: llvm-readobj %t.exe -sections -section-data | FileCheck %s
> +
> +# Section relocations against absolute symbols resolve to the last real
> ouput
> +# section index plus one.
> +
> +.text
> +.global main
> +main:
> +ret
> +
> +.section .rdata,"dr"
> +.secidx __guard_fids_table
> +
> +# CHECK: Sections [
> +# CHECK:   Section {
> +# CHECK:     Number: 1
> +# CHECK:     Name: .rdata (2E 72 64 61 74 61 00 00)
> +# CHECK:     SectionData (
> +# CHECK:       0000: 0300                                 |..|
> +# CHECK:     )
> +# CHECK:   }
> +# CHECK:   Section {
> +# CHECK:     Number: 2
> +# CHECK:     Name: .text (2E 74 65 78 74 00 00 00)
> +# CHECK:     VirtualSize: 0x1
> +# CHECK:     SectionData (
> +# CHECK:       0000: C3                                   |.|
> +# CHECK:     )
> +# CHECK:   }
> +# CHECK-NOT: Section
> +# CHECK: ]
>
> Added: lld/trunk/test/COFF/secrel-absolute.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/
> secrel-absolute.s?rev=306071&view=auto
> ============================================================
> ==================
> --- lld/trunk/test/COFF/secrel-absolute.s (added)
> +++ lld/trunk/test/COFF/secrel-absolute.s Thu Jun 22 18:33:04 2017
> @@ -0,0 +1,14 @@
> +# RUN: llvm-mc %s -filetype=obj -triple=x86_64-windows-msvc -o %t.obj
> +# RUN: not lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe 2>&1 |
> FileCheck %s
> +
> +# secrel relocations against absolute symbols are errors.
> +
> +# CHECK: SECREL relocation points to a non-regular symbol:
> __guard_fids_table
> +
> +.text
> +.global main
> +main:
> +ret
> +
> +.section .rdata,"dr"
> +.secrel32 __guard_fids_table
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170622/e2007772/attachment.html>


More information about the llvm-commits mailing list