Skip to content

Commit

Permalink
During ThinLTO, avoid merging static data with non-static data with t…
Browse files Browse the repository at this point in the history
…he same name

When doing ThinLTO, the linker updates LTO proxy atoms to point to the
right linker atoms. When there are atoms with different scope with the
same name (in the case where e.g. one translation unit has a static
instance of data and another has a global instance of data with the same
name), we may end up making all translation units using a symbol with
that name link against the one global symbol with that name.
Presumably, the same problem can happen with static/global functions
with the same name too.

When the data/code under those names happen to be identical, that's
fine, but when they're not, bad things happen.

So when dealing with LTO proxy atoms for the translation unit scope,
don't look up in the list of atoms from the linker (for global symbols).

Fixes tpoechtrager#58
  • Loading branch information
glandium committed Aug 22, 2018
1 parent 41561b8 commit 328c737
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions cctools/ld64/src/ld/parsers/lto_file.cpp
Expand Up @@ -1424,9 +1424,10 @@ bool Parser::optimize( const std::vector<const ld::Atom*>& allAtoms,
void Parser::AtomSyncer::doAtom(const ld::Atom& machoAtom)
{
static const bool log = false;
bool isLocal = machoAtom.scope() == ld::Atom::scopeTranslationUnit;
// update proxy atoms to point to real atoms and find new atoms
const char* name = machoAtom.name();
CStringToAtom::const_iterator pos = _llvmAtoms.find(name);
CStringToAtom::const_iterator pos = isLocal ? _llvmAtoms.end() : _llvmAtoms.find(name);
if ( pos != _llvmAtoms.end() ) {
// turn Atom into a proxy for this mach-o atom
pos->second->setCompiledAtom(machoAtom);
Expand All @@ -1436,7 +1437,7 @@ void Parser::AtomSyncer::doAtom(const ld::Atom& machoAtom)
}
else {
// an atom of this name was not in the allAtoms list the linker gave us
auto llvmAtom = _deadllvmAtoms.find(name);
auto llvmAtom = isLocal ? _deadllvmAtoms.end() : _deadllvmAtoms.find(name);
if ( llvmAtom != _deadllvmAtoms.end() ) {
// this corresponding to an atom that the linker coalesced away or marked not-live
if ( _options.linkerDeadStripping ) {
Expand Down Expand Up @@ -1481,16 +1482,19 @@ void Parser::AtomSyncer::doAtom(const ld::Atom& machoAtom)
// reference is not going through Atom proxy. Fix it here to ensure that all
// llvm symbol references always go through Atom proxy.
{
const char* targetName = fit->u.target->name();
CStringToAtom::const_iterator post = _llvmAtoms.find(targetName);
const ld::Atom* targetAtom = fit->u.target;
const char* targetName = targetAtom->name();
bool isLocal = targetAtom->scope() == ld::Atom::scopeTranslationUnit;
CStringToAtom::const_iterator post = isLocal ? _llvmAtoms.end() : _llvmAtoms.find(targetName);
if ( post != _llvmAtoms.end() ) {
const ld::Atom* t = post->second;
if (log) fprintf(stderr, " updating direct reference to %p to be ref to %p: %s\n", fit->u.target, t, targetName);
fit->u.target = t;
}
else {
// <rdar://problem/12859831> Don't unbind follow-on reference into by-name reference
if ( (_deadllvmAtoms.find(targetName) != _deadllvmAtoms.end()) && (fit->kind != ld::Fixup::kindNoneFollowOn) && (fit->u.target->scope() != ld::Atom::scopeTranslationUnit) ) {
auto llvmAtom = isLocal ? _deadllvmAtoms.end() : _deadllvmAtoms.find(targetName);
if ( (llvmAtom != _deadllvmAtoms.end()) && (fit->kind != ld::Fixup::kindNoneFollowOn) && (fit->u.target->scope() != ld::Atom::scopeTranslationUnit) ) {
// target was coalesed away and replace by mach-o atom from a non llvm .o file
fit->binding = ld::Fixup::bindingByNameUnbound;
fit->u.name = targetName;
Expand Down

0 comments on commit 328c737

Please sign in to comment.