[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)

Ben Shi via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 5 23:48:27 PST 2023


https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/74296

>From 65ce18117f99056aafcf58151b64f4243f4d5e26 Mon Sep 17 00:00:00 2001
From: Ben Shi <bennshi at tencent.com>
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/2] [clang][analyzer] Support `fflush` in the StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp    | 16 ++++++++++++++++
 .../Analysis/Inputs/system-header-simulator.h    |  1 +
 clang/test/Analysis/stream-error.c               |  9 +++++++++
 3 files changed, 26 insertions(+)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 925fc90e35543..2c725c01dc285 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"ftell"}, 1},
        {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
+      {{{"fflush"}, 1}, {&StreamChecker::preFflush, nullptr, 0}},
       {{{"rewind"}, 1},
        {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
       {{{"fgetpos"}, 2},
@@ -360,6 +361,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
                          CheckerContext &C,
                          const StreamErrorState &ErrorKind) const;
 
+  void preFflush(const FnDescription *Desc, const CallEvent &Call,
+                 CheckerContext &C) const;
+
   /// Check that the stream (in StreamVal) is not NULL.
   /// If it can only be NULL a fatal error is emitted and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
@@ -1191,6 +1195,18 @@ void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
   C.addTransition(State);
 }
 
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
+                              CheckerContext &C) const {
+  // Skip if the stream is NULL/nullptr, which means flush all streams.
+  if (!Call.getArgExpr(Desc->StreamArgNo)
+           ->isNullPointerConstant(C.getASTContext(),
+                                   Expr::NPC_ValueDependentIsNotNull)) {
+    ProgramStateRef State = C.getState();
+    if (State = ensureStreamOpened(getStreamArg(Desc, Call), C, State))
+      C.addTransition(State);
+  }
+}
+
 ProgramStateRef
 StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE,
                                    CheckerContext &C,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index 7089bd8bfc9d9..409a969a0d4cc 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -61,6 +61,7 @@ void clearerr(FILE *stream);
 int feof(FILE *stream);
 int ferror(FILE *stream);
 int fileno(FILE *stream);
+int fflush(FILE *stream);
 
 size_t strlen(const char *);
 
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index c8332bcbfa8ca..aa5b6be851773 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -299,6 +299,15 @@ void error_fseek_0(void) {
   fclose(F);
 }
 
+void error_fflush(void) {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  fclose(F);
+  fflush(F);    // expected-warning {{Stream might be already closed}}
+  fflush(NULL); // no-warning
+}
+
 void error_indeterminate(void) {
   FILE *F = fopen("file", "r+");
   if (!F)

>From dcb766b468a2f29df30451f8f196d7a2371fd038 Mon Sep 17 00:00:00 2001
From: Ben Shi <bennshi at tencent.com>
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/2] [clang][analyzer] Support `fflush` in the StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 49 ++++++++++++++++---
 clang/test/Analysis/stream-error.c            | 12 +++--
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 2c725c01dc285..a368619fd37d2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,7 +266,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"ftell"}, 1},
        {&StreamChecker::preDefault, &StreamChecker::evalFtell, 0}},
-      {{{"fflush"}, 1}, {&StreamChecker::preFflush, nullptr, 0}},
+      {{{"fflush"}, 1},
+       {&StreamChecker::preFflush, &StreamChecker::evalFflush, 0}},
       {{{"rewind"}, 1},
        {&StreamChecker::preDefault, &StreamChecker::evalRewind, 0}},
       {{{"fgetpos"}, 2},
@@ -364,6 +365,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void preFflush(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C) const;
 
+  void evalFflush(const FnDescription *Desc, const CallEvent &Call,
+                  CheckerContext &C) const;
+
   /// Check that the stream (in StreamVal) is not NULL.
   /// If it can only be NULL a fatal error is emitted and nullptr returned.
   /// Otherwise the return value is a new state where the stream is constrained
@@ -1197,14 +1201,45 @@ void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,
 
 void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent &Call,
                               CheckerContext &C) const {
-  // Skip if the stream is NULL/nullptr, which means flush all streams.
-  if (!Call.getArgExpr(Desc->StreamArgNo)
-           ->isNullPointerConstant(C.getASTContext(),
-                                   Expr::NPC_ValueDependentIsNotNull)) {
-    ProgramStateRef State = C.getState();
-    if (State = ensureStreamOpened(getStreamArg(Desc, Call), C, State))
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+  std::optional<DefinedSVal> Stream = StreamVal.getAs<DefinedSVal>();
+  if (!Stream)
+    return;
+
+  ConstraintManager::ProgramStatePair SP =
+      C.getConstraintManager().assumeDual(State, *Stream);
+  if (State = SP.first)
+    if (State = ensureStreamOpened(StreamVal, C, State))
       C.addTransition(State);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent &Call,
+                               CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+
+  // We will check the result even if the input is `NULL`,
+  // but do nothing if the input state is unknown.
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (StreamSym) {
+    const StreamState *OldSS = State->get<StreamMap>(StreamSym);
+    if (!OldSS)
+      return;
+    assertStreamStateOpened(OldSS);
   }
+
+  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  // `fflush` returns 0 on success, otherwise returns EOF.
+  ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+
+  // This function does not affect the stream state.
+  // Still we add success and failure state with the appropriate return value.
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailed);
 }
 
 ProgramStateRef
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index aa5b6be851773..94787874cf839 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -301,11 +301,17 @@ void error_fseek_0(void) {
 
 void error_fflush(void) {
   FILE *F = tmpfile();
-  if (!F)
+  int Ret;
+  fflush(NULL);                      // no-warning
+  if (!F) {
+    if ((Ret = fflush(F)) != EOF)    // no-warning
+      clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}}
     return;
+  }
+  if ((Ret = fflush(F)) != 0)
+    clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
   fclose(F);
-  fflush(F);    // expected-warning {{Stream might be already closed}}
-  fflush(NULL); // no-warning
+  fflush(F);                         // expected-warning {{Stream might be already closed}}
 }
 
 void error_indeterminate(void) {



More information about the cfe-commits mailing list