https://github.com/balazske created
https://github.com/llvm/llvm-project/pull/93676
The checker is made more exact (only pointer into array is allowed) and more
tests are added.
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] [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}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
