[clang] 8f78dd4 - [clang][analyzer] Add function 'ungetc' to StreamChecker. (#77331)

via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 10 00:09:55 PST 2024


Author: Balázs Kéri
Date: 2024-01-10T09:09:51+01:00
New Revision: 8f78dd4b92b44c490d263a4d161850853874859d

URL: https://github.com/llvm/llvm-project/commit/8f78dd4b92b44c490d263a4d161850853874859d
DIFF: https://github.com/llvm/llvm-project/commit/8f78dd4b92b44c490d263a4d161850853874859d.diff

LOG: [clang][analyzer] Add function 'ungetc' to StreamChecker. (#77331)

`StdLibraryFunctionsChecker` is updated too with `ungetc`.

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
    clang/test/Analysis/Inputs/system-header-simulator.h
    clang/test/Analysis/stream-error.c
    clang/test/Analysis/stream-noopen.c
    clang/test/Analysis/stream.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 15479906d22bdd..20872f7ddb81e9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1202,8 +1202,9 @@ Improvements
   (`c3a87ddad62a <https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c>`_,
   `0954dc3fb921 <https://github.com/llvm/llvm-project/commit/0954dc3fb9214b994623f5306473de075f8e3593>`_)
 
-- Improved the ``alpha.unix.Stream`` checker by modeling more functions like,
-  ``fflush``, ``fputs``, ``fgetc``, ``fputc``, ``fopen``, ``fdopen``, ``fgets``, ``tmpfile``.
+- Improved the ``alpha.unix.Stream`` checker by modeling more functions
+  ``fputs``, ``fputc``, ``fgets``, ``fgetc``, ``fdopen``, ``ungetc``, ``fflush``
+  and no not recognize alternative ``fopen`` and ``tmpfile`` implementations.
   (`#76776 <https://github.com/llvm/llvm-project/pull/76776>`_,
   `#74296 <https://github.com/llvm/llvm-project/pull/74296>`_,
   `#73335 <https://github.com/llvm/llvm-project/pull/73335>`_,
@@ -1211,7 +1212,8 @@ Improvements
   `#71518 <https://github.com/llvm/llvm-project/pull/71518>`_,
   `#72016 <https://github.com/llvm/llvm-project/pull/72016>`_,
   `#70540 <https://github.com/llvm/llvm-project/pull/70540>`_,
-  `#73638 <https://github.com/llvm/llvm-project/pull/73638>`_)
+  `#73638 <https://github.com/llvm/llvm-project/pull/73638>`_,
+  `#77331 <https://github.com/llvm/llvm-project/pull/77331>`_)
 
 - The ``alpha.security.taint.TaintPropagation`` checker no longer propagates
   taint on ``strlen`` and ``strnlen`` calls, unless these are marked

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 034825d88a44de..32a2deab871c79 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2201,6 +2201,25 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                   ErrnoNEZeroIrrelevant, GenericFailureMsg)
             .ArgConstraint(NotNull(ArgNo(0))));
 
+    // int ungetc(int c, FILE *stream);
+    addToFunctionSummaryMap(
+        "ungetc", Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
+        Summary(NoEvalCall)
+            .Case({ReturnValueCondition(BO_EQ, ArgNo(0)),
+                   ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
+                  ErrnoMustNotBeChecked, GenericSuccessMsg)
+            .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
+                   ArgumentCondition(0, WithinRange, {{EOFv, EOFv}})},
+                  ErrnoNEZeroIrrelevant,
+                  "Assuming that 'ungetc' fails because EOF was passed as "
+                  "character")
+            .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv)),
+                   ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})},
+                  ErrnoNEZeroIrrelevant, GenericFailureMsg)
+            .ArgConstraint(ArgumentCondition(
+                0, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}}))
+            .ArgConstraint(NotNull(ArgNo(1))));
+
     // int fseek(FILE *stream, long offset, int whence);
     // FIXME: It can be possible to get the 'SEEK_' values (like EOFv) and use
     // these for condition of arg 2.

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 25da3c18e8519f..fbfa101257d5e1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
       {{{"fputs"}, 2},
        {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
         std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
+      {{{"ungetc"}, 2},
+       {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
+        std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
       {{{"fseek"}, 3},
        {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
       {{{"ftell"}, 1},
@@ -332,6 +335,9 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
   void evalFputx(const FnDescription *Desc, const CallEvent &Call,
                  CheckerContext &C, bool IsSingleChar) const;
 
+  void evalUngetc(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,
@@ -916,6 +922,45 @@ void StreamChecker::evalFputx(const FnDescription *Desc, const CallEvent &Call,
   C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalUngetc(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);
+
+  // 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.
+  // Failure of 'ungetc' does not result in feof or ferror state.
+  // If the PutVal has value of EofVal the function should "fail", but this is
+  // the same transition as the success state.
+  // In this case only one state transition is added by the analyzer (the two
+  // new states may be similar).
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StateFailed =
+      StateFailed->set<StreamMap>(StreamSym, StreamState::getOpened(Desc));
+  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/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index 8c43c48c6a3e45..caae59c38a4c8e 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -53,6 +53,7 @@ int fgetc(FILE *stream);
 char *fgets(char *restrict str, int count, FILE *restrict stream);
 int fputc(int ch, FILE *stream);
 int fputs(const char *restrict s, FILE *restrict stream);
+int ungetc(int c, FILE *stream);
 int fseek(FILE *__stream, long int __off, int __whence);
 long int ftell(FILE *__stream);
 void rewind(FILE *__stream);

diff  --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 13c6684b5840af..c038348e799d29 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -191,6 +191,22 @@ void error_fputs(void) {
   fputs("ABC", F); // expected-warning {{Stream might be already closed}}
 }
 
+void error_ungetc() {
+  FILE *F = tmpfile();
+  if (!F)
+    return;
+  int Ret = ungetc('X', F);
+  clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  if (Ret == EOF) {
+    clang_analyzer_warnIfReached();          // expected-warning {{REACHABLE}}
+  } else {
+    clang_analyzer_eval(Ret == 'X');         // expected-warning {{TRUE}}
+  }
+  fputc('Y', F);                             // no-warning
+  fclose(F);
+  ungetc('A', F); // expected-warning {{Stream might be already closed}}
+}
+
 void write_after_eof_is_allowed(void) {
   FILE *F = tmpfile();
   if (!F)

diff  --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..8ad101ee1e8c13 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -138,6 +138,31 @@ void test_rewind(FILE *F) {
   rewind(F);
 }
 
+void test_ungetc(FILE *F) {
+  int Ret = ungetc('X', F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+  if (Ret == 'X') {
+    if (errno) {} // expected-warning {{undefined}}
+  } else {
+    clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+    clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  }
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_ungetc_EOF(FILE *F, int C) {
+  int Ret = ungetc(EOF, F);
+  clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+  clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+  clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+  Ret = ungetc(C, F);
+  if (Ret == EOF) {
+    clang_analyzer_eval(C == EOF); // expected-warning {{TRUE}}
+                                   // expected-warning at -1{{FALSE}}
+  }
+}
+
 void test_feof(FILE *F) {
   errno = 0;
   feof(F);

diff  --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 060d561c1fe1c2..d8026247697a50 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -39,6 +39,12 @@ void check_fputs(void) {
   fclose(fp);
 }
 
+void check_ungetc(void) {
+  FILE *fp = tmpfile();
+  ungetc('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}}


        


More information about the cfe-commits mailing list