[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/benshi001 closed https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske approved this pull request. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/benshi001 updated
https://github.com/llvm/llvm-project/pull/74296
>From fbfe3492b66492948c9b0220af38d59345c5a793 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/6] [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 925fc90e355431..2c725c01dc285b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public CheckerStreamArgNo)
+ ->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 ,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 7089bd8bfc9d98..409a969a0d4cce 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 c8332bcbfa8ca7..aa5b6be851773a 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 fab80da9cf06bd0fa73dfdca1d4f2ed23ad060ba Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/6] [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 2c725c01dc285b..a368619fd37d22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,7 +266,8 @@ class StreamChecker : public CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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 aa5b6be851773a..94787874cf8396 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) {
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,83 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull && !StateNull)
+ensureStreamOpened(StreamVal, C, StateNotNull);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ // Skip if the stream can be both NULL and non-NULL.
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull && StateNull)
+return;
+ if (StateNotNull && !StateNull)
+State = StateNotNull;
+ else
+State = StateNull;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ auto ClearErrorInNotFailed = [, Desc](SymbolRef Sym,
+ const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (StateNotNull && !StateNull) {
+// Skip if the input stream's state is unknown, open-failed or closed.
+if (SymbolRef StreamSym = StreamVal.getAsSymbol()) {
+ const StreamState *SS = State->get(StreamSym);
+ if (SS) {
+assert(SS->isOpened() && "Stream is expected to be opened");
+ClearErrorInNotFailed(StreamSym, SS);
+ }
balazske wrote:
A `else return` is better to add here (to skip add of state transitions).
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/benshi001 updated
https://github.com/llvm/llvm-project/pull/74296
>From fbfe3492b66492948c9b0220af38d59345c5a793 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/5] [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 925fc90e355431..2c725c01dc285b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public CheckerStreamArgNo)
+ ->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 ,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 7089bd8bfc9d98..409a969a0d4cce 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 c8332bcbfa8ca7..aa5b6be851773a 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 fab80da9cf06bd0fa73dfdca1d4f2ed23ad060ba Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/5] [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 2c725c01dc285b..a368619fd37d22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,7 +266,8 @@ class StreamChecker : public CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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 aa5b6be851773a..94787874cf8396 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) {
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,82 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ConstraintManager::ProgramStatePair SP =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (SP.first)
+ensureStreamOpened(StreamVal, C, SP.first);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ // Skip if the stream can be both NULL and non-NULL.
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull && StateNull)
+return;
+ if (StateNotNull && !StateNull)
+State = StateNotNull;
+ else
+State = StateNull;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ auto ClearError = [, Desc](SymbolRef Sym,
+const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (StateNotNull && !StateNull) {
+// Skip if the input stream's state is unknown, open-failed or closed.
+if (SymbolRef StreamSym = StreamVal.getAsSymbol()) {
+ const StreamState *SS = State->get(StreamSym);
+ if (SS && SS->isOpened()) {
balazske wrote:
Here an assert for `SS->isOpened()` can be added. The state should be checked
in `preFflush` and can be only opened.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -299,6 +299,74 @@ void error_fseek_0(void) {
fclose(F);
}
+void error_fflush_after_fclose(void) {
+ FILE *F = tmpfile();
+ int Ret;
+ fflush(NULL); // no-warning
+ if (!F)
+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}}
+}
+
+void error_fflush_on_open_failed_stream(void) {
+ FILE *F = tmpfile();
+ if (!F) {
+fflush(F); // no-warning
+return;
+ }
+ fclose(F);
+}
+
+void error_fflush_on_unknown_stream(FILE *F) {
+ fflush(F); // no-warning
balazske wrote:
A `fclose(F)` can be added (after `fflush`) to show that no state-split is made
in `fflush`, and no warning appears for NULL stream at the fclose call.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,82 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ConstraintManager::ProgramStatePair SP =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (SP.first)
+ensureStreamOpened(StreamVal, C, SP.first);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ // Skip if the stream can be both NULL and non-NULL.
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull && StateNull)
+return;
+ if (StateNotNull && !StateNull)
+State = StateNotNull;
+ else
+State = StateNull;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ auto ClearError = [, Desc](SymbolRef Sym,
+const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (StateNotNull && !StateNull) {
+// Skip if the input stream's state is unknown, open-failed or closed.
+if (SymbolRef StreamSym = StreamVal.getAsSymbol()) {
+ const StreamState *SS = State->get(StreamSym);
+ if (SS && SS->isOpened()) {
+ClearError(StreamSym, SS);
+C.addTransition(StateNotFailed);
+C.addTransition(StateFailed);
+ }
+}
+ } else {
+// Clear error states for all streams.
+const StreamMapTy = StateNotFailed->get();
+for (const auto : Map) {
+ SymbolRef Sym = I.first;
+ const StreamState = I.second;
+ if (SS.isOpened())
+ClearError(Sym, );
+}
+C.addTransition(StateNotFailed);
+C.addTransition(StateFailed);
balazske wrote:
These `addTransition` calls (and at line 1260) can be moved to after the `if`
statement.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -299,6 +299,74 @@ void error_fseek_0(void) {
fclose(F);
}
+void error_fflush_after_fclose(void) {
+ FILE *F = tmpfile();
+ int Ret;
+ fflush(NULL); // no-warning
+ if (!F)
+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}}
+}
+
+void error_fflush_on_open_failed_stream(void) {
+ FILE *F = tmpfile();
+ if (!F) {
+fflush(F); // no-warning
+return;
+ }
+ fclose(F);
+}
+
+void error_fflush_on_unknown_stream(FILE *F) {
+ fflush(F); // no-warning
+}
+
+void error_fflush_on_non_null_stream_clear_error_states(void) {
+ FILE *F0 = tmpfile(), *F1 = tmpfile();
+ // `fflush` clears a non-EOF stream's error state.
+ if (F0) {
+StreamTesterChecker_make_ferror_stream(F0);
+if (fflush(F0) == 0) { // no-warning
+ clang_analyzer_eval(ferror(F0)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(feof(F0)); // expected-warning {{FALSE}}
+}
+fclose(F0);
+ }
+ // `fflush` clears an EOF stream's error state.
+ if (F1) {
+StreamTesterChecker_make_ferror_stream(F1);
+StreamTesterChecker_make_feof_stream(F1);
balazske wrote:
These stream "make" functions are not very good,
`StreamTesterChecker_make_feof_stream` clears the error flag. Normally a stream
has not both flags set at the same time, the `make_ferror_stream` can be
removed. (In the StreamState, if both flags are set this is a "superposition"
of two states, not a stream that has both flags set at the same time.)
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,82 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ConstraintManager::ProgramStatePair SP =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (SP.first)
balazske wrote:
It is better to check for `SP.first && !SP.second`. If a stream pointer is
unknown (`SP.first` and `SP.second` is not null) there should be no
`StreamState` for it, so the `ensureStreamOpened` does nothing useful.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,82 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ConstraintManager::ProgramStatePair SP =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (SP.first)
+ensureStreamOpened(StreamVal, C, SP.first);
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ // Skip if the stream can be both NULL and non-NULL.
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull && StateNull)
+return;
+ if (StateNotNull && !StateNull)
+State = StateNotNull;
+ else
+State = StateNull;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ auto ClearError = [, Desc](SymbolRef Sym,
balazske wrote:
A better name could be `ClearErrorInNotFailed`. It can be misleading if a
function changes a value that is not passed to it (as argument) and the name
does not tell about this.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -299,6 +299,74 @@ void error_fseek_0(void) {
fclose(F);
}
+void error_fflush_after_fclose(void) {
+ FILE *F = tmpfile();
+ int Ret;
+ fflush(NULL); // no-warning
+ if (!F)
+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}}
+}
+
+void error_fflush_on_open_failed_stream(void) {
+ FILE *F = tmpfile();
+ if (!F) {
+fflush(F); // no-warning
+return;
+ }
+ fclose(F);
+}
+
+void error_fflush_on_unknown_stream(FILE *F) {
+ fflush(F); // no-warning
+}
+
+void error_fflush_on_non_null_stream_clear_error_states(void) {
+ FILE *F0 = tmpfile(), *F1 = tmpfile();
+ // `fflush` clears a non-EOF stream's error state.
+ if (F0) {
+StreamTesterChecker_make_ferror_stream(F0);
+if (fflush(F0) == 0) { // no-warning
+ clang_analyzer_eval(ferror(F0)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(feof(F0)); // expected-warning {{FALSE}}
+}
+fclose(F0);
+ }
+ // `fflush` clears an EOF stream's error state.
+ if (F1) {
+StreamTesterChecker_make_ferror_stream(F1);
+StreamTesterChecker_make_feof_stream(F1);
+if (fflush(F1) == 0) { // no-warning
+ clang_analyzer_eval(ferror(F1)); // expected-warning {{FALSE}}
+ clang_analyzer_eval(feof(F1)); // expected-warning {{TRUE}}
+}
+fclose(F1);
+ }
+}
+
+void error_fflush_on_null_stream_clear_error_states(void) {
+ FILE *F0 = tmpfile(), *F1 = tmpfile();
+ // `fflush` clears all stream's error states, while retains their EOF states.
+ if (F0 && F1) {
+StreamTesterChecker_make_ferror_stream(F0);
+StreamTesterChecker_make_ferror_stream(F1);
balazske wrote:
Because `StreamTesterChecker_make_feof_stream` removes the error flag, this
line is not useful. The test can check if the EOF flag remains active after
`fflush` and the error flag is not set.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske requested changes to this pull request. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske edited https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/benshi001 updated
https://github.com/llvm/llvm-project/pull/74296
>From fbfe3492b66492948c9b0220af38d59345c5a793 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/4] [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 925fc90e355431..2c725c01dc285b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public CheckerStreamArgNo)
+ ->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 ,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 7089bd8bfc9d98..409a969a0d4cce 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 c8332bcbfa8ca7..aa5b6be851773a 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 fab80da9cf06bd0fa73dfdca1d4f2ed23ad060ba Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/4] [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 2c725c01dc285b..a368619fd37d22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,7 +266,8 @@ class StreamChecker : public CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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 aa5b6be851773a..94787874cf8396 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) {
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull) {
+if (StateNotNull = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(StateNotNull);
+ } else if (StateNull) {
+const StreamState *SS = StateNull->get(StreamSym);
+if (!SS || !SS->isOpenFailed()) {
+ StateNull = StateNull->set(StreamSym,
+StreamState::getOpenFailed(Desc));
+ if (StateNull)
+C.addTransition(StateNull);
+}
+ }
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ const StreamState *SS = nullptr;
+
+ // Skip if the input stream's state is unknown.
+ // FIXME: We should skip non-NULL constant, such as `(FILE *) 0x1234`.
+ if (StreamSym) {
+if (!(SS = State->get(StreamSym)))
+ return;
+assert((SS->isOpened() || SS->isOpenFailed()) &&
+ "Stream is expected to opened or open-failed");
+ }
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ C.addTransition(StateFailed);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+ auto ClearError = [, Desc](SymbolRef Sym,
+const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (SS && SS->isOpened()) {
balazske wrote:
This decision can not be made with `SS`. If a stream pointer is known to be
non-null but unknown to the checker, `SS` is null, but not NULL is passed to
`fflush`. The check can be made with assumptions on `getStreamArg(Desc, Call)`.
If it can be both NULL and non-NULL, we can't do anything in the `evalCall`
(for the same reason as in `preFflush`), otherwise we can get if it is clearly
NULL or not NULL.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -299,6 +299,60 @@ void error_fseek_0(void) {
fclose(F);
}
+void error_fflush_0(void) {
+ FILE *F = tmpfile();
+ int Ret;
+ fflush(NULL); // no-warning
+ if (!F) {
+if ((Ret = fflush(F)) != EOF)
+ 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}}
+}
+
+void error_fflush_1(void) {
balazske wrote:
I prefer to have multiple tests (with a name that indicates what is tested) for
the different cases.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull) {
+if (StateNotNull = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(StateNotNull);
+ } else if (StateNull) {
+const StreamState *SS = StateNull->get(StreamSym);
+if (!SS || !SS->isOpenFailed()) {
+ StateNull = StateNull->set(StreamSym,
+StreamState::getOpenFailed(Desc));
+ if (StateNull)
+C.addTransition(StateNull);
+}
+ }
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ const StreamState *SS = nullptr;
+
+ // Skip if the input stream's state is unknown.
+ // FIXME: We should skip non-NULL constant, such as `(FILE *) 0x1234`.
+ if (StreamSym) {
+if (!(SS = State->get(StreamSym)))
+ return;
+assert((SS->isOpened() || SS->isOpenFailed()) &&
balazske wrote:
An open-failed stream is not acceptable here, it must be opened only.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske requested changes to this pull request. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske edited https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
balazske wrote:
My opinion is now that here we should not add any transitions (except the
possible errors). Otherwise there will be two branches with a known NULL and
non-NULL stream, even if the stream was unknown before the call. This causes
unwanted "noisiness" of the checker.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
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
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/3] [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 925fc90e355431..2c725c01dc285b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public CheckerStreamArgNo)
+ ->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 ,
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h
b/clang/test/Analysis/Inputs/system-header-simulator.h
index 7089bd8bfc9d98..409a969a0d4cce 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 c8332bcbfa8ca7..aa5b6be851773a 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
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/3] [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 2c725c01dc285b..a368619fd37d22 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,7 +266,8 @@ class StreamChecker : public CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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 aa5b6be851773a..94787874cf8396 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) {
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull)
+if (State = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(State);
+ if (StateNull) {
alejandro-alvarez-sonarsource wrote:
Due to this transition, these two test cases will behave differently:
```cpp
void test_fflush_2(FILE *F1) {
fflush(F1);
// Due to fflush, the analyzer follows a path where F1 is NULL, and another
where it isn't.
// Raises a "Stream pointer might be NULL" on the next line
if (fwrite("1", 1, 1, F1) == 0)
return;
fclose(F1);
}
void test_fflush_3(FILE *F1) {
// no fflush, the warning does not raise
if (fwrite("1", 1, 1, F1) == 0)
return;
fclose(F1);
}
```
I feel this could be noisy. What do you think about adding the StateNull
transition if, and only if, FILE* can be `NULL`?
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,84 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ SymbolRef StreamSym = StreamVal.getAsSymbol();
+ if (!Stream || !StreamSym)
+return;
+
+ ProgramStateRef StateNotNull, StateNull;
+ std::tie(StateNotNull, StateNull) =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (StateNotNull)
+if (State = ensureStreamOpened(StreamVal, C, StateNotNull))
+ C.addTransition(State);
+ if (StateNull) {
+const StreamState *SS = StateNull->get(StreamSym);
+if (!SS || !SS->isOpenFailed()) {
+ StateNull = StateNull->set(StreamSym,
+StreamState::getOpenFailed(Desc));
+ if (StateNull)
+C.addTransition(StateNull);
+}
+ }
+}
+
+void StreamChecker::evalFflush(const FnDescription *Desc, const CallEvent
,
+ CheckerContext ) 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();
+ const StreamState *SS;
+ if (StreamSym) {
+if (!(SS = State->get(StreamSym)))
+ return;
+assert((SS->isOpened() || SS->isOpenFailed()) &&
+ "Stream is expected to opened or open-failed");
+ }
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ // `fflush` returns EOF on failure, otherwise returns 0.
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ C.addTransition(StateFailed);
+
+ // Clear error states if `fflush` returns 0, but retain their EOF flags.
+ ProgramStateRef StateNotFailed = bindInt(0, State, C, CE);
+ auto ClearError = [, Desc](SymbolRef Sym,
+const StreamState *SS) {
+if (SS->ErrorState & ErrorFError) {
+ StreamErrorState NewES =
+ (SS->ErrorState & ErrorFEof) ? ErrorFEof : ErrorNone;
+ StreamState NewSS = StreamState::getOpened(Desc, NewES, false);
+ StateNotFailed = StateNotFailed->set(Sym, NewSS);
+}
+ };
+
+ if (SS->isOpened()) {
alejandro-alvarez-sonarsource wrote:
Unless I am mistaken, `SS` can be uninitialized here. If `StreamSym` is `NULL`,
SS does not get initialized.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
benshi001 wrote: I can not understand why the new test `error_fflush_1` failed on windows, it did succeed on my local linux. So shall we 1. Add a `#if _win32` in the test? 2. Or only commit `fflush(not_null_stream) == 0` part, and leave `fflush(NULL) == 0` in the next patch? I hope the second one, at least we can break down this change. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
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
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/3] [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 CheckerStreamArgNo)
+ ->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 ,
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
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/3] [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 CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
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
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/3] [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 CheckerStreamArgNo)
+ ->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 ,
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
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/3] [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 CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
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
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH 1/3] [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 CheckerStreamArgNo)
+ ->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 ,
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
Date: Wed, 6 Dec 2023 15:47:35 +0800
Subject: [PATCH 2/3] [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 CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -299,6 +299,21 @@ void error_fseek_0(void) {
fclose(F);
}
+void error_fflush(void) {
+ FILE *F = tmpfile();
+ 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}}
+}
+
balazske wrote:
If the stream error state is reset by `fflush` another test is needed to check
if the reset works.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,49 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
+ }
+
+ const CallExpr *CE = dyn_cast_or_null(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.
balazske wrote:
The stream state can be affected in some way, if the stream was in failed
(`ferror`) state or the position is indeterminate, probably the `fflush` can be
allowed but the error (and indeterminate position) should be reset to
non-error. If the argument is null we can do this reset for all of the known
streams. It is somewhat questionable what should happen at a failing `fflush`,
but probably it is OK to leave the error flags as it was before.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1191,6 +1199,49 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ ProgramStateRef State = C.getState();
+ SVal StreamVal = getStreamArg(Desc, Call);
+ std::optional Stream = StreamVal.getAs();
+ if (!Stream)
+return;
+
+ ConstraintManager::ProgramStatePair SP =
+ C.getConstraintManager().assumeDual(State, *Stream);
+ if (State = SP.first)
+if (State = ensureStreamOpened(StreamVal, C, State))
+ C.addTransition(State);
balazske wrote:
I think another `addTransition` is needed for `SP.second` if it is non-null.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
benshi001 wrote: > For now only the pre-condition is added, the `evalFflush` function is missing. I have also added `evalFflush`. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
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
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 CheckerStreamArgNo)
+ ->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 ,
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
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 CheckerStreamArgNo)
- ->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 Stream = StreamVal.getAs();
+ 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
,
+ CheckerContext ) 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(StreamSym);
+if (!OldSS)
+ return;
+assertStreamStateOpened(OldSS);
}
+
+ const CallExpr *CE = dyn_cast_or_null(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
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske requested changes to this pull request. For now only the pre-condition is added, the `evalFflush` function is missing. https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -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
+}
+
balazske wrote:
A new test is needed when the passed stream is NULL but not a null constant.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/balazske edited https://github.com/llvm/llvm-project/pull/74296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -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
balazske wrote:
The `fflush(NULL)` should be before `fflush(F)` (probably at the start of the
function) because analysis may stop when a warning is emitted (and following
code is not reachable).
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
@@ -1188,6 +1192,18 @@ void StreamChecker::evalSetFeofFerror(const
FnDescription *Desc,
C.addTransition(State);
}
+void StreamChecker::preFflush(const FnDescription *Desc, const CallEvent ,
+ CheckerContext ) const {
+ // Skip if the stream is NULL/nullptr, which means flush all streams.
+ if (!Call.getArgExpr(Desc->StreamArgNo)
balazske wrote:
This check does not work in this case:
```
FILE *F = NULL;
fflush(F);
```
I think you should copy function `ensureStreamNonNull` to here and change it
such that it makes a check for opened stream in the non-null case. In case of
null stream no error is needed, and at unknown stream no assumption is needed
about the non-nullness of the stream.
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
llvmbot wrote:
@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
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 CheckerStreamArgNo)
+ ->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 ,
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)
``
https://github.com/llvm/llvm-project/pull/74296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Support `fflush` in the StreamChecker (PR #74296)
https://github.com/benshi001 created
https://github.com/llvm/llvm-project/pull/74296
None
>From 2929f07a9ac2c462bf7aed9fe10307ef79659074 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Mon, 4 Dec 2023 15:51:20 +0800
Subject: [PATCH] [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 a4799b5f762ca..5744feba4d35b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -266,6 +266,7 @@ class StreamChecker : public CheckerStreamArgNo)
+ ->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 ,
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)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
