[PATCH] D54495: [LLD] [COFF] Remove empty sections before calculating the size of section headers

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Writer.cpp:888-892
+  auto IsEmpty = [](OutputSection *S) {
+    for (Chunk *C : S->Chunks)
+      if (C->getSize() > 0)
+        return false;
+    return true;
----------------
ruiu wrote:
> mstorsjo wrote:
> > ruiu wrote:
> > > It feels like `S->getVirtualSize()` should return 0 if all sections in it are empty. Can you fix the code computing output section size instead of computing a bogus value as a size and then remove such section here?
> > `S->getVirtualSize()` doesn't do any calculation at all, it only returns `Header.VirtualSize` (where `Header` is a `llvm::object::coff_section` which will be written to disk as is). If we'd make `getVirtualSize()` actually do the calculation, it'd redo it every time we query it later.
> > 
> > Or should we move this into a `isEmpty()` helper method in `OutputSection` instead?
> Yeah I know it is an accessor, but I wonder why we compute a non-zero value as a size even if all its members are of size zero.
We don't compute a non-zero value. If I switch execution order so that we run `removeEmptySections()` before `finalizeAddresses()`, we haven't actually calculated that value yet, so all sections have VirtualSize set to 0 still.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D54495