Index: lib/IR/SafepointIRVerifier.cpp =================================================================== --- lib/IR/SafepointIRVerifier.cpp +++ lib/IR/SafepointIRVerifier.cpp @@ -120,6 +120,31 @@ return false; } +/// If V is already base pointer returns V otherwise strips off one pointer cast +/// or GEP instruction and returns its input. +static const Value *getImmediateBasePointer(const Value *V) { + assert(V->getType()->isPointerTy() && + "Should never be invoked for non-pointer types!"); + if (auto *GEP = dyn_cast(V)) + return GEP->getPointerOperand(); + if (auto *BC = dyn_cast(V)) + return BC->stripPointerCasts(); + return V; +} + +static bool isDerivedPointerDef(const Value *V) { + if (!V->getType()->isPointerTy()) + return false; + return getImmediateBasePointer(V) != V; +} + +static const Value *getBasePointer(const Value *V) { + const Value *VBase = getImmediateBasePointer(V); + if (VBase == V) + return V; + return getBasePointer(VBase); +} + // Debugging aid -- prints a [Begin, End) range of values. template static void PrintValueSet(raw_ostream &OS, IteratorTy Begin, IteratorTy End) { @@ -168,7 +193,7 @@ const auto &Defs = BlockMap[DTN->getBlock()]->Contribution; Result.insert(Defs.begin(), Defs.end()); // If this block is 'Cleared', then nothing LiveIn to this block can be - // available after this block completes. Note: This turns out to be + // available after this block completes. Note: This turns out to be // really important for reducing memory consuption of the initial available // sets and thus peak memory usage by this verifier. if (BlockMap[DTN->getBlock()]->Cleared) @@ -182,7 +207,7 @@ /// Model the effect of an instruction on the set of available values. static void TransferInstruction(const Instruction &I, bool &Cleared, - DenseSet &Available) { + DenseSet &Available) { if (isStatepoint(I)) { Cleared = true; Available.clear(); @@ -197,7 +222,7 @@ static void TransferBlock(const BasicBlock *BB, BasicBlockState &BBS, bool FirstPass) { - const DenseSet &AvailableIn = BBS.AvailableIn; + const DenseSet &AvailableIn = BBS.AvailableIn; DenseSet &AvailableOut = BBS.AvailableOut; if (BBS.Cleared) { @@ -296,7 +321,7 @@ static void Verify(const Function &F, const DominatorTree &DT) { SpecificBumpPtrAllocator BSAllocator; DenseMap BlockMap; - + DEBUG(dbgs() << "Verifying gc pointers in function: " << F.getName() << "\n"); if (PrintOnly) dbgs() << "Verifying gc pointers in function: " << F.getName() << "\n"; @@ -364,6 +389,11 @@ // We destructively modify AvailableIn as we traverse the block instruction // by instruction. DenseSet &AvailableSet = BlockMap[&BB]->AvailableIn; + auto isBaseAvailable = [&AvailableSet] (const Value *V) { + // Derived pointer can be safely used only if its base is relocated. + // So we need to check only if base pointer is relocated or not. + return AvailableSet.count(getBasePointer(V)) != 0; + }; for (const Instruction &I : BB) { if (const PHINode *PN = dyn_cast(&I)) { if (containsGCPtrType(PN->getType())) @@ -372,18 +402,18 @@ const Value *InValue = PN->getIncomingValue(i); if (isNotExclusivelyConstantDerived(InValue) && - !BlockMap[InBB]->AvailableOut.count(InValue)) + !BlockMap[InBB]->AvailableOut.count(getBasePointer(InValue))) ReportInvalidUse(*InValue, *PN); } } else if (isa(I) && containsGCPtrType(I.getOperand(0)->getType())) { - Value *LHS = I.getOperand(0), *RHS = I.getOperand(1); + const Value *LHS = I.getOperand(0), *RHS = I.getOperand(1); enum BaseType baseTyLHS = getBaseType(LHS), baseTyRHS = getBaseType(RHS); // Returns true if LHS and RHS are unrelocated pointers and they are // valid unrelocated uses. - auto hasValidUnrelocatedUse = [&AvailableSet, baseTyLHS, baseTyRHS, &LHS, &RHS] () { + auto hasValidUnrelocatedUse = [&] () { // A cmp instruction has valid unrelocated pointer operands only if // both operands are unrelocated pointers. // In the comparison between two pointers, if one is an unrelocated @@ -391,7 +421,7 @@ // instruction to contain valid unrelocated uses. This unrelocated // use can be a null constant as well, or another unrelocated // pointer. - if (AvailableSet.count(LHS) || AvailableSet.count(RHS)) + if (isBaseAvailable(LHS) || isBaseAvailable(RHS)) return false; // Constant pointers (that are not exclusively null) may have // meaning in different VMs, so we cannot reorder the compare @@ -414,15 +444,20 @@ if (!hasValidUnrelocatedUse()) { // Print out all non-constant derived pointers that are unrelocated // uses, which are invalid. - if (baseTyLHS == BaseType::NonConstant && !AvailableSet.count(LHS)) + if (baseTyLHS == BaseType::NonConstant && !isBaseAvailable(LHS)) ReportInvalidUse(*LHS, I); - if (baseTyRHS == BaseType::NonConstant && !AvailableSet.count(RHS)) + if (baseTyRHS == BaseType::NonConstant && !isBaseAvailable(RHS)) ReportInvalidUse(*RHS, I); } - } else { + } else if (!isDerivedPointerDef(&I)) { + // Since isBaseAvailable checks availablility of base objects we don't + // need to validate this instructions (GEP/bitcast). Any use of + // unrelocated pointers in this instructions by itself is legal. If they + // are used in illegal way (currently the only legal way to use them is + // cmp) the verifier is guaranteed to report a failure. for (const Value *V : I.operands()) if (containsGCPtrType(V->getType()) && - isNotExclusivelyConstantDerived(V) && !AvailableSet.count(V)) + isNotExclusivelyConstantDerived(V) && !isBaseAvailable(V)) ReportInvalidUse(*V, I); } Index: test/SafepointIRVerifier/use-derived-unrelocated.ll =================================================================== --- /dev/null +++ test/SafepointIRVerifier/use-derived-unrelocated.ll @@ -0,0 +1,94 @@ +; RUN: opt -safepoint-ir-verifier-print-only -verify-safepoint-ir -S %s 2>&1 | FileCheck %s + +; Checking if verifier accepts chain of GEPs/bitcasts. +define void @test.deriving.ok(i32, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.deriving.ok +; CHECK-NEXT: No illegal uses found by SafepointIRVerifier in: test.deriving.ok + %ptr = getelementptr i8, i8 addrspace(1)* %base1, i64 4 + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base1) + %ptr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 8 + %ptr.i32 = bitcast i8 addrspace(1)* %ptr to i32 addrspace(1)* + %ptr2.i32 = bitcast i8 addrspace(1)* %ptr2 to i32 addrspace(1)* + ret void +} + +; Checking if verifier accepts cmp of two derived pointers when one defined +; before safepoint and one after and both have unrelocated base. +define void @test.cmp.ok(i32, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.cmp.ok +; CHECK-NEXT: No illegal uses found by SafepointIRVerifier in: test.cmp.ok + %ptr = getelementptr i8, i8 addrspace(1)* %base1, i64 4 + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base1) + %ptr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 8 + %c2 = icmp sgt i8 addrspace(1)* %ptr2, %ptr + ret void +} + +; Checking if verifier accepts cmp of two derived pointers when one defined +; before safepoint and one after and both have unrelocated base. One of pointers +; defined as a long chain of geps/bitcasts. +define void @test.cmp-long_chain.ok(i32, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.cmp-long_chain.ok +; CHECK-NEXT: No illegal uses found by SafepointIRVerifier in: test.cmp-long_chain.ok + %ptr = getelementptr i8, i8 addrspace(1)* %base1, i64 4 + %ptr.i32 = bitcast i8 addrspace(1)* %ptr to i32 addrspace(1)* + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base1) + %ptr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 8 + %ptr2.i32 = bitcast i8 addrspace(1)* %ptr2 to i32 addrspace(1)* + %ptr2.i32.2 = getelementptr i32, i32 addrspace(1)* %ptr2.i32, i64 4 + %ptr2.i32.3 = getelementptr i32, i32 addrspace(1)* %ptr2.i32.2, i64 8 + %ptr2.i32.4 = getelementptr i32, i32 addrspace(1)* %ptr2.i32.3, i64 8 + %ptr2.i32.5 = getelementptr i32, i32 addrspace(1)* %ptr2.i32.4, i64 8 + %ptr2.i32.6 = getelementptr i32, i32 addrspace(1)* %ptr2.i32.5, i64 8 + %ptr2.i32.6.i8 = bitcast i32 addrspace(1)* %ptr2.i32.6 to i8 addrspace(1)* + %ptr2.i32.6.i8.i32 = bitcast i8 addrspace(1)* %ptr2.i32.6.i8 to i32 addrspace(1)* + %ptr2.i32.6.i8.i32.2 = getelementptr i32, i32 addrspace(1)* %ptr2.i32.6.i8.i32, i64 8 + %c2 = icmp sgt i32 addrspace(1)* %ptr2.i32.6.i8.i32.2, %ptr.i32 + ret void +} + +; GEP and bitcast of unrelocated pointer is acceptable, but load by resulting +; pointer should be reported. +define void @test.load.fail(i32, i8 addrspace(1)* %base) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.load.fail + %ptr = getelementptr i8, i8 addrspace(1)* %base, i64 4 + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base) + %ptr.i32 = bitcast i8 addrspace(1)* %ptr to i32 addrspace(1)* ; it's ok +; CHECK-NEXT: Illegal use of unrelocated value found! +; CHECK-NEXT: Def: %ptr.i32 = bitcast i8 addrspace(1)* %ptr to i32 addrspace(1)* +; CHECK-NEXT: Use: %ptr.val = load i32, i32 addrspace(1)* %ptr.i32 + %ptr.val = load i32, i32 addrspace(1)* %ptr.i32 + ret void +} + +; Comparison between pointer derived from unrelocated one (though defined after +; safepoint) and relocated pointer should be reported. +define void @test.cmp.fail(i64 %arg, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.cmp.fail + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base2 , i32 -1, i32 0, i32 0, i32 0) + %base2.relocated = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %safepoint_token, i32 7, i32 7) ; base2, base2 + %addr1 = getelementptr i8, i8 addrspace(1)* %base1, i64 %arg +; CHECK-NEXT: Illegal use of unrelocated value found! +; CHECK-NEXT: Def: %addr1 = getelementptr i8, i8 addrspace(1)* %base1, i64 %arg +; CHECK-NEXT: Use: %cmp = icmp eq i8 addrspace(1)* %addr1, %base2.relocated + %cmp = icmp eq i8 addrspace(1)* %addr1, %base2.relocated + ret void +} + +define void @test.cmp-load.fail(i64 %arg, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.cmp-load.fail + %addr1 = getelementptr i8, i8 addrspace(1)* %base1, i64 %arg + %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %base2 , i32 -1, i32 0, i32 0, i32 0) + %addr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 8 + %cmp = icmp eq i8 addrspace(1)* %addr1, %addr2 +; CHECK-NEXT: Illegal use of unrelocated value found! +; CHECK-NEXT: Def: %addr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 8 +; CHECK-NEXT: Use: %val = load i8, i8 addrspace(1)* %addr2 + %val = load i8, i8 addrspace(1)* %addr2 + ret void +} + +; Function Attrs: nounwind +declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...) +declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32) +