[PATCH] D49366: Add support for /FUNCTIONPADMIN command-line option

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

Unless I'm missing something, doesn't this patch end up adding padding to the end of every code section rather than the beginning?

It seems like this ought to be implemented by changing the section layout code. I'm thinking that you would add code to http://llvm-cs.pcc.me.uk/tools/lld/COFF/Writer.cpp#742 that does this:

  if (C requires padding) {
    RawSize += Padding;
    VirtualSize += Padding;
  }

With that I don't think you need to add fields for storing the amount of padding required, you can just figure it out on the fly there.



================
Comment at: COFF/DriverUtils.cpp:271-273
+    } else {
+      error("/functionpadmin: invalid argument for this machine: " + Arg);
+    }
----------------
stefan_reinalter wrote:
> zturner wrote:
> > What about ARM64?  The documentation doesn't say what the default value is here, but it should be easy to figure out.
> Understood, I will try figuring this out.
And ARM32.


================
Comment at: COFF/PDB.cpp:790
+    // without any padding to mimic the behaviour of link.exe.
+    SC.Size = SecChunk->getRawSize();
+
----------------
stefan_reinalter wrote:
> zturner wrote:
> > That is quite odd.  Wouldn't this mean that the PDB has incorrect info?  I wonder how this could possibly be correct.  So either it's a bug in link.exe or there's something I don't understand about what PDB conumers use this information for.
> I don't think this means that the PDB has incorrect info, but rather that the padding bytes are not accounted for anywhere.
> E.g. take a function SimpleFunction at RVA 0x3190:
> 
> The public symbol stored will hold its address:
> PublicSymbol: [00003190][0001:00002190] ?SimpleFunction@@YAXXZ(void __cdecl SimpleFunction(void))
> 
> The function stored inside the compiland will hold the following info:
> Function       : static, [00003190][0001:00002190], len = 00000020, void __cdecl SimpleFunction(void)
> FuncDebugStart :   static, [00003194][0001:00002194]
> FuncDebugEnd   :   static, [000031A7][0001:000021A7]
> 
> This stores the address, the length, and end of the prologue as well as start of the epilogue. None of these are concerned about padding bytes. The contribution also shows 0x20 bytes being contributed, which does *not* include padding bytes:
> 00003190  0001:00002190  00000020  C:\SomePath\SimpleFunction.obj
> 
> The padding bytes between functions are 'unaccounted' for, so to speak.
> I think this is by design, otherwise you'd get different info from the compiland/function stream and the contribution stream.
> 
> Note that padding bytes are inserted in front of functions (=between section chunks), but function entry points/RVAs still point at the first code byte, not any padding.
> 
If you move the implementation of `/functionpadmin` to section layout I don't think you would need to worry about that here.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D49366