[clang][analyzer] Add "pedantic" mode to StreamChecker.

clang/docs/analyzer/checkers.rst

@@ -3138,10 +3138,16 @@ are detected:
   allowed in this state.
 * Invalid 3rd ("``whence``") argument to ``fseek``.
 
-The checker does not track the correspondence between integer file descriptors
-and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not
-treated specially and are therefore often not recognized (because these streams
-are usually not opened explicitly by the program, and are global variables).
+The stream operations are by this checker usually split into two cases, a success
+and a failure case. However, in the case of write operations (like ``fwrite``,
+``fprintf`` and even ``fsetpos``) this behavior could produce a large amount of
+unwanted reports on projects that don't have error checks around the write
+operations, so by default the checker assumes that write operations always succeed.
+This behavior can be controlled by the ``Pedantic`` flag: With
+``-analyzer-config alpha.unix.Stream:Pedantic=true`` the checker will model the
+cases where a write operation fails and report situations where this leads to
+erroneous behavior. (The default is ``Pedantic=false``, where write operations
+are assumed to succeed.)
 
 .. code-block:: c
 
@@ -3196,6 +3202,13 @@ are usually not opened explicitly by the program, and are global variables).
    fclose(p);
  }
 
+**Limitations**
+
+The checker does not track the correspondence between integer file descriptors
+and ``FILE *`` pointers. Operations on standard streams like ``stdin`` are not
+treated specially and are therefore often not recognized (because these streams
+are usually not opened explicitly by the program, and are global variables).
+
 .. _alpha-unix-cstring-BufferOverlap:
 
 alpha.unix.cstring.BufferOverlap (C)

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -604,6 +604,15 @@ def PthreadLockChecker : Checker<"PthreadLock">,
 def StreamChecker : Checker<"Stream">,
   HelpText<"Check stream handling functions">,
   WeakDependencies<[NonNullParamChecker]>,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "Pedantic",
+                  "If false, assume that stream operations which are often not "
+                  "checked for error do not fail."
+                  "fail.",
+                  "false",
+                  InAlpha>
+  ]>,
   Documentation<HasDocumentation>;
 
 def SimpleStreamChecker : Checker<"SimpleStream">,

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

@@ -297,6 +297,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   /// If true, evaluate special testing stream functions.
   bool TestMode = false;
 
+  /// If true, generate failure branches for cases that are often not checked.
+  bool PedanticMode = false;
+
 private:
   CallDescriptionMap<FnDescription> FnDescriptions = {
       {{{"fopen"}, 2}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
@@ -945,6 +948,10 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
   }
 
   // Add transition for the failed state.
+  // At write, add failure case only if "pedantic mode" is on.
+  if (!IsFread && !PedanticMode)
+    return;
+
   NonLoc RetVal = makeRetVal(C, E.CE).castAs<NonLoc>();
   ProgramStateRef StateFailed =
       State->BindExpr(E.CE, C.getLocationContext(), RetVal);
@@ -1057,6 +1064,9 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
     C.addTransition(StateNotFailed);
   }
 
+  if (!PedanticMode)
+    return;
+
   // Add transition for the failed state. The resulting value of the file
   // position indicator for the stream is indeterminate.
   ProgramStateRef StateFailed = E.bindReturnValue(State, C, *EofVal);
@@ -1092,6 +1102,9 @@ void StreamChecker::evalFprintf(const FnDescription *Desc,
       E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
   C.addTransition(StateNotFailed);
 
+  if (!PedanticMode)
+    return;
+
   // Add transition for the failed state. The resulting value of the file
   // position indicator for the stream is indeterminate.
   StateFailed = E.setStreamState(
@@ -1264,21 +1277,23 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call,
   if (!E.Init(Desc, Call, C, State))
     return;
 
-  // Bifurcate the state into failed and non-failed.
-  // Return zero on success, -1 on error.
+  // Add success state.
   ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, 0);
-  ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
-
   // No failure: Reset the state to opened with no error.
   StateNotFailed =
       E.setStreamState(StateNotFailed, StreamState::getOpened(Desc));
   C.addTransition(StateNotFailed);
 
+  if (!PedanticMode)
+    return;
+
+  // Add failure state.
   // At error it is possible that fseek fails but sets none of the error flags.
   // If fseek failed, assume that the file position becomes indeterminate in any
   // case.
   // It is allowed to set the position beyond the end of the file. EOF error
   // should not occur.
+  ProgramStateRef StateFailed = E.bindReturnValue(State, C, -1);
   StateFailed = E.setStreamState(
       StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
   C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
@@ -1316,6 +1331,10 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc,
 
   StateNotFailed = E.setStreamState(
       StateNotFailed, StreamState::getOpened(Desc, ErrorNone, false));
+  C.addTransition(StateNotFailed);
+
+  if (!PedanticMode)
+    return;
 
   // At failure ferror could be set.
   // The standards do not tell what happens with the file position at failure.
@@ -1324,7 +1343,6 @@ void StreamChecker::evalFsetpos(const FnDescription *Desc,
   StateFailed = E.setStreamState(
       StateFailed, StreamState::getOpened(Desc, ErrorNone | ErrorFError, true));
 
-  C.addTransition(StateNotFailed);
   C.addTransition(StateFailed, E.getFailureNoteTag(this, C));
 }
 
@@ -1794,7 +1812,9 @@ ProgramStateRef StreamChecker::checkPointerEscape(
 //===----------------------------------------------------------------------===//
 
 void ento::registerStreamChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<StreamChecker>();
+  auto *Checker = Mgr.registerChecker<StreamChecker>();
+  Checker->PedanticMode =
+      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "Pedantic");
 }
 
 bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) {

clang/test/Analysis/analyzer-config.c

@@ -12,6 +12,7 @@
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
+// CHECK-NEXT: alpha.unix.Stream:Pedantic = false
 // CHECK-NEXT: apply-fixits = false
 // CHECK-NEXT: assume-controlled-environment = false
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false

clang/test/Analysis/std-c-library-functions-vs-stream-checker.c

@@ -1,6 +1,7 @@
 // Check the case when only the StreamChecker is enabled.
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core,alpha.unix.Stream \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false \
 // RUN:   -triple x86_64-unknown-linux \
@@ -19,6 +20,7 @@
 // StdLibraryFunctionsChecker are enabled.
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core,alpha.unix.Stream \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \
 // RUN:   -analyzer-checker=debug.ExprInspection \

clang/test/Analysis/stream-errno-note.c

@@ -1,5 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.unix.Stream \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-checker=unix.Errno \
 // RUN:   -analyzer-checker=unix.StdCLibraryFunctions \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true \

clang/test/Analysis/stream-errno.c

@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.Errno,unix.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify %s
 
 #include "Inputs/system-header-simulator.h"

clang/test/Analysis/stream-error.c

@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN: -analyzer-checker=core \
 // RUN: -analyzer-checker=alpha.unix.Stream \
+// RUN: -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN: -analyzer-checker=debug.StreamTester \
 // RUN: -analyzer-checker=debug.ExprInspection
 

clang/test/Analysis/stream-note.c

@@ -1,6 +1,8 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -analyzer-output text \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.StdCLibraryFunctions -analyzer-output text \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=expected,stdargs %s
 
 #include "Inputs/system-header-simulator.h"

clang/test/Analysis/stream-pedantic.c

@@ -0,0 +1,95 @@
+// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=false -verify=nopedantic %s
+
+// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true -verify=pedantic %s
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_eval(int);
+
+void check_fwrite(void) {
+  char *Buf = "123456789";
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  size_t Ret = fwrite(Buf, 1, 10, Fp);
+  clang_analyzer_eval(Ret == 0); // nopedantic-warning {{FALSE}} \
+                                 // pedantic-warning {{FALSE}} \
+                                 // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fputc(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  int Ret = fputc('A', Fp);
+  clang_analyzer_eval(Ret == EOF); // nopedantic-warning {{FALSE}} \
+                                   // pedantic-warning {{FALSE}} \
+                                   // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fputs(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  int Ret = fputs("ABC", Fp);
+  clang_analyzer_eval(Ret == EOF); // nopedantic-warning {{FALSE}} \
+                                   // pedantic-warning {{FALSE}} \
+                                   // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fprintf(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  int Ret = fprintf(Fp, "ABC");
+  clang_analyzer_eval(Ret < 0); // nopedantic-warning {{FALSE}} \
+                                // pedantic-warning {{FALSE}} \
+                                // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fseek(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  int Ret = fseek(Fp, 0, 0);
+  clang_analyzer_eval(Ret == -1); // nopedantic-warning {{FALSE}} \
+                                  // pedantic-warning {{FALSE}} \
+                                  // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fseeko(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  int Ret = fseeko(Fp, 0, 0);
+  clang_analyzer_eval(Ret == -1); // nopedantic-warning {{FALSE}} \
+                                  // pedantic-warning {{FALSE}} \
+                                  // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}
+
+void check_fsetpos(void) {
+  FILE *Fp = tmpfile();
+  if (!Fp)
+    return;
+  fpos_t Pos;
+  int Ret = fsetpos(Fp, &Pos);
+  clang_analyzer_eval(Ret); // nopedantic-warning {{FALSE}} \
+                            // pedantic-warning {{FALSE}} \
+                            // pedantic-warning {{TRUE}}
+  fputc('A', Fp); // pedantic-warning {{might be 'indeterminate'}}
+  fclose(Fp);
+}

clang/test/Analysis/stream-stdlibraryfunctionargs.c

@@ -1,10 +1,13 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,unix.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stream,any %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.StdCLibraryFunctions,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true \
 // RUN:   -analyzer-config unix.StdCLibraryFunctions:ModelPOSIX=true -verify=stdfunc,any %s
 
 #include "Inputs/system-header-simulator.h"

clang/test/Analysis/stream.c

@@ -1,7 +1,11 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s
+// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s
+// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s
+// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection \
+// RUN:   -analyzer-config alpha.unix.Stream:Pedantic=true -verify %s
 
 #include "Inputs/system-header-simulator.h"
 #include "Inputs/system-header-simulator-for-malloc.h"