This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [COFF] Sort .relocs before all other discardable sections
ClosedPublic

Authored by mstorsjo on Jul 15 2018, 1:38 PM.

Details

Summary

If a binary is stripped, which can remove discardable sections (except for the .relocs section, which also is marked as discardable as it isn't loaded at runtime, only read by the loader), the .relocs section should be first of them, in order not to create gaps in the image.

Previously, binaries with relocations were broken if they were stripped by GNU binutils strip. Trying to execute such binaries produces an error about "xx is not a valid win32 application".

This fixes GNU binutils bug 23348.

Prior to SVN r329370 (which didn't intend to have functional changes), the code for moving discardable sections to the end didn't clearly express how other discardable sections should be ordered compared to .relocs, even though the change probably retained the same end result as before.

After SVN r329370, the code (and comments) more clearly indicate that it tries to make the .relocs section the absolutely last one; this patch changes that.

This matches how GNU binutils ld sorts .relocs compared to dwarf debug info sections.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Jul 15 2018, 1:38 PM
pcc added inline comments.Jul 18 2018, 3:16 PM
COFF/Writer.cpp
479 ↗(On Diff #155595)

I think you can make this

if (S->Header.Characteristics & IMAGE_SCN_MEM_DISCARDABLE)
  return 2;

and remove the part about RelocSec. The .reloc section will be sorted at the beginning of the discardable sections because we create it explicitly above (this is the same way that we ensure that the sections appear in the same order as link.exe); you can move your comment there.

mstorsjo updated this revision to Diff 156209.Jul 18 2018, 9:34 PM

Applied @pcc's suggestion

pcc accepted this revision.Jul 20 2018, 10:03 AM

LGTM

This revision is now accepted and ready to land.Jul 20 2018, 10:03 AM
This revision was automatically updated to reflect the committed changes.