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

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 18 06:57:14 PDT 2020


Szelethus added a comment.

I think the warning about EOF could be a separate patch, and it could be implemented for most stream operations. The patch in large looks great, but I'm just not sure why we handle fwrite so differently. Could you please explain?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:56
+  bool operator!=(const StreamErrorState &ES) const {
+    return NoError != ES.NoError || FEof != ES.FEof || FError != ES.FError;
+  }
----------------
How about `return !(*this == ES)`?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:456-463
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (Sym && State->get<StreamMap>(Sym)) {
+    const StreamState *SS = State->get<StreamMap>(Sym);
+    if (SS->ErrorState & ErrorFEof)
+      reportFEofWarning(C, State);
+  } else {
+    C.addTransition(State);
----------------
Do we not want this for `fwrite` as well?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:495
+    return;
+  Optional<NonLoc> CountVal = Call.getArgSVal(2).getAs<NonLoc>();
+  if (!CountVal)
----------------
`// The standard refers to this parameter as "nmemb", but almost everywhere else its called "count".`


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:523
+  // FEOF).
+  if (!IsFread || (SS->ErrorState != ErrorFEof)) {
+    ProgramStateRef StateNotFailed =
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015





More information about the cfe-commits mailing list