[clang][analyzer] Support fputc in StreamChecker

[benshi001]

No description provided.

[llvmbot]

@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/71518.diff

3 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+66-8)
• (modified) clang/test/Analysis/stream-error.c (+46)
• (modified) clang/test/Analysis/stream.c (+14)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 898906977ba9bb6..43ef9a2d377c512 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -245,11 +245,14 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
       {{{"fclose"}, 1},
        {&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
       {{{"fread"}, 4},
-       {std::bind(&StreamChecker::preFreadFwrite, _1, _2, _3, _4, true),
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
         std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
       {{{"fwrite"}, 4},
-       {std::bind(&StreamChecker::preFreadFwrite, _1, _2, _3, _4, false),
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
         std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
+      {{{"fputc"}, 2},
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+        &StreamChecker::evalFputc, 1}},
       {{{"fseek"}, 3},
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"ftell"}, 1},
@@ -305,12 +308,15 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void evalFclose(const FnDescription *Desc, const CallEvent &Call,
                   CheckerContext &C) const;
 
-  void preFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
-                      CheckerContext &C, bool IsFread) const;
+  void preReadWrite(const FnDescription *Desc, const CallEvent &Call,
+                    CheckerContext &C, bool IsRead) const;
 
   void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
                        CheckerContext &C, bool IsFread) const;
 
+  void evalFputc(const FnDescription *Desc, const CallEvent &Call,
+                 CheckerContext &C) const;
+
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
                 CheckerContext &C) const;
   void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -634,9 +640,9 @@ void StreamChecker::evalFclose(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(StateFailure);
 }
 
-void StreamChecker::preFreadFwrite(const FnDescription *Desc,
-                                   const CallEvent &Call, CheckerContext &C,
-                                   bool IsFread) const {
+void StreamChecker::preReadWrite(const FnDescription *Desc,
+                                 const CallEvent &Call, CheckerContext &C,
+                                 bool IsRead) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
   State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
@@ -650,7 +656,7 @@ void StreamChecker::preFreadFwrite(const FnDescription *Desc,
   if (!State)
     return;
 
-  if (!IsFread) {
+  if (!IsRead) {
     C.addTransition(State);
     return;
   }
@@ -745,6 +751,58 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc,
     C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalFputc(const FnDescription *Desc, const CallEvent &Call,
+                              CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+  if (!StreamSym)
+    return;
+
+  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  const StreamState *OldSS = State->get<StreamMap>(StreamSym);
+  if (!OldSS)
+    return;
+
+  assertStreamStateOpened(OldSS);
+
+  // `fputc` returns the written character on success, otherwise returns EOF.
+
+  // Generate a transition for the success state.
+  std::optional<NonLoc> PutVal = Call.getArgSVal(0).getAs<NonLoc>();
+  if (!PutVal)
+    return;
+  ProgramStateRef StateNotFailed =
+      State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+      StateNotFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateNotFailed);
+
+  // Add transition for the failed state.
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc RetVal = makeRetVal(C, CE).castAs<NonLoc>();
+  ProgramStateRef StateFailed =
+      State->BindExpr(CE, C.getLocationContext(), RetVal);
+  auto Cond = SVB.evalBinOp(State, BO_EQ, RetVal,
+                            SVB.makeIntVal(*EofVal, C.getASTContext().IntTy),
+                            SVB.getConditionType())
+                  .getAs<DefinedOrUnknownSVal>();
+  if (!Cond)
+    return;
+  StateFailed = StateFailed->assume(*Cond, true);
+  if (!StateFailed)
+    return;
+
+  // If a (non-EOF) error occurs, the resulting value of the file position
+  // indicator for the stream is indeterminate.
+  StreamState NewSS = StreamState::getOpened(
+      Desc, ErrorFError, /*IsFilePositionIndeterminate*/ true);
+  StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS);
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
                              CheckerContext &C) const {
   ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 3c00e59bb6bd19d..7e895f3fe947158 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -241,6 +241,23 @@ void error_indeterminate_clearerr(void) {
   fclose(F);
 }
 
+void error_indeterminate_fputc(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+    return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+    if (feof(F)) {
+      fputc('X', F); // no warning
+    } else if (ferror(F)) {
+      fputc('C', F); // expected-warning {{might be 'indeterminate'}}
+    } else {
+      fputc('E', F); // expected-warning {{might be 'indeterminate'}}
+    }
+  }
+  fclose(F);
+}
+
 void error_indeterminate_feof1(void) {
   FILE *F = fopen("file", "r+");
   if (!F)
@@ -268,3 +285,32 @@ void error_indeterminate_feof2(void) {
   }
   fclose(F);
 }
+
+void error_indeterminate_feof3(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+    return;
+  char Buf[10];
+  if (fread(Buf, 1, 10, F) < 10) {
+    if (feof(F)) {
+      // error is feof, should be non-indeterminate
+      fputc(';', F); // no warning
+    }
+    if (ferror(F)) {
+      fputc('=', F); // expected-warning {{might be 'indeterminate'}}
+    }
+  }
+  fclose(F);
+}
+
+void error_indeterminate_feof4(void) {
+  FILE *F = fopen("file", "r+");
+  if (!F)
+    return;
+  if (fputc('Y', F) == EOF) {
+    fputc('W', F); // expected-warning {{might be 'indeterminate'}}
+  } else {
+    fputc('H', F); // no warning
+  }
+  fclose(F);
+}
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index a01310cfef5dd8a..a2eaaa2bd0142c5 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -14,6 +14,12 @@ void check_fwrite(void) {
   fclose(fp);
 }
 
+void check_fgetc(void) {
+  FILE *fp = tmpfile();
+  fputc('A', fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
 void check_fseek(void) {
   FILE *fp = tmpfile();
   fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
@@ -111,6 +117,14 @@ void f_use_after_close(void) {
   clearerr(p); // expected-warning {{Stream might be already closed}}
 }
 
+void f_write_after_close(void) {
+  FILE *p = fopen("foo", "r");
+  if (!p)
+    return;
+  fclose(p);
+  fputc('A', p); // expected-warning {{Stream might be already closed}}
+}
+
 void f_open_after_close(void) {
   FILE *p = fopen("foo", "r");
   if (!p)

[benshi001]

fgetc should also be supported, and I will do that next week.