[clang] [analyzer] Don't display the offset value in underflows (PR #98621)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Donát Nagy (NagyDonat)

<details>
<summary>Changes</summary>

Previously alpha.security.ArrayBoundV2 displayed the (negative) offset value when it reported an underflow, but this produced lots of very similar and redundant reports in certain situations.

After this commit the offset won't be printed so the usual deduplication will handle these reports as equivalent (and print only one of them).

See https://github.com/llvm/llvm-project/issues/86969 for background.

---
Full diff: https://github.com/llvm/llvm-project/pull/98621.diff


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp (+8-7) 
- (modified) clang/test/Analysis/out-of-bounds-diagnostics.c (+24-2) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index f82288f1099e8..f177041abc411 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -374,13 +374,14 @@ static std::optional<int64_t> getConcreteValue(std::optional<NonLoc> SV) {
 
 static Messages getPrecedesMsgs(const SubRegion *Region, NonLoc Offset) {
   std::string RegName = getRegionName(Region);
-  SmallString<128> Buf;
-  llvm::raw_svector_ostream Out(Buf);
-  Out << "Access of " << RegName << " at negative byte offset";
-  if (auto ConcreteIdx = Offset.getAs<nonloc::ConcreteInt>())
-    Out << ' ' << ConcreteIdx->getValue();
+
+  // We're not reporting the Offset, because we don't want to spam the user
+  // with similar reports that differ only in different offset values.
+  // See https://github.com/llvm/llvm-project/issues/86969 for details.
+  (void)Offset;
+
   return {formatv("Out of bound access to memory preceding {0}", RegName),
-          std::string(Buf)};
+          formatv("Access of {0} at negative byte offset", RegName)};
 }
 
 /// Try to divide `Val1` and `Val2` (in place) by `Divisor` and return true if
@@ -609,7 +610,7 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext &C) const {
   // CHECK UPPER BOUND
   DefinedOrUnknownSVal Size = getDynamicExtent(State, Reg, SVB);
   if (auto KnownSize = Size.getAs<NonLoc>()) {
-    // In a situation where both overflow and overflow are possible (but the
+    // In a situation where both underflow and overflow are possible (but the
     // index is either tainted or known to be invalid), the logic of this
     // checker will first assume that the offset is non-negative, and then
     // (with this additional assumption) it will detect an overflow error.
diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c
index 92f983d8b1561..10cfe95a79a55 100644
--- a/clang/test/Analysis/out-of-bounds-diagnostics.c
+++ b/clang/test/Analysis/out-of-bounds-diagnostics.c
@@ -6,7 +6,7 @@ int TenElements[10];
 void arrayUnderflow(void) {
   TenElements[-3] = 5;
   // expected-warning at -1 {{Out of bound access to memory preceding 'TenElements'}}
-  // expected-note at -2 {{Access of 'TenElements' at negative byte offset -12}}
+  // expected-note at -2 {{Access of 'TenElements' at negative byte offset}}
 }
 
 int underflowWithDeref(void) {
@@ -14,7 +14,29 @@ int underflowWithDeref(void) {
   --p;
   return *p;
   // expected-warning at -1 {{Out of bound access to memory preceding 'TenElements'}}
-  // expected-note at -2 {{Access of 'TenElements' at negative byte offset -4}}
+  // expected-note at -2 {{Access of 'TenElements' at negative byte offset}}
+}
+
+int rng(void);
+int getIndex(void) {
+  switch (rng()) {
+    case 1: return -152;
+    case 2: return -160;
+    case 3: return -168;
+    default: return -172;
+  }
+}
+
+void gh86959(void) {
+  // Previously code like this produced many almost-identical bug reports that
+  // only differed in the byte offset value (which was reported by the checker
+  // at that time). Verify that now we only see one report.
+
+  // expected-note at +1 {{Entering loop body}}
+  while (rng())
+    TenElements[getIndex()] = 10;
+  // expected-warning at -1 {{Out of bound access to memory preceding 'TenElements'}}
+  // expected-note at -2 {{Access of 'TenElements' at negative byte offset}}
 }
 
 int scanf(const char *restrict fmt, ...);

``````````

</details>


https://github.com/llvm/llvm-project/pull/98621