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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 21 00:23:34 PST 2020


grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lld/test/ELF/shuffle-sections-init-fini.s:11
+# CHECK:      Hex dump of section '.fini_array'
+# CHECK-NEXT: {{....}}ff
+
----------------
MaskRay wrote:
> grimar wrote:
> > I think this tests nothing. If you have the same result with different seeds, it probably means that
> > `--shuffle-sections=` does not work as expected.
> > 
> > What I think can be done to test it:
> > Run LLD without `--shuffle-sections` and then check that with this option, the result is just different.
> > e.g.:
> > 
> > ```
> > # COMMON:       Hex dump of section '.init_array'
> > # WITHOUT-NEXT: ff0100
> > # WITH-NOT:     ff0100
> > ...
> > ```
> > 
> Random funtion implementations are different across platforms. We may have to create a large number of sections to ensure the result will be different.
Yes, I think it is fine for now. As far I understand std::mt19937  (i.e. a random number engine based on Mersenne Twister algorithm) should produce the same result on different platforms, because implementation should base on the same algorithm.
As was said in the different thread, the problem is probably comes from `std::shuffle`, then if we one day get llvm's implementation of one,
it will be possible to simplify this test.
Anyways the number of sections you are adding in this test looks sufficient and not that large to me.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74887





More information about the llvm-commits mailing list