[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

pr34404 points out that given

  class C {
    struct {
      int x;
    };
  };

, taking pointer-to-member `&C::x` would crash the analyzer. We were not ready to stumble upon an `IndirectFieldDecl`, which is used for representing a field within an anonymous structure (or union) nested into a class.

Even if it wasn't anonymous, our new pointer-to-member support is not yet enabled for this case, so the value of the expression would be a conjured symbol of type `void *`. Being quite vague, it fits equally poorly to the anonymous case (in general this behavior makes very little sense, since //pointer-to-member must be a `NonLoc`//), so for the purposes of fixing the crash i guess i'd just keep it.

Actually constructing a `nonloc::PointerToMember` in the regular field case should be trivial. However, in the anonymous field case it requires more work, because `IndirectFieldDecl` is not a `DeclaratorDecl`. And simply calling `getAnonField()` over `IndirectFieldDecl` to obtain `FieldDecl` is incorrect, because it'd discard the offset to the anonymous structure within the class, leading to incorrect results on the attached tests for `m` and `n`.


https://reviews.llvm.org/D39800

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/pointer-to-member.cpp


Index: test/Analysis/pointer-to-member.cpp
===================================================================
--- test/Analysis/pointer-to-member.cpp
+++ test/Analysis/pointer-to-member.cpp
@@ -230,3 +230,42 @@
   clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int R2::*>(static_cast<int R1::*>(&B::f)))) == 4); // expected-warning {{TRUE}}
 }
 } // end of testPointerToMemberDiamond namespace
+
+namespace testAnonymousMember {
+struct A {
+  struct {
+    int x;
+  };
+  struct {
+    struct {
+      int y;
+    };
+  };
+  struct {
+    union {
+      int z;
+    };
+  };
+};
+
+void test() {
+  clang_analyzer_eval(&A::x); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::y); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&A::z); // expected-warning{{TRUE}}
+
+  // FIXME: These should be true.
+  int A::*l = &A::x, A::*m = &A::y, A::*n = &A::z;
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(m); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n); // expected-warning{{UNKNOWN}}
+
+  // FIXME: These should be true as well.
+  A a;
+  a.x = 1;
+  clang_analyzer_eval(a.*l == 1); // expected-warning{{UNKNOWN}}
+  a.y = 2;
+  clang_analyzer_eval(a.*m == 2); // expected-warning{{UNKNOWN}}
+  a.z = 3;
+  clang_analyzer_eval(a.*n == 3); // expected-warning{{UNKNOWN}}
+}
+} // end of testAnonymousMember namespace
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2108,7 +2108,7 @@
                       ProgramPoint::PostLValueKind);
     return;
   }
-  if (isa<FieldDecl>(D)) {
+  if (isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D)) {
     // FIXME: Compute lvalue of field pointers-to-member.
     // Right now we just use a non-null void pointer, so that it gives proper
     // results in boolean contexts.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39800.122087.patch
Type: text/x-patch
Size: 1940 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171108/e49f5503/attachment.bin>