[clang] cab91ec - [clang][analyzer] Improve PointerSubChecker (#96501)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 1 03:56:28 PDT 2024
Author: Balázs Kéri
Date: 2024-08-01T12:56:25+02:00
New Revision: cab91ecffd7a6cb94fa27e7fe8cd93dfc4d9a672
URL: https://github.com/llvm/llvm-project/commit/cab91ecffd7a6cb94fa27e7fe8cd93dfc4d9a672
DIFF: https://github.com/llvm/llvm-project/commit/cab91ecffd7a6cb94fa27e7fe8cd93dfc4d9a672.diff
LOG: [clang][analyzer] Improve PointerSubChecker (#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.
Added:
Modified:
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
clang/test/Analysis/pointer-sub.c
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 05d3f4d4018f7..55832d20bd27a 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2498,15 +2498,49 @@ Check for pointer arithmetic on locations other than array elements.
alpha.core.PointerSub (C)
"""""""""""""""""""""""""
-Check for pointer subtractions on two pointers pointing to
diff erent memory chunks.
+Check for pointer subtractions on two pointers pointing to
diff erent memory
+chunks. According to the C standard §6.5.6 only subtraction of pointers that
+point into (or one past the end) the same array object is valid (for this
+purpose non-array variables are like arrays of size 1).
.. code-block:: c
void test() {
- int x, y;
- int d = &y - &x; // warn
+ int a, b, c[10], d[10];
+ int x = &c[3] - &c[1];
+ x = &d[4] - &c[1]; // warn: 'c' and 'd' are
diff erent arrays
+ x = (&a + 1) - &a;
+ x = &b - &a; // warn: 'a' and 'b' are
diff erent variables
+ x = (&a + 2) - &a; // warn: for a variable it is only valid to have a pointer
+ // to one past the address of it
+ x = &c[10] - &c[0];
+ x = &c[11] - &c[0]; // warn: index larger than one past the end
+ x = &c[-1] - &c[0]; // warn: negative index
}
+ struct S {
+ int x[10];
+ int y[10];
+ };
+
+ void test1() {
+ struct S a[10];
+ struct S b;
+ int d = &a[4] - &a[6];
+ d = &a[0].x[3] - &a[0].x[1];
+ d = a[0].y - a[0].x; // warn: 'S.b' and 'S.a' are
diff erent objects
+ d = (char *)&b.y - (char *)&b.x; // warn:
diff erent members of the same object
+ d = (char *)&b.y - (char *)&b; // warn: object of type S is not the same array as a member of it
+ }
+
+There may be existing applications that use code like above for calculating
+offsets of members in a structure, using pointer subtractions. This is still
+undefined behavior according to the standard and code like this can be replaced
+with the `offsetof` macro.
+
+The checker only reports cases where stack-allocated objects are involved (no
+warnings on pointers to memory allocated by `malloc`).
+
.. _alpha-core-StackAddressAsyncEscape:
alpha.core.StackAddressAsyncEscape (C)
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index eea93a41f1384..b856b0edc6151 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;
@@ -40,6 +41,11 @@ class PointerSubChecker
"Indexing the address of a variable with other than 1 at this place "
"is undefined behavior.";
+ /// Check that an array is indexed in the allowed range that is 0 to "one
+ /// after the end". The "array" can be address of a non-array variable.
+ /// @param E Expression of the pointer subtraction.
+ /// @param ElemReg An indexed region in the subtraction expression.
+ /// @param Reg Region of the other side of the expression.
bool checkArrayBounds(CheckerContext &C, const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
@@ -49,12 +55,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,10 +86,10 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E,
};
ProgramStateRef State = C.getState();
- const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder &SVB = C.getSValBuilder();
if (SuperReg == Reg) {
+ // Case like `(&x + 1) - &x`. Only 1 or 0 is allowed as index.
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
ReportBug(Msg_BadVarIndex);
@@ -121,8 +143,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);
@@ -159,12 +182,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 88e6dec2d172f..194c891889952 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,40 @@ void f7(int *p) {
}
void f8(int n) {
- int a[10];
- int d = a[n] - a[0];
+ int a[10] = {1};
+ int d = a[n] - a[0]; // no-warning
+}
+
+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
+}
+
+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