[clang] [clang][analyzer] Improve PointerSubChecker (PR #96501)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created https://github.com/llvm/llvm-project/pull/96501

The checker could report false positives if pointer arithmetic was done on pointers to non-array data before pointer subtraction. Another problem is fixed that could cause false positive if members of the same structure but in different memory objects are subtracted.

>From b431151f83fa2980e4a132191ccf5713ab69806b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 24 Jun 2024 16:48:54 +0200
Subject: [PATCH] [clang][analyzer] Improve PointerSubChecker

The checker could report false positives if pointer arithmetic was
done on pointers to non-array data before pointer subtraction.
Another problem is fixed that could cause false positive if members
of the same structure but in different memory objects are subtracted.
---
 .../Checkers/PointerSubChecker.cpp            | 22 ++++++++++++--
 clang/test/Analysis/pointer-sub.c             | 29 +++++++++++++------
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index eea93a41f1384..63ed4df67d6d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -49,12 +49,28 @@ class PointerSubChecker
 };
 }
 
+static bool isArrayVar(const MemRegion *R) {
+  while (R) {
+    if (isa<VarRegion>(R))
+      return true;
+    if (const auto *ER = dyn_cast<ElementRegion>(R))
+      R = ER->getSuperRegion();
+    else
+      return false;
+  }
+  return false;
+}
+
 bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E,
                                          const ElementRegion *ElemReg,
                                          const MemRegion *Reg) const {
   if (!ElemReg)
     return true;
 
+  const MemRegion *SuperReg = ElemReg->getSuperRegion();
+  if (!isArrayVar(SuperReg))
+    return true;
+
   auto ReportBug = [&](const llvm::StringLiteral &Msg) {
     if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
       auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
@@ -64,7 +80,6 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E,
   };
 
   ProgramStateRef State = C.getState();
-  const MemRegion *SuperReg = ElemReg->getSuperRegion();
   SValBuilder &SVB = C.getSValBuilder();
 
   if (SuperReg == Reg) {
@@ -121,8 +136,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
   if (LR == RR)
     return;
 
-  // No warning if one operand is unknown.
-  if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR))
+  // No warning if one operand is unknown or resides in a region that could be
+  // equal to the other.
+  if (LR->getSymbolicBase() || RR->getSymbolicBase())
     return;
 
   const auto *ElemLR = dyn_cast<ElementRegion>(LR);
diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c
index 88e6dec2d172f..404b8530b89c0 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core.PointerSub -analyzer-output=text-minimal -verify %s
 
 void f1(void) {
   int x, y, z[10];
@@ -73,15 +73,15 @@ void f4(void) {
   d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
 }
 
-typedef struct {
+struct S {
   int a;
   int b;
   int c[10]; // expected-note2{{Array at the right-hand side of subtraction}}
   int d[10]; // expected-note2{{Array at the left-hand side of subtraction}}
-} S;
+};
 
 void f5(void) {
-  S s;
+  struct S s;
   int y;
   int d;
 
@@ -92,18 +92,18 @@ void f5(void) {
   d = &s.d[3] - &s.c[2]; // expected-warning{{Subtraction of two pointers that}}
   d = s.d - s.c; // expected-warning{{Subtraction of two pointers that}}
 
-  S sa[10];
+  struct S sa[10];
   d = &sa[2] - &sa[1];
   d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers that}}
 }
 
 void f6(void) {
-  long long l;
+  long long l = 2;
   char *a1 = (char *)&l;
   int d = a1[3] - l;
 
-  long long la1[3]; // expected-note{{Array at the right-hand side of subtraction}}
-  long long la2[3]; // expected-note{{Array at the left-hand side of subtraction}}
+  long long la1[3] = {1}; // expected-note{{Array at the right-hand side of subtraction}}
+  long long la2[3] = {1}; // expected-note{{Array at the left-hand side of subtraction}}
   char *pla1 = (char *)la1;
   char *pla2 = (char *)la2;
   d = pla1[1] - pla1[0];
@@ -117,6 +117,17 @@ void f7(int *p) {
 }
 
 void f8(int n) {
-  int a[10];
+  int a[10] = {1};
   int d = a[n] - a[0];
 }
+
+int f9(const char *p1) {
+  const char *p2 = p1;
+  --p1;
+  ++p2;
+  return p1 - p2; // no-warning
+}
+
+int f10(struct S *p1, struct S *p2) {
+  return &p1->c[5] - &p2->c[5]; // no-warning
+}