subject:"\[clang\] \[clang\]\[analyzer\] Add function 'fprintf' to StreamChecker. \(PR #77613\)"

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-25 Thread Balázs Kéri via cfe-commits



@@ -926,6 +932,49 @@ void StreamChecker::evalFputx(const FnDescription *Desc, 
const CallEvent ,
   C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalFprintf(const FnDescription *Desc,
+const CallEvent ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  if (Call.getNumArgs() < 2)
+return;
+  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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);

balazske wrote:

Yes, some fixes can be done with `fprintf` and `fscanf` and the argument 
invalidation. I plan to do this with the following patches.

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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-24 Thread Balazs Benics via cfe-commits


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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-24 Thread Balázs Benics via cfe-commits



@@ -926,6 +932,49 @@ void StreamChecker::evalFputx(const FnDescription *Desc, 
const CallEvent ,
   C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalFprintf(const FnDescription *Desc,
+const CallEvent ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  if (Call.getNumArgs() < 2)
+return;
+  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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);

balazs-benics-sonarsource wrote:

In addition to this, we should also invalidate any buffers passed to such 
calls. This also sounds like a regression compared to default eval calling 
"fpintf". (Look at the format string specifier `%n`)

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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-24 Thread Balazs Benics via cfe-commits



@@ -926,6 +932,49 @@ void StreamChecker::evalFputx(const FnDescription *Desc, 
const CallEvent ,
   C.addTransition(StateFailed);
 }
 
+void StreamChecker::evalFprintf(const FnDescription *Desc,
+const CallEvent ,
+CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  if (Call.getNumArgs() < 2)
+return;
+  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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);

steakhal wrote:

Downstream we used `State->assumeInclusiveRange()` for this.

As a sideffect, we bound an upperbound to the `RetVal` with the number of 
arguments we had for the call.
Have you considered setting the upperbound here too?

I was thinking if the std library functions checker applies this constraint, 
but AFAIK it does not, nor we have a proposed patch for doing so. And anyways 
I'd prefer setting both upper and lowerbounds at the same checker if possible. 
Thus, I write to this PR.

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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-12 Thread Balázs Kéri via cfe-commits


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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-12 Thread Balázs Kéri via cfe-commits


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

From 96a786f07ca5fb84b372e0b7c60f371bf4cdfd7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 10 Jan 2024 10:55:27 +0100
Subject: [PATCH] [clang][analyzer] Add function 'fprintf' to StreamChecker.

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 49 +++
 clang/test/Analysis/stream-error.c| 17 +++
 clang/test/Analysis/stream.c  | 11 +++--
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 742426a628e065..95c7503e49e0d3 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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
+
+  StateNotFailed =
+  StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateNotFailed);
+
+  // Add transition for the failed state. The resulting value of the file
+  // position indicator for the stream is indeterminate.
+  StateFailed = StateFailed->set(
+  StreamSym, StreamState::getOpened(Desc, ErrorFError, true));
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent 
,
CheckerContext ) const {
   ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c 
b/clang/test/Analysis/stream-error.c
index 2d03c0bbe7c474..0f7fdddc0dd4cd 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -191,6 +191,23 @@ void error_fputs(void) {
   fputs("ABC", F); // expected-warning {{Stream might be already closed}}
 }
 
+void error_fprintf(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fprintf(F, "aaa");
+  if (Ret >= 0) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+fprintf(F, "bbb"); // no-warning
+  } else {
+clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+fprintf(F, "bbb");  // expected-warning {{might be 
'indeterminate'}}
+  }
+  fclose(F);
+  fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}}
+}
+
 void error_ungetc() {
   FILE *F = tmpfile();
   if (!F)
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index d8026247697a50..e8f06922bdb2f3 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -39,6 +39,12 @@ void check_fputs(void) {
   fclose(fp);
 }
 
+void check_fprintf(void) {
+  FILE *fp = tmpfile();
+  fprintf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
 void check_ungetc(void) {
   FILE *fp = tmpfile();
   ungetc('A', fp); // expected-warning {{Stream pointer might be NULL}}
@@ -271,9 +277,8 @@ void check_escape4(void) {
 return;
   fwrite("1", 1, 1, F); // may fail
 
-  // no escape at (non-StreamChecker-handled) system call
-  // FIXME: all such calls should be handled by the checker
-  fprintf(F, "0");
+  // no escape at a non-StreamChecker-handled system call
+  setbuf(F, "0");
 
   fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}}
   fclose(F);

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-10 Thread via cfe-commits


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

I don't see any issue.

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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-10 Thread via cfe-commits


llvmbot wrote:




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

Author: Balázs Kéri (balazske)


Changes

[clang][analyzer] Add function 'fprintf' to StreamChecker.

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


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+49) 
- (modified) clang/test/Analysis/stream-error.c (+17) 
- (modified) clang/test/Analysis/stream.c (+8-3) 


``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 25da3c18e8519f..d6df5199ece2d7 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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
+
+  StateNotFailed =
+  StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateNotFailed);
+
+  // Add transition for the failed state. The resulting value of the file
+  // position indicator for the stream is indeterminate.
+  StateFailed = StateFailed->set(
+  StreamSym, StreamState::getOpened(Desc, ErrorFError, true));
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c 
b/clang/test/Analysis/stream-error.c
index 13c6684b5840af..abe391b21a4d07 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -191,6 +191,23 @@ void error_fputs(void) {
   fputs("ABC", F); // expected-warning {{Stream might be already closed}}
 }
 
+void error_fprintf(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fprintf(F, "aaa");
+  if (Ret >= 0) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+fprintf(F, "bbb"); // no-warning
+  } else {
+clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+fprintf(F, "bbb");  // expected-warning {{might be 
'indeterminate'}}
+  }
+  fclose(F);
+  fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}}
+}
+
 void write_after_eof_is_allowed(void) {
   FILE *F = tmpfile();
   if (!F)
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 060d561c1fe1c2..bd1509f31bc0d0 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -39,6 +39,12 @@ void check_fputs(void) {
   fclose(fp);
 }
 
+void check_fprintf(void) {
+  FILE *fp = tmpfile();
+  fprintf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
 void check_fseek(void) {
   FILE *fp = tmpfile();
   fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
@@ -265,9 +271,8 @@ void check_escape4(void) {
 return;
   fwrite("1", 1, 1, F); // may fail
 
-  // no escape at (non-StreamChecker-handled) system call
-  // FIXME: all such calls should be handled by the checker
-  fprintf(F, "0");
+  // no escape at a non-StreamChecker-handled system call
+  setbuf(F, "0");
 
   fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}}
   fclose(F);

``




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

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

2024-01-10 Thread Balázs Kéri via cfe-commits


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

[clang][analyzer] Add function 'fprintf' to StreamChecker.

From 574816e425d623b3b07674422b901879cc3c83c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= 
Date: Wed, 10 Jan 2024 10:55:27 +0100
Subject: [PATCH] [clang][analyzer] Add function 'fprintf' to StreamChecker.

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 49 +++
 clang/test/Analysis/stream-error.c| 17 +++
 clang/test/Analysis/stream.c  | 11 +++--
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 25da3c18e8519f..d6df5199ece2d7 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);
+
+  NonLoc RetVal = makeRetVal(C, CE).castAs();
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+  SValBuilder  = C.getSValBuilder();
+  auto  = C.getASTContext();
+  auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+  .getAs();
+  if (!Cond)
+return;
+  ProgramStateRef StateNotFailed, StateFailed;
+  std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
+
+  StateNotFailed =
+  StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
+  C.addTransition(StateNotFailed);
+
+  // Add transition for the failed state. The resulting value of the file
+  // position indicator for the stream is indeterminate.
+  StateFailed = StateFailed->set(
+  StreamSym, StreamState::getOpened(Desc, ErrorFError, true));
+  C.addTransition(StateFailed);
+}
+
 void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
diff --git a/clang/test/Analysis/stream-error.c 
b/clang/test/Analysis/stream-error.c
index 13c6684b5840af..abe391b21a4d07 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -191,6 +191,23 @@ void error_fputs(void) {
   fputs("ABC", F); // expected-warning {{Stream might be already closed}}
 }
 
+void error_fprintf(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  int Ret = fprintf(F, "aaa");
+  if (Ret >= 0) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+fprintf(F, "bbb"); // no-warning
+  } else {
+clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+fprintf(F, "bbb");  // expected-warning {{might be 
'indeterminate'}}
+  }
+  fclose(F);
+  fprintf(F, "ccc"); // expected-warning {{Stream might be already closed}}
+}
+
 void write_after_eof_is_allowed(void) {
   FILE *F = tmpfile();
   if (!F)
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 060d561c1fe1c2..bd1509f31bc0d0 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -39,6 +39,12 @@ void check_fputs(void) {
   fclose(fp);
 }
 
+void check_fprintf(void) {
+  FILE *fp = tmpfile();
+  fprintf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
 void check_fseek(void) {
   FILE *fp = tmpfile();
   fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
@@ -265,9 +271,8 @@ void check_escape4(void) {
 return;
   fwrite("1", 1, 1, F); // may fail
 
-  // no escape at (non-StreamChecker-handled) system call
-  // FIXME: all such calls should be handled by the checker
-  fprintf(F, "0");
+  // no escape at a non-StreamChecker-handled system call
+  setbuf(F, "0");
 
   fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}}
   fclose(F);

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

9 matches

Site Navigation

Mail list logo

Footer information