Skip to content

Commit 26224ca

Browse files
balazskebalazske
authored
[clang][analyzer] Improved PointerSubChecker (#93676)
The checker is made more exact (only pointer into array is allowed, check array index) and more tests are added.
1 parent c0b65a2 commit 26224ca

File tree

4 files changed

+222
-27
lines changed

4 files changed

+222
-27
lines changed

‎clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp

+96-15
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "clang/StaticAnalyzer/Core/Checker.h"
1818
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
1919
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
20+
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
2021
#include "llvm/ADT/StringRef.h"
2122

2223
using namespace clang;
@@ -26,16 +27,88 @@ namespace {
2627
class PointerSubChecker
2728
: public Checker< check::PreStmt<BinaryOperator> > {
2829
const BugType BT{this, "Pointer subtraction"};
30+
const llvm::StringLiteral Msg_MemRegionDifferent =
31+
"Subtraction of two pointers that do not point into the same array "
32+
"is undefined behavior.";
33+
const llvm::StringLiteral Msg_LargeArrayIndex =
34+
"Using an array index greater than the array size at pointer subtraction "
35+
"is undefined behavior.";
36+
const llvm::StringLiteral Msg_NegativeArrayIndex =
37+
"Using a negative array index at pointer subtraction "
38+
"is undefined behavior.";
39+
const llvm::StringLiteral Msg_BadVarIndex =
40+
"Indexing the address of a variable with other than 1 at this place "
41+
"is undefined behavior.";
42+
43+
bool checkArrayBounds(CheckerContext &C, const Expr *E,
44+
const ElementRegion *ElemReg,
45+
const MemRegion *Reg) const;
46+
void reportBug(CheckerContext &C, const Expr *E,
47+
const llvm::StringLiteral &Msg) const;
2948

3049
public:
3150
void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
3251
};
3352
}
3453

54+
bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E,
55+
const ElementRegion *ElemReg,
56+
const MemRegion *Reg) const {
57+
if (!ElemReg)
58+
return true;
59+
60+
ProgramStateRef State = C.getState();
61+
const MemRegion *SuperReg = ElemReg->getSuperRegion();
62+
SValBuilder &SVB = C.getSValBuilder();
63+
64+
if (SuperReg == Reg) {
65+
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
66+
I && (!I->isOne() && !I->isZero()))
67+
reportBug(C, E, Msg_BadVarIndex);
68+
return false;
69+
}
70+
71+
DefinedOrUnknownSVal ElemCount =
72+
getDynamicElementCount(State, SuperReg, SVB, ElemReg->getElementType());
73+
auto IndexTooLarge = SVB.evalBinOp(C.getState(), BO_GT, ElemReg->getIndex(),
74+
ElemCount, SVB.getConditionType())
75+
.getAs<DefinedOrUnknownSVal>();
76+
if (IndexTooLarge) {
77+
ProgramStateRef S1, S2;
78+
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
79+
if (S1 && !S2) {
80+
reportBug(C, E, Msg_LargeArrayIndex);
81+
return false;
82+
}
83+
}
84+
auto IndexTooSmall = SVB.evalBinOp(State, BO_LT, ElemReg->getIndex(),
85+
SVB.makeZeroVal(SVB.getArrayIndexType()),
86+
SVB.getConditionType())
87+
.getAs<DefinedOrUnknownSVal>();
88+
if (IndexTooSmall) {
89+
ProgramStateRef S1, S2;
90+
std::tie(S1, S2) = State->assume(*IndexTooSmall);
91+
if (S1 && !S2) {
92+
reportBug(C, E, Msg_NegativeArrayIndex);
93+
return false;
94+
}
95+
}
96+
return true;
97+
}
98+
99+
void PointerSubChecker::reportBug(CheckerContext &C, const Expr *E,
100+
const llvm::StringLiteral &Msg) const {
101+
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
102+
auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
103+
R->addRange(E->getSourceRange());
104+
C.emitReport(std::move(R));
105+
}
106+
}
107+
35108
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
36109
CheckerContext &C) const {
37110
// When doing pointer subtraction, if the two pointers do not point to the
38-
// same memory chunk, emit a warning.
111+
// same array, emit a warning.
39112
if (B->getOpcode() != BO_Sub)
40113
return;
41114

@@ -44,28 +117,36 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
44117

45118
const MemRegion *LR = LV.getAsRegion();
46119
const MemRegion *RR = RV.getAsRegion();
47-
48-
if (!(LR && RR))
120+
if (!LR || !RR)
49121
return;
50122

51-
const MemRegion *BaseLR = LR->getBaseRegion();
52-
const MemRegion *BaseRR = RR->getBaseRegion();
123+
// Allow subtraction of identical pointers.
124+
if (LR == RR)
125+
return;
53126

54-
if (BaseLR == BaseRR)
127+
// No warning if one operand is unknown.
128+
if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR))
55129
return;
56130

57-
// Allow arithmetic on different symbolic regions.
58-
if (isa<SymbolicRegion>(BaseLR) || isa<SymbolicRegion>(BaseRR))
131+
const auto *ElemLR = dyn_cast<ElementRegion>(LR);
132+
const auto *ElemRR = dyn_cast<ElementRegion>(RR);
133+
134+
if (!checkArrayBounds(C, B->getLHS(), ElemLR, RR))
135+
return;
136+
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
59137
return;
60138

61-
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
62-
constexpr llvm::StringLiteral Msg =
63-
"Subtraction of two pointers that do not point to the same memory "
64-
"chunk may cause incorrect result.";
65-
auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
66-
R->addRange(B->getSourceRange());
67-
C.emitReport(std::move(R));
139+
if (ElemLR && ElemRR) {
140+
const MemRegion *SuperLR = ElemLR->getSuperRegion();
141+
const MemRegion *SuperRR = ElemRR->getSuperRegion();
142+
if (SuperLR == SuperRR)
143+
return;
144+
// Allow arithmetic on different symbolic regions.
145+
if (isa<SymbolicRegion>(SuperLR) || isa<SymbolicRegion>(SuperRR))
146+
return;
68147
}
148+
149+
reportBug(C, B, Msg_MemRegionDifferent);
69150
}
70151

71152
void ento::registerPointerSubChecker(CheckerManager &mgr) {

‎clang/test/Analysis/casts.c

+4
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ void multiDimensionalArrayPointerCasts(void) {
138138
clang_analyzer_eval(y1 == y2); // expected-warning{{TRUE}}
139139

140140
// FIXME: should be FALSE (i.e. equal pointers).
141+
// FIXME: pointer subtraction warning might be incorrect
141142
clang_analyzer_eval(y1 - y2); // expected-warning{{UNKNOWN}}
143+
// expected-warning@-1{{Subtraction of two pointers that do not point into the same array is undefined behavior}}
142144
// FIXME: should be TRUE (i.e. same symbol).
143145
clang_analyzer_eval(*y1 == *y2); // expected-warning{{UNKNOWN}}
144146

@@ -147,7 +149,9 @@ void multiDimensionalArrayPointerCasts(void) {
147149
clang_analyzer_eval(y1 == y3); // expected-warning{{TRUE}}
148150

149151
// FIXME: should be FALSE (i.e. equal pointers).
152+
// FIXME: pointer subtraction warning might be incorrect
150153
clang_analyzer_eval(y1 - y3); // expected-warning{{UNKNOWN}}
154+
// expected-warning@-1{{Subtraction of two pointers that do not point into the same array is undefined behavior}}
151155
// FIXME: should be TRUE (i.e. same symbol).
152156
clang_analyzer_eval(*y1 == *y3); // expected-warning{{UNKNOWN}}
153157

‎clang/test/Analysis/pointer-sub.c

+120
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s
2+
3+
void f1(void) {
4+
int x, y, z[10];
5+
int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}}
6+
d = z - &y; // expected-warning{{Subtraction of two pointers that do not point into the same array is undefined behavior}}
7+
d = &x - &x; // no-warning (subtraction of any two identical pointers is allowed)
8+
d = (long *)&x - (long *)&x;
9+
d = (&x + 1) - &x; // no-warning ('&x' is like a single-element array)
10+
d = &x - (&x + 1); // no-warning
11+
d = (&x + 0) - &x; // no-warning
12+
d = (&x - 1) - &x; // expected-warning{{Indexing the address of a variable with other than 1 at this place is undefined behavior}}
13+
d = (&x + 2) - &x; // expected-warning{{Indexing the address of a variable with other than 1 at this place is undefined behavior}}
14+
15+
d = (z + 9) - z; // no-warning (pointers to same array)
16+
d = (z + 10) - z; // no-warning (pointer to "one after the end")
17+
d = (z + 11) - z; // expected-warning{{Using an array index greater than the array size at pointer subtraction is undefined behavior}}
18+
d = (z - 1) - z; // expected-warning{{Using a negative array index at pointer subtraction is undefined behavior}}
19+
}
20+
21+
void f2(void) {
22+
int a[10], b[10], c;
23+
int *p = &a[2];
24+
int *q = &a[8];
25+
int d = q - p; // no-warning (pointers into the same array)
26+
27+
q = &b[3];
28+
d = q - p; // expected-warning{{Subtraction of two pointers that}}
29+
30+
q = a + 10;
31+
d = q - p; // no warning (use of pointer to one after the end is allowed)
32+
q = a + 11;
33+
d = q - a; // expected-warning{{Using an array index greater than the array size at pointer subtraction is undefined behavior}}
34+
35+
d = &a[4] - a; // no-warning
36+
d = &a[2] - p; // no-warning
37+
d = &c - p; // expected-warning{{Subtraction of two pointers that}}
38+
39+
d = (int *)((char *)(&a[4]) + sizeof(int)) - &a[4]; // no-warning (pointers into the same array data)
40+
d = (int *)((char *)(&a[4]) + 1) - &a[4]; // expected-warning{{Subtraction of two pointers that}}
41+
}
42+
43+
void f3(void) {
44+
int a[3][4];
45+
int d;
46+
47+
d = &(a[2]) - &(a[1]);
48+
d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
49+
d = a[1] - a[1];
50+
d = &(a[1][2]) - &(a[1][0]);
51+
d = &(a[1][2]) - &(a[0][0]); // expected-warning{{Subtraction of two pointers that}}
52+
53+
d = (int *)((char *)(&a[2][2]) + sizeof(int)) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}}
54+
d = (int *)((char *)(&a[2][2]) + 1) - &a[2][2]; // expected-warning{{Subtraction of two pointers that}}
55+
d = (int (*)[4])((char *)&a[2] + sizeof(int (*)[4])) - &a[2]; // expected-warning{{Subtraction of two pointers that}}
56+
d = (int (*)[4])((char *)&a[2] + 1) - &a[2]; // expected-warning{{Subtraction of two pointers that}}
57+
}
58+
59+
void f4(void) {
60+
int n = 4, m = 3;
61+
int a[n][m];
62+
int (*p)[m] = a; // p == &a[0]
63+
p += 1; // p == &a[1]
64+
65+
// FIXME: This is a known problem with -Wpointer-arith (https://github.com/llvm/llvm-project/issues/28328)
66+
int d = p - a; // d == 1 // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}}
67+
68+
// FIXME: This is a known problem with -Wpointer-arith (https://github.com/llvm/llvm-project/issues/28328)
69+
d = &(a[2]) - &(a[1]); // expected-warning{{subtraction of pointers to type 'int[m]' of zero size has undefined behavior}}
70+
71+
d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}}
72+
}
73+
74+
typedef struct {
75+
int a;
76+
int b;
77+
int c[10];
78+
int d[10];
79+
} S;
80+
81+
void f5(void) {
82+
S s;
83+
int y;
84+
int d;
85+
86+
d = &s.b - &s.a; // expected-warning{{Subtraction of two pointers that}}
87+
d = &s.c[0] - &s.a; // expected-warning{{Subtraction of two pointers that}}
88+
d = &s.b - &y; // expected-warning{{Subtraction of two pointers that}}
89+
d = &s.c[3] - &s.c[2];
90+
d = &s.d[3] - &s.c[2]; // expected-warning{{Subtraction of two pointers that}}
91+
d = s.d - s.c; // expected-warning{{Subtraction of two pointers that}}
92+
93+
S sa[10];
94+
d = &sa[2] - &sa[1];
95+
d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers that}}
96+
}
97+
98+
void f6(void) {
99+
long long l;
100+
char *a1 = (char *)&l;
101+
int d = a1[3] - l;
102+
103+
long long la1[3];
104+
long long la2[3];
105+
char *pla1 = (char *)la1;
106+
char *pla2 = (char *)la2;
107+
d = pla1[1] - pla1[0];
108+
d = (long long *)&pla1[1] - &l; // expected-warning{{Subtraction of two pointers that}}
109+
d = &pla2[3] - &pla1[3]; // expected-warning{{Subtraction of two pointers that}}
110+
}
111+
112+
void f7(int *p) {
113+
int a[10];
114+
int d = &a[10] - p; // no-warning ('p' is unknown, even if it cannot point into 'a')
115+
}
116+
117+
void f8(int n) {
118+
int a[10];
119+
int d = a[n] - a[0];
120+
}

‎clang/test/Analysis/ptr-arith.c

+2-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// 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
2-
// 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
1+
// 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
2+
// 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
33

44
void clang_analyzer_eval(int);
55
void clang_analyzer_dump(int);
@@ -35,16 +35,6 @@ domain_port (const char *domain_b, const char *domain_e,
3535
return port;
3636
}
3737

38-
void f3(void) {
39-
int x, y;
40-
int d = &y - &x; // expected-warning{{Subtraction of two pointers that do not point to the same memory chunk may cause incorrect result}}
41-
42-
int a[10];
43-
int *p = &a[2];
44-
int *q = &a[8];
45-
d = q-p; // no-warning
46-
}
47-
4838
void f4(void) {
4939
int *p;
5040
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}}

0 commit comments

Comments
 (0)