[clang][analyzer] Add "pedantic" mode to StreamChecker. (#87322)

The checker may create failure branches for all stream write operations
only if the new option "pedantic" is set to true.
Result of the write operations is often not checked in typical code. If
failure branches are created the checker will warn for unchecked write
operations and generate a lot of "false positives" (these are valid
warnings but the programmer does not care about this problem).

12 files changed, 166 insertions, 14 deletions

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index f188f18ba555..fb748d23a53d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/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)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 5fe5c9286dab..9aa1c6ddfe44 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/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">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 069e3a633c12..31c756ab0c58 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -297,6 +297,9 @@ public:
   /// 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) {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 2167a2b32f59..23e37a856d09 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/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
diff --git a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c b/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
index 281fbaaffe70..cac3fe5c5151 100644
--- a/clang/test/Analysis/std-c-library-functions-vs-stream-checker.c
+++ b/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 \
diff --git a/clang/test/Analysis/stream-errno-note.c b/clang/test/Analysis/stream-errno-note.c
index 2411a2d9a00a..fb12f0bace93 100644
--- a/clang/test/Analysis/stream-errno-note.c
+++ b/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 \
diff --git a/clang/test/Analysis/stream-errno.c b/clang/test/Analysis/stream-errno.c
index 5f0a58032fa2..08382eaf6abf 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/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"
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 7f9116ff4014..2abf4b900a04 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/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
 
diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index 54ea699f4667..03a8ff4e468f 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/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"
diff --git a/clang/test/Analysis/stream-pedantic.c b/clang/test/Analysis/stream-pedantic.c
new file mode 100644
index 000000000000..2bbea81d47ef
--- /dev/null
+++ b/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);
+}
diff --git a/clang/test/Analysis/stream-stdlibraryfunctionargs.c b/clang/test/Analysis/stream-stdlibraryfunctionargs.c
index 0053510163ef..2ea6a8c472c6 100644
--- a/clang/test/Analysis/stream-stdlibraryfunctionargs.c
+++ b/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"
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index ba5e66a4102e..93ed555c89eb 100644
--- a/clang/test/Analysis/stream.c
+++ b/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"