[llvm-branch-commits] [llvm] 61c8cf9 - [Attributor][FIX] Do not use assumed information for UB detection
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Feb 8 21:27:45 PST 2022
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 }
More information about the llvm-branch-commits
mailing list