[analyzer] Fix note for member reference

[spaits]

In the following code:

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:

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.

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Gábor Spaits (spaits)

Changes

In the following code:

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 de reference happens to a pointer member:

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/68691.diff

1 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (+43-14)

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

[NagyDonat]

LGTM; my only suggestions are comment / naming clarifications.