Fix broken export table if .rdata is merged with .text.

[ruiu]

Summary

Previously, we assumed that .rdata is zero-filled, so when writing
an COFF import table, we didn't write anything if the data is zero.
That assumption was wrong because .rdata can be merged with .text.
If .rdata is merged with .text, they are initialized with 0xcc, which
is a trap instruction.

This patch removes that assumption from code.

Should be merged to 8.0 branch as this is a regression.

Fixes https://bugs.llvm.org/show_bug.cgi?id=39826

[rnk]

lgtm

lld/COFF/DLL.cpp

475–476 ↗

(On Diff #183336)

I think we don't need this dead store if we modify the loop below to write zero unconditionally if !E.Noname. Maybe it's worth asserting Config->Exports.size() == Size to ensure there is no padding.

[ruiu]

lld/COFF/DLL.cpp

475–476 ↗

(On Diff #183336)

Actually this store seems completely dead. I'll remove before submitting.

COFF/DLL.cpp

public:
 
   size_t getSize() const override {
     // Starts with 2 byte Hint field, followed by a null-terminated string,
     // ends with 0 or 1 byte padding.
     return alignTo(Name.size() + 3, 2);
   }
 
   void writeTo(uint8_t *Buf) const override {
+    memset(Buf + OutputSectionOff, 0, getSize());
     write16le(Buf + OutputSectionOff, Hint);
     memcpy(Buf + OutputSectionOff + 2, Name.data(), Name.size());
   }
 
 private:
   StringRef Name;
   uint16_t Hint;
 };
 
 // A chunk for the import descriptor table.
 class LookupChunk : public Chunk {
 public:
   explicit LookupChunk(Chunk *C) : HintName(C) { Alignment = Config->Wordsize; }
   size_t getSize() const override { return Config->Wordsize; }
 
   void writeTo(uint8_t *Buf) const override {
+    if (Config->is64())
+      write64le(Buf + OutputSectionOff, HintName->getRVA());
+    else
       write32le(Buf + OutputSectionOff, HintName->getRVA());
   }
 
   Chunk *HintName;
 };
 
 // A chunk for the import descriptor table.
 // This chunk represent import-by-ordinal symbols.
 // See Microsoft PE/COFF spec 7.1. Import Header for details.

 
 // A chunk for the import descriptor table.
 class ImportDirectoryChunk : public Chunk {
 public:
   explicit ImportDirectoryChunk(Chunk *N) : DLLName(N) {}
   size_t getSize() const override { return sizeof(ImportDirectoryTableEntry); }
 
   void writeTo(uint8_t *Buf) const override {
+    memset(Buf + OutputSectionOff, 0, getSize());
+
     auto *E = (coff_import_directory_table_entry *)(Buf + OutputSectionOff);
     E->ImportLookupTableRVA = LookupTab->getRVA();
     E->NameRVA = DLLName->getRVA();
     E->ImportAddressTableRVA = AddressTab->getRVA();
   }
 
   Chunk *DLLName;
   Chunk *LookupTab;
   Chunk *AddressTab;
 };
 
 // A chunk representing null terminator in the import table.
 // Contents of this chunk is always null bytes.
 class NullChunk : public Chunk {
 public:
   explicit NullChunk(size_t N) : Size(N) {}
   bool hasData() const override { return false; }
   size_t getSize() const override { return Size; }
 
+  void writeTo(uint8_t *Buf) const override {
+    memset(Buf + OutputSectionOff, 0, Size);
+  }
+
 private:
   size_t Size;
 };
 
 static std::vector<std::vector<DefinedImportData *>>
 binImports(const std::vector<DefinedImportData *> &Imports) {
   // Group DLL-imported symbols by DLL name because that's how
   // symbols are layed out in the import descriptor table.

 public:
   explicit DelayDirectoryChunk(Chunk *N) : DLLName(N) {}
 
   size_t getSize() const override {
     return sizeof(delay_import_directory_table_entry);
   }
 
   void writeTo(uint8_t *Buf) const override {
+    memset(Buf + OutputSectionOff, 0, getSize());
+
     auto *E = (delay_import_directory_table_entry *)(Buf + OutputSectionOff);
     E->Attributes = 1;
     E->Name = DLLName->getRVA();
     E->ModuleHandle = ModuleHandle->getRVA();
     E->DelayImportAddressTable = AddressTab->getRVA();
     E->DelayImportNameTable = NameTab->getRVA();
   }
 
ExportDirectoryChunk(int I, int J, Chunk *D, Chunk *A, Chunk *N, Chunk *O)
       : MaxOrdinal(I), NameTabSize(J), DLLName(D), AddressTab(A), NameTab(N),
         OrdinalTab(O) {}
 
   size_t getSize() const override {
     return sizeof(export_directory_table_entry);
   }
 
   void writeTo(uint8_t *Buf) const override {
+    memset(Buf + OutputSectionOff, 0, getSize());
+
     auto *E = (export_directory_table_entry *)(Buf + OutputSectionOff);
     E->NameRVA = DLLName->getRVA();
     E->OrdinalBase = 0;
     E->AddressTableEntries = MaxOrdinal + 1;
     E->NumberOfNamePointers = NameTabSize;
     E->ExportAddressTableRVA = AddressTab->getRVA();
     E->NamePointerRVA = NameTab->getRVA();
     E->OrdinalTableRVA = OrdinalTab->getRVA();

test/COFF/imports.test

 IMPORT:      Import {
 IMPORT-NEXT:   Name: std64.dll
 IMPORT-NEXT:   ImportLookupTableRVA: 0x2028
 IMPORT-NEXT:   ImportAddressTableRVA: 0x2048
 IMPORT-NEXT:   Symbol: ExitProcess (0)
 IMPORT-NEXT:   Symbol:  (50)
 IMPORT-NEXT:   Symbol: MessageBoxA (1)
 IMPORT-NEXT: }
+
+# RUN: lld-link /out:%t.exe /entry:main /subsystem:console /merge:.rdata=.text \
+# RUN:   %p/Inputs/hello64.obj %p/Inputs/std64.lib /include:ExitProcess
+# RUN: llvm-readobj -coff-imports %t.exe | FileCheck -check-prefix=MERGE %s
+
+MERGE:      Import {
+MERGE-NEXT:   Name: std64.dll
+MERGE-NEXT:   ImportLookupTableRVA: 0x1090
+MERGE-NEXT:   ImportAddressTableRVA: 0x10B0
+MERGE-NEXT:   Symbol: ExitProcess (0)
+MERGE-NEXT:   Symbol:  (50)
+MERGE-NEXT:   Symbol: MessageBoxA (1)
+MERGE-NEXT: }