[COFF] Allow debug info to relocate against discarded symbols

[rnk]

Summary

In order to do this without switching on the symbol kind multiple times,
I created Defined::getChunkAndOffset and use that instead of
SymbolBody::getRVA in the inner relocation loop.

Now we get the symbol's chunk before switching over relocation types, so
we can test if it has been discarded outside the inner relocation type
switch. This also simplifies application of section relative
relocations. Previously we would switch on symbol kind to compute the
RVA, then the relocation type, and then the symbol kind again to get the
output section so we could subtract that from the symbol RVA. Now we
*always* have an OutputSection, so applying SECREL and SECTION
relocations isn't as much of a special case.

I'm still not quite happy with the cleanliness of this code. I'm not
sure what offsets and bases we should be using during the relocation
processing loop: VA, RVA, or OutputSectionOffset. I started work on the
FIXME to add a fake Chunk and OutputSection for absolute and __ImageBase
symbols so that we never need null checks for these pointers, but I
decided to send out this patch for comments before undertaking that
large effort.

[rnk]

ptal

This was the last change I needed to produce a PDB for a program that links in the CRT.

[majnemer]

Do they emit a S_DISCARDED for the symbol?

[rnk]

In D34650#791277, @majnemer wrote:

Do they emit a S_DISCARDED for the symbol?

Nope. It's not nearly as bad as DWARF, though, which has tons of relocations against discarded sections.

[rnk]

In D34650#791386, @rnk wrote:

In D34650#791277, @majnemer wrote:

Do they emit a S_DISCARDED for the symbol?

Nope. It's not nearly as bad as DWARF, though, which has tons of relocations against discarded sections.

It's worth pointing out that this only seems to be a problem for globals, not functions. Functions use associative comdats to drop debug info describing discarded functions. The only place I've seen these bad relocations is in these S_GDATA32 records for external globals. MSVC represents function declarations with LF_FUNC_ID type records, which are merged across TUs and don't contain relocations.

[rnk]

• clean up code, test fatal error emission

[rnk]

Hopefully this version of the patch is cleaner. The error checking should be more streamlined, and we now have a test for the error case of a relocation against a discarded section.

[ruiu]

lld/COFF/Chunks.cpp

56–66 ↗

(On Diff #104272)

Ah, this is much more elegant than I was thinking. Good job!

lld/COFF/MarkLive.cpp

57 ↗

(On Diff #104272)

... a common error in debug sections?

[rnk]

lld/COFF/Chunks.cpp

56–66 ↗

(On Diff #104272)

Thanks!

lld/COFF/MarkLive.cpp

57 ↗

(On Diff #104272)

I guess this is a bad comment. It's more that I didn't want to duplicate the error message in two places when we can just continue here.

Concretely, we need this code because if we don't have it, we will fail the next assert, because isLive() is defined as:

return Live && !Discarded;

lld/test/COFF/reloc-discarded.s

28 ↗

(On Diff #104272)

This is the invalid input test that shows how to get into this situation, FWIW.

[rnk]

• better comment

[ruiu]

LGTM

[rnk]

The last patch didn't pass tests, and fixing the tests required some surgery.
PTAL

[ruiu]

LGTM

lld/trunk/COFF/Chunks.h

public:
   size_t getSize() const override { return Header->SizeOfRawData; }
   ArrayRef<uint8_t> getContents() const;
   void writeTo(uint8_t *Buf) const override;
   bool hasData() const override;
   uint32_t getPermissions() const override;
   StringRef getSectionName() const override { return SectionName; }
   void getBaserels(std::vector<Baserel> *Res) override;
   bool isCOMDAT() const;
-  void applyRelX64(uint8_t *Off, uint16_t Type, Defined *Sym, uint64_t P) const;
-  void applyRelX86(uint8_t *Off, uint16_t Type, Defined *Sym, uint64_t P) const;
-  void applyRelARM(uint8_t *Off, uint16_t Type, Defined *Sym, uint64_t P) const;
+  void applyRelX64(uint8_t *Off, uint16_t Type, OutputSection *OS, uint64_t S,
+                   uint64_t P) const;
+  void applyRelX86(uint8_t *Off, uint16_t Type, OutputSection *OS, uint64_t S,
+                   uint64_t P) const;
+  void applyRelARM(uint8_t *Off, uint16_t Type, OutputSection *OS, uint64_t S,
+                   uint64_t P) const;
 
   // Called if the garbage collector decides to not include this chunk
   // in a final output. It's supposed to print out a log message to stdout.
   void printDiscardedMessage() const;
 
   // Adds COMDAT associative sections to this COMDAT section. A chunk
   // and its children are treated as a group by the garbage collector.
   void addAssociative(SectionChunk *Child);

lld/trunk/COFF/Chunks.cpp

 //===- Chunks.cpp ---------------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Chunks.h"
 #include "Error.h"
 #include "InputFiles.h"
 #include "Symbols.h"
+#include "Writer.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
SectionChunk::SectionChunk(ObjectFile *F, const coff_section *H)
   Live = !Config->DoGC || !isCOMDAT();
 }
 
 static void add16(uint8_t *P, int16_t V) { write16le(P, read16le(P) + V); }
 static void add32(uint8_t *P, int32_t V) { write32le(P, read32le(P) + V); }
 static void add64(uint8_t *P, int64_t V) { write64le(P, read64le(P) + V); }
 static void or16(uint8_t *P, uint16_t V) { write16le(P, read16le(P) | V); }
 
-static void applySecRel(const SectionChunk *Sec, uint8_t *Off, Defined *Sym) {
-  // Don't apply section relative relocations to absolute symbols in codeview
-  // debug info sections. MSVC does not treat such relocations as fatal errors,
-  // and they can be found in the standard library for linker-provided symbols
-  // like __guard_fids_table and __safe_se_handler_table.
-  if (!(isa<DefinedAbsolute>(Sym) && Sec->isCodeView()))
-    add32(Off, Sym->getSecrel());
+static void applySecRel(const SectionChunk *Sec, uint8_t *Off,
+                        OutputSection *OS, uint64_t S) {
+  if (!OS) {
+    if (Sec->isCodeView())
+      return;
+    fatal("SECREL relocation cannot be applied to absolute symbols");
+  }
+  uint64_t SecRel = S - OS->getRVA();
+  assert(SecRel < INT32_MAX && "overflow in SECREL relocation");
+  add32(Off, SecRel);
 }
 
-void SectionChunk::applyRelX64(uint8_t *Off, uint16_t Type, Defined *Sym,
-                               uint64_t P) const {
-  uint64_t S = Sym->getRVA();
+static void applySecIdx(uint8_t *Off, OutputSection *OS) {
+  // If we have no output section, this must be an absolute symbol. Use the
+  // sentinel absolute symbol section index.
+  uint16_t SecIdx = OS ? OS->SectionIndex : DefinedAbsolute::OutputSectionIndex;
+  add16(Off, SecIdx);
+}
+
+void SectionChunk::applyRelX64(uint8_t *Off, uint16_t Type, OutputSection *OS,
+                               uint64_t S, uint64_t P) const {
   switch (Type) {
   case IMAGE_REL_AMD64_ADDR32:   add32(Off, S + Config->ImageBase); break;
   case IMAGE_REL_AMD64_ADDR64:   add64(Off, S + Config->ImageBase); break;
   case IMAGE_REL_AMD64_ADDR32NB: add32(Off, S); break;
   case IMAGE_REL_AMD64_REL32:    add32(Off, S - P - 4); break;
   case IMAGE_REL_AMD64_REL32_1:  add32(Off, S - P - 5); break;
   case IMAGE_REL_AMD64_REL32_2:  add32(Off, S - P - 6); break;
   case IMAGE_REL_AMD64_REL32_3:  add32(Off, S - P - 7); break;
   case IMAGE_REL_AMD64_REL32_4:  add32(Off, S - P - 8); break;
   case IMAGE_REL_AMD64_REL32_5:  add32(Off, S - P - 9); break;
-  case IMAGE_REL_AMD64_SECTION:  add16(Off, Sym->getSectionIndex()); break;
-  case IMAGE_REL_AMD64_SECREL:   applySecRel(this, Off, Sym); break;
+  case IMAGE_REL_AMD64_SECTION:  applySecIdx(Off, OS); break;
+  case IMAGE_REL_AMD64_SECREL:   applySecRel(this, Off, OS, S); break;
   default:
     fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
   }
 }
 
-void SectionChunk::applyRelX86(uint8_t *Off, uint16_t Type, Defined *Sym,
-                               uint64_t P) const {
-  uint64_t S = Sym->getRVA();
+void SectionChunk::applyRelX86(uint8_t *Off, uint16_t Type, OutputSection *OS,
+                               uint64_t S, uint64_t P) const {
   switch (Type) {
   case IMAGE_REL_I386_ABSOLUTE: break;
   case IMAGE_REL_I386_DIR32:    add32(Off, S + Config->ImageBase); break;
   case IMAGE_REL_I386_DIR32NB:  add32(Off, S); break;
   case IMAGE_REL_I386_REL32:    add32(Off, S - P - 4); break;
-  case IMAGE_REL_I386_SECTION:  add16(Off, Sym->getSectionIndex()); break;
-  case IMAGE_REL_I386_SECREL:   applySecRel(this, Off, Sym); break;
+  case IMAGE_REL_I386_SECTION:  applySecIdx(Off, OS); break;
+  case IMAGE_REL_I386_SECREL:   applySecRel(this, Off, OS, S); break;
   default:
     fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
   }
 }
 
 static void applyMOV(uint8_t *Off, uint16_t V) {
   write16le(Off, (read16le(Off) & 0xfbf0) | ((V & 0x800) >> 1) | ((V >> 12) & 0xf));
   write16le(Off + 2, (read16le(Off + 2) & 0x8f00) | ((V & 0x700) << 4) | (V & 0xff));
static void applyBranch24T(uint8_t *Off, int32_t V) {
   uint32_t S = V < 0 ? 1 : 0;
   uint32_t J1 = ((~V >> 23) & 1) ^ S;
   uint32_t J2 = ((~V >> 22) & 1) ^ S;
   or16(Off, (S << 10) | ((V >> 12) & 0x3ff));
   // Clear out the J1 and J2 bits which may be set.
   write16le(Off + 2, (read16le(Off + 2) & 0xd000) | (J1 << 13) | (J2 << 11) | ((V >> 1) & 0x7ff));
 }
 
-void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, Defined *Sym,
-                               uint64_t P) const {
-  uint64_t S = Sym->getRVA();
+void SectionChunk::applyRelARM(uint8_t *Off, uint16_t Type, OutputSection *OS,
+                               uint64_t S, uint64_t P) const {
   // Pointer to thumb code must have the LSB set.
-  if (Sym->isExecutable())
-    S |= 1;
+  uint64_t SX = S;
+  if (OS && (OS->getPermissions() & IMAGE_SCN_MEM_EXECUTE))
+    SX |= 1;
   switch (Type) {
-  case IMAGE_REL_ARM_ADDR32:    add32(Off, S + Config->ImageBase); break;
-  case IMAGE_REL_ARM_ADDR32NB:  add32(Off, S); break;
-  case IMAGE_REL_ARM_MOV32T:    applyMOV32T(Off, S + Config->ImageBase); break;
-  case IMAGE_REL_ARM_BRANCH20T: applyBranch20T(Off, S - P - 4); break;
-  case IMAGE_REL_ARM_BRANCH24T: applyBranch24T(Off, S - P - 4); break;
-  case IMAGE_REL_ARM_BLX23T:    applyBranch24T(Off, S - P - 4); break;
-  case IMAGE_REL_ARM_SECREL:    applySecRel(this, Off, Sym); break;
+  case IMAGE_REL_ARM_ADDR32:    add32(Off, SX + Config->ImageBase); break;
+  case IMAGE_REL_ARM_ADDR32NB:  add32(Off, SX); break;
+  case IMAGE_REL_ARM_MOV32T:    applyMOV32T(Off, SX + Config->ImageBase); break;
+  case IMAGE_REL_ARM_BRANCH20T: applyBranch20T(Off, SX - P - 4); break;
+  case IMAGE_REL_ARM_BRANCH24T: applyBranch24T(Off, SX - P - 4); break;
+  case IMAGE_REL_ARM_BLX23T:    applyBranch24T(Off, SX - P - 4); break;
+  case IMAGE_REL_ARM_SECTION:   applySecIdx(Off, OS); break;
+  case IMAGE_REL_ARM_SECREL:    applySecRel(this, Off, OS, S); break;
   default:
     fatal("unsupported relocation type 0x" + Twine::utohexstr(Type));
   }
 }
 
 void SectionChunk::writeTo(uint8_t *Buf) const {
   if (!hasData())
     return;
   // Copy section contents from source object file to output file.
   ArrayRef<uint8_t> A = getContents();
   memcpy(Buf + OutputSectionOff, A.data(), A.size());
 
   // Apply relocations.
   for (const coff_relocation &Rel : Relocs) {
     uint8_t *Off = Buf + OutputSectionOff + Rel.VirtualAddress;
+
+    // Get the output section of the symbol for this relocation.  The output
+    // section is needed to compute SECREL and SECTION relocations used in debug
+    // info.
     SymbolBody *Body = File->getSymbolBody(Rel.SymbolTableIndex);
     Defined *Sym = cast<Defined>(Body);
+    Chunk *C = Sym->getChunk();
+    OutputSection *OS = C ? C->getOutputSection() : nullptr;
+
+    // Only absolute and __ImageBase symbols lack an output section. For any
+    // other symbol, this indicates that the chunk was discarded.  Normally
+    // relocations against discarded sections are an error.  However, debug info
+    // sections are not GC roots and can end up with these kinds of relocations.
+    // Skip these relocations.
+    if (!OS && !isa<DefinedAbsolute>(Sym) && !isa<DefinedSynthetic>(Sym)) {
+      if (isCodeView())
+        continue;
+      fatal("relocation against symbol in discarded section: " +
+            Sym->getName());
+    }
+    uint64_t S = Sym->getRVA();
+
+    // Compute the RVA of the relocation for relative relocations.
     uint64_t P = RVA + Rel.VirtualAddress;
     switch (Config->Machine) {
     case AMD64:
-      applyRelX64(Off, Rel.Type, Sym, P);
+      applyRelX64(Off, Rel.Type, OS, S, P);
       break;
     case I386:
-      applyRelX86(Off, Rel.Type, Sym, P);
+      applyRelX86(Off, Rel.Type, OS, S, P);
       break;
     case ARMNT:
-      applyRelARM(Off, Rel.Type, Sym, P);
+      applyRelARM(Off, Rel.Type, OS, S, P);
       break;
     default:
       llvm_unreachable("unknown machine type");
     }
   }
 }
 
 void SectionChunk::addAssociative(SectionChunk *Child) {

lld/trunk/COFF/MarkLive.cpp

void markLive(const std::vector<Chunk *> &Chunks) {
   };
 
   // Add GC root chunks.
   for (SymbolBody *B : Config->GCRoot)
     AddSym(B);
 
   while (!Worklist.empty()) {
     SectionChunk *SC = Worklist.pop_back_val();
+
+    // If this section was discarded, there are relocations referring to
+    // discarded sections. Ignore these sections to avoid crashing. They will be
+    // diagnosed during relocation processing.
+    if (SC->isDiscarded())
+      continue;
+
     assert(SC->isLive() && "We mark as live when pushing onto the worklist!");
 
     // Mark all symbols listed in the relocation table for this section.
     for (SymbolBody *B : SC->symbols())
       AddSym(B);
 
     // Mark associative sections if any.
     for (SectionChunk *C : SC->children())
       Enqueue(C);
   }
 }
 
 }
 }

lld/trunk/COFF/Symbols.h

public:
   static bool classof(const SymbolBody *S) {
     return S->kind() <= LastDefinedKind;
   }
 
   // Returns the RVA (relative virtual address) of this symbol. The
   // writer sets and uses RVAs.
   uint64_t getRVA();
 
-  // Returns the RVA relative to the beginning of the output section.
-  // Used to implement SECREL relocation type.
-  uint32_t getSecrel();
-
-  // Returns the output section index.
-  // Used to implement SECTION relocation type.
-  uint16_t getSectionIndex();
-
-  // Returns true if this symbol points to an executable (e.g. .text) section.
-  // Used to implement ARM relocations.
-  bool isExecutable();
+  // Returns the chunk containing this symbol. Absolute symbols and __ImageBase
+  // do not have chunks, so this may return null.
+  Chunk *getChunk();
 };
 
 // Symbols defined via a COFF object file or bitcode file.  For COFF files, this
 // stores a coff_symbol_generic*, and names of internal symbols are lazily
 // loaded through that. For bitcode files, Sym is nullptr and the name is stored
 // as a StringRef.
 class DefinedCOFF : public Defined {
   friend SymbolBody;
public:
   static bool classof(const SymbolBody *S) {
     return S->kind() == DefinedRegularKind;
   }
 
   uint64_t getRVA() { return (*Data)->getRVA() + Sym->Value; }
   bool isCOMDAT() { return IsCOMDAT; }
   SectionChunk *getChunk() { return *Data; }
   uint32_t getValue() { return Sym->Value; }
-  uint32_t getSecrel();
 
 private:
   SectionChunk **Data;
 };
 
 class DefinedCommon : public DefinedCOFF {
 public:
   DefinedCommon(InputFile *F, StringRef N, uint64_t Size,
                 const coff_symbol_generic *S = nullptr,
                 CommonChunk *C = nullptr)
       : DefinedCOFF(DefinedCommonKind, F, N, S), Data(C), Size(Size) {
     this->IsExternal = true;
   }
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == DefinedCommonKind;
   }
 
   uint64_t getRVA() { return Data->getRVA(); }
-  uint32_t getSecrel() { return Data->OutputSectionOff; }
-  uint16_t getSectionIndex();
+  Chunk *getChunk() { return Data; }
 
 private:
   friend SymbolTable;
   uint64_t getSize() const { return Size; }
   CommonChunk *Data;
   uint64_t Size;
 };
 
public:
 
   uint64_t getRVA() { return VA - Config->ImageBase; }
   void setVA(uint64_t V) { VA = V; }
 
   // The sentinel absolute symbol section index. Section index relocations
   // against absolute symbols resolve to this 16 bit number, and it is the
   // largest valid section index plus one. This is written by the Writer.
   static uint16_t OutputSectionIndex;
+  uint16_t getSecIdx() { return OutputSectionIndex; }
 
 private:
   uint64_t VA;
 };
 
 // This symbol is used for linker-synthesized symbols like __ImageBase and
 // __safe_se_handler_table.
 class DefinedSynthetic : public Defined {
 public:
   explicit DefinedSynthetic(StringRef Name, Chunk *C)
       : Defined(DefinedSyntheticKind, Name), C(C) {}
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == DefinedSyntheticKind;
   }
 
   // A null chunk indicates that this is __ImageBase. Otherwise, this is some
   // other synthesized chunk, like SEHTableChunk.
-  uint32_t getRVA() const { return C ? C->getRVA() : 0; }
-  uint32_t getSecrel() const { return C ? C->OutputSectionOff : 0; }
-  Chunk *getChunk() const { return C; }
+  uint32_t getRVA() { return C ? C->getRVA() : 0; }
+  Chunk *getChunk() { return C; }
 
 private:
   Chunk *C;
 };
 
 // This class represents a symbol defined in an archive file. It is
 // created from an archive file header, and it knows how to load an
 // object file from an archive to replace itself with a defined
DefinedImportData(StringRef N, ImportFile *F)
       : Defined(DefinedImportDataKind, N), File(F) {
   }
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == DefinedImportDataKind;
   }
 
   uint64_t getRVA() { return File->Location->getRVA(); }
+  Chunk *getChunk() { return File->Location; }
+  void setLocation(Chunk *AddressTable) { File->Location = AddressTable; }
+
   StringRef getDLLName() { return File->DLLName; }
   StringRef getExternalName() { return File->ExternalName; }
-  void setLocation(Chunk *AddressTable) { File->Location = AddressTable; }
   uint16_t getOrdinal() { return File->Hdr->OrdinalHint; }
 
   ImportFile *File;
 };
 
 // This class represents a symbol for a jump table entry which jumps
 // to a function in a DLL. Linker are supposed to create such symbols
 // without "__imp_" prefix for all function symbols exported from
case DefinedRegularKind:
     return cast<DefinedRegular>(this)->getRVA();
   case LazyKind:
   case UndefinedKind:
     llvm_unreachable("Cannot get the address for an undefined symbol.");
   }
   llvm_unreachable("unknown symbol kind");
 }
 
+inline Chunk *Defined::getChunk() {
+  switch (kind()) {
+  case DefinedRegularKind:
+    return cast<DefinedRegular>(this)->getChunk();
+  case DefinedAbsoluteKind:
+    return nullptr;
+  case DefinedSyntheticKind:
+    return cast<DefinedSynthetic>(this)->getChunk();
+  case DefinedImportDataKind:
+    return cast<DefinedImportData>(this)->getChunk();
+  case DefinedImportThunkKind:
+    return cast<DefinedImportThunk>(this)->getChunk();
+  case DefinedLocalImportKind:
+    return cast<DefinedLocalImport>(this)->getChunk();
+  case DefinedCommonKind:
+    return cast<DefinedCommon>(this)->getChunk();
+  case LazyKind:
+  case UndefinedKind:
+    llvm_unreachable("Cannot get the chunk of an undefined symbol.");
+  }
+  llvm_unreachable("unknown symbol kind");
+}
+
 // A real symbol object, SymbolBody, is usually stored within a Symbol. There's
 // always one Symbol for each symbol name. The resolver updates the SymbolBody
 // stored in the Body field of this object as it resolves symbols. Symbol also
 // holds computed properties of symbol names.
 struct Symbol {
   // True if this symbol was referenced by a regular (non-bitcode) object.
   unsigned IsUsedInRegularObj : 1;
 

lld/trunk/COFF/Writer.cpp

if (StringTableOff) {
     sprintf(Hdr->Name, "/%d", StringTableOff);
   } else {
     assert(!Config->Debug || Name.size() <= COFF::NameSize);
     strncpy(Hdr->Name, Name.data(),
             std::min(Name.size(), (size_t)COFF::NameSize));
   }
 }
 
-uint32_t Defined::getSecrel() {
-  assert(this);
-  switch (kind()) {
-  case DefinedRegularKind:
-    return cast<DefinedRegular>(this)->getSecrel();
-  case DefinedCommonKind:
-    return cast<DefinedCommon>(this)->getSecrel();
-  case DefinedSyntheticKind:
-    return cast<DefinedSynthetic>(this)->getSecrel();
-  default:
-    break;
-  }
-  fatal("SECREL relocation points to a non-regular symbol: " + toString(*this));
-}
-
-uint32_t DefinedRegular::getSecrel() {
-  assert(getChunk()->isLive() && "relocation against discarded section");
-  uint64_t Diff = getRVA() - getChunk()->getOutputSection()->getRVA();
-  assert(Diff < UINT32_MAX && "section offset too large");
-  return (uint32_t)Diff;
-}
-
-uint16_t Defined::getSectionIndex() {
-  if (auto *D = dyn_cast<DefinedRegular>(this))
-    return D->getChunk()->getOutputSection()->SectionIndex;
-  if (isa<DefinedAbsolute>(this))
-    return DefinedAbsolute::OutputSectionIndex;
-  if (auto *D = dyn_cast<DefinedCommon>(this))
-    return D->getSectionIndex();
-  if (auto *D = dyn_cast<DefinedSynthetic>(this)) {
-    if (!D->getChunk())
-      return 0;
-    return D->getChunk()->getOutputSection()->SectionIndex;
-  }
-  fatal("SECTION relocation points to a non-regular symbol: " +
-        toString(*this));
-}
-
-uint16_t DefinedCommon::getSectionIndex() {
-  return Data->getOutputSection()->SectionIndex;
-}
-
-bool Defined::isExecutable() {
-  const auto X = IMAGE_SCN_MEM_EXECUTE;
-  if (auto *D = dyn_cast<DefinedRegular>(this))
-    return D->getChunk()->getOutputSection()->getPermissions() & X;
-  return isa<DefinedImportThunk>(this);
-}
-
 } // namespace coff
 } // namespace lld
 
 // The main function of the writer.
 void Writer::run() {
   createSections();
   createMiscChunks();
   createImportTables();

lld/trunk/test/COFF/Inputs/pdb-global-gc.s

+.section .data,"dw",one_only,__wc_mb_cur
+.global __wc_mb_cur
+__wc_mb_cur:
+.long 42

lld/trunk/test/COFF/pdb-global-gc.yaml

+# RUN: yaml2obj %s -o %t.obj
+# RUN: llvm-mc %S/Inputs/pdb-global-gc.s -triple x86_64-windows-msvc -filetype=obj -o %t2.obj
+# RUN: lld-link %t.obj %t2.obj -debug -entry:main \
+# RUN:          -nodefaultlib -debug -out:%t.exe -pdb:%t.pdb -verbose
+# RUN: llvm-pdbutil dump -symbols %t.pdb | FileCheck %s
+
+# This tests the case where an __imp_ chunk is discarded by linker GC. The debug
+# info may refer to the __imp_ symbol still.
+
+# Compile this code with MSVC to regenerate the test case:
+#   extern char __declspec(dllimport) __wc_mb_cur;
+#   int discarded() { return __wc_mb_cur; }
+#   int main() { return g2; }
+
+# CHECK:                           Symbols
+# CHECK: ============================================================
+# CHECK:   Mod 0000 | `{{.*}}pdb-global-gc.yaml.tmp.obj`:
+# CHECK:   - S_GDATA32 [size = 28] `__wc_mb_cur`
+# CHECK-NEXT:       type = 0x0070 (char), addr = 0000:0000
+# CHECK:   Mod 0001 | `{{.*}}pdb-global-gc.yaml.tmp2.obj`:
+# CHECK:   Mod 0002 | `* Linker *`:
+
+--- !COFF
+header:          
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:        
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    Subsections:     
+      - !Symbols
+        Records:         
+          - Kind:            S_GDATA32
+            DataSym:         
+              Type:            112
+              DisplayName:     __wc_mb_cur
+      - !StringTable
+        Strings:         
+    Relocations:     
+      - VirtualAddress:  20
+        SymbolName:      __wc_mb_cur
+        Type:            IMAGE_REL_AMD64_SECREL
+      - VirtualAddress:  24
+        SymbolName:      __wc_mb_cur
+        Type:            IMAGE_REL_AMD64_SECTION
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     0FBE0500000000C3
+    Relocations:     
+      - VirtualAddress:  3
+        SymbolName:      __wc_mb_cur
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     B82A000000C3
+symbols:         
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          240
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          11
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        2906070869
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          6
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2139436471
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            discarded
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            main
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            __wc_mb_cur
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

lld/trunk/test/COFF/pdb-import-gc.yaml

+# RUN: yaml2obj %s -o %t.obj
+# RUN: lld-link %t.obj %S/Inputs/pdb-import-gc.lib -debug -entry:main \
+# RUN:          -nodefaultlib -debug -out:%t.exe -pdb:%t.pdb
+# RUN: llvm-pdbutil dump -symbols %t.pdb | FileCheck %s
+
+# This tests the case where an __imp_ chunk is discarded by linker GC. The debug
+# info may refer to the __imp_ symbol still.
+
+# Compile this code with MSVC to regenerate the test case:
+#   extern char __declspec(dllimport) __wc_mb_cur;
+#   int discarded() { return __wc_mb_cur; }
+#   int main() { return g2; }
+
+# CHECK:                           Symbols
+# CHECK: ============================================================
+# CHECK:   Mod 0000 | `{{.*}}pdb-import-gc.yaml.tmp.obj`:
+# CHECK:   - S_GDATA32 [size = 32] `__imp___wc_mb_cur`
+# CHECK-NEXT:       type = 0x0070 (char), addr = 0000:0000
+# CHECK:   Mod 0001 | `* Linker *`:
+
+--- !COFF
+header:          
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:        
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    Subsections:     
+      - !Symbols
+        Records:         
+          - Kind:            S_GDATA32
+            DataSym:         
+              Type:            112
+              DisplayName:     __imp___wc_mb_cur
+      - !StringTable
+        Strings:         
+    Relocations:     
+      - VirtualAddress:  20
+        SymbolName:      __imp___wc_mb_cur
+        Type:            IMAGE_REL_AMD64_SECREL
+      - VirtualAddress:  24
+        SymbolName:      __imp___wc_mb_cur
+        Type:            IMAGE_REL_AMD64_SECTION
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     488B05000000000FBE00C3
+    Relocations:     
+      - VirtualAddress:  3
+        SymbolName:      __imp___wc_mb_cur
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     B82A000000C3
+symbols:         
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          240
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          11
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        2906070869
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          6
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        2139436471
+      Number:          0
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            discarded
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            main
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            __imp___wc_mb_cur
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

lld/trunk/test/COFF/reloc-discarded.s

+# RUN: echo -e '.section .bss,"bw",discard,main_global\n.global main_global\n main_global:\n .long 0' | \
+# RUN:     llvm-mc - -filetype=obj -o %t1.obj -triple x86_64-windows-msvc
+# RUN: llvm-mc %s -filetype=obj -o %t2.obj -triple x86_64-windows-msvc
+
+# LLD should report an error and not assert regardless of whether we are doing
+# GC.
+
+# RUN: not lld-link -entry:main -nodefaultlib %t1.obj %t2.obj -out:%t.exe -opt:ref   2>&1 | FileCheck %s
+# RUN: not lld-link -entry:main -nodefaultlib %t1.obj %t2.obj -out:%t.exe -opt:noref 2>&1 | FileCheck %s
+
+# CHECK: error: relocation against symbol in discarded section: assoc_global
+
+  .section  .bss,"bw",discard,main_global
+  .globl  main_global
+  .p2align  2
+main_global:
+  .long 0
+
+  .section  .CRT$XCU,"dr",associative,main_global
+  .p2align  3
+  .globl assoc_global
+assoc_global:
+  .quad main_global
+
+  .text
+  .globl main
+main:
+  movq assoc_global(%rip), %rax
+  movl (%rax), %eax
+  retq

lld/trunk/test/COFF/secrel-absolute.s

 # RUN: llvm-mc %s -filetype=obj -triple=x86_64-windows-msvc -o %t.obj
 # RUN: not lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe 2>&1 | FileCheck %s
 
 # secrel relocations against absolute symbols are errors.
 
-# CHECK: SECREL relocation points to a non-regular symbol: __guard_fids_table
+# CHECK: SECREL relocation cannot be applied to absolute symbols
 
 .text
 .global main
 main:
 ret
 
 .section .rdata,"dr"
 .secrel32 __guard_fids_table