[llvm] [llvm] Fix __builtin_object_size interaction between Negative Offset … (PR #111827)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 13 12:28:08 PDT 2024
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/111827
>From b4804bc0b3af528e7189a7525c17cf475f0f8699 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 10 Oct 2024 13:59:22 +0200
Subject: [PATCH] [llvm] Fix __builtin_object_size interaction between Negative
Offset and Select/Phi
When picking a SizeOffsetAPInt through combineSizeOffset, the current
constant offset that has been extracted should be taken into account to
perform the right decision, otherwise in the presence of nullptr, we may
perform the wrong decision.
Fix #111709
---
llvm/include/llvm/Analysis/MemoryBuiltins.h | 2 +
llvm/lib/Analysis/MemoryBuiltins.cpp | 41 ++++--
.../builtin-object-size-phi.ll | 130 ++++++++++++++++++
3 files changed, 165 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index 7b48844cc9e8e9..01c642d4f48abd 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -160,6 +160,7 @@ struct ObjectSizeOpts {
/// though they can't be evaluated. Otherwise, null is always considered to
/// point to a 0 byte region of memory.
bool NullIsUnknownSize = false;
+
/// If set, used for more accurate evaluation
AAResults *AA = nullptr;
};
@@ -230,6 +231,7 @@ class ObjectSizeOffsetVisitor
ObjectSizeOpts Options;
unsigned IntTyBits;
APInt Zero;
+ APInt ConstantOffset;
SmallDenseMap<Instruction *, SizeOffsetAPInt, 8> SeenInsts;
unsigned InstructionsVisited;
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index e1abf5e4d885ec..b84841d4ba80c5 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -570,6 +570,14 @@ static APInt getSizeWithOverflow(const SizeOffsetAPInt &Data) {
return Size - Offset;
}
+static SizeOffsetAPInt applyConstantOffset(SizeOffsetAPInt SOT,
+ APInt ConstantOffset) {
+ if (ConstantOffset.isZero())
+ return SOT;
+ return {SOT.Size, SOT.Offset.getBitWidth() > 1 ? SOT.Offset + ConstantOffset
+ : SOT.Offset};
+}
+
/// Compute the size of the object pointed by Ptr. Returns true and the
/// object size in Size if successful, and false otherwise.
/// If RoundToAlign is true, then Size is rounded up to the alignment of
@@ -697,7 +705,8 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
// the index type size and if we stripped address space casts we have to
// readjust the APInt as we pass it upwards in order for the APInt to match
// the type the caller passed in.
- APInt Offset(InitialIntTyBits, 0);
+
+ APInt Offset = APInt{InitialIntTyBits, 0};
V = V->stripAndAccumulateConstantOffsets(
DL, Offset, /* AllowNonInbounds */ true, /* AllowInvariantGroup */ true);
@@ -706,7 +715,11 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
Zero = APInt::getZero(IntTyBits);
+ APInt PrevConstantOffset = ConstantOffset;
+
+ ConstantOffset = ConstantOffset.sextOrTrunc(Offset.getBitWidth()) + Offset;
SizeOffsetAPInt SOT = computeValue(V);
+ ConstantOffset = PrevConstantOffset;
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
if (!IndexTypeSizeChanged && Offset.isZero())
@@ -723,8 +736,7 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
SOT.Offset = APInt();
}
// If the computed offset is "unknown" we cannot add the stripped offset.
- return {SOT.Size,
- SOT.Offset.getBitWidth() > 1 ? SOT.Offset + Offset : SOT.Offset};
+ return applyConstantOffset(SOT, Offset);
}
SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
@@ -981,18 +993,28 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
SizeOffsetAPInt RHS) {
if (!LHS.bothKnown() || !RHS.bothKnown())
return ObjectSizeOffsetVisitor::unknown();
+ SizeOffsetAPInt AdjustedLHS = applyConstantOffset(LHS, ConstantOffset);
+ SizeOffsetAPInt AdjustedRHS = applyConstantOffset(RHS, ConstantOffset);
switch (Options.EvalMode) {
case ObjectSizeOpts::Mode::Min:
- return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
+ return (getSizeWithOverflow(AdjustedLHS)
+ .slt(getSizeWithOverflow(AdjustedRHS)))
+ ? LHS
+ : RHS;
case ObjectSizeOpts::Mode::Max:
- return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
+ return (getSizeWithOverflow(AdjustedLHS)
+ .sgt(getSizeWithOverflow(AdjustedRHS)))
+ ? LHS
+ : RHS;
case ObjectSizeOpts::Mode::ExactSizeFromOffset:
- return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
+ return (getSizeWithOverflow(AdjustedLHS)
+ .eq(getSizeWithOverflow(AdjustedRHS)))
? LHS
: ObjectSizeOffsetVisitor::unknown();
case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
- return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ return AdjustedLHS == AdjustedRHS ? LHS
+ : ObjectSizeOffsetVisitor::unknown();
}
llvm_unreachable("missing an eval mode");
}
@@ -1000,11 +1022,14 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
SizeOffsetAPInt ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
if (PN.getNumIncomingValues() == 0)
return ObjectSizeOffsetVisitor::unknown();
+
auto IncomingValues = PN.incoming_values();
+
return std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
computeImpl(*IncomingValues.begin()),
[this](SizeOffsetAPInt LHS, Value *VRHS) {
- return combineSizeOffset(LHS, computeImpl(VRHS));
+ SizeOffsetAPInt RHS = computeImpl(VRHS);
+ return combineSizeOffset(LHS, RHS);
});
}
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index 4f4d6a88e1693b..d9e7d628ce6cbb 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -117,3 +117,133 @@ if.end:
%size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 true, i1 true, i1 false)
ret i64 %size
}
+
+define i64 @pick_negative_offset(i32 %n) {
+; CHECK-LABEL: @pick_negative_offset(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
+; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: [[BUFFER1:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED1:%.*]] = getelementptr i8, ptr [[BUFFER1]], i64 20
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[OFFSETED1]], [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[POFFSETED:%.*]] = getelementptr i8, ptr [[P]], i64 -4
+; CHECK-NEXT: ret i64 4
+;
+entry:
+ %buffer0 = alloca i8, i64 20
+ %offseted0 = getelementptr i8, ptr %buffer0, i64 20
+ %cond = icmp eq i32 %n, 0
+ br i1 %cond, label %if.else, label %if.end
+
+if.else:
+ %buffer1 = alloca i8, i64 20
+ %offseted1 = getelementptr i8, ptr %buffer1, i64 20
+ br label %if.end
+
+if.end:
+ %p = phi ptr [ %offseted1, %if.else ], [ %offseted0, %entry ]
+ %poffseted = getelementptr i8, ptr %p, i64 -4
+ %size = call i64 @llvm.objectsize.i64.p0(ptr %poffseted, i1 false, i1 false, i1 false)
+ ret i64 %size
+}
+
+define i64 @pick_negative_offset_with_nullptr(i32 %n) {
+; CHECK-LABEL: @pick_negative_offset_with_nullptr(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
+; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[P0:%.*]] = phi ptr [ [[OFFSETED0]], [[ENTRY:%.*]] ], [ null, [[IF_ELSE]] ]
+; CHECK-NEXT: [[P1:%.*]] = phi ptr [ null, [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY]] ]
+; CHECK-NEXT: [[P0OFFSETED:%.*]] = getelementptr i8, ptr [[P0]], i64 -4
+; CHECK-NEXT: [[P1OFFSETED:%.*]] = getelementptr i8, ptr [[P1]], i64 -4
+; CHECK-NEXT: ret i64 4
+;
+entry:
+ %buffer0 = alloca i8, i64 20
+ %offseted0 = getelementptr i8, ptr %buffer0, i64 20
+ %cond = icmp eq i32 %n, 0
+ br i1 %cond, label %if.else, label %if.end
+
+if.else:
+ br label %if.end
+
+if.end:
+ %p0 = phi ptr [ %offseted0, %entry ], [ null, %if.else ]
+ %p1 = phi ptr [ null, %if.else ], [ %offseted0, %entry ]
+ %p0offseted = getelementptr i8, ptr %p0, i64 -4
+ %p1offseted = getelementptr i8, ptr %p1, i64 -4
+ %size0 = call i64 @llvm.objectsize.i64.p0(ptr %p0offseted, i1 false, i1 false, i1 false)
+ %size1 = call i64 @llvm.objectsize.i64.p0(ptr %p1offseted, i1 false, i1 false, i1 false)
+ %size = select i1 %cond, i64 %size0, i64 %size1
+ ret i64 %size
+}
+
+define i64 @pick_negative_offset_with_unsized_nullptr(i32 %n) {
+; CHECK-LABEL: @pick_negative_offset_with_unsized_nullptr(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
+; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[P0:%.*]] = phi ptr [ [[OFFSETED0]], [[ENTRY:%.*]] ], [ null, [[IF_ELSE]] ]
+; CHECK-NEXT: [[P1:%.*]] = phi ptr [ null, [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY]] ]
+; CHECK-NEXT: [[P0OFFSETED:%.*]] = getelementptr i8, ptr [[P0]], i64 -4
+; CHECK-NEXT: [[P1OFFSETED:%.*]] = getelementptr i8, ptr [[P1]], i64 -4
+; CHECK-NEXT: ret i64 -1
+;
+entry:
+ %buffer0 = alloca i8, i64 20
+ %offseted0 = getelementptr i8, ptr %buffer0, i64 20
+ %cond = icmp eq i32 %n, 0
+ br i1 %cond, label %if.else, label %if.end
+
+if.else:
+ br label %if.end
+
+if.end:
+ %p0 = phi ptr [ %offseted0, %entry ], [ null, %if.else ]
+ %p1 = phi ptr [ null, %if.else ], [ %offseted0, %entry ]
+ %p0offseted = getelementptr i8, ptr %p0, i64 -4
+ %p1offseted = getelementptr i8, ptr %p1, i64 -4
+ %size0 = call i64 @llvm.objectsize.i64.p0(ptr %p0offseted, i1 false, i1 true, i1 false)
+ %size1 = call i64 @llvm.objectsize.i64.p0(ptr %p1offseted, i1 false, i1 true, i1 false)
+ %size = select i1 %cond, i64 %size0, i64 %size1
+ ret i64 %size
+}
+
+define i64 @chain_pick_negative_offset_with_nullptr(i32 %x) {
+; CHECK-LABEL: @chain_pick_negative_offset_with_nullptr(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[ARRAY:%.*]] = alloca [4 x i32], align 4
+; CHECK-NEXT: [[C:%.*]] = icmp eq i32 [[X:%.*]], 0
+; CHECK-NEXT: [[P:%.*]] = getelementptr i8, ptr [[ARRAY]], i64 8
+; CHECK-NEXT: [[COND:%.*]] = select i1 [[C]], ptr [[P]], ptr null
+; CHECK-NEXT: [[P4:%.*]] = getelementptr i8, ptr [[COND]], i64 8
+; CHECK-NEXT: [[COND6:%.*]] = select i1 [[C]], ptr [[P4]], ptr null
+; CHECK-NEXT: [[P7:%.*]] = getelementptr i8, ptr [[COND6]], i64 -4
+; CHECK-NEXT: ret i64 4
+;
+entry:
+ %array = alloca [4 x i32]
+ %c = icmp eq i32 %x, 0
+ %p = getelementptr i8, ptr %array, i64 8
+ %cond = select i1 %c, ptr %p, ptr null
+ %p4 = getelementptr i8, ptr %cond, i64 8
+ %cond6 = select i1 %c, ptr %p4, ptr null
+ %p7 = getelementptr i8, ptr %cond6, i64 -4
+ %size = call i64 @llvm.objectsize.i64.p0(ptr %p7, i1 false, i1 false, i1 false)
+ ret i64 %size
+}
More information about the llvm-commits
mailing list