[PATCH] D82883: [LLD][COFF] Deduplicate .pdata entries

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D82883#2147461 <https://reviews.llvm.org/D82883#2147461>, @aganea wrote:

> @mstorsjo This happens on VS2019 libs. I don't know if this happens on VS2017. I don't have an exact repro, but the test below exhibits the same effect as the problem I was observing. I will dig more into this.


Right, so in the object file bits that end up linked in statically? Interesting.

What does MSVC link.exe do in that case - does it also dedup them, or keep the duplicates?



================
Comment at: lld/COFF/Writer.cpp:1869
+    // the PE image, but that should be fine.
+    memset(end - removed, 0, removed * sizeof(Entry));
+
----------------
aganea wrote:
> mstorsjo wrote:
> > Would it be possible to adjust the virtual size of the .pdata section as well, to exclude the pruned bits? I could imagine that some runtime introspection tools locate the section directly instead of using the data directory.
> Unfortunately the .pdata is merged into .rdata, there're other things following the merged .pdata in the output, and at this point all the RVAs have been calculated and sections already merged & written.
> 
> I've tried pruning earlier, but it's a tail biting snake. We need the entire merged, reallocated & sorted .pdata stream, and the computed RVAs, but we don't have that unless all sections have been written. But once they're written, we can't prune them, unless we would re-write again.
> 
> If we did a two-step "writeSection" and perhaps computed everything in advance, before writing, it could maybe be possible to resize the output section, but that would involve some sizeable refactoring. I figured it wasn't worth it, but if anyone has a different view on this, please let me know.
Right, I agree it's probably a sensible tradeoff to keep it like this - it's probably not worth rearchitecting things just to be able to prune it better.

To see that I understand the issue correctly: Before we've fixed the layout and set RVAs, the .pdata section is unsorted. At that stage, we should still be able to check which pdata entries actually point to the same symbol (although it might be messy, having to dereference the relocations?) - but without sorting it, a duplicate check would end up rather costly - and without RVAs we can't really sort it. Am I understanding the situation correctly?

Given the (presumed) small extent of the issue, this solution does sound like a good tradeoff.

Also, right, if merging .pdata into another section, there's no need to update section sizes (and in that case, a few noop bytes in the middle of the .rdata are just orphaned bytes anyway).

IIRC with mingw setups, .pdata doesn't normally end up merged though. Is it (within reasonable effort) possible to check if `.pdata` is left unmerged, and in that case fix the section size? Or is the association from the enclosing section lost earlier at some point? If not, then this is clearly good enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82883/new/

https://reviews.llvm.org/D82883