NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs, baloghadamsoftware.
In the provided test case the expression `(int *&)*(int *)p` is evaluated as follows: 1. Take an lvalue `p` of type `char *`. Its symbolic value will be `&p`. 2. Load an rvalue `p` of type `char *`. The resulting value is `&SymRegion{reg_$0<p>}`. 3. Cast it to rvalue `(int *)p` of type `int *`. The resulting value is `&element{SymRegion{reg_$0<p>}, 0, int}`. 4. Treat it as lvalue `*(int *)p` of type `int`. The resulting value is still `&element{SymRegion{reg_$0<p>}, 0, int}` because the analyzer doesn't discriminate between lvalues and respective pointer rvalues - both are simply "Loc". 5. Cast it to lvalue `(int *&)*(int *)p` of type `(int *)`. The resulting value is computed incorrectly as `&element{SymRegion{reg_$0<p>}, 0, int}`. Note: on the last step we take an //lvalue// of type `int` and cast it to //lvalue// of type `int *`. There are no references in the expression types, but there's a reference in the type-as-written of the outer cast-expression. Then we try to evaluate a cast from `int` to `int *&`, because the analyzer is clever enough to realize that it needs to take type-as-written for the explicit cast. The cast, of course, misbehaves - there's no intended behavior in converting a `Loc` from an integer to a reference-to-a-pointer. I tried to detect that situation and work around it by saying that we should instead evaluate a cast from `int &` to `int *&`. I.e., add an artificial `&` to the original expression's type. It seems to work correctly (i.e., the resulting value becomes `&element{SymRegion{reg_$0<p>}, 0, int *}` which seems reasonable) and it should work because it's a reasonable requirement for `evalCast()` to support such casts. That said, i didn't investigate all various ASTs carefully yet, in order to figure out if my pattern-matching is correct, so i might have missed some corner cases, so i'll most likely try to investigate it more carefully, or at least see if it causes regressions on large codebases, before committing. Repository: rC Clang https://reviews.llvm.org/D46224 Files: lib/StaticAnalyzer/Core/ExprEngineC.cpp test/Analysis/casts.cpp Index: test/Analysis/casts.cpp =================================================================== --- test/Analysis/casts.cpp +++ test/Analysis/casts.cpp @@ -21,3 +21,17 @@ break; } } + +int *&castToIntPtrLValueRef(char *p) { + return (int *&)*(int *)p; +} +bool testCastToIntPtrLValueRef(char *p, int *s) { + return castToIntPtrLValueRef(p) != s; // no-crash +} + +int *&&castToIntPtrRValueRef(char *p) { + return (int *&&)*(int *)p; +} +bool testCastToIntPtrRValueRef(char *p, int *s) { + return castToIntPtrRValueRef(p) != s; // no-crash +} Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -257,6 +257,13 @@ ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx, QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr, ExplodedNode* Pred) { + if (T->isLValueReferenceType()) { + assert(!CastE->getType()->isLValueReferenceType()); + ExTy = getContext().getLValueReferenceType(ExTy); + } else if (T->isRValueReferenceType()) { + assert(!CastE->getType()->isRValueReferenceType()); + ExTy = getContext().getRValueReferenceType(ExTy); + } // Delegate to SValBuilder to process. SVal OrigV = state->getSVal(Ex, LCtx); SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
Index: test/Analysis/casts.cpp =================================================================== --- test/Analysis/casts.cpp +++ test/Analysis/casts.cpp @@ -21,3 +21,17 @@ break; } } + +int *&castToIntPtrLValueRef(char *p) { + return (int *&)*(int *)p; +} +bool testCastToIntPtrLValueRef(char *p, int *s) { + return castToIntPtrLValueRef(p) != s; // no-crash +} + +int *&&castToIntPtrRValueRef(char *p) { + return (int *&&)*(int *)p; +} +bool testCastToIntPtrRValueRef(char *p, int *s) { + return castToIntPtrRValueRef(p) != s; // no-crash +} Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -257,6 +257,13 @@ ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx, QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr, ExplodedNode* Pred) { + if (T->isLValueReferenceType()) { + assert(!CastE->getType()->isLValueReferenceType()); + ExTy = getContext().getLValueReferenceType(ExTy); + } else if (T->isRValueReferenceType()) { + assert(!CastE->getType()->isRValueReferenceType()); + ExTy = getContext().getRValueReferenceType(ExTy); + } // Delegate to SValBuilder to process. SVal OrigV = state->getSVal(Ex, LCtx); SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits