NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
struct FnDescription;
using FnCheck = std::function<void(const StreamChecker *, const FnDescription
*,
const CallEvent &, CheckerContext &)>;
----------------
balazske wrote:
> NoQ wrote:
> > `llvm::function_ref`?
> `function_ref`'s documentation says:
> > This class does not own the callable, so it is not in general safe to store
> > a function_ref.
> The `FnDescription` stores these functions so it is not safe to use
> `llvm::function_ref`?
>
>
>
I think you're using it only for global function pointers, no?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:366
+
+ State = State->set<StreamMap>(StreamSym, StreamState::getOpened());
+ C.addTransition(State);
----------------
balazske wrote:
> NoQ wrote:
> > So what exactly happens when you're calling `clearerr` on a closed stream?
> > Does it actually become opened? Also what happens when you're calling it on
> > an open-failed stream?
> The stream gets always in opened and error-free state. The `preDefault`
> checks that the stream is opened (not closed and not open-failed) and creates
> error node if not. If this create fails that is a problem case, the stream
> will be opened after `clearerr`. Should check in the eval functions for
> closed stream even if the pre-functions (in `preCall` callback) have normally
> checked this?
Aha, ok, got it.
Let's assert it then?
================
Comment at: clang/test/Analysis/stream-error.c:33-34
+ int ch = fputc('a', F);
+ if (ch == EOF) {
+ // FIXME: fputc not handled by checker yet, should expect TRUE
+ clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
----------------
balazske wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Sure, we don't, but the bigger issue here is that we wouldn't mark `F` to
> > > be in EOF even if we did handle it, we would also need to understand that
> > > `ch == EOF` is the telling sign. But that also ins't in the scope of this
> > > patch :)
> > Yeah, that's gonna be fun.
> If there is a `fputc` call (for example) that is recognized (has recognized
> stream and no functions "fail") there is already an error and non-error
> branch created for it. On the error branch the return value of `fputc` would
> be **EOF** and the stream state is set to error. (The `fputc` should be later
> modeled by this checker, not done yet.)
>
> If the `ch == EOF` is found then to figure out if this `ch` comes from an
> `fputc` that is called on a stream that is not recognized (it may be function
> parameter) and then make a "tracked" stream with error state for it, this is
> another thing.
Aha, fair enough, that's probably a well-justified state split.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
https://reviews.llvm.org/D75682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
