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