One place where this seems to matter is to make sure the .rsrc section comes after .text. The Win32 UpdateResource() function can change the contents of .rsrc. It will move the sections that come after, but if .text gets moved, the entry point header will not get updated and the executable breaks. This was found by a test in Chromium.
Details
Diff Detail
- Repository
- rLLD LLVM Linker
Event Timeline
COFF/Writer.cpp | ||
---|---|---|
678 | 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. |
COFF/Writer.cpp | ||
---|---|---|
648–649 | std::vector<StringRef> is perhaps better. | |
651 | i -> I | |
678 | 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. |
Peter has a good point that .rsrc should come after any sections not named in the list in my first patch. Also we should probably make sure .reloc really comes last too.
Rather than just moving .rsrc to the end though, I think there is a point in trying to match link.exe's section order.
I've uploaded a new patch version. Please let me know what you think.
Feel free to let me know if this comment is off topic here, but I was just investigating a similar issue. I have an object file with this section list:
0 .text 0000001c 0000000000000000 TEXT 1 .data 00000000 0000000000000000 DATA 2 .bss 00000000 0000000000000000 BSS 3 .xdata 00000008 0000000000000000 DATA 4 .drectve 00000030 0000000000000000 5 .debug$S 00000144 0000000000000000 DATA 6 .debug$T 00000044 0000000000000000 DATA 7 .pdata 0000000c 0000000000000000 DATA
When I link this with lld, I get this section list:
0 .pdata 0000000c 0000000140001000 DATA 1 .text 0000001c 0000000140002000 TEXT 2 .xdata 00000008 0000000140003000 DATA 3 .rdata 00000063 0000000140004000 DATA
When I link with link.exe, I get this section list:
0 .text 0000001c 0000000140001000 TEXT 1 .rdata 00000134 0000000140002000 DATA 2 .pdata 0000000c 0000000140003000 DATA
First of all, using link.exe .pdata comes at the end (or maybe more accurately, it appears in the same order as it does in the object file).
Secondly, we are outputting a .xdata section where link.exe is not.
Are these fixes also within the scope of this patch?
LGTM
COFF/Writer.cpp | ||
---|---|---|
660–662 | This is a nit but I'd perhaps use 99, 100 and 50 or something like that for these numbers instead of super large numbers. |
Oh I didn't notice that the discussion was going on while reviewing.
As to mimicking the order of MSVC linker's section output, it is indeed a curgo cult, but I'd probably do that if I start writing a Windows linker now, as the format is not entirely documented, and many tools are proprietary and can't change.
COFF/Writer.cpp | ||
---|---|---|
661 | Have you confirmed that UpdateResource updates the .reloc address in the data directory if it is moved? |
COFF/Writer.cpp | ||
---|---|---|
661 | Yes it looks like it does (I compared Chromium's mini_installer.exe and next_version_mini_installer.exe) |
I'll go ahead and commit. Please feel free to follow up in your timezone if you like to make any changes.
std::vector<StringRef> is perhaps better.