Skip to content

Commit c68bc17

Browse files
authored
[analyzer] Fix note for member reference (#68691)
In the following code: ```cpp int main() { struct Wrapper {char c; int &ref; }; Wrapper w = {.c = 'a', .ref = *(int *)0 }; w.ref = 1; } ``` The clang static analyzer will produce the following warnings and notes: ``` test.cpp:12:11: warning: Dereference of null pointer [core.NullDereference] 12 | w.ref = 1; | ~~~~~~^~~ test.cpp:11:5: note: 'w' initialized here 11 | Wrapper w = {.c = 'a', .ref = *(int *)0 }; | ^~~~~~~~~ test.cpp:12:11: note: Dereference of null pointer 12 | w.ref = 1; | ~~~~~~^~~ 1 warning generated. ``` In the line where `w` is created, the note gives information about the initialization of `w` instead of `w.ref`. Let's compare it to a similar case where a null pointer dereference happens to a pointer member: ```cpp int main() { struct Wrapper {char c; int *ptr; }; Wrapper w = {.c = 'a', .ptr = nullptr }; *w.ptr = 1; } ``` Here the following error and notes are seen: ``` test.cpp:18:12: warning: Dereference of null pointer (loaded from field 'ptr') [core.NullDereference] 18 | *w.ptr = 1; | ~~~ ^ test.cpp:17:5: note: 'w.ptr' initialized to a null pointer value 17 | Wrapper w = {.c = 'a', .ptr = nullptr }; | ^~~~~~~~~ test.cpp:18:12: note: Dereference of null pointer (loaded from field 'ptr') 18 | *w.ptr = 1; | ~~~ ^ 1 warning generated. ``` Here the note that shows the initialization the initialization of `w.ptr` in shown instead of `w`. This commit is here to achieve similar notes for member reference as the notes of member pointers, so the report looks like the following: ``` test.cpp:12:11: warning: Dereference of null pointer [core.NullDereference] 12 | w.ref = 1; | ~~~~~~^~~ test.cpp:11:5: note: 'w.ref' initialized to a null pointer value 11 | Wrapper w = {.c = 'a', .ref = *(int *)0 }; | ^~~~~~~~~ test.cpp:12:11: note: Dereference of null pointer 12 | w.ref = 1; | ~~~~~~^~~ 1 warning generated. ``` Here the initialization of `w.ref` is shown instead of `w`. --------- Authored-by: Gábor Spaits <[email protected]> Reviewed-by: Donát Nagy <[email protected]>
1 parent 3ab536f commit c68bc17

File tree

2 files changed

+71
-14
lines changed

2 files changed

+71
-14
lines changed

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,16 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) {
132132
}
133133
// Pattern match for a few useful cases: a[0], p->f, *p etc.
134134
else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
135+
// This handles the case when the dereferencing of a member reference
136+
// happens. This is needed, because the AST for dereferencing a
137+
// member reference looks like the following:
138+
// |-MemberExpr
139+
// `-DeclRefExpr
140+
// Without this special case the notes would refer to the whole object
141+
// (struct, class or union variable) instead of just the relevant member.
142+
143+
if (ME->getMemberDecl()->getType()->isReferenceType())
144+
break;
135145
E = ME->getBase();
136146
} else if (const auto *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) {
137147
E = IvarRef->getBase();
@@ -157,26 +167,42 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) {
157167
return E;
158168
}
159169

170+
static const VarDecl *getVarDeclForExpression(const Expr *E) {
171+
if (const auto *DR = dyn_cast<DeclRefExpr>(E))
172+
return dyn_cast<VarDecl>(DR->getDecl());
173+
return nullptr;
174+
}
175+
160176
static const MemRegion *
161177
getLocationRegionIfReference(const Expr *E, const ExplodedNode *N,
162178
bool LookingForReference = true) {
163-
if (const auto *DR = dyn_cast<DeclRefExpr>(E)) {
164-
if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) {
165-
if (LookingForReference && !VD->getType()->isReferenceType())
166-
return nullptr;
167-
return N->getState()
168-
->getLValue(VD, N->getLocationContext())
169-
.getAsRegion();
179+
if (const auto *ME = dyn_cast<MemberExpr>(E)) {
180+
// This handles null references from FieldRegions, for example:
181+
// struct Wrapper { int &ref; };
182+
// Wrapper w = { *(int *)0 };
183+
// w.ref = 1;
184+
const Expr *Base = ME->getBase();
185+
const VarDecl *VD = getVarDeclForExpression(Base);
186+
if (!VD)
187+
return nullptr;
188+
189+
const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
190+
if (!FD)
191+
return nullptr;
192+
193+
if (FD->getType()->isReferenceType()) {
194+
SVal StructSVal = N->getState()->getLValue(VD, N->getLocationContext());
195+
return N->getState()->getLValue(FD, StructSVal).getAsRegion();
170196
}
197+
return nullptr;
171198
}
172199

173-
// FIXME: This does not handle other kinds of null references,
174-
// for example, references from FieldRegions:
175-
// struct Wrapper { int &ref; };
176-
// Wrapper w = { *(int *)0 };
177-
// w.ref = 1;
178-
179-
return nullptr;
200+
const VarDecl *VD = getVarDeclForExpression(E);
201+
if (!VD)
202+
return nullptr;
203+
if (LookingForReference && !VD->getType()->isReferenceType())
204+
return nullptr;
205+
return N->getState()->getLValue(VD, N->getLocationContext()).getAsRegion();
180206
}
181207

182208
/// Comparing internal representations of symbolic values (via

clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,34 @@ int testRefToNullPtr2() {
4141
return *p2; //expected-warning {{Dereference of null pointer}}
4242
// expected-note@-1{{Dereference of null pointer}}
4343
}
44+
45+
void testMemberNullPointerDeref() {
46+
struct Wrapper {char c; int *ptr; };
47+
Wrapper w = {'a', nullptr}; // expected-note {{'w.ptr' initialized to a null pointer value}}
48+
*w.ptr = 1; //expected-warning {{Dereference of null pointer}}
49+
// expected-note@-1{{Dereference of null pointer}}
50+
}
51+
52+
void testMemberNullReferenceDeref() {
53+
struct Wrapper {char c; int &ref; };
54+
Wrapper w = {.c = 'a', .ref = *(int *)0 }; // expected-note {{'w.ref' initialized to a null pointer value}}
55+
// expected-warning@-1 {{binding dereferenced null pointer to reference has undefined behavior}}
56+
w.ref = 1; //expected-warning {{Dereference of null pointer}}
57+
// expected-note@-1{{Dereference of null pointer}}
58+
}
59+
60+
void testReferenceToPointerWithNullptr() {
61+
int *i = nullptr; // expected-note {{'i' initialized to a null pointer value}}
62+
struct Wrapper {char c; int *&a;};
63+
Wrapper w {'c', i}; // expected-note{{'w.a' initialized here}}
64+
*(w.a) = 25; // expected-warning {{Dereference of null pointer}}
65+
// expected-note@-1 {{Dereference of null pointer}}
66+
}
67+
68+
void testNullReferenceToPointer() {
69+
struct Wrapper {char c; int *&a;};
70+
Wrapper w {'c', *(int **)0 }; // expected-note{{'w.a' initialized to a null pointer value}}
71+
// expected-warning@-1 {{binding dereferenced null pointer to reference has undefined behavior}}
72+
w.a = nullptr; // expected-warning {{Dereference of null pointer}}
73+
// expected-note@-1 {{Dereference of null pointer}}
74+
}

0 commit comments

Comments
 (0)