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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateFailed);

steakhal wrote:

Thanks, makes sense.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via cfe-commits]

@@ -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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateFailed);

balazske wrote:

`ungetc` should not cause EOF or read error ("ferror") to be set. Documentation 
is not fully clear but this operation should change data only in the file 
buffer and this can not cause EOF or read error.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 



@@ -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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateFailed);

steakhal wrote:

Why did we not set `ErrorFError | ErrorFEof` errors for the failure state?

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource deleted 
https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Benics via cfe-commits]

@@ -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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateFailed);

balazs-benics-sonarsource wrote:

Why did we not set `ErrorFError | ErrorFEof` errors for the failure state?

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via cfe-commits]

https://github.com/balazske closed 
https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


NagyDonat wrote:

Thanks for the update, I think it's reasonable to merge this after waiting a 
bit for potential other reviews.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


NagyDonat wrote:

Thanks for the update, I think it's reasonable to merge this after waiting a 
bit for potential other reviews.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via cfe-commits]

@@ -916,6 +922,44 @@ 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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);

balazske wrote:

`NewSS` is now removed.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balázs Kéri via cfe-commits]

@@ -916,6 +922,44 @@ 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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?

balazske wrote:

I did check it, at least with EOF concrete value only one branch is added, this 
is fixed in the comment.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mitch Phillips via cfe-commits]

=?utf-8?q?Balázs_Kéri?= ,
=?utf-8?q?Balázs_Kéri?= 
Message-ID:
In-Reply-To: 


https://github.com/hctim updated https://github.com/llvm/llvm-project/pull/77331

>From 9bcc43b5c62ba969f91c495d4d570c5c4337aca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 8 Jan 2024 16:42:58 +0100
Subject: [PATCH 1/3] [clang][analyzer] Add 'ungetc' to StreamChecker.

---
 clang/docs/ReleaseNotes.rst   |  4 +-
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 19 
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 44 +++
 .../Analysis/Inputs/system-header-simulator.h |  1 +
 clang/test/Analysis/stream-error.c| 16 +++
 clang/test/Analysis/stream-noopen.c   | 25 +++
 clang/test/Analysis/stream.c  |  6 +++
 7 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescripti

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

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/77331

From 9bcc43b5c62ba969f91c495d4d570c5c4337aca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 8 Jan 2024 16:42:58 +0100
Subject: [PATCH 1/3] [clang][analyzer] Add 'ungetc' to StreamChecker.

---
 clang/docs/ReleaseNotes.rst   |  4 +-
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 19 
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 44 +++
 .../Analysis/Inputs/system-header-simulator.h |  1 +
 clang/test/Analysis/stream-error.c| 16 +++
 clang/test/Analysis/stream-noopen.c   | 25 +++
 clang/test/Analysis/stream.c  |  6 +++
 7 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
  CheckerConte

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

[Balázs Kéri via cfe-commits]

https://github.com/balazske updated 
https://github.com/llvm/llvm-project/pull/77331

From 9bcc43b5c62ba969f91c495d4d570c5c4337aca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 8 Jan 2024 16:42:58 +0100
Subject: [PATCH 1/2] [clang][analyzer] Add 'ungetc' to StreamChecker.

---
 clang/docs/ReleaseNotes.rst   |  4 +-
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 19 
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 44 +++
 .../Analysis/Inputs/system-header-simulator.h |  1 +
 clang/test/Analysis/stream-error.c| 16 +++
 clang/test/Analysis/stream-noopen.c   | 25 +++
 clang/test/Analysis/stream.c  |  6 +++
 7 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call,
  CheckerConte

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

[via cfe-commits]

@@ -916,6 +922,44 @@ 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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);

NagyDonat wrote:

Is there any reason why you're introducing `NewSS` as a temporary variable 
while you're inlining the `getOpened()` call a few lines above this?

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

https://github.com/NagyDonat approved this pull request.

At first glance LGTM.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -916,6 +922,44 @@ 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(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?

NagyDonat wrote:

I strongly suspect that StateFailed == StateNotFailed (more precisely semantic 
equivalence of the states, I don't know about the actual result of `operator==` 
on `ProgramStateRef` values) is possible in the case when the first argument of 
`ungetc` is a concrete `EOF` value.

I'd personally guess that this doesn't cause too much harm (at most there'll be 
a redundantly duplicated edge that doesn't disturb the graph traversal), but if 
you want to be on the safe side, you could investigate this with a testcase and 
some exploded graph dumps.

https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

https://github.com/NagyDonat edited 
https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

llvmbot wrote:




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

Author: Balázs Kéri (balazske)


Changes

`StdLibraryFunctionsChecker` is updated too with `ungetc`.

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


7 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+2-2) 
- (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
(+19) 
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+44) 
- (modified) clang/test/Analysis/Inputs/system-header-simulator.h (+1) 
- (modified) clang/test/Analysis/stream-error.c (+16) 
- (modified) clang/test/Analysis/stream-noopen.c (+25) 
- (modified) clang/test/Analysis/stream.c (+6) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(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/t

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)


Changes

`StdLibraryFunctionsChecker` is updated too with `ungetc`.

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


7 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+2-2) 
- (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
(+19) 
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+44) 
- (modified) clang/test/Analysis/Inputs/system-header-simulator.h (+1) 
- (modified) clang/test/Analysis/stream-error.c (+16) 
- (modified) clang/test/Analysis/stream-noopen.c (+25) 
- (modified) clang/test/Analysis/stream.c (+6) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(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/Input

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

[Balázs Kéri via cfe-commits]

https://github.com/balazske created 
https://github.com/llvm/llvm-project/pull/77331

`StdLibraryFunctionsChecker` is updated too with `ungetc`.

From 9bcc43b5c62ba969f91c495d4d570c5c4337aca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Mon, 8 Jan 2024 16:42:58 +0100
Subject: [PATCH] [clang][analyzer] Add 'ungetc' to StreamChecker.

---
 clang/docs/ReleaseNotes.rst   |  4 +-
 .../Checkers/StdLibraryFunctionsChecker.cpp   | 19 
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 44 +++
 .../Analysis/Inputs/system-header-simulator.h |  1 +
 clang/test/Analysis/stream-error.c| 16 +++
 clang/test/Analysis/stream-noopen.c   | 25 +++
 clang/test/Analysis/stream.c  |  6 +++
 7 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..3bfcd7997faa19 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1162,8 +1162,8 @@ Improvements
   (`c3a87ddad62a 
`_,
   `0954dc3fb921 
`_)
 
-- 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``.
   (`#76776 `_,
   `#74296 `_,
   `#73335 `_,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..e16bf9feb5b1c4 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..a3d1f34d3d1a9a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -263,6 +263,9 @@ class StreamChecker : public Checker(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  const StreamState *OldSS = State->get(StreamSym);
+  if (!OldSS)
+return;
+
+  assertStreamStateOpened(OldSS);
+
+  // Generate a transition for the success state.
+  std::optional PutVal = Call.getArgSVal(0).getAs();
+  if (!PutVal)
+return;
+  ProgramStateRef StateNotFailed =
+  State->BindExpr(CE, C.getLocationContext(), *PutVal);
+  StateNotFailed =
+  StateNotFailed->set(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.
+  // FIXME: Is it possible that StateFailed == StateNotFailed ?
+  ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+  StreamState NewSS = StreamState::getOpened(Desc);
+  StateFailed = StateFailed->set(StreamSym, NewSS);
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const Cal