[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Wed May 20 01:35:10 PDT 2020

balazske marked 2 inline comments as done.
balazske added a comment.

I want to test the StreamChecker for false positives. Page http://clang.llvm.org/analyzer/open_projects.html says that it has too much false positives because state splitting (I did not see this page before.)

There is an alternative way of implementation: Store a (ordered) list of previous stream operations and data what we know about the operation. This data contains if the operation failed or not, the symbol that is the return value of the function, error state before and after the operation, what bug report was made already. This list can be populated during analysis, without state splits. At every time when a new information is put into the list it is possible to check for problems.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:523
+  // FEOF).
+  if (!IsFread || (SS->ErrorState != ErrorFEof)) {
+    ProgramStateRef StateNotFailed =
----------------
Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > Aaaah okay it took me a while. I think `SS->ErrorState != ErrorFEof` isn't as talkative as `isConstrained.*`, could you add just a line like "if we **know** the state to be EOF..."?
> > > 
> > > On another note, why is fwrite so special in this context?
> > If it is an `fwrite`, the function call can always succeed (even after a previously EOF or error).
> > If it is an `fread`, it can succeed but not after an exactly-EOF state.
> > 
> > A success transition is not needed in an `fread` with exactly EOF condition. (If we have fread with non-exactly EOF, for example `ErrorFEof` and `ErrorFError`, the success is needed because the read in `ErrorFError` can succeed.)
> > Aaaah okay it took me a while. I think SS->ErrorState != ErrorFEof isn't as talkative as isConstrained.*, could you add just a line like "if we know the state to be EOF..."?
> 
> Could you please add these few words before commiting?
It is now "If we know the state to be FEOF at fread, do not add a success state.".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015