[PATCH] D46224: [analyzer] pr37209: Fix casts of lvalues to references.

[Artem Dergachev via Phabricator via cfe-commits]

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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46224.144429.patch
Type: text/x-patch
Size: 1406 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180428/7fe63bb4/attachment.bin>