Fix for nasty LTO issue that's most obvious in ObjectiveC.

[freik]

Summary

The LTO module linker is dropping the 'isExternallyInitialized' attribute from GlobalVariable's as it's copying them into the One Module to Rule Them All. This results in later phases believing that values can be copy-prop'ed or pre-evaluated. The second change is the one that actually fixes the problem, but the first change looks like a similar oversight, so I also changed it to include the attribute.

This bug prevents LTO from being usable by the Facebook iOS app.

[freik]

Missed the new test file...

[rnk]

This seems like an omission from GlobalVariable::copyAttributesFrom(), which the linker already calls.

[freik]

@rnk: I considered both, and am new to the codebase, so the 'correct' place to fix this wasn't apparent. Does anyone else have an opinion, here? Either way is effective, and pretty straightforward as well.

[majnemer]

In D6145#11, @freik wrote:

@rnk: I considered both, and am new to the codebase, so the 'correct' place to fix this wasn't apparent. Does anyone else have an opinion, here? Either way is effective, and pretty straightforward as well.

I think the idea is that copyAttributesFrom is intended to handle all attributes which are defaulted when a GlobalObject is created.

Now that I am reviewing GlobalObject::copyAttributesFrom, it looks like we forgot to handle Comdat there as well.

[freik]

The reason I wasn't sure which was correct is because isExternallyIntialized is a default ctor parameter, so if someone's specifying a different value, then calling the copyAttributesFrom to fill in everything else, it's a bug. From a design perspective, having an API like copyAttributesFrom for a type with a ctor that has default parameters seems incredibly confusing. Perhaps I should eliminate the default parameter? And maybe this discussion should move over to llvmdev?

[freik]

Fair enough. I'll go ahead & update the diff with the fix moved into copyAttributesFrom, and fix up handful of missing attributes in GlobalObject and GlobalValue, too.

[freik]

[Responding to feedback]
Updated the test case to be better directed (specifically testing externally_initialized and appending attributes). Moved the fix to be in GlobalVariable::copyAttributesFrom instead of at the copy site.

[• rafael]

LGTM. Do you need me to commit it?

[freik]

Yup. Thanks!

[• rafael]

221620 .

lib/IR/Globals.cpp

 
 /// copyAttributesFrom - copy all additional attributes (those not needed to
 /// create a GlobalVariable) from the GlobalVariable Src to this one.
 void GlobalVariable::copyAttributesFrom(const GlobalValue *Src) {
   assert(isa<GlobalVariable>(Src) && "Expected a GlobalVariable!");
   GlobalObject::copyAttributesFrom(Src);
   const GlobalVariable *SrcVar = cast<GlobalVariable>(Src);
   setThreadLocalMode(SrcVar->getThreadLocalMode());
+  setExternallyInitialized(SrcVar->isExternallyInitialized());
 }
 
 
 //===----------------------------------------------------------------------===//
 // GlobalAlias Implementation
 //===----------------------------------------------------------------------===//
 
 GlobalAlias::GlobalAlias(Type *Ty, unsigned AddressSpace, LinkageTypes Link,

test/Linker/lto-attributes.ll

+; RUN: llvm-link -S %s -o - | FileCheck %s
+
+; CHECK: @foo = private externally_initialized global i8* null
+@foo = private externally_initialized global i8* null
+; CHECK: @array = appending global [7 x i8] c"abcdefg", align 1
+@array = appending global [7 x i8] c"abcdefg", align 1
+