From 99fca5de96d3435e8eb7c84e8366cee98ef5416a Mon Sep 17 00:00:00 2001
From: Mikhail Glushenkov <foldr@codedgers.com>
Date: Tue, 19 Oct 2010 16:47:23 +0000
Subject: [PATCH] GlobalOpt: EvaluateFunction() must not evaluate stores to
 weak_odr globals.

Fixes PR8389.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@116812 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/llvm/GlobalVariable.h                 | 29 ++++++++++++++++++-
 lib/Transforms/IPO/GlobalOpt.cpp              | 13 +++++----
 .../GlobalOpt/2010-10-19-WeakOdr.ll           | 16 ++++++++++
 3 files changed, 51 insertions(+), 7 deletions(-)
 create mode 100644 test/Transforms/GlobalOpt/2010-10-19-WeakOdr.ll

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<ConstantArray>(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<GlobalVariable>(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<ConstantExpr>(C))
     // Handle a constantexpr gep.
@@ -2072,9 +2073,9 @@ static bool isSimpleEnoughPointerToCommit(Constant *C) {
         isa<GlobalVariable>(CE->getOperand(0)) &&
         cast<GEPOperator>(CE)->isInBounds()) {
       GlobalVariable *GV = cast<GlobalVariable>(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
+}
+
+