[llvm] [llvm] Fix __builtin_object_size interaction between Negative Offset … (PR #111827)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 05:19:23 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-transforms
Author: None (serge-sans-paille)
<details>
<summary>Changes</summary>
…and Select/Phi
When picking a SizeOffsetAPInt through combineSizeOffset, the behavior differs if we're going to apply a constant offset that's positive or negative: If it's positive, then we need to compare the remaining bytes (i.e. Size
- Offset), but if it's negative, we need to compare the preceding bytes (i.e. Offset).
Fix #<!-- -->111709
---
Full diff: https://github.com/llvm/llvm-project/pull/111827.diff
3 Files Affected:
- (modified) llvm/include/llvm/Analysis/MemoryBuiltins.h (+2)
- (modified) llvm/lib/Analysis/MemoryBuiltins.cpp (+45-13)
- (modified) llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll (+107)
``````````diff
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..eb6e139a6b9d6a 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 APInt getOffsetWithOverflow(const SizeOffsetAPInt &Data) {
+ APInt Size = Data.Size;
+ APInt Offset = Data.Offset;
+ if (Offset.isNegative())
+ return APInt(Size.getBitWidth(), 0);
+ return 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,9 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
Zero = APInt::getZero(IntTyBits);
+ std::swap(Offset, ConstantOffset);
SizeOffsetAPInt SOT = computeValue(V);
+ std::swap(Offset, ConstantOffset);
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
if (!IndexTypeSizeChanged && Offset.isZero())
@@ -981,18 +992,39 @@ ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
SizeOffsetAPInt RHS) {
if (!LHS.bothKnown() || !RHS.bothKnown())
return ObjectSizeOffsetVisitor::unknown();
-
- switch (Options.EvalMode) {
- case ObjectSizeOpts::Mode::Min:
- return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::Max:
- return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::ExactSizeFromOffset:
- return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
- ? LHS
- : ObjectSizeOffsetVisitor::unknown();
- case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
- return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ // If the ConstantOffset we add in the end is negative, then we're actually
+ // interested in selecting the nodes based on their offset rather than their
+ // size.
+ if (ConstantOffset.isNegative()) {
+ switch (Options.EvalMode) {
+ case ObjectSizeOpts::Mode::Min:
+ return (getOffsetWithOverflow(LHS).slt(getOffsetWithOverflow(RHS))) ? LHS
+ : RHS;
+ case ObjectSizeOpts::Mode::Max:
+ return (getOffsetWithOverflow(LHS).sgt(getOffsetWithOverflow(RHS))) ? LHS
+ : RHS;
+ case ObjectSizeOpts::Mode::ExactSizeFromOffset:
+ return (getOffsetWithOverflow(LHS).eq(getOffsetWithOverflow(RHS)))
+ ? LHS
+ : ObjectSizeOffsetVisitor::unknown();
+ case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
+ return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ }
+ } else {
+ switch (Options.EvalMode) {
+ case ObjectSizeOpts::Mode::Min:
+ return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS
+ : RHS;
+ case ObjectSizeOpts::Mode::Max:
+ return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS
+ : RHS;
+ case ObjectSizeOpts::Mode::ExactSizeFromOffset:
+ return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
+ ? LHS
+ : ObjectSizeOffsetVisitor::unknown();
+ case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
+ return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ }
}
llvm_unreachable("missing an eval mode");
}
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index 4f4d6a88e1693b..27cbc391d52c3a 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -117,3 +117,110 @@ 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: [[SIZE:%.*]] = select i1 [[COND]], i64 0, i64 4
+; CHECK-NEXT: ret i64 [[SIZE]]
+;
+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
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/111827
More information about the llvm-commits
mailing list