Author: Johannes Doerfert Date: 2022-02-08T21:27:13-08:00 New Revision: 61c8cf97479f37c63724f63fd4c727cd2b433ae9
URL: https://github.com/llvm/llvm-project/commit/61c8cf97479f37c63724f63fd4c727cd2b433ae9 DIFF: https://github.com/llvm/llvm-project/commit/61c8cf97479f37c63724f63fd4c727cd2b433ae9.diff LOG: [Attributor][FIX] Do not use assumed information for UB detection The helper `Attributor::checkForAllReturnedValuesAndReturnInsts` simplifies the returned value optimistically. In `AAUndefinedBehavior` we cannot use such optimistic values when deducing UB. As a result, we assumed UB for the return value of a function because we initially (=optimistically) thought the function return is `undef`. While we later adjusted this properly, the `AAUndefinedBehavior` was under the impression the return value is "known" (=fix) and could never change. To correct this we use `Attributor::checkForAllInstructions` and then manually to perform simplification of the return value, only allowing known values to be used. This actually matches the other UB deductions. Fixes #53647 (cherry picked from commit dd101c808b85aad8edb48ab6d5f754cc6527fcff) Added: Modified: llvm/lib/Transforms/IPO/AttributorAttributes.cpp llvm/test/Transforms/Attributor/undefined_behavior.ll Removed: ################################################################################ diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp index 2d88e329e093..db2ba91bea58 100644 --- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp +++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp @@ -2691,40 +2691,38 @@ struct AAUndefinedBehaviorImpl : public AAUndefinedBehavior { return true; }; - auto InspectReturnInstForUB = - [&](Value &V, const SmallSetVector<ReturnInst *, 4> RetInsts) { - // Check if a return instruction always cause UB or not - // Note: It is guaranteed that the returned position of the anchor - // scope has noundef attribute when this is called. - // We also ensure the return position is not "assumed dead" - // because the returned value was then potentially simplified to - // `undef` in AAReturnedValues without removing the `noundef` - // attribute yet. - - // When the returned position has noundef attriubte, UB occur in the - // following cases. - // (1) Returned value is known to be undef. - // (2) The value is known to be a null pointer and the returned - // position has nonnull attribute (because the returned value is - // poison). - bool FoundUB = false; - if (isa<UndefValue>(V)) { - FoundUB = true; - } else { - if (isa<ConstantPointerNull>(V)) { - auto &NonNullAA = A.getAAFor<AANonNull>( - *this, IRPosition::returned(*getAnchorScope()), - DepClassTy::NONE); - if (NonNullAA.isKnownNonNull()) - FoundUB = true; - } - } + auto InspectReturnInstForUB = [&](Instruction &I) { + auto &RI = cast<ReturnInst>(I); + // Either we stopped and the appropriate action was taken, + // or we got back a simplified return value to continue. + Optional<Value *> SimplifiedRetValue = + stopOnUndefOrAssumed(A, RI.getReturnValue(), &I); + if (!SimplifiedRetValue.hasValue() || !SimplifiedRetValue.getValue()) + return true; - if (FoundUB) - for (ReturnInst *RI : RetInsts) - KnownUBInsts.insert(RI); - return true; - }; + // Check if a return instruction always cause UB or not + // Note: It is guaranteed that the returned position of the anchor + // scope has noundef attribute when this is called. + // We also ensure the return position is not "assumed dead" + // because the returned value was then potentially simplified to + // `undef` in AAReturnedValues without removing the `noundef` + // attribute yet. + + // When the returned position has noundef attriubte, UB occurs in the + // following cases. + // (1) Returned value is known to be undef. + // (2) The value is known to be a null pointer and the returned + // position has nonnull attribute (because the returned value is + // poison). + if (isa<ConstantPointerNull>(*SimplifiedRetValue)) { + auto &NonNullAA = A.getAAFor<AANonNull>( + *this, IRPosition::returned(*getAnchorScope()), DepClassTy::NONE); + if (NonNullAA.isKnownNonNull()) + KnownUBInsts.insert(&I); + } + + return true; + }; bool UsedAssumedInformation = false; A.checkForAllInstructions(InspectMemAccessInstForUB, *this, @@ -2747,8 +2745,9 @@ struct AAUndefinedBehaviorImpl : public AAUndefinedBehavior { auto &RetPosNoUndefAA = A.getAAFor<AANoUndef>(*this, ReturnIRP, DepClassTy::NONE); if (RetPosNoUndefAA.isKnownNoUndef()) - A.checkForAllReturnedValuesAndReturnInsts(InspectReturnInstForUB, - *this); + A.checkForAllInstructions(InspectReturnInstForUB, *this, + {Instruction::Ret}, UsedAssumedInformation, + /* CheckBBLivenessOnly */ true); } } diff --git a/llvm/test/Transforms/Attributor/undefined_behavior.ll b/llvm/test/Transforms/Attributor/undefined_behavior.ll index bb5a7ef19bbe..40d6340c185b 100644 --- a/llvm/test/Transforms/Attributor/undefined_behavior.ll +++ b/llvm/test/Transforms/Attributor/undefined_behavior.ll @@ -1,6 +1,6 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals -; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM -; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM +; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM +; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM ; RUN: opt -attributor-cgscc -enable-new-pm=0 -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM @@ -793,6 +793,39 @@ define i32* @violate_noundef_pointer() { %ret = call i32* @argument_noundef2(i32* undef) ret i32* %ret } + +define internal noundef i32 @assumed_undef_is_ok(i1 %c, i32 %arg) { +; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn +; IS__CGSCC____-LABEL: define {{[^@]+}}@assumed_undef_is_ok +; IS__CGSCC____-SAME: (i1 [[C:%.*]]) #[[ATTR0]] { +; IS__CGSCC____-NEXT: br i1 [[C]], label [[REC:%.*]], label [[RET:%.*]] +; IS__CGSCC____: rec: +; IS__CGSCC____-NEXT: br label [[RET]] +; IS__CGSCC____: ret: +; IS__CGSCC____-NEXT: ret i32 undef +; + %stack = alloca i32 + store i32 %arg, i32* %stack + br i1 %c, label %rec, label %ret +rec: + %call = call i32 @assumed_undef_is_ok(i1 false, i32 0) + store i32 %call, i32* %stack + br label %ret +ret: + %l = load i32, i32* %stack + ret i32 %l +} + +define noundef i32 @assumed_undef_is_ok_caller(i1 %c) { +; CHECK: Function Attrs: nofree norecurse nosync nounwind readnone willreturn +; CHECK-LABEL: define {{[^@]+}}@assumed_undef_is_ok_caller +; CHECK-SAME: (i1 [[C:%.*]]) #[[ATTR0]] { +; CHECK-NEXT: ret i32 0 +; + %call = call i32 @assumed_undef_is_ok(i1 %c, i32 undef) + ret i32 %call +} + ;. ; IS__TUNIT____: attributes #[[ATTR0]] = { nofree norecurse nosync nounwind readnone willreturn } ; IS__TUNIT____: attributes #[[ATTR1]] = { nofree norecurse nosync nounwind null_pointer_is_valid readnone willreturn } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
