[clang] [analyzer] Fix note for member reference (PR #68691)
Gábor Spaits via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 12 04:59:26 PDT 2023
https://github.com/spaits updated https://github.com/llvm/llvm-project/pull/68691
>From bb9cb77cab7b073d45c0b998c926a0b60a75a35e Mon Sep 17 00:00:00 2001
From: Gabor Spaits <gabor.spaits at ericsson.com>
Date: Tue, 10 Oct 2023 13:40:05 +0200
Subject: [PATCH 1/3] [analyzer] Fix note for member reference
---
.../Core/BugReporterVisitors.cpp | 57 ++++++++++++++-----
1 file changed, 43 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 42d03f67510cf88..914011f367d8f2e 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -132,6 +132,18 @@ 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 of a
+ // member reference looks like the following:
+ // |-MemberExpr
+ // `-DeclRefExpr
+ // This branch without the special case just takes out the DeclRefExpr
+ // of the struct, class or union.
+ // This is wrong, because this DeclRefExpr will be passed
+ // to the bug reporting and the notes will refer to wrong variable
+ // (the struct instead of the member).
+ if (ME->getMemberDecl()->getType()->isReferenceType())
+ break;
E = ME->getBase();
} else if (const auto *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) {
E = IvarRef->getBase();
@@ -157,26 +169,43 @@ 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 other kinds of null references,
+ // for example, references from FieldRegions:
+ // 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
>From 5bf3ba5f3b66d972e3c26c8f782ae077700e459a Mon Sep 17 00:00:00 2001
From: Gabor Spaits <gabor.spaits at ericsson.com>
Date: Tue, 10 Oct 2023 15:26:28 +0200
Subject: [PATCH 2/3] Add test cases for null reference reports
---
.../deref-track-symbolic-region.cpp | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
index e258a60aa966a53..2442851da989b16 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 aaa() {
+ 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
>From 6f9ddb9d6f598a65ed3fca602329aa26bb5e3458 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A1bor=20Spaits?=
<48805437+spaits at users.noreply.github.com>
Date: Thu, 12 Oct 2023 13:59:19 +0200
Subject: [PATCH 3/3] Update test function name
Co-authored-by: DonatNagyE <donat.nagy at ericsson.com>
---
clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp b/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
index 2442851da989b16..e9f62c2407e88b4 100644
--- a/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
+++ b/clang/test/Analysis/diagnostics/deref-track-symbolic-region.cpp
@@ -65,7 +65,7 @@ void testReferenceToPointerWithNullptr() {
// expected-note at -1 {{Dereference of null pointer}}
}
-void aaa() {
+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}}
More information about the cfe-commits
mailing list