This is an archive of the discontinued LLVM Phabricator instance.

[lld/mac] Fix nondeterminism in output section ordering
ClosedPublic

Authored by thakis on Jun 28 2021, 12:39 PM.

Details

Reviewers
int3
gkm
Group Reviewers
Restricted Project
Commits
rGf1969b74a7e7: [lld/mac] Fix nondeterminism in output section ordering
Summary

The two different thread_local_regular sections (__thread_data and
more_thread_data) had nondeterminstic ordering for two reasons:

  1. https://reviews.llvm.org/D102972 changed concatOutputSections from MapVector to DenseMap, so when we iterate it to make output segments, we would add the two sections to the __DATA output segment in nondeterministic order.
  1. The same change also moved the two stable_sort()s for segments and sections to sort(). Since sections with assigned priority (such as TLV data) have the same priority for all sections, this is incorrect -- we must use stable_sort() so that the initial (input-order-based) order remains.

As a side effect, we now (deterministically) put the common
section in front of
bss (while previously we happened to
put it after it). (common and bss are both zerofill so
both have order INT_MAX, but common symbols are added to
inputSections before normal sections are collected.)

Makes lld/test/MachO/tlv.s and lld/test/MachO/tlv-dylib.s pass with
LLVM_ENABLE_EXPENSIVE_CHECKS=ON.

Diff Detail

Event Timeline

thakis created this revision.Jun 28 2021, 12:39 PM
Herald added a reviewer: gkm. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: mgrang. · View Herald Transcript
thakis requested review of this revision.Jun 28 2021, 12:39 PM
aganea added a subscriber: aganea.Jun 28 2021, 1:04 PM
tschuett added inline comments.
lld/MachO/OutputSegment.cpp
143

Do you want to add a comment here: it has to be stable_sort ...

lld/MachO/Writer.cpp
81

Do you want to add a comment here: The order of bla has to be deterministic or whatever.

thakis updated this revision to Diff 355030.Jun 28 2021, 1:59 PM
thakis retitled this revision from [lld/mac] Make lld/test/MachO/tlv.s pass with expensive tests to [lld/mac] Make lld/test/MachO/tlv.s,tlv-dylib.s pass with expensive tests.
thakis edited the summary of this revision. (Show Details)

comments, tlv-dylib.s

thakis marked 2 inline comments as done.Jun 28 2021, 1:59 PM
int3 accepted this revision.Jun 28 2021, 2:33 PM

D'oh, sorry about that, and thanks for the fix!

This revision is now accepted and ready to land.Jun 28 2021, 2:33 PM

Make lld/test/MachO/tlv.s,tlv-dylib.s pass with expensive tests

I think the subject needs to mention that it fixes a non-determinism, otherwise people might think this just adjusts fragile tests.

thakis retitled this revision from [lld/mac] Make lld/test/MachO/tlv.s,tlv-dylib.s pass with expensive tests to [lld/mac] Fix nondeterminism in output section ordering.Jun 28 2021, 3:32 PM
thakis edited the summary of this revision. (Show Details)

Make lld/test/MachO/tlv.s,tlv-dylib.s pass with expensive tests

I think the subject needs to mention that it fixes a non-determinism, otherwise people might think this just adjusts fragile tests.

Good point, done.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2021, 3:41 PM