[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus requested changes to this revision.
Szelethus added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:152-158
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
   assert(Desc && Desc->StreamArgNo != ArgNone &&
          "Try to get a non-existing stream argument.");
   return Call.getArgSVal(Desc->StreamArgNo);
 }
----------------
If we add methods to `FnDescription`, we might as well add this as well, but I don't insist.


================
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},
----------------
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.


================
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);
----------------
Isn't the state change redundant? We have a `preCall` to this function and we assert this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374