Page MenuHomePhabricator

[ELF] Change --shuffle-sections=<seed> to --shuffle-sections=<section-glob>=<seed>
ClosedPublic

Authored by MaskRay on Mar 15 2021, 8:53 PM.

Details

Summary

--shuffle-sections=<seed> applies to all sections. The new
--shuffle-sections=<section-glob>=<seed> makes shuffling selective. To the
best of my knowledge, the option is only used as debugging, so just drop the
original form.

--shuffle-sections '.init_array*=-1' --shuffle-sections '.fini_array*=-1'.
reverses static constructors/destructors of the same priority.
Useful to detect some static initialization order fiasco.

--shuffle-sections '.data*=-1'
reverses .data* sections. Useful to detect unfunded pointer comparison results
of two unrelated objects.

If certain sections have an intrinsic order, the old form cannot be used.

Diff Detail

Event Timeline

MaskRay created this revision.Mar 15 2021, 8:53 PM
MaskRay requested review of this revision.Mar 15 2021, 8:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2021, 8:53 PM
MaskRay updated this revision to Diff 330876.Mar 15 2021, 8:59 PM

add error tests

I've not got the time to review this whole change right now, but I've checked and it should be safe from our point of view to change the command-line option. An alternative non-breaking approach would be to treat a shuffle-sections argument without an equals sign as the old-style, but that might be unnecessary additional complexity.

lld/docs/ReleaseNotes.rst
33

I think this sounds better.

MaskRay updated this revision to Diff 331032.Mar 16 2021, 10:14 AM
MaskRay marked an inline comment as done.

Rebase

Fix wording

MaskRay updated this revision to Diff 331033.Mar 16 2021, 10:14 AM

fix wording

Harbormaster completed remote builds in B94086: Diff 331033.

Pre-merge bots indicate there's another test using --shuffle-sections which needs updating.

lld/ELF/Driver.cpp
1154

It would probably be helpful if this error message included what was actually the option, in case they have multiple --shuffle-sections, to allow them to find the right one. Example:

error: --shuffle-sections=: expected <section_glob>=<seed>, but was "abcdef"
lld/ELF/Writer.cpp
1301–1303

Was wondering if there was a way to do this loop in a way using the existing built-in algorithms, but the best I've got is the following:

std::copy_if(sections.begin(), sections.end(), std::back_inserter(matched),
      [&](InputSectionBase *sec) { return patAndSeed.first.match(sec->name); });

which doesn't really seem any clearer to me.

lld/test/ELF/shuffle-sections.s
48

Perhaps also --shuffle-sections=a, (i.e. where the '=' is missing for the option value, but the string is non-empty).

MaskRay updated this revision to Diff 331296.Mar 17 2021, 9:46 AM
MaskRay marked 2 inline comments as done.

rebase

address comments

jhenderson added inline comments.Mar 18 2021, 1:01 AM
lld/test/ELF/shuffle-sections.s
51

I think it would be a good idea to check the exact string here, to show we're not printing garbage. You can use FileCheck -D to easily do that without needing to duplicate this line per case.

MaskRay updated this revision to Diff 331482.Mar 18 2021, 1:13 AM

check exact message

MaskRay marked an inline comment as done.Mar 18 2021, 1:13 AM
jhenderson accepted this revision.Mar 18 2021, 1:20 AM

LGTM, thanks!

This revision is now accepted and ready to land.Mar 18 2021, 1:20 AM