[clang] [analyzer] Implement potential underflow warnings (PR #216077)

[Donát Nagy via cfe-commits]

https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/216077

>From 1e4bdff18bacff7fa9ece3a0dbf21298a93051c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 13 Aug 2026 16:57:44 +0200
Subject: [PATCH 1/2] [analyzer] Implement potiential underflow warnings

The `security.ArrayBound` checker is to report potential out-of-bounds
access when it detects that the accessed offset is tainted (potentially
attacker-controlled).

However, until now this only reported cases where _overflow_ was
possible with the tainted offset. (This is probably an accidental
oversight -- in the old implementation it was easy to forget adding a
second check that would report the "potential underflow with tainted
offset" case.)

This commit corrects this oversight and ensures that potential underflow
with a tainted offset is also correctly reported.
---
 .../Checkers/ArrayBoundChecker.cpp            | 20 +++++++++----------
 .../test/Analysis/ArrayBound/verbose-tests.c  | 17 ++++++++++++++++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 9e4e8e83749ad..256b17154cd44 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -334,13 +334,15 @@ static BugDescription describeInvalidAccess(bounds::CheckResult Res,
           std::string(Buf)};
 }
 
-static BugDescription describeTaintBug(StringRef RegName, StringRef OffsetName,
-                                       bool AlsoMentionUnderflow) {
+static BugDescription describeTaintBug(bounds::CheckResult Res,
+                                       StringRef RegName,
+                                       StringRef OffsetName) {
   return {formatv("Potential out of bound access to {0} with tainted {1}",
                   RegName, OffsetName),
-          formatv("Access of {0} with a tainted {1} that may be {2}too large",
-                  RegName, OffsetName,
-                  AlsoMentionUnderflow ? "negative or " : "")};
+          formatv("Access of {0} with a tainted {1} that may be{2}{3}{4}.",
+                  RegName, OffsetName, Res.mayUnderflow() ? " negative" : "",
+                  (Res.mayUnderflow() && Res.mayOverflow()) ? " or" : "",
+                  Res.mayOverflow() ? " too large" : "")};
 }
 
 /// When the access was ambiguous (that is, mayBeInBounds() && mayBeInvalid()),
@@ -480,10 +482,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
       return;
     }
 
-    // FIXME: Remove `Res.mayOverflow()` and provide diagnostics for the case
-    // when the tainted access operation cannot overflow but can underflow.
-    // (This is an NFC commit, so I cannot include this improvement.)
-    if (Res.mayOverflow() && isTainted(State, ByteOffset)) {
+    if (isTainted(State, ByteOffset)) {
       // Diagnostic detail: saying "tainted offset" is always correct, but
       // the common case is that 'idx' is tainted in 'arr[idx]' and then it's
       // nicer to say "tainted index".
@@ -492,8 +491,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
         if (isTainted(State, ASE->getIdx(), C.getStackFrame()))
           OffsetName = "index";
 
-      BugDescription Desc =
-          describeTaintBug(RegName, OffsetName, Res.mayUnderflow());
+      BugDescription Desc = describeTaintBug(Res, RegName, OffsetName);
       reportOOB(C, State, Desc, ByteOffset, Extent, /*IsTaintBug=*/true);
       return;
     }
diff --git a/clang/test/Analysis/ArrayBound/verbose-tests.c b/clang/test/Analysis/ArrayBound/verbose-tests.c
index c0b1f2a8ae6be..81b5001c60f89 100644
--- a/clang/test/Analysis/ArrayBound/verbose-tests.c
+++ b/clang/test/Analysis/ArrayBound/verbose-tests.c
@@ -100,6 +100,23 @@ void taintedIndexNonneg(void) {
   // expected-note at -2 {{Access of 'TenElements' with a tainted index that may be too large}}
 }
 
+void taintedIndexNonlarge(void) {
+  int index;
+  scanf("%d", &index);
+  // expected-note at -1 {{Taint originated here}}
+  // expected-note at -2 {{Taint propagated to the 2nd argument}}
+
+  // expected-note at +2 {{Assuming 'index' is < 10}}
+  // expected-note at +1 {{Taking false branch}}
+  if (index >= 10)
+    return;
+
+  TenElements[index] = 5;
+  // expected-warning at -1 {{Potential out of bound access to 'TenElements' with tainted index}}
+  // expected-note at -2 {{Access of 'TenElements' with a tainted index that may be negative}}
+}
+
+
 void taintedIndexUnsigned(void) {
   unsigned index;
   scanf("%u", &index);

>From 90f88089250caa8dfdfb74915817d67062314720 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 24 Aug 2026 10:45:12 +0200
Subject: [PATCH 2/2] Remove a newline
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Balázs Benics <benicsbalazs at gmail.com>
---
 clang/test/Analysis/ArrayBound/verbose-tests.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/test/Analysis/ArrayBound/verbose-tests.c b/clang/test/Analysis/ArrayBound/verbose-tests.c
index 81b5001c60f89..bbc32e52a9853 100644
--- a/clang/test/Analysis/ArrayBound/verbose-tests.c
+++ b/clang/test/Analysis/ArrayBound/verbose-tests.c
@@ -116,7 +116,6 @@ void taintedIndexNonlarge(void) {
   // expected-note at -2 {{Access of 'TenElements' with a tainted index that may be negative}}
 }
 
-
 void taintedIndexUnsigned(void) {
   unsigned index;
   scanf("%u", &index);