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

[NoQ]

Summary

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.

[kromanenkov]

@NoQ Do we need to change a DeclaratorDecl field in PointerToMember SVal to something more common, like ValueDecl, to support IndirectFieldDecl as well?
Of course in another patch, this one LGTM.

[dcoughlin]

LGTM.

I suppose we could move the logic that constructs pointers to members for fields here (right now that logic is in ExprEngine::VisitUnaryOperator()'s handling of '&'). However, since the AST's DeclRefExpr for the field is marked as an lvalue this would be slightly odd -- we would have the value of an lvalue expression be a non-Loc.

[xazax.hun]

LGTM!

[alexfh]

Is anything holding this patch?

[NoQ]

Sorry, i wanted to quickly look at why this thing is lvalue, but this didn't seem to be happening, so i guess i'd just commit for now.

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

if (const EnumConstantDecl *ED = dyn_cast<EnumConstantDecl>(D)) {
     return;
   }
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     SVal V = svalBuilder.getFunctionPointer(FD);
     Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
                       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.
+    // FIXME: Maybe delegate this to the surrounding operator&.
+    // Note how this expression is lvalue, however pointer-to-member is NonLoc.
     SVal V = svalBuilder.conjureSymbolVal(Ex, LCtx, getContext().VoidPtrTy,
                                           currBldrCtx->blockCount());
     state = state->assume(V.castAs<DefinedOrUnknownSVal>(), true);
     Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
           ProgramPoint::PostLValueKind);
     return;
   }
 

cfe/trunk/test/Analysis/pointer-to-member.cpp

void double_diamond() {
   static_cast<R1 *>(static_cast<R2 *>(&d2))->f = 4;
 
   clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int L2::*>(static_cast<int L1::*>(&B::f)))) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int R2::*>(static_cast<int L1::*>(&B::f)))) == 2); // expected-warning {{TRUE}}
   clang_analyzer_eval(d2.*(static_cast<int D2::*>(static_cast<int L2::*>(static_cast<int R1::*>(&B::f)))) == 3); // expected-warning {{TRUE}}
   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