Index: lib/IR/SafepointIRVerifier.cpp =================================================================== --- lib/IR/SafepointIRVerifier.cpp +++ lib/IR/SafepointIRVerifier.cpp @@ -120,6 +120,22 @@ return false; } +static bool isDerivedPointerDef(const Value *Val) { + return isa(Val) || isa(Val); +} + +static const Value *getBasePointer(const Value *Val) { + if (!Val->getType()->isPointerTy()) + return Val; + + while (isDerivedPointerDef(Val)) + if (auto *GEP = dyn_cast(Val)) + Val = GEP->getPointerOperand(); + else if (auto *CI = dyn_cast(Val)) + Val = CI->stripPointerCasts(); + return Val; +} + // Debugging aid -- prints a [Begin, End) range of values. template static void PrintValueSet(raw_ostream &OS, IteratorTy Begin, IteratorTy End) { @@ -168,7 +184,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 +198,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 +213,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 +312,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 +380,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 it's base is relocated. + // So we need to check only is base pointer 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 +393,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 +412,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 +435,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,81 @@ +; RUN: opt -safepoint-ir-verifier-print-only -verify-safepoint-ir -S %s 2>&1 | FileCheck %s + +define void @test.simple.ok(i32, i8 addrspace(1)* %base1, i8 addrspace(1)* %base2) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.simple.ok +; CHECK-NEXT: No illegal uses found by SafepointIRVerifier in: test.simple.ok +.b0: + %ptr = getelementptr i8, i8 addrspace(1)* %base1, i64 4 + br label %.b1 + +.b1: + %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) + br label %.b2 + +.b2: + %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)* + %c = icmp sgt i32 addrspace(1)* %ptr.i32, %ptr2.i32 + ret void +} + +define void @test.simple.fail(i32, i8 addrspace(1)* %base) gc "statepoint-example" { +; CHECK-LABEL: Verifying gc pointers in function: test.simple.fail +.b0: + %ptr = getelementptr i8, i8 addrspace(1)* %base, i64 4 + br label %.b1 + +.b1: + %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) + br label %.b2 + +.b2: + %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 +} + +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.ok(i64 %arg, 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 + %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) + %another_offset = add i64 %arg, 8 + %addr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 %another_offset + %cmp = icmp eq i8 addrspace(1)* %addr1, %addr2 + 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) + %another_offset = add i64 %arg, 8 + %addr2 = getelementptr i8, i8 addrspace(1)* %base2, i64 %another_offset + %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 %another_offset +; 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) +