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.
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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits