Charusso added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731
+  }
+  return C.getNoteTag([Text, Name](BugReport &BR) -> std::string {
+      SmallString<256> Msg;
----------------
baloghadamsoftware wrote:
> Szelethus wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > Szelethus wrote:
> > > > > NoQ wrote:
> > > > > > baloghadamsoftware wrote:
> > > > > > > NoQ wrote:
> > > > > > > > You'll need to check whether the container is actually of 
> > > > > > > > interest to the bug report. We don't want notes to be added 
> > > > > > > > about changes to irrelevant containers.
> > > > > > > > 
> > > > > > > > You can use a combination of "Report `BR` was emitted by one of 
> > > > > > > > the iterator checkers" and "The memory region of the container 
> > > > > > > > is marked as interesting" (while also actually marking it as 
> > > > > > > > interesting in the checker).
> > > > > > > > 
> > > > > > > > Ideally we should instead make a new generic storage inside the 
> > > > > > > > `BugReport` object, in order to pass down the interesting 
> > > > > > > > information from the call site of `emitReport` ("Hi, i'm an 
> > > > > > > > iterator checker who emitted this report and i'm interested in 
> > > > > > > > changes made to the size of this container").
> > > > > > > Are you sure in this? I already wondered how it works so I added 
> > > > > > > a test that checks one container and changes another one and 
> > > > > > > there were no note tags displayed for the one we did not check 
> > > > > > > but change. See the last test.
> > > > > > That's because you didn't do
> > > > > > ```lang=c++
> > > > > >   V2.cbegin();
> > > > > >   V2.cend();
> > > > > > ```
> > > > > > in the beginning.
> > > > > A similar conversation sparked up recently in between @boga95, 
> > > > > @steakhal and me regarding reporting taintedness. Bug reports are 
> > > > > fine up to the point where (in reverse) the first propagation 
> > > > > happens, but finding out which value tainted the one that caused the 
> > > > > report isn't handled at the moment. One idea was to mark the initial 
> > > > > (again, in reverse) value as interesting, create a `NoteTag` at the 
> > > > > point of propagation, where we should know which value was the cause 
> > > > > of the spread, mark that interesting as well, etc.
> > > > > 
> > > > > If `NoteTag`s only emit a message when the concerning value is 
> > > > > interesting, this should theoretically solve that problem. I guess 
> > > > > you could say that we're propagating interestingness in reverse.
> > > > > 
> > > > > I'm not immediately sure if this idea was ever mentioned or 
> > > > > implemented here.
> > > > Yes, that's the intended solution to such problems. 
> > > > `trackExpressionValue` works similarly, just with assignments instead 
> > > > of taint propagations. And in both cases note tags are a much more 
> > > > straightforward solution to the problem.
> > > Yes, you are right. My problem now is that how to mark interesting when 
> > > debugging? I I filter for interesting containers only, I lose my ability 
> > > to debug. Should I create a debug function just for marking a container 
> > > as interesting. Or is there such function already?
> > I'm not immediately sure how interetingness ties into debugging, what 
> > specific scenario are you thinking about?
> In the test of the modeling checker we use debug checkers. They should be 
> able to mark the container interesting to be able to test the not tags. I 
> managed to solve problem, even in a somewhat unorthodox way.
The core issue with NoteTag it does not know about interestingness and nor 
about MemRegion. I believe everything based on MemRegions already and when you 
emit the report you know exactly which MemRegion raised an error. So I think 
first we need to solve that the NoteTags only report on given MemRegions and 
those regions of course mega-interesting: we do not need to keep around the 
interestingness then.


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

https://reviews.llvm.org/D73720



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

Reply via email to