Page MenuHomePhabricator

[ThinLTO] Avoid backwards references to renamed locals in distributed backend case
AbandonedPublic

Authored by tejohnson on Jul 18 2016, 10:41 AM.

Details

Reviewers
pcc
mehdi_amini
Summary

Address another issue related to distributed backends and
--start-lib/--end-lib (see D22356 for more background).

We can also run into trouble when importing the prevailing copy of a
weak symbol if it references a local value. The local value will be
promoted and renamed using the module hash of the prevailing copy. If
that prevailing copy is not needed by the final native link, due to
intervening imports/inlines, that renamed copy of the local value may
not be linked in, and any importing module in a library listed later on
the link line will have an undefined reference.

Avoid this via a new PreventBackwardsModuleRefs flag to the importer
which prevents linking in any multiply-defined symbol if it references
a local needing renaming. We pass true for the flag in the
thinlto_index_only case (where we will have distributed backends and a
separate native link process).

I considered a couple of other solutions:

  1. Always link in the last copy of a multiply defined symbol. However,

in the non-odr case this may change the program semantics as we would be
importing a different copy than the linker would normally select.

  1. Force import the referenced local and as a local copy (don't use the

promoted copy). This is the better long term fix (and I have added a
FIXME to gold-plugin to that effect), but it also relies on doing
importing in the distributed backeds which is not yet there (will come
in via D21545). In that case we would still need to detect this case in
the importer, but force import the referenced locals instead of
preventing the referencing weak/linkonce import. We would also need to
either mark all copies of the referenced local for internalization after
importing, or only those copies in the distributed backend individual index
files that don't also have a singly-defined reference to the source module
(meaning the source module must be after the importing module on the
link line).

Added a new test for this case.

Depends on D22356.

Diff Detail

Event Timeline

tejohnson updated this revision to Diff 64348.Jul 18 2016, 10:41 AM
tejohnson retitled this revision from to [ThinLTO] Avoid backwards references to renamed locals in distributed backend case.
tejohnson updated this object.
tejohnson added reviewers: mehdi_amini, pcc.
tejohnson added a subscriber: llvm-commits.
tejohnson abandoned this revision.Jul 19 2016, 6:18 AM

See discussion in D22356 - will be addressing this by having ThinLink communicate which files to include to the final link.