dcoughlin added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+ case CK_ReinterpretMemberPointer: {
+ const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+ assert(isa<UnaryOperator>(UOExpr) &&
----------------
kromanenkov wrote:
> dcoughlin wrote:
> > dcoughlin wrote:
> > > dcoughlin wrote:
> > > > I don't think pattern matching on the sub expression to find the
> > > > referred-to declaration is the right thing to do here. It isn't always
> > > > the case that the casted expression will be a unary pointer to member
> > > > operation. For example, this is perfectly fine and triggers an
> > > > assertion failure on your patch:
> > > >
> > > > ```
> > > > struct B {
> > > > int f;
> > > > };
> > > >
> > > > struct D : public B {
> > > > int g;
> > > > };
> > > >
> > > > void foo() {
> > > > D d;
> > > > d.f = 7;
> > > >
> > > > int B::* pfb = &B::f;
> > > > int D::* pfd = pfb;
> > > > int v = d.*pfd;
> > > > }
> > > > ```
> > > > Note that you can't just propagate the value already computed for the
> > > > subexpression. Here is a particularly annoying example from the C++
> > > > spec:
> > > >
> > > > ```
> > > > struct B {
> > > > int f;
> > > > };
> > > > struct L : public B { };
> > > > struct R : public B { };
> > > > struct D : public L, R { };
> > > >
> > > > void foo() {
> > > > D d;
> > > >
> > > > int B::* pb = &B::f;
> > > > int L::* pl = pb;
> > > > int R::* pr = pb;
> > > >
> > > > int D::* pdl = pl;
> > > > int D::* pdr = pr;
> > > >
> > > > clang_analyzer_eval(pdl == pdr); // FALSE
> > > > clang_analyzer_eval(pb == pl); // TRUE
> > > > }
> > > > ```
> > > > My guess is this will require accumulating CXXBasePath s or something
> > > > similar for each cast. I don't know how to do this efficiently.
> > > > I don't know how to do this efficiently.
> > >
> > > Jordan suggested storing this in a bump-pointer allocated object with a
> > > lifetime of the AnalysisDeclContext. The common case is no multiple
> > > inheritance, so that should be the fast case.
> > >
> > > Maybe the Data could be a pointer union between a DeclaratorDecl and an
> > > immutable linked list (with sharing) of CXXBaseSpecifiers from the
> > > CastExprs in the AST. The storage for this could be managed with a new
> > > manager in SValBuilder.
> > (The bump pointer-allocated thing would have to have the Decl as well.)
> >
> > This could also probably live in BasicValueFactory. The extra data would be
> > similar to LazyCompoundValData.
> My understanding is that PointerToMember SVal should be represented similar
> to LazyCompoundVal SVal. If I got you right, PointerToMember SVal should be
> constructed in VisitUnaryOperator and VisitCast
> (CK_BaseToDerivedMemberPointer and so on) just add CXXBaseSpecifiers to this
> SVal?
> What do you mean by sharing of immutable linked list?
SVal has very strict size limitations -- you can only store a single pointer
for its Data. Since you'll need to have multiple CXXBaseSpecifiers in a single
SVal (for example, consider a multiple inheritance hierarchy with a double
diamond), the storage for this container will have to live elsewhere.
One way to do this is to bump-pointer allocate linked list nodes for the "path"
of casts that the value has taken. If you do this then you can share the memory
for the nodes for a shared path prefix for two paths with a common prefix.
https://reviews.llvm.org/D25475
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
