[clang] [clang][analyzer] Improve PointerSubChecker (PR #96501)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 25 07:00:55 PDT 2024
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/96501
>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 1/2] [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
+}
>From ce80876f7364fba57de3cf5377d9f3673d6744b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Thu, 27 Jun 2024 10:26:20 +0200
Subject: [PATCH 2/2] improved note messages
---
.../Checkers/PointerSubChecker.cpp | 17 ++++++++-----
clang/test/Analysis/pointer-sub.c | 25 ++++++++++++++++++-
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index 63ed4df67d6d5..6025a3e810f41 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -19,6 +19,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
using namespace clang;
using namespace ento;
@@ -175,12 +176,16 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
// do_something(&a.array[5] - &b.array[5]);
// In this case don't emit notes.
if (DiffDeclL != DiffDeclR) {
- if (DiffDeclL)
- R->addNote("Array at the left-hand side of subtraction",
- {DiffDeclL, C.getSourceManager()});
- if (DiffDeclR)
- R->addNote("Array at the right-hand side of subtraction",
- {DiffDeclR, C.getSourceManager()});
+ auto AddNote = [&R, &C](const ValueDecl *D, StringRef SideStr) {
+ if (D) {
+ std::string Msg = llvm::formatv(
+ "{0} at the {1}-hand side of subtraction",
+ D->getType()->isArrayType() ? "Array" : "Object", SideStr);
+ R->addNote(Msg, {D, C.getSourceManager()});
+ }
+ };
+ AddNote(DiffDeclL, "left");
+ AddNote(DiffDeclR, "right");
}
C.emitReport(std::move(R));
}
diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c
index 404b8530b89c0..194c891889952 100644
--- a/clang/test/Analysis/pointer-sub.c
+++ b/clang/test/Analysis/pointer-sub.c
@@ -118,7 +118,7 @@ void f7(int *p) {
void f8(int n) {
int a[10] = {1};
- int d = a[n] - a[0];
+ int d = a[n] - a[0]; // no-warning
}
int f9(const char *p1) {
@@ -131,3 +131,26 @@ int f9(const char *p1) {
int f10(struct S *p1, struct S *p2) {
return &p1->c[5] - &p2->c[5]; // no-warning
}
+
+struct S1 {
+ int a;
+ int b; // expected-note{{Object at the right-hand side of subtraction}}
+};
+
+int f11() {
+ struct S1 s; // expected-note{{Object at the left-hand side of subtraction}}
+ return (char *)&s - (char *)&s.b; // expected-warning{{Subtraction of two pointers that}}
+}
+
+struct S2 {
+ char *p1;
+ char *p2;
+};
+
+void init_S2(struct S2 *);
+
+int f12() {
+ struct S2 s;
+ init_S2(&s);
+ return s.p1 - s.p2; // no-warning (pointers are unknown)
+}
More information about the cfe-commits
mailing list