https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/93676

From a896030e71d09ebe7239d6fab343606918ee4c1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Wed, 29 May 2024 14:28:43 +0200
Subject: [PATCH 1/3] [clang][analyzer] Improved PointerSubChecker

The checker is made more exact (only pointer into array is allowed)
and more tests are added.
---
 .../Checkers/PointerSubChecker.cpp            | 34 +++++----
 clang/test/Analysis/pointer-sub.c             | 74 +++++++++++++++++++
 clang/test/Analysis/ptr-arith.c               | 14 +---
 3 files changed, 96 insertions(+), 26 deletions(-)
 create mode 100644 clang/test/Analysis/pointer-sub.c

diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index 2438cf30b39b5..4caa9ded93ed4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -35,7 +35,7 @@ class PointerSubChecker
 void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
                                      CheckerContext &C) const {
   // When doing pointer subtraction, if the two pointers do not point to the
-  // same memory chunk, emit a warning.
+  // same array, emit a warning.
   if (B->getOpcode() != BO_Sub)
     return;
 
@@ -44,24 +44,30 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator 
*B,
 
   const MemRegion *LR = LV.getAsRegion();
   const MemRegion *RR = RV.getAsRegion();
-
-  if (!(LR && RR))
-    return;
-
-  const MemRegion *BaseLR = LR->getBaseRegion();
-  const MemRegion *BaseRR = RR->getBaseRegion();
-
-  if (BaseLR == BaseRR)
+  if (!LR || !RR)
     return;
 
-  // Allow arithmetic on different symbolic regions.
-  if (isa<SymbolicRegion>(BaseLR) || isa<SymbolicRegion>(BaseRR))
-    return;
+  const auto *ElemLR = dyn_cast<ElementRegion>(LR);
+  const auto *ElemRR = dyn_cast<ElementRegion>(RR);
+  // FIXME: We want to verify that these are elements of an array.
+  // Because behavior of ElementRegion it may be confused with a cast.
+  // There is not a simple way to distinguish it from array element (check the
+  // types?). Because this missing check a warning is missing in the rare case
+  // when two casted pointers to the same region (variable) are subtracted.
+  if (ElemLR && ElemRR) {
+    const MemRegion *SuperLR = ElemLR->getSuperRegion();
+    const MemRegion *SuperRR = ElemRR->getSuperRegion();
+    if (SuperLR == SuperRR)
+      return;
+    // Allow arithmetic on different symbolic regions.
+    if (isa<SymbolicRegion>(SuperLR) || isa<SymbolicRegion>(SuperRR))
+      return;
+  }
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     constexpr llvm::StringLiteral Msg =
-        "Subtraction of two pointers that do not point to the same memory "
-        "chunk may cause incorrect result.";
+        "Subtraction of two pointers that do not point into the same array "
+        "is undefined behavior.";
     auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
     R->addRange(B->getSourceRange());
     C.emitReport(std::move(R));
diff --git a/clang/test/Analysis/pointer-sub.c 
b/clang/test/Analysis/pointer-sub.c
new file mode 100644
index 0000000000000..010c739d6ad10
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub.c
@@ -0,0 +1,74 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s
+
+void f1(void) {
+  int x, y, z[10];
+  int d = &y - &x; // expected-warning{{Subtraction of two pointers that do 
not point into the same array is undefined behavior}}
+  d = z - &y; // expected-warning{{Subtraction of two pointers that do not 
point into the same array is undefined behavior}}
+  d = &x - &x; // expected-warning{{Subtraction of two pointers that do not 
point into the same array is undefined behavior}}
+  d = (long*)&x - (long*)&x;
+}
+
+void f2(void) {
+  int a[10], b[10], c;
+  int *p = &a[2];
+  int *q = &a[8];
+  int d = q - p; // no-warning
+
+  q = &b[3];
+  d = q - p; // expected-warning{{Subtraction of two pointers that}}
+
+  q = a + 10;
+  d = q - p; // no warning (use of pointer to one after the end is allowed)
+  d = &a[4] - a; // no warning
+
+  q = a + 11;
+  d = q - a; // ?
+
+  d = &c - p; // expected-warning{{Subtraction of two pointers that}}
+}
+
+void f3(void) {
+  int a[3][4];
+  int d;
+
+  d = &(a[2]) - &(a[1]);
+  d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
+  d = a[1] - a[1];
+  d = &(a[1][2]) - &(a[1][0]);
+  d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two 
pointers that}}
+}
+
+void f4(void) {
+  int n = 4, m = 3;
+  int a[n][m];
+  int (*p)[m] = a; // p == &a[0]
+  p += 1; // p == &a[1]
+  int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to 
type 'int[m]' of zero size has undefined behavior}}
+
+  d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 
'int[m]' of zero size has undefined behavior}}
+  d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
+}
+
+typedef struct {
+  int a;
+  int b;
+  int c[10];
+  int d[10];
+} S;
+
+void f5(void) {
+  S s;
+  int y;
+  int d;
+
+  d = &s.b - &s.a; // expected-warning{{Subtraction of two pointers that}}
+  d = &s.c[0] - &s.a; // expected-warning{{Subtraction of two pointers that}}
+  d = &s.b - &y; // expected-warning{{Subtraction of two pointers that}}
+  d = &s.c[3] - &s.c[2];
+  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];
+  d = &sa[2] - &sa[1];
+  d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers 
that}}
+}
diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c
index 40c8188704e81..f99dfabb07366 100644
--- a/clang/test/Analysis/ptr-arith.c
+++ b/clang/test/Analysis/ptr-arith.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection
 -Wno-pointer-to-int-cast -verify -triple x86_64-apple-darwin9 
-Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection
 -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 
-Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,debug.ExprInspection
 -Wno-pointer-to-int-cast -verify -triple x86_64-apple-darwin9 
-Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,debug.ExprInspection
 -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 
-Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
 void clang_analyzer_dump(int);
@@ -35,16 +35,6 @@ domain_port (const char *domain_b, const char *domain_e,
   return port;
 }
 
-void f3(void) {
-  int x, y;
-  int d = &y - &x; // expected-warning{{Subtraction of two pointers that do 
not point to the same memory chunk may cause incorrect result}}
-
-  int a[10];
-  int *p = &a[2];
-  int *q = &a[8];
-  d = q-p; // no-warning
-}
-
 void f4(void) {
   int *p;
   p = (int*) 0x10000; // expected-warning{{Using a fixed address is not 
portable because that address will probably not be valid in all environments or 
platforms}}

From 7c2e5604f20c9fe1089008bd346577426bd92234 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Thu, 30 May 2024 17:56:37 +0200
Subject: [PATCH 2/3] improved checker in some cases, added tests

---
 .../Checkers/PointerSubChecker.cpp            | 13 ++--
 clang/test/Analysis/casts.c                   |  4 ++
 clang/test/Analysis/pointer-sub.c             | 63 +++++++++++++++++--
 3 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index 4caa9ded93ed4..87764aa369f83 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -47,13 +47,16 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator 
*B,
   if (!LR || !RR)
     return;
 
+  // Allow subtraction of identical pointers.
+  if (LR == RR)
+    return;
+
+  // No warning if one operand is unknown.
+  if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR))
+    return;
+
   const auto *ElemLR = dyn_cast<ElementRegion>(LR);
   const auto *ElemRR = dyn_cast<ElementRegion>(RR);
-  // FIXME: We want to verify that these are elements of an array.
-  // Because behavior of ElementRegion it may be confused with a cast.
-  // There is not a simple way to distinguish it from array element (check the
-  // types?). Because this missing check a warning is missing in the rare case
-  // when two casted pointers to the same region (variable) are subtracted.
   if (ElemLR && ElemRR) {
     const MemRegion *SuperLR = ElemLR->getSuperRegion();
     const MemRegion *SuperRR = ElemRR->getSuperRegion();
diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c
index 30cd74be564fd..7dad4edfa89b9 100644
--- a/clang/test/Analysis/casts.c
+++ b/clang/test/Analysis/casts.c
@@ -138,7 +138,9 @@ void multiDimensionalArrayPointerCasts(void) {
   clang_analyzer_eval(y1 == y2); // expected-warning{{TRUE}}
 
   // FIXME: should be FALSE (i.e. equal pointers).
+  // FIXME: pointer subtraction warning might be incorrect
   clang_analyzer_eval(y1 - y2); // expected-warning{{UNKNOWN}}
+  // expected-warning@-1{{Subtraction of two pointers that do not point into 
the same array is undefined behavior}}
   // FIXME: should be TRUE (i.e. same symbol).
   clang_analyzer_eval(*y1 == *y2); // expected-warning{{UNKNOWN}}
 
@@ -147,7 +149,9 @@ void multiDimensionalArrayPointerCasts(void) {
   clang_analyzer_eval(y1 == y3); // expected-warning{{TRUE}}
 
   // FIXME: should be FALSE (i.e. equal pointers).
+  // FIXME: pointer subtraction warning might be incorrect
   clang_analyzer_eval(y1 - y3); // expected-warning{{UNKNOWN}}
+  // expected-warning@-1{{Subtraction of two pointers that do not point into 
the same array is undefined behavior}}
   // FIXME: should be TRUE (i.e. same symbol).
   clang_analyzer_eval(*y1 == *y3); // expected-warning{{UNKNOWN}}
 
diff --git a/clang/test/Analysis/pointer-sub.c 
b/clang/test/Analysis/pointer-sub.c
index 010c739d6ad10..93033d23cd14a 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -1,30 +1,54 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s
 
+#if 0
+
+void f(void) {
+  int a[3][4];
+  int d;
+  d = (int *)(((char *)(&a[2][2]) + 1) - 1) - &a[2][2];
+  d = (int *)(((char *)(&a[2][2]) + 1) - 1) - (int *)(((char *)(&a[1][1]) + 1) 
- 1);
+
+  long long l;
+  char *a1 = (char *)&l;
+  d = a1[3] - l;
+
+  long long la1[3];
+  long long la2[3];
+  char *a2 = (char *)la1;
+  d = &a2[3] - (char *)&la2[2];
+}
+
+#else
+
 void f1(void) {
   int x, y, z[10];
   int d = &y - &x; // expected-warning{{Subtraction of two pointers that do 
not point into the same array is undefined behavior}}
   d = z - &y; // expected-warning{{Subtraction of two pointers that do not 
point into the same array is undefined behavior}}
-  d = &x - &x; // expected-warning{{Subtraction of two pointers that do not 
point into the same array is undefined behavior}}
-  d = (long*)&x - (long*)&x;
+  d = &x - &x; // no-warning (subtraction of any two identical pointers is 
allowed)
+  d = (long *)&x - (long *)&x;
+  d = (&x + 1) - &x; // expected-warning{{Subtraction of two pointers that do 
not point into the same array is undefined behavior}}
 }
 
 void f2(void) {
   int a[10], b[10], c;
   int *p = &a[2];
   int *q = &a[8];
-  int d = q - p; // no-warning
+  int d = q - p; // no-warning (pointers into the same array)
 
   q = &b[3];
   d = q - p; // expected-warning{{Subtraction of two pointers that}}
 
   q = a + 10;
   d = q - p; // no warning (use of pointer to one after the end is allowed)
-  d = &a[4] - a; // no warning
-
   q = a + 11;
-  d = q - a; // ?
+  d = q - a; // no-warning (no check for past-the-end array access in this 
checker)
 
+  d = &a[4] - a; // no-warning
+  d = &a[2] - p; // no-warning
   d = &c - p; // expected-warning{{Subtraction of two pointers that}}
+
+  d = (int *)((char *)(&a[4]) + 4) - &a[4]; // no-warning (pointers into the 
same array data)
+  d = (int *)((char *)(&a[4]) + 3) - &a[4]; // expected-warning{{Subtraction 
of two pointers that}}
 }
 
 void f3(void) {
@@ -36,6 +60,10 @@ void f3(void) {
   d = a[1] - a[1];
   d = &(a[1][2]) - &(a[1][0]);
   d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two 
pointers that}}
+
+  // FIXME: This warning is wrong:
+  // 2-dimensional array is internally converted into one-dimensional by the 
analyzer
+  d = (int *)(((char *)(&a[2][2]) + 2) - 1) - &a[2][2]; // 
expected-warning{{Subtraction of two pointers that}}
 }
 
 void f4(void) {
@@ -43,9 +71,13 @@ void f4(void) {
   int a[n][m];
   int (*p)[m] = a; // p == &a[0]
   p += 1; // p == &a[1]
+
+  // FIXME: This warning is not needed
   int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to 
type 'int[m]' of zero size has undefined behavior}}
 
+  // FIXME: This warning is not needed
   d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 
'int[m]' of zero size has undefined behavior}}
+
   d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
 }
 
@@ -72,3 +104,22 @@ void f5(void) {
   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;
+  char *a1 = (char *)&l;
+  int d = a1[3] - l;
+
+  long long la1[3];
+  long long la2[3];
+  char *pla1 = (char *)la1;
+  char *pla2 = (char *)la2;
+  d = &pla2[3] - &pla1[3]; // expected-warning{{Subtraction of two pointers 
that}}
+}
+
+void f7(int *p) {
+  int a[10];
+  int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point 
into 'a')
+}
+
+#endif

From d981a59cca2f4e3c55b3f80bd61969bf07f0b9fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Fri, 31 May 2024 16:09:50 +0200
Subject: [PATCH 3/3] fixes and additions in tests

---
 clang/test/Analysis/pointer-sub.c | 37 ++++++++-----------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/clang/test/Analysis/pointer-sub.c 
b/clang/test/Analysis/pointer-sub.c
index 93033d23cd14a..423292124a309 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -1,25 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s
 
-#if 0
-
-void f(void) {
-  int a[3][4];
-  int d;
-  d = (int *)(((char *)(&a[2][2]) + 1) - 1) - &a[2][2];
-  d = (int *)(((char *)(&a[2][2]) + 1) - 1) - (int *)(((char *)(&a[1][1]) + 1) 
- 1);
-
-  long long l;
-  char *a1 = (char *)&l;
-  d = a1[3] - l;
-
-  long long la1[3];
-  long long la2[3];
-  char *a2 = (char *)la1;
-  d = &a2[3] - (char *)&la2[2];
-}
-
-#else
-
 void f1(void) {
   int x, y, z[10];
   int d = &y - &x; // expected-warning{{Subtraction of two pointers that do 
not point into the same array is undefined behavior}}
@@ -41,14 +21,14 @@ void f2(void) {
   q = a + 10;
   d = q - p; // no warning (use of pointer to one after the end is allowed)
   q = a + 11;
-  d = q - a; // no-warning (no check for past-the-end array access in this 
checker)
+  d = q - a; // no-warning (no check for past-the-end array pointers in this 
checker)
 
   d = &a[4] - a; // no-warning
   d = &a[2] - p; // no-warning
   d = &c - p; // expected-warning{{Subtraction of two pointers that}}
 
-  d = (int *)((char *)(&a[4]) + 4) - &a[4]; // no-warning (pointers into the 
same array data)
-  d = (int *)((char *)(&a[4]) + 3) - &a[4]; // expected-warning{{Subtraction 
of two pointers that}}
+  d = (int *)((char *)(&a[4]) + sizeof(int)) - &a[4]; // no-warning (pointers 
into the same array data)
+  d = (int *)((char *)(&a[4]) + 1) - &a[4]; // expected-warning{{Subtraction 
of two pointers that}}
 }
 
 void f3(void) {
@@ -61,9 +41,10 @@ void f3(void) {
   d = &(a[1][2]) - &(a[1][0]);
   d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two 
pointers that}}
 
-  // FIXME: This warning is wrong:
-  // 2-dimensional array is internally converted into one-dimensional by the 
analyzer
-  d = (int *)(((char *)(&a[2][2]) + 2) - 1) - &a[2][2]; // 
expected-warning{{Subtraction of two pointers that}}
+  d = (int *)((char *)(&a[2][2]) + sizeof(int)) - &a[2][2]; // 
expected-warning{{Subtraction of two pointers that}}
+  d = (int *)((char *)(&a[2][2]) + 1) - &a[2][2]; // 
expected-warning{{Subtraction of two pointers that}}
+  d = (int (*)[4])((char *)&a[2] + sizeof(int (*)[4])) - &a[2]; // 
expected-warning{{Subtraction of two pointers that}}
+  d = (int (*)[4])((char *)&a[2] + 1) - &a[2]; // 
expected-warning{{Subtraction of two pointers that}}
 }
 
 void f4(void) {
@@ -114,6 +95,8 @@ void f6(void) {
   long long la2[3];
   char *pla1 = (char *)la1;
   char *pla2 = (char *)la2;
+  d = pla1[1] - pla1[0];
+  d = (long long *)&pla1[1] - &l; // expected-warning{{Subtraction of two 
pointers that}}
   d = &pla2[3] - &pla1[3]; // expected-warning{{Subtraction of two pointers 
that}}
 }
 
@@ -121,5 +104,3 @@ void f7(int *p) {
   int a[10];
   int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point 
into 'a')
 }
-
-#endif

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to