[lld] r336652 - [COFF] Store import symbol pointers as pointers to the base class

Martin Storsjo via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 10 03:40:11 PDT 2018


Author: mstorsjo
Date: Tue Jul 10 03:40:11 2018
New Revision: 336652

URL: http://llvm.org/viewvc/llvm-project?rev=336652&view=rev
Log:
[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

Added:
    lld/trunk/test/COFF/Inputs/otherFunc.s
    lld/trunk/test/COFF/thunk-replace.s
Modified:
    lld/trunk/COFF/InputFiles.cpp
    lld/trunk/COFF/InputFiles.h
    lld/trunk/COFF/SymbolTable.cpp
    lld/trunk/COFF/SymbolTable.h
    lld/trunk/COFF/Writer.cpp

Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=336652&r1=336651&r2=336652&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Tue Jul 10 03:40:11 2018
@@ -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<DefinedImportData>(ImpSym), Hdr->Machine);
 }
 
 void BitcodeFile::parse() {

Modified: lld/trunk/COFF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=336652&r1=336651&r2=336652&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.h (original)
+++ lld/trunk/COFF/InputFiles.h Tue Jul 10 03:40:11 2018
@@ -207,8 +207,8 @@ public:
 
   static std::vector<ImportFile *> Instances;
 
-  DefinedImportData *ImpSym = nullptr;
-  DefinedImportThunk *ThunkSym = nullptr;
+  Symbol *ImpSym = nullptr;
+  Symbol *ThunkSym = nullptr;
   std::string DLLName;
 
 private:

Modified: lld/trunk/COFF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=336652&r1=336651&r2=336652&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.cpp (original)
+++ lld/trunk/COFF/SymbolTable.cpp Tue Jul 10 03:40:11 2018
@@ -342,30 +342,29 @@ Symbol *SymbolTable::addCommon(InputFile
   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<Undefined>(S) || isa<Lazy>(S)) {
     replaceSymbol<DefinedImportData>(S, N, F);
-    return cast<DefinedImportData>(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<Undefined>(S) || isa<Lazy>(S)) {
     replaceSymbol<DefinedImportThunk>(S, Name, ID, Machine);
-    return cast<DefinedImportThunk>(S);
+    return S;
   }
 
   reportDuplicate(S, ID->File);

Modified: lld/trunk/COFF/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=336652&r1=336651&r2=336652&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.h (original)
+++ lld/trunk/COFF/SymbolTable.h Tue Jul 10 03:40:11 2018
@@ -92,9 +92,9 @@ public:
   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);
 

Modified: lld/trunk/COFF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Writer.cpp?rev=336652&r1=336651&r2=336652&view=diff
==============================================================================
--- lld/trunk/COFF/Writer.cpp (original)
+++ lld/trunk/COFF/Writer.cpp Tue Jul 10 03:40:11 2018
@@ -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<DefinedImportThunk>(File->ThunkSym))
+        fatal(toString(*File->ThunkSym) + " was replaced");
+      DefinedImportThunk *Thunk = cast<DefinedImportThunk>(File->ThunkSym);
       if (File->ThunkLive)
         TextSec->addChunk(Thunk->getChunk());
+    }
 
+    if (File->ImpSym && !isa<DefinedImportData>(File->ImpSym))
+      fatal(toString(*File->ImpSym) + " was replaced");
+    DefinedImportData *ImpSym = cast_or_null<DefinedImportData>(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);
     }
   }
 

Added: lld/trunk/test/COFF/Inputs/otherFunc.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/otherFunc.s?rev=336652&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/otherFunc.s (added)
+++ lld/trunk/test/COFF/Inputs/otherFunc.s Tue Jul 10 03:40:11 2018
@@ -0,0 +1,7 @@
+.global otherFunc
+.global MessageBoxA
+.text
+otherFunc:
+  ret
+MessageBoxA:
+  ret

Added: lld/trunk/test/COFF/thunk-replace.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/thunk-replace.s?rev=336652&view=auto
==============================================================================
--- lld/trunk/test/COFF/thunk-replace.s (added)
+++ lld/trunk/test/COFF/thunk-replace.s Tue Jul 10 03:40:11 2018
@@ -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




More information about the llvm-commits mailing list