[PATCH] D104925: [Analyzer] Improve report of file read at end-of-file condition.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:429-435
+    const ExplodedNode *N = BR.getErrorNode();
+    while (N && N != NotePosition) {
+      const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym);
+      if (!StreamS || !StreamS->ErrorState.FEof)
+        return "";
+      N = N->getFirstPred();
+    }
----------------
balazske wrote:
> balazske wrote:
> > NoQ wrote:
> > > This code says "If there exists a state below in the path where the stream is not at EOF, drop the note". The report will not be emitted at all if the stream is not at EOF at the end, right? Are you trying to account for the situation when the stream gets reset into a non-EOF state and therefore you don't need to emit the note above that point?
> > > 
> > > In such cases the intended solution is to add another note tag to mark the stream uninteresting once it gets reset, because all the history before that point is irrelevant to our report entirely (whatever the previous position was, its get completely overwritten by the reset).
> > > 
> > > The API for marking objects uninteresting is not yet there but it's suggested in D104300. I believe you should rebase your patch on top of it and take advantage of it as it lands.
> > It was unknown to me if the visit of `NoteTag`'s happens from the bug path end to begin or in different way. If it is from end to begin it may be enough to remove the interestingness in the current `NoteTag` function when a message is produced, it should find exactly the last place where that EOF flag is set to true. And `NotePosition` is not needed.
> I tried it out, seems to work. But to continue D104300 should be finished or the "reset of interestingness" must be split to a separate change (or into this patch), is this possible?
> I tried it out, seems to work. But to continue D104300 should be finished or the "reset of interestingness" must be split to a separate change (or into this patch), is this possible?

Yes, I think it would be the logical way forward if we want a smooth development with this change. So let's create a parent patch that implements solely `markNotInteresting` from D104300. And that new patch should be the parent of this patch, also D104300 could be rebased once the new patch of `markNotInteresting` is landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104925