diff --git a/include/llvm/GlobalVariable.h b/include/llvm/GlobalVariable.h index a8a2bd6a522..597583b2cee 100644 --- a/include/llvm/GlobalVariable.h +++ b/include/llvm/GlobalVariable.h @@ -80,7 +80,21 @@ public: inline bool hasInitializer() const { return !isDeclaration(); } /// hasDefinitiveInitializer - Whether the global variable has an initializer, - /// and this is the initializer that will be used in the final executable. + /// and any other instances of the global (this can happen due to weak + /// linkage) are guaranteed to have the same initializer. + /// + /// Note that if you want to transform a global, you must use + /// hasUniqueInitializer() instead, because of the *_odr linkage type. + /// + /// Example: + /// + /// @a = global SomeType* null - Initializer is both definitive and unique. + /// + /// @b = global weak SomeType* null - Initializer is neither definitive nor + /// unique. + /// + /// @c = global weak_odr SomeType* null - Initializer is definitive, but not + /// unique. inline bool hasDefinitiveInitializer() const { return hasInitializer() && // The initializer of a global variable with weak linkage may change at @@ -88,6 +102,19 @@ public: !mayBeOverridden(); } + /// hasUniqueInitializer - Whether the global variable has an initializer, and + /// any changes made to the initializer will turn up in the final executable. + inline bool hasUniqueInitializer() const { + return hasInitializer() && + // It's not safe to modify initializers of global variables with weak + // linkage, because the linker might choose to discard the initializer and + // use the initializer from another instance of the global variable + // instead. It is wrong to modify the initializer of a global variable + // with *_odr linkage because then different instances of the global may + // have different initializers, breaking the One Definition Rule. + !isWeakForLinker(); + } + /// getInitializer - Return the initializer for this global variable. It is /// illegal to call this method if the global is external, because we cannot /// tell what the value is initialized to! diff --git a/lib/Transforms/IPO/GlobalOpt.cpp b/lib/Transforms/IPO/GlobalOpt.cpp index c1a175fce74..213f9caf817 100644 --- a/lib/Transforms/IPO/GlobalOpt.cpp +++ b/lib/Transforms/IPO/GlobalOpt.cpp @@ -1944,8 +1944,9 @@ GlobalVariable *GlobalOpt::FindGlobalCtors(Module &M) { FTy->isVarArg() || FTy->getNumParams() != 0) return 0; - // Verify that the initializer is simple enough for us to handle. - if (!I->hasDefinitiveInitializer()) return 0; + // Verify that the initializer is simple enough for us to handle. We are + // only allowed to optimize the initializer if it is unique. + if (!I->hasUniqueInitializer()) return 0; ConstantArray *CA = dyn_cast(I->getInitializer()); if (!CA) return 0; for (User::op_iterator i = CA->op_begin(), e = CA->op_end(); i != e; ++i) @@ -2062,9 +2063,9 @@ static bool isSimpleEnoughPointerToCommit(Constant *C) { return false; if (GlobalVariable *GV = dyn_cast(C)) - // Do not allow weak/linkonce/dllimport/dllexport linkage or + // Do not allow weak/*_odr/linkonce/dllimport/dllexport linkage or // external globals. - return GV->hasDefinitiveInitializer(); + return GV->hasUniqueInitializer(); if (ConstantExpr *CE = dyn_cast(C)) // Handle a constantexpr gep. @@ -2072,9 +2073,9 @@ static bool isSimpleEnoughPointerToCommit(Constant *C) { isa(CE->getOperand(0)) && cast(CE)->isInBounds()) { GlobalVariable *GV = cast(CE->getOperand(0)); - // Do not allow weak/linkonce/dllimport/dllexport linkage or + // Do not allow weak/*_odr/linkonce/dllimport/dllexport linkage or // external globals. - if (!GV->hasDefinitiveInitializer()) + if (!GV->hasUniqueInitializer()) return false; // The first index must be zero. diff --git a/test/Transforms/GlobalOpt/2010-10-19-WeakOdr.ll b/test/Transforms/GlobalOpt/2010-10-19-WeakOdr.ll new file mode 100644 index 00000000000..ad5b440a5ab --- /dev/null +++ b/test/Transforms/GlobalOpt/2010-10-19-WeakOdr.ll @@ -0,0 +1,16 @@ +; RUN: opt < %s -globalopt -S | FileCheck %s + +; PR8389: Globals with weak_odr linkage type must not be modified + +; CHECK: weak_odr global i32 0 + +@SomeVar = weak_odr global i32 0 + +@llvm.global_ctors = appending global [1 x { i32, void ()* }] [ { i32, void ()* } { i32 65535, void ()* @CTOR } ] + +define internal void @CTOR() { + store i32 23, i32* @SomeVar + ret void +} + +