Skip to content

[clang][analyzer] Support fflush in the StreamChecker #74296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 21, 2023
Merged

[clang][analyzer] Support fflush in the StreamChecker #74296

merged 6 commits into from
Dec 21, 2023

Conversation

benshi001
Copy link
Member

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Dec 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ben Shi (benshi001)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/74296.diff

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+16)
  • (modified) clang/test/Analysis/Inputs/system-header-simulator.h (+1)
  • (modified) clang/test/Analysis/stream-error.c (+9)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index a4799b5f762ca..5744feba4d35b 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
@@ -1188,6 +1192,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)

Copy link
Collaborator

@balazske balazske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now only the pre-condition is added, the evalFflush function is missing.

@benshi001 benshi001 requested a review from balazske December 6, 2023 07:50
@benshi001
Copy link
Member Author

For now only the pre-condition is added, the evalFflush function is missing.

I have also added evalFflush.

@benshi001 benshi001 requested a review from balazske December 7, 2023 08:34
@benshi001 benshi001 requested a review from balazske December 14, 2023 06:33
@benshi001 benshi001 requested a review from balazske December 21, 2023 02:36
@benshi001 benshi001 requested a review from balazske December 21, 2023 09:28
@benshi001 benshi001 merged commit 73948ec into llvm:main Dec 21, 2023
@benshi001 benshi001 deleted the csa-fflush branch December 21, 2023 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants