balazske marked 2 inline comments as done.
balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:222
{StreamState::FEof, StreamState::FError, StreamState::NoError}}},
- {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0, {}}},
+ // Note: ftell sets errno only.
+ {{"ftell", 1},
----------------
Szelethus wrote:
> Szelethus wrote:
> > C'99 standard [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
> > | §7.19.9.4.3]], about the `ftell function`:
> >
> > > If successful, the `ftell` function returns the current value of the file
> > > position indicator for the stream. On failure, the `ftell` function
> > > returns `-1L` and stores an implementation-defined positive value in
> > > `errno`.
> >
> > So we need an evalCall for this.
> I mean, this function can only fail if the stream itself is an erroneous or
> closed state, right? So we can associate the -1 return value with the stream
> being in that, and the other with the stream being opened.
The `ftell` is part of a later patch that is adding `ftell` (and probably other
functions). Only the "PossibleErrors" was updated in this patch because its
meaning was changed (it contains value even if only one error kind is possible,
before it was used only if at least two errors are possible).
It is not sure how the file operations work, maybe the `ftell` needs access to
some data that is not available at the moment (and can fail even if the stream
was not OK). If the stream is already in failed state the question is: Do ftell
fail (then the we can make it return -1) or it does not fail but gives an
unusable value (then generate a checker warning and stop the analysis).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:485
+ State = bindInt(0, State, C, CE);
+ State = State->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(State);
----------------
Szelethus wrote:
> Isn't the state change redundant? We have a `preCall` to this function and we
> assert this as well.
We need to clarify what file operations are valid if the stream is in failed
state or in EOF state. Again the question is if the function will simply fail
again or it does not fail but does wrong things, or it will work correctly.
Currently the initial stream error state is not taken into account for `fread`
and `fwrite`, this needs to be fixed. For example for `fread`: "If an error
occurs, the resulting value of the file position indicator for the stream is
indeterminate." So at least a next read or write or `ftell` can not work
correct after this (or works but with unusable result).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78374/new/
https://reviews.llvm.org/D78374
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
