vrnithinkumar added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:408-412
+        SmallString<128> Msg;
+        llvm::raw_svector_ostream Out(Msg);
+        TagDetails.trackValidExpr(BR);
+        TagDetails.explainSmartPtrAction(Out);
+        return std::string(Out.str());
----------------
NoQ wrote:
> NoQ wrote:
> > Ok, note that note tags are attached to nodes independently of bug reports; 
> > when the report is thrown, only then we know what are the smart pointers 
> > that should be explained.
> > 
> > So there are two checks that you should do here:
> > 
> > 1. Check that the bug report is emitted by your checker (eg., by comparing 
> > bug types). If not, don't add notes.
> > 
> > 2. Check that the region about which the note speaks is related to your 
> > report (i.e., it's not a completely unrelated smart pointer). You can do 
> > that by marking the smart pointer as "interesting" (i.e., 
> > `PathSensitiveBugReport::markIntersting()`) when you emit the report, and 
> > then in the lambda you check whether the smart pointer is interesting 
> > before you emit a note. Additionally, you can carry over interestingness 
> > when smart pointers are copied.
> > 
> > This is what i was trying to accomplish with this code snippet that i 
> > included in the examples in the other comment:
> > ```lang=c++
> >   if (&BR.getBugType() != &NullDereferenceBugType || !R->isInteresting())
> >     return "";
> > ```
> (i strongly recommend having test cases for both of these issues)
I was stuck on how to check the 2 cases from `SmartPtrModeling`.

1. I was not able to figure out how to access `NullDereferenceBugType` defined 
in the `SmartPtrChecker` in `SmartPtrModeling` to check `&BR.getBugType() != 
&NullDereferenceBugType`. Since `NullDereferenceBugType` is part of the 
`SmartPtrChecker` I could not access it from `PathSensitiveBugReport`.  One way 
I figured out is to use `getCheckerName()` on BugType and compare the string. I 
feel this one as little hacky.


2. I got stuck on how will we implement the `!R->isInteresting()` in case of 
the bug report is added by some other checker on some other region. For below 
test case, bug report is added on a raw pointer by `CallAndMessageChecker` and 
the `!R->isInteresting()` will not satisfy and we will not be adding note tags 
where `unique_ptr` is released. I tried getting the LHS region for `A *AP = 
P.release();` assignment operation and check if the region is interesting but 
not sure whether its gonna work for some complex assignment cases.

```
void derefOnReleasedNullRawPtr() {
  std::unique_ptr<A> P;
  A *AP = P.release(); // expected-note {{'AP' initialized to a null pointer 
value}}
  // expected-note@-1 {{Smart pointer 'P' is released and set to null}}
  AP->foo(); // expected-warning {{Called C++ object pointer is null 
[core.CallAndMessage]}}
  // expected-note@-1{{Called C++ object pointer is null}}
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84600

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to