[llvm] r215457 - Don't upgrade global constructors when reading bitcode
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Aug 12 09:46:37 PDT 2014
Author: dexonsmith
Date: Tue Aug 12 11:46:37 2014
New Revision: 215457
URL: http://llvm.org/viewvc/llvm-project?rev=215457&view=rev
Log:
Don't upgrade global constructors when reading bitcode
An optional third field was added to `llvm.global_ctors` (and
`llvm.global_dtors`) in r209015. Most of the code has been changed to
deal with both versions of the variables. Users of the C API might
create either version, the helper functions in LLVM create the two-field
version, and clang now creates the three-field version.
However, the BitcodeReader was changed to always upgrade to the
three-field version. This created an unnecessary inconsistency in the
IR before/after serializing to bitcode.
This commit resolves the inconsistency by making the third field truly
optional (and not upgrading in the bitcode reader). Since `llvm-link`
was relying on this upgrade code, rather than deleting it I've moved it
into `ModuleLinker`, where it upgrades these arrays as necessary to
resolve inconsistencies between modules.
The ideal resolution would be to remove the 2-field version and make the
third field required. I filed PR20506 to track that.
I changed `test/Bitcode/upgrade-global-ctors.ll` to a negative test and
duplicated the `llvm-link` check in `test/Linker/global_ctors.ll` to
check both upgrade directions.
Since I came across this as part of PR5680 (serializing use-list order),
I've also added the missing `verify-uselistorder` RUN line to
`test/Bitcode/metadata-2.ll`.
Modified:
llvm/trunk/lib/IR/AutoUpgrade.cpp
llvm/trunk/lib/Linker/LinkModules.cpp
llvm/trunk/test/Bitcode/metadata-2.ll
llvm/trunk/test/Bitcode/upgrade-global-ctors.ll
llvm/trunk/test/Linker/global_ctors.ll
Modified: llvm/trunk/lib/IR/AutoUpgrade.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/AutoUpgrade.cpp?rev=215457&r1=215456&r2=215457&view=diff
==============================================================================
--- llvm/trunk/lib/IR/AutoUpgrade.cpp (original)
+++ llvm/trunk/lib/IR/AutoUpgrade.cpp Tue Aug 12 11:46:37 2014
@@ -173,62 +173,7 @@ bool llvm::UpgradeIntrinsicFunction(Func
return Upgraded;
}
-static bool UpgradeGlobalStructors(GlobalVariable *GV) {
- ArrayType *ATy = dyn_cast<ArrayType>(GV->getType()->getElementType());
- StructType *OldTy =
- ATy ? dyn_cast<StructType>(ATy->getElementType()) : nullptr;
-
- // Only upgrade an array of a two field struct with the appropriate field
- // types.
- if (!OldTy || OldTy->getNumElements() != 2)
- return false;
-
- // Get the upgraded 3 element type.
- PointerType *VoidPtrTy = Type::getInt8Ty(GV->getContext())->getPointerTo();
- Type *Tys[3] = {
- OldTy->getElementType(0),
- OldTy->getElementType(1),
- VoidPtrTy
- };
- StructType *NewTy =
- StructType::get(GV->getContext(), Tys, /*isPacked=*/false);
-
- // Build new constants with a null third field filled in.
- Constant *OldInitC = GV->getInitializer();
- ConstantArray *OldInit = dyn_cast<ConstantArray>(OldInitC);
- if (!OldInit && !isa<ConstantAggregateZero>(OldInitC))
- return false;
- std::vector<Constant *> Initializers;
- if (OldInit) {
- for (Use &U : OldInit->operands()) {
- ConstantStruct *Init = cast<ConstantStruct>(&U);
- Constant *NewInit =
- ConstantStruct::get(NewTy, Init->getOperand(0), Init->getOperand(1),
- Constant::getNullValue(VoidPtrTy), nullptr);
- Initializers.push_back(NewInit);
- }
- }
- assert(Initializers.size() == ATy->getNumElements());
-
- // Replace the old GV with a new one.
- ATy = ArrayType::get(NewTy, Initializers.size());
- Constant *NewInit = ConstantArray::get(ATy, Initializers);
- GlobalVariable *NewGV = new GlobalVariable(
- *GV->getParent(), ATy, GV->isConstant(), GV->getLinkage(), NewInit, "",
- GV, GV->getThreadLocalMode(), GV->getType()->getAddressSpace(),
- GV->isExternallyInitialized());
- NewGV->copyAttributesFrom(GV);
- NewGV->takeName(GV);
- assert(GV->use_empty() && "program cannot use initializer list");
- GV->eraseFromParent();
- return true;
-}
-
bool llvm::UpgradeGlobalVariable(GlobalVariable *GV) {
- if (GV->getName() == "llvm.global_ctors" ||
- GV->getName() == "llvm.global_dtors")
- return UpgradeGlobalStructors(GV);
-
// Nothing to do yet.
return false;
}
Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=215457&r1=215456&r2=215457&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Aug 12 11:46:37 2014
@@ -469,6 +469,9 @@ namespace {
void computeTypeMapping();
+ void upgradeMismatchedGlobalArray(StringRef Name);
+ void upgradeMismatchedGlobals();
+
bool linkAppendingVarProto(GlobalVariable *DstGV, GlobalVariable *SrcGV);
bool linkGlobalProto(GlobalVariable *SrcGV);
bool linkFunctionProto(Function *SrcF);
@@ -816,6 +819,83 @@ void ModuleLinker::computeTypeMapping()
TypeMap.linkDefinedTypeBodies();
}
+static void upgradeGlobalArray(GlobalVariable *GV) {
+ ArrayType *ATy = cast<ArrayType>(GV->getType()->getElementType());
+ StructType *OldTy = cast<StructType>(ATy->getElementType());
+ assert(OldTy->getNumElements() == 2 && "Expected to upgrade from 2 elements");
+
+ // Get the upgraded 3 element type.
+ PointerType *VoidPtrTy = Type::getInt8Ty(GV->getContext())->getPointerTo();
+ Type *Tys[3] = {OldTy->getElementType(0), OldTy->getElementType(1),
+ VoidPtrTy};
+ StructType *NewTy = StructType::get(GV->getContext(), Tys, false);
+
+ // Build new constants with a null third field filled in.
+ Constant *OldInitC = GV->getInitializer();
+ ConstantArray *OldInit = dyn_cast<ConstantArray>(OldInitC);
+ if (!OldInit && !isa<ConstantAggregateZero>(OldInitC))
+ // Invalid initializer; give up.
+ return;
+ std::vector<Constant *> Initializers;
+ if (OldInit && OldInit->getNumOperands()) {
+ Value *Null = Constant::getNullValue(VoidPtrTy);
+ for (Use &U : OldInit->operands()) {
+ ConstantStruct *Init = cast<ConstantStruct>(U.get());
+ Initializers.push_back(ConstantStruct::get(
+ NewTy, Init->getOperand(0), Init->getOperand(1), Null, nullptr));
+ }
+ }
+ assert(Initializers.size() == ATy->getNumElements() &&
+ "Failed to copy all array elements");
+
+ // Replace the old GV with a new one.
+ ATy = ArrayType::get(NewTy, Initializers.size());
+ Constant *NewInit = ConstantArray::get(ATy, Initializers);
+ GlobalVariable *NewGV = new GlobalVariable(
+ *GV->getParent(), ATy, GV->isConstant(), GV->getLinkage(), NewInit, "",
+ GV, GV->getThreadLocalMode(), GV->getType()->getAddressSpace(),
+ GV->isExternallyInitialized());
+ NewGV->copyAttributesFrom(GV);
+ NewGV->takeName(GV);
+ assert(GV->use_empty() && "program cannot use initializer list");
+ GV->eraseFromParent();
+}
+
+void ModuleLinker::upgradeMismatchedGlobalArray(StringRef Name) {
+ // Look for the global arrays.
+ auto *DstGV = dyn_cast_or_null<GlobalVariable>(DstM->getNamedValue(Name));
+ if (!DstGV)
+ return;
+ auto *SrcGV = dyn_cast_or_null<GlobalVariable>(SrcM->getNamedValue(Name));
+ if (!SrcGV)
+ return;
+
+ // Check if the types already match.
+ auto *DstTy = cast<ArrayType>(DstGV->getType()->getElementType());
+ auto *SrcTy =
+ cast<ArrayType>(TypeMap.get(SrcGV->getType()->getElementType()));
+ if (DstTy == SrcTy)
+ return;
+
+ // Grab the element types. We can only upgrade an array of a two-field
+ // struct. Only bother if the other one has three-fields.
+ auto *DstEltTy = cast<StructType>(DstTy->getElementType());
+ auto *SrcEltTy = cast<StructType>(SrcTy->getElementType());
+ if (DstEltTy->getNumElements() == 2 && SrcEltTy->getNumElements() == 3) {
+ upgradeGlobalArray(DstGV);
+ return;
+ }
+ if (DstEltTy->getNumElements() == 3 && SrcEltTy->getNumElements() == 2)
+ upgradeGlobalArray(SrcGV);
+
+ // We can't upgrade any other differences.
+}
+
+void ModuleLinker::upgradeMismatchedGlobals() {
+ upgradeMismatchedGlobalArray("llvm.global_ctors");
+ upgradeMismatchedGlobalArray("llvm.global_dtors");
+}
+
/// linkAppendingVarProto - If there were any appending global variables, link
/// them together now. Return true on error.
bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
@@ -1454,6 +1534,9 @@ bool ModuleLinker::run() {
ComdatsChosen[&C] = std::make_pair(SK, LinkFromSrc);
}
+ // Upgrade mismatched global arrays.
+ upgradeMismatchedGlobals();
+
// Insert all of the globals in src into the DstM module... without linking
// initializers (which could refer to functions not yet mapped over).
for (Module::global_iterator I = SrcM->global_begin(),
Modified: llvm/trunk/test/Bitcode/metadata-2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/metadata-2.ll?rev=215457&r1=215456&r2=215457&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/metadata-2.ll (original)
+++ llvm/trunk/test/Bitcode/metadata-2.ll Tue Aug 12 11:46:37 2014
@@ -1,4 +1,5 @@
; RUN: llvm-as < %s | llvm-dis -disable-output
+; RUN: verify-uselistorder < %s -preserve-bc-use-list-order
%0 = type { %object.ModuleInfo.__vtbl*, i8*, %"byte[]", %1, %"ClassInfo[]", i32, void ()*, void ()*, void ()*, i8*, void ()* } ; type %0
%1 = type { i64, %object.ModuleInfo* } ; type %1
%2 = type { i32, void ()* } ; type %2
Modified: llvm/trunk/test/Bitcode/upgrade-global-ctors.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/upgrade-global-ctors.ll?rev=215457&r1=215456&r2=215457&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/upgrade-global-ctors.ll (original)
+++ llvm/trunk/test/Bitcode/upgrade-global-ctors.ll Tue Aug 12 11:46:37 2014
@@ -1,4 +1,5 @@
; RUN: llvm-dis < %s.bc| FileCheck %s
; RUN: verify-uselistorder < %s.bc -preserve-bc-use-list-order
-; CHECK: @llvm.global_ctors = appending global [0 x { i32, void ()*, i8* }] zeroinitializer
+; Global constructors should no longer be upgraded when reading bitcode.
+; CHECK: @llvm.global_ctors = appending global [0 x { i32, void ()* }] zeroinitializer
Modified: llvm/trunk/test/Linker/global_ctors.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/global_ctors.ll?rev=215457&r1=215456&r2=215457&view=diff
==============================================================================
--- llvm/trunk/test/Linker/global_ctors.ll (original)
+++ llvm/trunk/test/Linker/global_ctors.ll Tue Aug 12 11:46:37 2014
@@ -1,5 +1,6 @@
; RUN: llvm-as %s -o %t.new.bc
; RUN: llvm-link %t.new.bc %S/Inputs/old_global_ctors.3.4.bc | llvm-dis | FileCheck %s
+; RUN: llvm-link %S/Inputs/old_global_ctors.3.4.bc %t.new.bc | llvm-dis | FileCheck %s
; old_global_ctors.3.4.bc contains the following LLVM IL, assembled into
; bitcode by llvm-as from 3.4. It uses a two element @llvm.global_ctors array.
More information about the llvm-commits
mailing list