[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[wangxin via Phabricator via cfe-commits]

wangxindsb marked 3 inline comments as done.
wangxindsb added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:29
 namespace {
+enum class ObjectState : bool { CtorCalled, DtorCalled };
+} // end namespace
----------------
NoQ wrote:
> Please remind me what was wrong with ascending over `StackFrameContext`s to see if we're inside a constructor or destructor call? Why did we need a state trait here? Was it too hard to obtain this-values? (Not sure if there's time to fix this, but it might be worth it to leave a FIXME around).
I have implement this method before, the code is on this link, [[ https://github.com/wangxindsb/VirtualCallChecker/blob/master/VirtualCallChecker-fullstack.cpp | virtualcallchecker using stackframe]]. I thought it was harder and slower than the state trait, so I gave up this method. Also, I thought this method maybe hard to add visitor. I will work on this soon.


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:110
+  auto ThiSVal =
+      State->getSVal(SVB.getCXXThis(MD, LCtx->getCurrentStackFrame()));
+  const MemRegion *Reg = ThiSVal.getAsRegion();
----------------
NoQ wrote:
> I don't think this is working. CXXThisRegion is never a region of a C++ object; it's the segment of the stack where the pointer is passed, you need to dereference its value to get the actual object region.
> 
> Probably tests for the visitor might make things more clear.
```
class Y {
public:
  virtual void foobar();
  void fooY() {  
    F f1;
    foobar();
  }
  Y() {
    fooY();
  }
};

```
I thought this test is for this situation. If the visitor is correct, it will issued "Called from this constructor 'Y'", else it will issued "Called from this constructor 'F'".



https://reviews.llvm.org/D34275