From 19ddafafdf131aed40abbdaf5af1fb7b59c1e8ac Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton@mozilla.com>
Date: Sat, 23 Nov 2024 07:38:37 +0000
Subject: [PATCH] =?UTF-8?q?[llvm]=20Fix=20ObjectSizeOffsetVisitor=20behavi?=
 =?UTF-8?q?or=20in=20exact=20mode=20upon=20negati=E2=80=A6=20(#116955)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

…ve offset

In Exact mode, the approximation of returning (0,0) is invalid. It only
holds in min/max mode.
---
 llvm/lib/Analysis/MemoryBuiltins.cpp          | 15 ++++---
 .../BoundsChecking/negative.ll                | 45 +++++++++++++++++++
 2 files changed, 53 insertions(+), 7 deletions(-)
 create mode 100644 llvm/test/Instrumentation/BoundsChecking/negative.ll

diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index cd8594d670502d..4028d5f4e2e1b8 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -565,10 +565,7 @@ static APInt getSizeWithOverflow(const SizeOffsetAPInt &Data) {
   APInt Size = Data.Size;
   APInt Offset = Data.Offset;
 
-  assert(!Offset.isNegative() &&
-         "size for a pointer before the allocated object is ambiguous");
-
-  if (Size.ult(Offset))
+  if (Offset.isNegative() || Size.ult(Offset))
     return APInt::getZero(Size.getBitWidth());
 
   return Size - Offset;
@@ -756,10 +753,14 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
   }
 
   // We end up pointing on a location that's outside of the original object.
-  // This is UB, and we'd rather return an empty location then.
   if (ORT.knownBefore() && ORT.Before.isNegative()) {
-    ORT.Before = APInt::getZero(ORT.Before.getBitWidth());
-    ORT.After = APInt::getZero(ORT.Before.getBitWidth());
+    // 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());
+    }
+    // Otherwise it's fine, caller can handle negative offset.
   }
   return ORT;
 }
diff --git a/llvm/test/Instrumentation/BoundsChecking/negative.ll b/llvm/test/Instrumentation/BoundsChecking/negative.ll
new file mode 100644
index 00000000000000..d8fb117bd13af8
--- /dev/null
+++ b/llvm/test/Instrumentation/BoundsChecking/negative.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; Check that negative oob gep do not generate invalid check.
+; RUN: opt < %s -passes=bounds-checking -S | FileCheck %s
+target datalayout = "e-p:64:64:64-p1:16:16:16-p2:64:64:64:48-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+
+@str = global [100 x i8] zeroinitializer, align 1
+
+define i16 @main() {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_COND:%.*]]
+; CHECK:       for.cond:
+; CHECK-NEXT:    [[I_0:%.*]] = phi i8 [ 65, [[ENTRY:%.*]] ], [ [[INC:%.*]], [[TMP4:%.*]] ]
+; CHECK-NEXT:    [[EXITCOND_NOT:%.*]] = icmp eq i8 [[I_0]], 76
+; CHECK-NEXT:    br i1 [[EXITCOND_NOT]], label [[FOR_END:%.*]], label [[TMP4]]
+; CHECK:       for.inc:
+; CHECK-NEXT:    [[I_0_C:%.*]] = sext i8 [[I_0]] to i64
+; CHECK-NEXT:    [[TMP0:%.*]] = add i64 -65, [[I_0_C]]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i8, ptr getelementptr (i8, ptr @str, i8 -65), i8 [[I_0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i64 100, [[TMP0]]
+; CHECK-NEXT:    store i8 [[I_0]], ptr [[GEP]], align 1
+; CHECK-NEXT:    [[INC]] = add nuw nsw i8 [[I_0]], 1
+; CHECK-NEXT:    br label [[FOR_COND]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret i16 0
+;
+entry:
+  br label %for.cond
+
+for.cond:
+  %i.0 = phi i8 [ 65, %entry ], [ %inc, %for.inc ]
+  %exitcond.not = icmp eq i8 %i.0, 76
+  br i1 %exitcond.not, label %for.end, label %for.inc
+
+for.inc:                                          ; preds = %for.cond
+  %gep = getelementptr i8, ptr getelementptr (i8, ptr @str, i8 -65), i8 %i.0
+  store i8 %i.0, ptr %gep, align 1
+  %inc = add nuw nsw i8 %i.0, 1
+  br label %for.cond
+
+for.end:
+  ret i16 0
+}
+