Skip to content

Commit a2ccd5d

Browse files
authored
[FunctionAttrs] Fix incorrect noundef inference with poison attrs (#89348)
Currently, when inferring noundef, we only check that the return value is not undef/poison. However, we fail to account for the possibility that a poison-generating return attribute will convert the value to poison, and then violate the noundef attribute, resulting in immediate UB. For the relevant return attributes (align, nonnull and range), check whether we can trivially re-prove the relevant property, otherwise do not infer noundef. This fixes the FunctionAttrs side of #88026.
1 parent 07b1177 commit a2ccd5d

File tree

2 files changed

+27
-9
lines changed

2 files changed

+27
-9
lines changed

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,7 +1287,8 @@ static void addNoUndefAttrs(const SCCNodeSet &SCCNodes,
12871287
// values.
12881288
for (Function *F : SCCNodes) {
12891289
// Already noundef.
1290-
if (F->getAttributes().hasRetAttr(Attribute::NoUndef))
1290+
AttributeList Attrs = F->getAttributes();
1291+
if (Attrs.hasRetAttr(Attribute::NoUndef))
12911292
continue;
12921293

12931294
// We can infer and propagate function attributes only when we know that the
@@ -1305,10 +1306,30 @@ static void addNoUndefAttrs(const SCCNodeSet &SCCNodes,
13051306
if (F->getReturnType()->isVoidTy())
13061307
continue;
13071308

1308-
if (all_of(*F, [](BasicBlock &BB) {
1309+
const DataLayout &DL = F->getParent()->getDataLayout();
1310+
if (all_of(*F, [&](BasicBlock &BB) {
13091311
if (auto *Ret = dyn_cast<ReturnInst>(BB.getTerminator())) {
13101312
// TODO: perform context-sensitive analysis?
1311-
return isGuaranteedNotToBeUndefOrPoison(Ret->getReturnValue());
1313+
Value *RetVal = Ret->getReturnValue();
1314+
if (!isGuaranteedNotToBeUndefOrPoison(RetVal))
1315+
return false;
1316+
1317+
// We know the original return value is not poison now, but it
1318+
// could still be converted to poison by another return attribute.
1319+
// Try to explicitly re-prove the relevant attributes.
1320+
if (Attrs.hasRetAttr(Attribute::NonNull) &&
1321+
!isKnownNonZero(RetVal, DL))
1322+
return false;
1323+
1324+
if (MaybeAlign Align = Attrs.getRetAlignment())
1325+
if (RetVal->getPointerAlignment(DL) < *Align)
1326+
return false;
1327+
1328+
Attribute Attr = Attrs.getRetAttr(Attribute::Range);
1329+
if (Attr.isValid() &&
1330+
!Attr.getRange().contains(
1331+
computeConstantRange(RetVal, /*ForSigned=*/false)))
1332+
return false;
13121333
}
13131334
return true;
13141335
})) {

llvm/test/Transforms/FunctionAttrs/noundef.ll

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,8 @@ define i64 @test_trunc_with_constexpr() {
167167
ret i64 %conv
168168
}
169169

170-
; FIXME: This is a miscompile.
171170
define align 4 ptr @maybe_not_aligned(ptr noundef %p) {
172-
; CHECK-LABEL: define noundef align 4 ptr @maybe_not_aligned(
171+
; CHECK-LABEL: define align 4 ptr @maybe_not_aligned(
173172
; CHECK-SAME: ptr noundef readnone returned [[P:%.*]]) #[[ATTR0]] {
174173
; CHECK-NEXT: ret ptr [[P]]
175174
;
@@ -184,9 +183,8 @@ define align 4 ptr @definitely_aligned(ptr noundef align 4 %p) {
184183
ret ptr %p
185184
}
186185

187-
; FIXME: This is a miscompile.
188186
define nonnull ptr @maybe_not_nonnull(ptr noundef %p) {
189-
; CHECK-LABEL: define noundef nonnull ptr @maybe_not_nonnull(
187+
; CHECK-LABEL: define nonnull ptr @maybe_not_nonnull(
190188
; CHECK-SAME: ptr noundef readnone returned [[P:%.*]]) #[[ATTR0]] {
191189
; CHECK-NEXT: ret ptr [[P]]
192190
;
@@ -201,9 +199,8 @@ define nonnull ptr @definitely_nonnull(ptr noundef nonnull %p) {
201199
ret ptr %p
202200
}
203201

204-
; FIXME: This is a miscompile.
205202
define range(i8 0, 10) i8 @maybe_not_in_range(i8 noundef %v) {
206-
; CHECK-LABEL: define noundef range(i8 0, 10) i8 @maybe_not_in_range(
203+
; CHECK-LABEL: define range(i8 0, 10) i8 @maybe_not_in_range(
207204
; CHECK-SAME: i8 noundef returned [[V:%.*]]) #[[ATTR0]] {
208205
; CHECK-NEXT: ret i8 [[V]]
209206
;

0 commit comments

Comments
 (0)