[PATCH] D86029: [analyzer] Add modeling for unque_ptr::get()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

This patch looks correct to me at a glance. I think we should land it as is and debug/improve later.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:362-363
+  const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
+  if (!InnerPointVal)
+    return;
+
----------------
vrnithinkumar wrote:
> vrnithinkumar wrote:
> > xazax.hun wrote:
> > > NoQ wrote:
> > > > You'll have to actively handle this case, sooner or later. Consider the following test cases that won't work until you do:
> > > > ```lang=c++
> > > > void foo(std::unique_ptr<A> p) {
> > > >   A *x = p.get();
> > > >   A *y = p.get();
> > > >   clang_analyzer_eval(x == y); // expected-warning{{TRUE}}
> > > >   if (!x) {
> > > >     y->foo(); // expected-warning{{Called C++ object pointer is null}}
> > > >   }
> > > > }
> > > > 
> > > > ```
> > > You mean the case where we do not have an inner pointer registered in the state yet, right?
> > > 
> > > I believe we might also have to handle similar cases for `operator bool()` as well. 
> > Added the above test case. 
> > Using conjureSymbolVal in case of missing inner pointer value
> ```
> void foo(std::unique_ptr<A> P) {
>   A *X = P.get();
>   if (!X) {
>     P->foo(); // expected-warning {{Dereference of null smart pointer 'Pl' [alpha.cplusplus.SmartPtr]}}
>   }
> }
> ```
> I was trying to add the above use case. Since we are using conjureSymbolVal in case of missing inner pointer value.
> 
> But still the inner pointer value is constrained to [0, 0] in false branch, `InnerPointVal->isZeroConstant()` returning false. 
> Also I tried `State->isNull(*InnerPointVal).isConstrainedTrue();` This is also not working.
> How should we check whether the conjureSymbolVal for inner pointer value is constrained to [0, 0]?
> How should we check whether the conjureSymbolVal for inner pointer value is constrained to [0, 0]?

View exploded graphs. That's literally the only reasonable answer to every such question. In particular, it shows you constraints for all symbols at every moment of time, and given that you implemented `printState()` it also shows you inner pointer values that you keep track of at every moment of time.

Check if it's still the same symbol. Check that the symbol lives long enough - or does it get forgotten about in the middle? - if so you might need to get your `checkLiveSymbols` callback right.

Please let us know if you still can't seem to debug it on your own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86029