[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
NoQ added a comment. 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. Repository: rL LLVM https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319055: [analyzer] pr34404: Fix a crash on modeling pointers
to indirect members. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D39800?vs=122087=124404#toc
Repository:
rL LLVM
https://reviews.llvm.org/D39800
Files:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/pointer-to-member.cpp
Index: cfe/trunk/test/Analysis/pointer-to-member.cpp
===
--- cfe/trunk/test/Analysis/pointer-to-member.cpp
+++ cfe/trunk/test/Analysis/pointer-to-member.cpp
@@ -230,3 +230,42 @@
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::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(::x); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::y); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::z); // expected-warning{{TRUE}}
+
+ // FIXME: These should be true.
+ int A::*l = ::x, A::*m = ::y, A::*n = ::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: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2108,10 +2108,12 @@
ProgramPoint::PostLValueKind);
return;
}
- if (isa(D)) {
+ if (isa(D) || isa(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(), true);
Index: cfe/trunk/test/Analysis/pointer-to-member.cpp
===
--- cfe/trunk/test/Analysis/pointer-to-member.cpp
+++ cfe/trunk/test/Analysis/pointer-to-member.cpp
@@ -230,3 +230,42 @@
clang_analyzer_eval(d2.*(static_cast(static_cast(static_cast(::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(::x); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::y); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::z); // expected-warning{{TRUE}}
+
+ // FIXME: These should be true.
+ int A::*l = ::x, A::*m = ::y, A::*n = ::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: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2108,10 +2108,12 @@
ProgramPoint::PostLValueKind);
return;
}
- if (isa(D)) {
+ if (isa(D) || isa(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(), true);
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
alexfh added a comment. Is anything holding this patch? https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
xazax.hun accepted this revision. xazax.hun added a comment. LGTM! https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
dcoughlin accepted this revision. dcoughlin added a comment. 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. https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
kromanenkov added a comment. @NoQ Do we need to change a `DeclaratorDecl` field in PointerToMember SVal to something more common, like `ValueDecl`, to support `IndirectFieldDecl` as well? https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.
NoQ created this revision.
pr34404 points out that given
class C {
struct {
int x;
};
};
, taking pointer-to-member `::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(static_cast(static_cast(::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(::x); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::y); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::z); // expected-warning{{TRUE}}
+
+ // FIXME: These should be true.
+ int A::*l = ::x, A::*m = ::y, A::*n = ::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(D)) {
+ if (isa(D) || isa(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(static_cast(static_cast(::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(::x); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::y); // expected-warning{{TRUE}}
+ clang_analyzer_eval(::z); // expected-warning{{TRUE}}
+
+ // FIXME: These should be true.
+ int A::*l = ::x, A::*m = ::y, A::*n = ::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(D)) {
+ if (isa(D) || isa(D)) {
// FIXME: Compute lvalue of field pointers-to-member.
// Right now we just use a non-null void pointer, so that it gives proper
//
