[PATCH] D45260: COFF: Layout sections in the same order as link.exe

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: COFF/Writer.cpp:670
+  // Put the sections in an order that matches what link.exe does. In
+  // particular, .rsrc must come after .text, otherwise the entry point could
+  // end up referring to garbage after .rsrc is updated with UpdateResource and
----------------
ruiu wrote:
> pcc wrote:
> > pcc wrote:
> > > What if I have a `.rsrc` section as well as a section whose name does not appear in the list above? It seems like this could still lay out `.rsrc` before my section, which would break things.
> > > 
> > > To me it seems like the correct fix would be to just move `.rsrc` to the end.
> > That is what D45273 does.
> I think I agree with Peter about ".rsrc" section ordering, but I believe sorting all output sections in the same order as MSVC does is beneficial if there's no reason to keep the current section ordering. Changing section ordering shouldn't affect anything, but in reality it should reduce "surprise" like this.
Not sure that I agree, it seems a little cargo cultish to me. But I'll leave it up to you folks.


================
Comment at: COFF/Writer.cpp:688
   // the loader cannot handle holes.
   std::stable_partition(
       OutputSections.begin(), OutputSections.end(), [](OutputSection *S) {
----------------
You can probably merge this into the stable_sort like I was doing in D45273.


https://reviews.llvm.org/D45260