[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked an inline comment as done.
balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:382-384
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailedWithFError);
+  C.addTransition(StateFailedWithoutFError);
----------------
baloghadamsoftware wrote:
> Szelethus wrote:
> > balazske wrote:
> > > Szelethus wrote:
> > > > This seems a bit excessive, we could merge the last two into `FSeekError`.
> > > There are 3 cases:
> > >  - `fseek` did not fail at all. Return value is zero. This is `StateNotFailed`.
> > >  - `fseek` failed but none of the error flags is true afterwards. Return value is nonzero but `feof` and `ferror` are not true. This is `StateFailedWithoutFError`.
> > >  - `fseek` failed and we have `feof` or `ferror` set (never both). Return value is nonzero and `feof` or `ferror` will be true. This is `StateFailedWithFError`. And an use of `AnyError`, otherwise we need here 2 states, one for `feof` and one for `ferror`. But it is not important here if `feof` or `ferror` is actually true, so the special value `AnyError` is used and only one new state instead of two.
> > Sure, but from the point of the analyzer the latter two are the same, isn't it? Its never a good idea to use a stream on which `fseek` failed without checking.
> > 
> > State splits are the most expensive things the analyzer can make, which is why I'm cautious here.
> Somehow, this should be done with just two new states. Maybe there should be an error state "Unknown" instead of `OtherError` (or `FeoFOrFError` what I suggested at the other patch) which can be any of the errors plus the `NoError` value. Later, when `ferror()` of `feof()` is checked we can do a second state split, but not earlier.
Yes it is possible to use two new states only. For example by having a new `FSeekError` that can "manifest" in feof, ferror or no error. Better is to have only one "last-operation-failed-with-any-error" state and determine later if needed what errors are possible (based on the last operation). For `fseek` it is possible to get feof, ferror, or none of these (but still failed `fseek`). For reading operations the error can be feof or ferror. After write only ferror is possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75851