[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 5 inline comments as done.
baloghadamsoftware added a comment.

Very slow progress, but lots of open questions. @NoQ, please try to answer them.



================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:614
+          SVB.makeLoc(MRMgr.getParamRegion(ICC->getInheritingConstructor(),
+                                           Idx, CalleeCtx));
+        Bindings.push_back(std::make_pair(ParamLoc, ArgVal));
----------------
Is this the correct handling of this kind of calls?


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2466
+      // FIXME: What to do with function parameters captured by a block?
+      //        Searching the function call in the call stack may fail.
+      if (!BD || Index < BD->param_size()) {
----------------
What to do with function parameters captured by a block? I tried to search for the owner function backwards in the chain of stack frames, but I did not find it.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2474
+              if (CMD->isOverloadedOperator())
+              ++Index;
+            }
----------------
Here we have no `CallEvent` so no better came to my mind.


================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:204
+    llvm_unreachable("Maybe we forgot something...");
+  }
+
----------------
This whole  branch should be moved to a separate function called e.g. `getArgExpr()`. But what to do with param `0` of `CXXNewExpr()`? It is the size parameter for which we do not have an `Expr`. For the greater indices we subtract `1` from the `Index`.


================
Comment at: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp:601-603
+    if (LCtx->getAnalysis<RelaxedLiveVariables>()->isLive(Loc,
+                                                          PR->getOriginExpr()))
+      return true;
----------------
baloghadamsoftware wrote:
> NoQ wrote:
> > > I implemented the basic `isLive()` and `getBinding()` functions which reduced the number of failing tests by `0`.
> > 
> > This line in particular is very incorrect. In fact, most of the time the result will be `false`.
> OK, but what should I check for liveness here? We do not have a `Decl`. We could retrieve the `Expr` for the `Arg`, except for `CXXNewOperator` because there the size argument cannot be retrieved.
Here could help the function `getArgExpr()` of `ParamRegion`, I suppose. We should put it into the place of `getOriginExpr()`. Is this correct?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77229/new/

https://reviews.llvm.org/D77229