[PATCH] D47391: [COFF] Don't crash when emitting symbol table for SafeSEH input files

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: COFF/Writer.cpp:614
   if (auto *D = dyn_cast<DefinedRegular>(Def))
-    if (D->getChunk()->isCodeView())
+    if (D->getChunk()->isCodeView() || D->getChunk()->isSXData())
       return None;
----------------
rnk wrote:
> smeenai wrote:
> > rnk wrote:
> > > smeenai wrote:
> > > > Does this need to be generalized to all RVA table sections, e.g. the control flow guard sections? We aren't using that right now, so it's not an immediate concern, but just wondering in general.
> > > It does. I think these are all the cases where we pull out chunks and set them aside so they don't make it to the final output:
> > >   if (C->isCodeView())
> > >     DebugChunks.push_back(C);
> > >   else if (Config->GuardCF != GuardCFLevel::Off && Name == ".gfids$y")
> > >     GuardFidChunks.push_back(C);
> > >   else if (Config->GuardCF != GuardCFLevel::Off && Name == ".gljmp$y")
> > >     GuardLJmpChunks.push_back(C);
> > >   else if (Name == ".sxdata")
> > >     SXDataChunks.push_back(C);
> > > 
> > > They are still marked as "live" so we can figure out which debug info (or RVA tables) to toss and which to keep, but none of them will ever make it into the output.
> > Okay. Sounds like we should be setting some property on SectionChunk to indicate that, to avoid having to duplicate all that logic in Writer? Something along the lines of WrittenToOutputDirectly (since the section's contents do get written to the output, just in a debug info/RVA table form)? Alternatively we could leave CodeView sections as is and mark the other ones as RVATableChunkInput or something along those lines, and then ignore those sections in the Writer (in addition to the existing ignoring of CodeView sections).
> I think we'd actually want to set that in `Writer::assignAddresses`, since it would subsume the `isLive` check as well.
> 
> Actually, remind me why we can't just use `Def->getChunk()->getOutputSection()` (with some null checks) here? Why are we searching the output sections by RVA anyway? I think it's only a few cases. If there's no OutputSection, then we shouldn't write it to the symbol table.
Good question. I did some archeology, and the search for output sections by RVA actually dates all the way back to https://reviews.llvm.org/D11023, which first introduced symbol tables. Let me try changing that and seeing if tests are happy.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47391