[llvm] [llvm] Bail out when meeting pointer with negative offset instead of … (PR #120424)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 18 15:00:42 PST 2024
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/120424
>From ab73245ab7e8d2b258d9639b9351adfe539a2f10 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Wed, 18 Dec 2024 14:38:40 +0100
Subject: [PATCH 1/2] [llvm] Bail out when meeting pointer with negative offset
instead of generating empty location
Fix the regression detected by https://github.com/llvm/llvm-test-suite/pull/188
---
llvm/lib/Analysis/MemoryBuiltins.cpp | 3 +--
.../builtin-object-size-phi.ll | 2 +-
.../builtin-object-size-range.ll | 22 ++++++++++++++++++-
.../objectsize_basic.ll | 4 ++--
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 57b97999b08860..cc70e4a1e056e1 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -841,8 +841,7 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
// This is UB, and we'd rather return an empty location then.
if (Options.EvalMode == ObjectSizeOpts::Mode::Min ||
Options.EvalMode == ObjectSizeOpts::Mode::Max) {
- ORT.Before = APInt::getZero(ORT.Before.getBitWidth());
- ORT.After = APInt::getZero(ORT.Before.getBitWidth());
+ return ObjectSizeOffsetVisitor::unknown();
}
// Otherwise it's fine, caller can handle negative offset.
}
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index cba4da073ff2aa..564311da64a81f 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -143,7 +143,7 @@ define dso_local i64 @pick_max_one_oob(i1 %c0, i1 %c1) {
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[P_END:%.*]] = phi ptr [ [[P_ELSE]], [[IF_ELSE]] ], [ [[P_THEN]], [[IF_THEN]] ]
-; CHECK-NEXT: [[OBJSIZE:%.*]] = select i1 [[C1:%.*]], i64 1, i64 0
+; CHECK-NEXT: [[OBJSIZE:%.*]] = select i1 [[C1:%.*]], i64 -1, i64 0
; CHECK-NEXT: ret i64 [[OBJSIZE]]
;
%p = alloca [2 x i8], align 1
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-range.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-range.ll
index f84ebee1442893..891a585724e655 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-range.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-range.ll
@@ -78,7 +78,8 @@ define i64 @select_neg_oob_offset(i1 %c0, i1 %c1) {
; CHECK-NEXT: [[PTR:%.*]] = alloca i8, i64 10, align 1
; CHECK-NEXT: [[OFFSET:%.*]] = select i1 [[C0:%.*]], i64 -3, i64 -4
; CHECK-NEXT: [[PTR_SLIDE:%.*]] = getelementptr inbounds i8, ptr [[PTR]], i64 [[OFFSET]]
-; CHECK-NEXT: ret i64 0
+; CHECK-NEXT: [[RES:%.*]] = select i1 [[C1:%.*]], i64 -1, i64 0
+; CHECK-NEXT: ret i64 [[RES]]
;
%ptr = alloca i8, i64 10
%offset = select i1 %c0, i64 -3, i64 -4
@@ -106,4 +107,23 @@ define i64 @select_gep_offsets(i1 %cond) {
ret i64 %res
}
+define i64 @select_gep_oob_offsets(i1 %cond) {
+; CHECK-LABEL: @select_gep_oob_offsets(
+; CHECK-NEXT: [[BASE1:%.*]] = alloca [288 x i8], align 16
+; CHECK-NEXT: [[SELECT0:%.*]] = select i1 [[COND:%.*]], i64 -4, i64 -64
+; CHECK-NEXT: [[SELECT1:%.*]] = select i1 [[COND]], i64 16, i64 64
+; CHECK-NEXT: [[GEP0:%.*]] = getelementptr inbounds nuw i8, ptr [[BASE1]], i64 [[SELECT1]]
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr inbounds i8, ptr [[GEP0]], i64 [[SELECT0]]
+; CHECK-NEXT: ret i64 -1
+;
+ %base1 = alloca [288 x i8], align 16
+ %select0 = select i1 %cond, i64 -4, i64 -64
+ %select1 = select i1 %cond, i64 16, i64 64
+ %gep0 = getelementptr inbounds nuw i8, ptr %base1, i64 %select1
+ %gep1 = getelementptr inbounds i8, ptr %gep0, i64 %select0
+ %call = call i64 @llvm.objectsize.i64.p0(ptr %gep1, i1 false, i1 true, i1 false)
+ ret i64 %call
+}
+
+
attributes #0 = { nounwind allocsize(0) }
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/objectsize_basic.ll b/llvm/test/Transforms/LowerConstantIntrinsics/objectsize_basic.ll
index 212b4a432db3c4..0eec7f75014eb3 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/objectsize_basic.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/objectsize_basic.ll
@@ -214,7 +214,7 @@ define i64 @wrapping_gep_neg(i1 %c) {
; CHECK-NEXT: [[OBJ:%.*]] = alloca i8, i64 4, align 1
; CHECK-NEXT: [[SLIDE:%.*]] = getelementptr i8, ptr [[OBJ]], i64 9223372036854775807
; CHECK-NEXT: [[SLIDE_BIS:%.*]] = getelementptr i8, ptr [[SLIDE]], i64 9223372036854775807
-; CHECK-NEXT: ret i64 0
+; CHECK-NEXT: ret i64 -1
;
%obj = alloca i8, i64 4
%slide = getelementptr i8, ptr %obj, i64 9223372036854775807
@@ -269,7 +269,7 @@ define i64 @out_of_bound_negative_gep(i1 %c) {
; CHECK-LABEL: @out_of_bound_negative_gep(
; CHECK-NEXT: [[OBJ:%.*]] = alloca i8, i32 4, align 1
; CHECK-NEXT: [[SLIDE:%.*]] = getelementptr i8, ptr [[OBJ]], i8 -8
-; CHECK-NEXT: ret i64 0
+; CHECK-NEXT: ret i64 -1
;
%obj = alloca i8, i32 4
%slide = getelementptr i8, ptr %obj, i8 -8
>From b228a0bb79bcd8cee99c461f3453fc5848c326d4 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 19 Dec 2024 00:00:20 +0100
Subject: [PATCH 2/2] fixup! [llvm] Bail out when meeting pointer with negative
offset instead of generating empty location
---
llvm/lib/Analysis/MemoryBuiltins.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index cc70e4a1e056e1..809d4e406bc157 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -838,7 +838,11 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
// We end up pointing on a location that's outside of the original object.
if (ORT.knownBefore() && ORT.Before.isNegative()) {
- // This is UB, and we'd rather return an empty location then.
+ // This means that we *may* be accessing memory before the allocation. It's
+ // unsure though, so bail out instead of returning a potentially misleading
+ // result.
+ // TODO: working with ranges instead of value would make it possible to take
+ // a better decision.
if (Options.EvalMode == ObjectSizeOpts::Mode::Min ||
Options.EvalMode == ObjectSizeOpts::Mode::Max) {
return ObjectSizeOffsetVisitor::unknown();
More information about the llvm-commits
mailing list