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.spa...@ericsson.com> Reviewed-by: Donát Nagy <donat.n...@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@-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@-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@-1 {{binding dereferenced null pointer to reference has undefined behavior}} + w.ref = 1; //expected-warning {{Dereference of null pointer}} + // expected-note@-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@-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@-1 {{binding dereferenced null pointer to reference has undefined behavior}} + w.a = nullptr; // expected-warning {{Dereference of null pointer}} + // expected-note@-1 {{Dereference of null pointer}} +} \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
