[clang] c68bc17 - [analyzer] Fix note for member reference (#68691)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 16 01:55:38 PDT 2023
Author: Gábor Spaits
Date: 2023-10-16T10:55:31+02:00
New Revision: c68bc1726c1c14a297c75cae597dab00e9e7e905
URL: https://github.com/llvm/llvm-project/commit/c68bc1726c1c14a297c75cae597dab00e9e7e905
DIFF: https://github.com/llvm/llvm-project/commit/c68bc1726c1c14a297c75cae597dab00e9e7e905.diff
LOG: [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 <gabor.spaits at ericsson.com>
Reviewed-by: Donát Nagy <donat.nagy at ericsson.com>
Added:
Modified:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 42d03f67510cf88..2d184d529513253 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -132,6 +132,16 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) {
}
// Pattern match for a few useful cases: a[0], p->f, *p etc.
else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+ // This handles the case when the dereferencing of a member reference
+ // happens. This is needed, because the AST for dereferencing a
+ // member reference looks like the following:
+ // |-MemberExpr
+ // `-DeclRefExpr
+ // Without this special case the notes would refer to the whole object
+ // (struct, class or union variable) instead of just the relevant member.
+
+ if (ME->getMemberDecl()->getType()->isReferenceType())
+ break;
E = ME->getBase();
} else if (const auto *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) {
E = IvarRef->getBase();
@@ -157,26 +167,42 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) {
return E;
}
+static const VarDecl *getVarDeclForExpression(const Expr *E) {
+ if (const auto *DR = dyn_cast<DeclRefExpr>(E))
+ return dyn_cast<VarDecl>(DR->getDecl());
+ return nullptr;
+}
+
static const MemRegion *
getLocationRegionIfReference(const Expr *E, const ExplodedNode *N,
bool LookingForReference = true) {
- if (const auto *DR = dyn_cast<DeclRefExpr>(E)) {
- if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) {
- if (LookingForReference && !VD->getType()->isReferenceType())
- return nullptr;
- return N->getState()
- ->getLValue(VD, N->getLocationContext())
- .getAsRegion();
+ if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+ // This handles null references from FieldRegions, for example:
+ // struct Wrapper { int &ref; };
+ // Wrapper w = { *(int *)0 };
+ // w.ref = 1;
+ const Expr *Base = ME->getBase();
+ const VarDecl *VD = getVarDeclForExpression(Base);
+ if (!VD)
+ return nullptr;
+
+ const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
+ if (!FD)
+ return nullptr;
+
+ if (FD->getType()->isReferenceType()) {
+ SVal StructSVal = N->getState()->getLValue(VD, N->getLocationContext());
+ return N->getState()->getLValue(FD, StructSVal).getAsRegion();
}
+ return nullptr;
}
- // FIXME: This does not handle other kinds of null references,
- // for example, references from FieldRegions:
- // struct Wrapper { int &ref; };
- // Wrapper w = { *(int *)0 };
- // w.ref = 1;
-
- return nullptr;
+ const VarDecl *VD = getVarDeclForExpression(E);
+ if (!VD)
+ return nullptr;
+ if (LookingForReference && !VD->getType()->isReferenceType())
+ return nullptr;
+ return N->getState()->getLValue(VD, N->getLocationContext()).getAsRegion();
}
/// Comparing internal representations of symbolic values (via
diff --git a/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
index e258a60aa966a53..e9f62c2407e88b4 100644
--- a/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
+++ b/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
@@ -41,3 +41,34 @@ int testRefToNullPtr2() {
return *p2; //expected-warning {{Dereference of null pointer}}
// expected-note at -1{{Dereference of null pointer}}
}
+
+void testMemberNullPointerDeref() {
+ struct Wrapper {char c; int *ptr; };
+ Wrapper w = {'a', nullptr}; // expected-note {{'w.ptr' initialized to a null pointer value}}
+ *w.ptr = 1; //expected-warning {{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+void testMemberNullReferenceDeref() {
+ struct Wrapper {char c; int &ref; };
+ Wrapper w = {.c = 'a', .ref = *(int *)0 }; // expected-note {{'w.ref' initialized to a null pointer value}}
+ // expected-warning at -1 {{binding dereferenced null pointer to reference has undefined behavior}}
+ w.ref = 1; //expected-warning {{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+void testReferenceToPointerWithNullptr() {
+ int *i = nullptr; // expected-note {{'i' initialized to a null pointer value}}
+ struct Wrapper {char c; int *&a;};
+ Wrapper w {'c', i}; // expected-note{{'w.a' initialized here}}
+ *(w.a) = 25; // expected-warning {{Dereference of null pointer}}
+ // expected-note at -1 {{Dereference of null pointer}}
+}
+
+void testNullReferenceToPointer() {
+ struct Wrapper {char c; int *&a;};
+ Wrapper w {'c', *(int **)0 }; // expected-note{{'w.a' initialized to a null pointer value}}
+ // expected-warning at -1 {{binding dereferenced null pointer to reference has undefined behavior}}
+ w.a = nullptr; // expected-warning {{Dereference of null pointer}}
+ // expected-note at -1 {{Dereference of null pointer}}
+}
\ No newline at end of file
More information about the cfe-commits
mailing list