[PATCH] D74887: [ELF] Shuffle .init_array/.fini_array with --shuffle-sections=

[Rafael Avila de Espindola via Phabricator via llvm-commits]

respindola added inline comments.


================
Comment at: lld/ELF/Writer.cpp:1364
+  // by --symbol-ordering-file or --shuffle-sections=.
+  if (!order.empty())
+    for (BaseCommand *b : sec->sectionCommands)
----------------
MaskRay wrote:
> respindola wrote:
> > MaskRay wrote:
> > > respindola wrote:
> > > > This will break code that uses init_priority, no?
> > > > 
> > > > I think we can only shuffle sections of equal priority.
> > > > 
> > > Priorities are respected. Sections with the same priority can be shuffled.
> > > 
> > > See the test below, `.init_array.1` is the first.
> > > 
> > > ```
> > > # CHECK:      Hex dump of section '.init_array'
> > > # CHECK-NEXT: 0x{{[0-9a-f]+}} ff
> > > # ORDERED-SAME: 000102 03040506 0708090a 0b
> > > # SHUFFLED-NOT: 000102 03040506 0708090a 0b
> > > ```
> > Oh, I see. I was somehow reading a return after sortISDBySectionOrder, sorry.
> > What happens is that we first shuffle and then depend on functions like sortInitFini producing a valid order. Sounds reasonable.
> > 
> > What about the script->hasSectionsCommand case? Don't we have to skip sortISDBySectionOrder in when it is true?
> > 
> I think we can also sort for the `script->hasSectionsCommand` case. All priorities are ignored. All SHT_INIT_ARRAY sections are shuffled together.
> 
> Added a test case.
Sounds reasonable. When using sections the linker script is probably what is responsible for producing the right order and we only sort inside each ISD.

This LGTM, but should probably also get approved by someone more recently active in lld.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74887/new/

https://reviews.llvm.org/D74887