From 88e0f9206b4dccb56dee931adab08f89ff80525a Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Thu, 25 Jun 2015 03:31:47 +0000 Subject: [PATCH] COFF: Fix a bug of __imp_ symbol. The change I made in r240620 was not correct. If a symbol foo is defined, and if you use __imp_foo, __imp_foo symbol is automatically defined as a pointer (not just an alias) to foo. Now that we need to create a chunk for automatically-created symbols. I defined LocalImportChunk class for them. llvm-svn: 240622 --- lld/COFF/Chunks.cpp | 4 ++++ lld/COFF/Chunks.h | 12 ++++++++++++ lld/COFF/SymbolTable.cpp | 4 +++- lld/COFF/SymbolTable.h | 3 +++ lld/COFF/Symbols.h | 25 +++++++++++++++++++++++++ lld/COFF/Writer.cpp | 9 +++++++++ lld/COFF/Writer.h | 1 + lld/test/COFF/locally-imported.test | 19 ++++++++++++++----- 8 files changed, 71 insertions(+), 6 deletions(-) diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp index 527ad3b00dcc..95626fd50485 100644 --- a/lld/COFF/Chunks.cpp +++ b/lld/COFF/Chunks.cpp @@ -253,6 +253,10 @@ BaserelChunk::BaserelChunk(uint32_t Page, uint32_t *Begin, uint32_t *End) { } } +void LocalImportChunk::writeTo(uint8_t *Buf) { + write32le(Buf + FileOff, Sym->getRVA()); +} + void BaserelChunk::writeTo(uint8_t *Buf) { memcpy(Buf + FileOff, Data.data(), Data.size()); } diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h index f5512bf6b815..c993c63268e9 100644 --- a/lld/COFF/Chunks.h +++ b/lld/COFF/Chunks.h @@ -211,6 +211,18 @@ private: Defined *ImpSymbol; }; +// Windows-specific. +// See comments for DefinedLocalImport class. +class LocalImportChunk : public Chunk { +public: + explicit LocalImportChunk(Defined *S) : Sym(S) {} + size_t getSize() const override { return 4; } + void writeTo(uint8_t *Buf) override; + +private: + Defined *Sym; +}; + // Windows-specific. // This class represents a block in .reloc section. // See the PE/COFF spec 5.6 for details. diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp index e76937fda6d2..fb847f64d12e 100644 --- a/lld/COFF/SymbolTable.cpp +++ b/lld/COFF/SymbolTable.cpp @@ -85,7 +85,9 @@ bool SymbolTable::reportRemainingUndefines() { // This odd rule is for compatibility with MSVC linker. if (Name.startswith("__imp_")) { if (Defined *Imp = find(Name.substr(strlen("__imp_")))) { - Sym->Body = Imp; + auto *S = new (Alloc) DefinedLocalImport(Name, Imp); + LocalImportChunks.push_back(S->getChunk()); + Sym->Body = S; continue; } } diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h index 623ab285e521..595a97ced31b 100644 --- a/lld/COFF/SymbolTable.h +++ b/lld/COFF/SymbolTable.h @@ -80,6 +80,9 @@ public: // Rename From -> To in the symbol table. std::error_code rename(StringRef From, StringRef To); + // A list of chunks which to be added to .rdata. + std::vector LocalImportChunks; + private: std::error_code resolve(SymbolBody *Body); std::error_code resolveLazy(StringRef Name); diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h index b0af70c7cdbd..e7edffda8331 100644 --- a/lld/COFF/Symbols.h +++ b/lld/COFF/Symbols.h @@ -49,6 +49,7 @@ public: DefinedAbsoluteKind, DefinedImportDataKind, DefinedImportThunkKind, + DefinedLocalImportKind, DefinedCommonKind, DefinedCOMDATKind, DefinedRegularKind, @@ -315,6 +316,30 @@ private: ImportThunkChunk Data; }; +// If you have a symbol "__imp_foo" in your object file, a symbol name +// "foo" becomes automatically available as a pointer to "__imp_foo". +// This class is for such automatically-created symbols. +// Yes, this is an odd feature. We didn't intend to implement that. +// This is here just for compatibility with MSVC. +class DefinedLocalImport : public Defined { +public: + DefinedLocalImport(StringRef N, Defined *S) + : Defined(DefinedLocalImportKind), Name(N), Data(S) {} + + static bool classof(const SymbolBody *S) { + return S->kind() == DefinedLocalImportKind; + } + + StringRef getName() override { return Name; } + uint64_t getRVA() override { return Data.getRVA(); } + uint64_t getFileOff() override { return Data.getFileOff(); } + Chunk *getChunk() { return &Data; } + +private: + StringRef Name; + LocalImportChunk Data; +}; + class DefinedBitcode : public Defined { public: DefinedBitcode(StringRef N, bool R) diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp index 17f91ea1cae6..4e8ae1af1aa2 100644 --- a/lld/COFF/Writer.cpp +++ b/lld/COFF/Writer.cpp @@ -43,6 +43,7 @@ std::error_code Writer::write(StringRef OutputPath) { markLive(); dedupCOMDATs(); createSections(); + createMiscChunks(); createImportTables(); createExportTable(); if (Config->Relocatable) @@ -169,6 +170,14 @@ void Writer::createSections() { } } +void Writer::createMiscChunks() { + if (Symtab->LocalImportChunks.empty()) + return; + OutputSection *Sec = createSection(".rdata"); + for (Chunk *C : Symtab->LocalImportChunks) + Sec->addChunk(C); +} + // Create .idata section for the DLL-imported symbol table. // The format of this section is inherently Windows-specific. // IdataContents class abstracted away the details for us, diff --git a/lld/COFF/Writer.h b/lld/COFF/Writer.h index e81902a5aac3..4d4d0befd2cc 100644 --- a/lld/COFF/Writer.h +++ b/lld/COFF/Writer.h @@ -81,6 +81,7 @@ private: void markLive(); void dedupCOMDATs(); void createSections(); + void createMiscChunks(); void createImportTables(); void createExportTable(); void assignAddresses(); diff --git a/lld/test/COFF/locally-imported.test b/lld/test/COFF/locally-imported.test index 2d8e53232458..e075ee4dbdec 100644 --- a/lld/test/COFF/locally-imported.test +++ b/lld/test/COFF/locally-imported.test @@ -1,7 +1,12 @@ # RUN: yaml2obj < %s > %t.obj # RUN: lld -flavor link2 /out:%t.exe %t.obj +# RUN: llvm-objdump -s %t.exe | FileCheck %s -# The linker automatically removes __imp_ prefix if it helps to resolve all symbols. +# CHECK: Contents of section .text: +# CHECK-NEXT: 1000 00200000 + +# CHECK: Contents of section .rdata: +# CHECK-NEXT: 2000 04100000 --- header: @@ -11,7 +16,11 @@ sections: - Name: .text Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] Alignment: 4 - SectionData: B82A000000C3 + SectionData: 00000000 + Relocations: + - VirtualAddress: 0 + SymbolName: __imp_mainCRTStartup + Type: IMAGE_REL_AMD64_ADDR32NB symbols: - Name: .text Value: 0 @@ -20,14 +29,14 @@ symbols: ComplexType: IMAGE_SYM_DTYPE_NULL StorageClass: IMAGE_SYM_CLASS_STATIC SectionDefinition: - Length: 6 - NumberOfRelocations: 0 + Length: 4 + NumberOfRelocations: 1 NumberOfLinenumbers: 0 CheckSum: 0 Number: 0 Selection: IMAGE_COMDAT_SELECT_ANY - Name: mainCRTStartup - Value: 0 + Value: 4 SectionNumber: 1 SimpleType: IMAGE_SYM_TYPE_NULL ComplexType: IMAGE_SYM_DTYPE_FUNCTION