diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -1993,53 +1993,62 @@ AAUndefinedBehaviorImpl(const IRPosition &IRP) : AAUndefinedBehavior(IRP) {} /// See AbstractAttribute::updateImpl(...). + // TODO: We should not only check instructions that access memory + // through a pointer (i.e. also branches etc.) ChangeStatus updateImpl(Attributor &A) override { - size_t PrevSize = NoUBLoads.size(); + const size_t PrevSize = NoUBMemAccessInsts.size(); - // TODO: We should not only check for load instructions. - auto InspectLoadForUB = [&](Instruction &I) { + auto InspectMemAccessInstForUB = [&](Instruction &I) { // Skip instructions that are already saved. - if (NoUBLoads.count(&I) || UBLoads.count(&I)) + if (NoUBMemAccessInsts.count(&I) || UBMemAccessInsts.count(&I)) return true; - Value *PtrOp = cast(&I)->getPointerOperand(); + // `InspectMemAccessInstForUB` is only called on instructions + // for which getPointerOperand() should give us their + // pointer operand unless they're volatile. + const Value *PtrOp = getPointerOperand(&I); + if (!PtrOp) + return true; - // A load is considered UB only if it dereferences a constant - // null pointer. + // A memory access through a pointer is considered UB + // only if the pointer has constant null value. + // TODO: Expand it to not only check constant values. if (!isa(PtrOp)) { - NoUBLoads.insert(&I); + NoUBMemAccessInsts.insert(&I); return true; } - Type *PtrTy = PtrOp->getType(); + const Type *PtrTy = PtrOp->getType(); - // Because we only consider loads inside functions, + // Because we only consider instructions inside functions, // assume that a parent function exists. const Function *F = I.getFunction(); - // A dereference on constant null is only considered UB - // if null dereference is _not_ defined for the target platform. - // TODO: Expand it to not only check constant values. + // A memory access using constant null pointer is only considered UB + // if null pointer is _not_ defined for the target platform. if (!llvm::NullPointerIsDefined(F, PtrTy->getPointerAddressSpace())) - UBLoads.insert(&I); + UBMemAccessInsts.insert(&I); else - NoUBLoads.insert(&I); + NoUBMemAccessInsts.insert(&I); return true; }; - A.checkForAllInstructions(InspectLoadForUB, *this, {Instruction::Load}); - if (PrevSize != NoUBLoads.size()) + A.checkForAllInstructions(InspectMemAccessInstForUB, *this, + {Instruction::Load, Instruction::Store, + Instruction::AtomicCmpXchg, + Instruction::AtomicRMW}); + if (PrevSize != NoUBMemAccessInsts.size()) return ChangeStatus::CHANGED; return ChangeStatus::UNCHANGED; } bool isAssumedToCauseUB(Instruction *I) const override { - return UBLoads.count(I); + return UBMemAccessInsts.count(I); } ChangeStatus manifest(Attributor &A) override { - if (!UBLoads.size()) + if (!UBMemAccessInsts.size()) return ChangeStatus::UNCHANGED; - for (Instruction *I : UBLoads) + for (Instruction *I : UBMemAccessInsts) changeToUnreachable(I, /* UseLLVMTrap */ false); return ChangeStatus::CHANGED; } @@ -2050,20 +2059,21 @@ } protected: - // A set of all the (live) load instructions that _are_ assumed to cause UB. - SmallPtrSet UBLoads; + // A set of all the (live) memory accessing instructions that _are_ assumed to + // cause UB. + SmallPtrSet UBMemAccessInsts; private: - // A set of all the (live) load instructions that are _not_ assumed to cause - // UB. + // A set of all the (live) memory accessing instructions + // that are _not_ assumed to cause UB. // Note: The correctness of the procedure depends on the fact that this // set stops changing after some point. "Change" here means that the size // of the set changes. The size of this set is monotonically increasing - // (we only add items to it) and is upper bounded by the number of load - // instructions in the processed function (we can never save more elements - // in this set than this number). Hence, the size of this set, at some - // point, will stop increasing, effectively reaching a fixpoint. - SmallPtrSet NoUBLoads; + // (we only add items to it) and is upper bounded by the number of memory + // accessing instructions in the processed function (we can never save more + // elements in this set than this number). Hence, the size of this set, at + // some point, will stop increasing, effectively reaching a fixpoint. + SmallPtrSet NoUBMemAccessInsts; }; struct AAUndefinedBehaviorFunction final : AAUndefinedBehaviorImpl { @@ -2075,7 +2085,7 @@ STATS_DECL(UndefinedBehaviorInstruction, Instruction, "Number of instructions known to have UB"); BUILD_STAT_NAME(UndefinedBehaviorInstruction, Instruction) += - UBLoads.size(); + UBMemAccessInsts.size(); } }; @@ -5573,6 +5583,8 @@ case Instruction::Invoke: case Instruction::CleanupRet: case Instruction::CatchSwitch: + case Instruction::AtomicRMW: + case Instruction::AtomicCmpXchg: case Instruction::Resume: case Instruction::Ret: IsInterestingOpcode = true; @@ -5812,9 +5824,9 @@ } template -raw_ostream & -llvm::operator<<(raw_ostream &OS, - const IntegerStateBase &S) { +raw_ostream &llvm:: +operator<<(raw_ostream &OS, + const IntegerStateBase &S) { return OS << "(" << S.getKnown() << "-" << S.getAssumed() << ")" << static_cast(S); } diff --git a/llvm/test/Transforms/Attributor/undefined_behavior.ll b/llvm/test/Transforms/Attributor/undefined_behavior.ll --- a/llvm/test/Transforms/Attributor/undefined_behavior.ll +++ b/llvm/test/Transforms/Attributor/undefined_behavior.ll @@ -6,22 +6,23 @@ ; We want to verify that whenever undefined behavior is assumed, the code becomes unreachable. ; We use FIXME's to indicate problems and missing attributes. -; ATTRIBUTOR: define void @wholly_unreachable() +; -- Load tests -- + +; ATTRIBUTOR-LABEL: define void @load_wholly_unreachable() +define void @load_wholly_unreachable() { ; ATTRIBUTOR-NEXT: unreachable -define void @wholly_unreachable() { %a = load i32, i32* null ret void } -; ATTRIBUTOR: define void @single_bb_unreachable(i1 %cond) -; ATTRIBUTOR-NEXT: br i1 %cond, label %t, label %e -; ATTRIBUTOR-EMPTY: -; ATTRIBUTOR-NEXT: t: -; ATTRIBUTOR-NEXT: unreachable -; ATTRIBUTOR-EMPTY: -; ATTRIBUTOR-NEXT: e: -; ATTRIBUTOR-NEXT: ret void -define void @single_bb_unreachable(i1 %cond) { +define void @load_single_bb_unreachable(i1 %cond) { +; ATTRIBUTOR-LABEL: @load_single_bb_unreachable( +; ATTRIBUTOR-NEXT: br i1 [[COND:%.*]], label [[T:%.*]], label [[E:%.*]] +; ATTRIBUTOR: t: +; ATTRIBUTOR-NEXT: unreachable +; ATTRIBUTOR: e: +; ATTRIBUTOR-NEXT: ret void +; br i1 %cond, label %t, label %e t: %b = load i32, i32* null @@ -30,9 +31,116 @@ ret void } -; ATTRIBUTOR: define void @null_pointer_is_defined() -; ATTRIBUTOR-NEXT: %a = load i32, i32* null -define void @null_pointer_is_defined() "null-pointer-is-valid"="true" { +define void @load_null_pointer_is_defined() "null-pointer-is-valid"="true" { +; ATTRIBUTOR-LABEL: @load_null_pointer_is_defined( +; ATTRIBUTOR-NEXT: [[A:%.*]] = load i32, i32* null +; ATTRIBUTOR-NEXT: ret void +; %a = load i32, i32* null ret void } + +; -- Store tests -- + +define void @store_wholly_unreachable() { +; ATTRIBUTOR-LABEL: @store_wholly_unreachable( +; ATTRIBUTOR-NEXT: unreachable +; + store i32 5, i32* null + ret void +} + +define void @store_single_bb_unreachable(i1 %cond) { +; ATTRIBUTOR-LABEL: @store_single_bb_unreachable( +; ATTRIBUTOR-NEXT: br i1 [[COND:%.*]], label [[T:%.*]], label [[E:%.*]] +; ATTRIBUTOR: t: +; ATTRIBUTOR-NEXT: unreachable +; ATTRIBUTOR: e: +; ATTRIBUTOR-NEXT: ret void +; + br i1 %cond, label %t, label %e +t: + store i32 5, i32* null + br label %e +e: + ret void +} + +define void @store_null_pointer_is_defined() "null-pointer-is-valid"="true" { +; ATTRIBUTOR-LABEL: @store_null_pointer_is_defined( +; ATTRIBUTOR-NEXT: store i32 5, i32* null +; ATTRIBUTOR-NEXT: ret void +; + store i32 5, i32* null + ret void +} + +; -- AtomicRMW tests -- + +define void @atomicrmw_wholly_unreachable() { +; ATTRIBUTOR-LABEL: @atomicrmw_wholly_unreachable( +; ATTRIBUTOR-NEXT: unreachable +; + %a = atomicrmw add i32* null, i32 1 acquire + ret void +} + +define void @atomicrmw_single_bb_unreachable(i1 %cond) { +; ATTRIBUTOR-LABEL: @atomicrmw_single_bb_unreachable( +; ATTRIBUTOR-NEXT: br i1 [[COND:%.*]], label [[T:%.*]], label [[E:%.*]] +; ATTRIBUTOR: t: +; ATTRIBUTOR-NEXT: unreachable +; ATTRIBUTOR: e: +; ATTRIBUTOR-NEXT: ret void +; + br i1 %cond, label %t, label %e +t: + %a = atomicrmw add i32* null, i32 1 acquire + br label %e +e: + ret void +} + +define void @atomicrmw_null_pointer_is_defined() "null-pointer-is-valid"="true" { +; ATTRIBUTOR-LABEL: @atomicrmw_null_pointer_is_defined( +; ATTRIBUTOR-NEXT: [[A:%.*]] = atomicrmw add i32* null, i32 1 acquire +; ATTRIBUTOR-NEXT: ret void +; + %a = atomicrmw add i32* null, i32 1 acquire + ret void +} + +; -- AtomicCmpXchg tests -- + +define void @atomiccmpxchg_wholly_unreachable() { +; ATTRIBUTOR-LABEL: @atomiccmpxchg_wholly_unreachable( +; ATTRIBUTOR-NEXT: unreachable +; + %a = cmpxchg i32* null, i32 2, i32 3 acq_rel monotonic + ret void +} + +define void @atomiccmpxchg_single_bb_unreachable(i1 %cond) { +; ATTRIBUTOR-LABEL: @atomiccmpxchg_single_bb_unreachable( +; ATTRIBUTOR-NEXT: br i1 [[COND:%.*]], label [[T:%.*]], label [[E:%.*]] +; ATTRIBUTOR: t: +; ATTRIBUTOR-NEXT: unreachable +; ATTRIBUTOR: e: +; ATTRIBUTOR-NEXT: ret void +; + br i1 %cond, label %t, label %e +t: + %a = cmpxchg i32* null, i32 2, i32 3 acq_rel monotonic + br label %e +e: + ret void +} + +define void @atomiccmpxchg_null_pointer_is_defined() "null-pointer-is-valid"="true" { +; ATTRIBUTOR-LABEL: @atomiccmpxchg_null_pointer_is_defined( +; ATTRIBUTOR-NEXT: [[A:%.*]] = cmpxchg i32* null, i32 2, i32 3 acq_rel monotonic +; ATTRIBUTOR-NEXT: ret void +; + %a = cmpxchg i32* null, i32 2, i32 3 acq_rel monotonic + ret void +}