From 474be005dbf5ee7fd658237f417192aad4696a4f Mon Sep 17 00:00:00 2001 From: Martin Storsjo Date: Tue, 10 Jul 2018 10:40:11 +0000 Subject: [PATCH] [COFF] Store import symbol pointers as pointers to the base class Future symbol insertions can potentially change the type of these symbols - keep pointers to the base class to reflect this, and use dynamic casts to inspect them before using as the subclass type. This fixes crashes that were possible before, by touching these symbols that now are populated as e.g. a DefinedRegular, via the old pointers with DefinedImportThunk type. Differential Revision: https://reviews.llvm.org/D48953 llvm-svn: 336652 --- lld/COFF/InputFiles.cpp | 3 ++- lld/COFF/InputFiles.h | 4 ++-- lld/COFF/SymbolTable.cpp | 11 +++++------ lld/COFF/SymbolTable.h | 6 +++--- lld/COFF/Writer.cpp | 15 +++++++++++---- lld/test/COFF/Inputs/otherFunc.s | 7 +++++++ lld/test/COFF/thunk-replace.s | 15 +++++++++++++++ 7 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 lld/test/COFF/Inputs/otherFunc.s create mode 100644 lld/test/COFF/thunk-replace.s diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp index 9e2345b0a51bc..86843211aa735 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -431,7 +431,8 @@ void ImportFile::parse() { // address pointed by the __imp_ symbol. (This allows you to call // DLL functions just like regular non-DLL functions.) if (Hdr->getType() == llvm::COFF::IMPORT_CODE) - ThunkSym = Symtab->addImportThunk(Name, ImpSym, Hdr->Machine); + ThunkSym = Symtab->addImportThunk( + Name, cast_or_null(ImpSym), Hdr->Machine); } void BitcodeFile::parse() { diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h index 9f4db4559097b..4ee4b363886f3 100644 --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -207,8 +207,8 @@ class ImportFile : public InputFile { static std::vector Instances; - DefinedImportData *ImpSym = nullptr; - DefinedImportThunk *ThunkSym = nullptr; + Symbol *ImpSym = nullptr; + Symbol *ThunkSym = nullptr; std::string DLLName; private: diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp index a3d1fcdabab37..b286d865caafc 100644 --- a/lld/COFF/SymbolTable.cpp +++ b/lld/COFF/SymbolTable.cpp @@ -342,30 +342,29 @@ Symbol *SymbolTable::addCommon(InputFile *F, StringRef N, uint64_t Size, return S; } -DefinedImportData *SymbolTable::addImportData(StringRef N, ImportFile *F) { +Symbol *SymbolTable::addImportData(StringRef N, ImportFile *F) { Symbol *S; bool WasInserted; std::tie(S, WasInserted) = insert(N); S->IsUsedInRegularObj = true; if (WasInserted || isa(S) || isa(S)) { replaceSymbol(S, N, F); - return cast(S); + return S; } reportDuplicate(S, F); return nullptr; } -DefinedImportThunk *SymbolTable::addImportThunk(StringRef Name, - DefinedImportData *ID, - uint16_t Machine) { +Symbol *SymbolTable::addImportThunk(StringRef Name, DefinedImportData *ID, + uint16_t Machine) { Symbol *S; bool WasInserted; std::tie(S, WasInserted) = insert(Name); S->IsUsedInRegularObj = true; if (WasInserted || isa(S) || isa(S)) { replaceSymbol(S, Name, ID, Machine); - return cast(S); + return S; } reportDuplicate(S, ID->File); diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h index 55481e6475bb7..30cb1a5410c36 100644 --- a/lld/COFF/SymbolTable.h +++ b/lld/COFF/SymbolTable.h @@ -92,9 +92,9 @@ class SymbolTable { Symbol *addCommon(InputFile *F, StringRef N, uint64_t Size, const llvm::object::coff_symbol_generic *S = nullptr, CommonChunk *C = nullptr); - DefinedImportData *addImportData(StringRef N, ImportFile *F); - DefinedImportThunk *addImportThunk(StringRef Name, DefinedImportData *S, - uint16_t Machine); + Symbol *addImportData(StringRef N, ImportFile *F); + Symbol *addImportThunk(StringRef Name, DefinedImportData *S, + uint16_t Machine); void reportDuplicate(Symbol *Existing, InputFile *NewFile); diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp index c6e17eea11598..e9b21df478aae 100644 --- a/lld/COFF/Writer.cpp +++ b/lld/COFF/Writer.cpp @@ -544,17 +544,24 @@ void Writer::createImportTables() { if (Config->DLLOrder.count(DLL) == 0) Config->DLLOrder[DLL] = Config->DLLOrder.size(); - if (DefinedImportThunk *Thunk = File->ThunkSym) + if (File->ThunkSym) { + if (!isa(File->ThunkSym)) + fatal(toString(*File->ThunkSym) + " was replaced"); + DefinedImportThunk *Thunk = cast(File->ThunkSym); if (File->ThunkLive) TextSec->addChunk(Thunk->getChunk()); + } + if (File->ImpSym && !isa(File->ImpSym)) + fatal(toString(*File->ImpSym) + " was replaced"); + DefinedImportData *ImpSym = cast_or_null(File->ImpSym); if (Config->DelayLoads.count(StringRef(File->DLLName).lower())) { if (!File->ThunkSym) fatal("cannot delay-load " + toString(File) + - " due to import of data: " + toString(*File->ImpSym)); - DelayIdata.add(File->ImpSym); + " due to import of data: " + toString(*ImpSym)); + DelayIdata.add(ImpSym); } else { - Idata.add(File->ImpSym); + Idata.add(ImpSym); } } diff --git a/lld/test/COFF/Inputs/otherFunc.s b/lld/test/COFF/Inputs/otherFunc.s new file mode 100644 index 0000000000000..ae8b9228a61ad --- /dev/null +++ b/lld/test/COFF/Inputs/otherFunc.s @@ -0,0 +1,7 @@ +.global otherFunc +.global MessageBoxA +.text +otherFunc: + ret +MessageBoxA: + ret diff --git a/lld/test/COFF/thunk-replace.s b/lld/test/COFF/thunk-replace.s new file mode 100644 index 0000000000000..2d47fcc64837e --- /dev/null +++ b/lld/test/COFF/thunk-replace.s @@ -0,0 +1,15 @@ +# REQUIRES: x86 + +# RUN: llvm-mc -triple=x86_64-win32 %s -filetype=obj -o %t.main.obj +# RUN: llvm-mc -triple=x86_64-win32 %p/Inputs/otherFunc.s -filetype=obj -o %t.other.obj +# RUN: llvm-ar rcs %t.other.lib %t.other.obj +# RUN: not lld-link -out:%t.exe -entry:main %t.main.obj %p/Inputs/std64.lib %t.other.lib -opt:noref 2>&1 | FileCheck %s +# CHECK: MessageBoxA was replaced + +.global main +.text +main: + callq MessageBoxA + callq ExitProcess + callq otherFunc + ret