[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
https://github.com/benshi001 closed https://github.com/llvm/llvm-project/pull/76207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
balazske wrote: I do not see much benefit of adding `fileno` to the checker in the current state. If the `StdLibraryFunctionsChecker` is turned on it will anyway do a similar modeling of `fileno` (this applied to `ftell` too). The `fileno` and `ftell` are semantically different and `fileno` can be more complicated if we want to ensure that the returned file numbers are different from each other, and special values for `stdin`, `stdout`, `stderr` should be taken into account. Adding `fileno` in the current way makes a small improvement for the case when `StdLibraryFunctionsChecker` is not used. A combined function for `fileno` and `ftell` can be used, but in a way that works with any function that returns -1 on error and >=0 value on success. The type of the return value can be get from the declaration of the function or from the `CallEvent` in some way, a type parameter to the function is not needed. Adding the standard streams to the checker has some difficulties and requires more work (at least there must be a checker parameter to assume that standard streams are not changed by the program, otherwise no modeling can be done because these are global variables). https://github.com/llvm/llvm-project/pull/76207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
https://github.com/benshi001 updated
https://github.com/llvm/llvm-project/pull/76207
>From 0a46fedc497a124d3aca4295b8c18ae60df186e9 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Fri, 22 Dec 2023 14:47:48 +0800
Subject: [PATCH] [clang][analyzer] Improve modeling of `fileno` in the
StreamChecker
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 30 ---
clang/test/Analysis/stream-errno.c| 7 +++--
clang/test/Analysis/stream-noopen.c | 12
3 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 254b36ed03968d..80ac910060720f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,7 +265,11 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -342,8 +345,8 @@ class StreamChecker : public Checker();
ProgramStateRef StateNotFailed =
State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
-SVB.makeZeroVal(C.getASTContext().LongTy),
-SVB.getConditionType())
- .getAs();
+ auto Cond =
+ SVB.evalBinOp(State, BO_GE, RetVal,
+SVB.makeZeroVal(IsRetLongTy ? C.getASTContext().LongTy
+: C.getASTContext().IntTy),
+SVB.getConditionType())
+ .getAs();
if (!Cond)
return;
StateNotFailed = StateNotFailed->assume(*Cond, true);
@@ -1078,7 +1084,9 @@ void StreamChecker::evalFtell(const FnDescription *Desc,
const CallEvent ,
return;
ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1,
C.getASTContext().LongTy));
+ CE, C.getLocationContext(),
+ SVB.makeIntVal(-1, IsRetLongTy ? C.getASTContext().LongTy
+ : C.getASTContext().IntTy));
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
diff --git a/clang/test/Analysis/stream-errno.c
b/clang/test/Analysis/stream-errno.c
index bf0a61db2424f9..62befbc78e5a75 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -216,9 +216,10 @@ void check_fileno(void) {
int N = fileno(F);
if (N == -1) {
clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
-if (errno) {} // no-warning
-fclose(F);
-return;
+if (errno) {}// no-warning
+ } else {
+clang_analyzer_eval(N >= 0); // expected-warning{{TRUE}}
}
if (errno) {} // expected-warning{{An undefined value may be read from
'errno'}}
+ fclose(F);
}
diff --git a/clang/test/Analysis/stream-noopen.c
b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..198b5dc45e35da 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -129,6 +129,18 @@ void check_ftell(FILE *F) {
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void check_fileno(FILE *F) {
+ int Ret = fileno(F);
+ clang_analyzer_eval(F != NULL);// expected-warning{{TRUE}}
+ if (!(Ret >= 0)) {
+clang_analyzer_eval(Ret == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {}// no-warning
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(ferror(F));// expected-warning{{UNKNOWN}}
+}
+
void test_rewind(FILE *F) {
errno = 0;
rewind(F);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
benshi001 wrote: The reported format error is in another file, not related to my patch. https://github.com/llvm/llvm-project/pull/76207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
benshi001 wrote: `fileno` and `ftell` are quite similar, 1. both of them return `0` on success, and `-1` on failure. 2. both of them set `errno` on failure. The differences are 1. `fileno` returns `int` type but `ftell` returns `long` type. 2. `ftell` will not affect the value of `error` on success, but this is not mentioned for `fileno`. https://github.com/llvm/llvm-project/pull/76207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Ben Shi (benshi001)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/76207.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+19-11)
- (modified) clang/test/Analysis/stream-errno.c (+4-3)
- (modified) clang/test/Analysis/stream-noopen.c (+12)
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 254b36ed03968d..80ac910060720f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,7 +265,11 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -342,8 +345,8 @@ class StreamChecker : public Checker();
ProgramStateRef StateNotFailed =
State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
-SVB.makeZeroVal(C.getASTContext().LongTy),
-SVB.getConditionType())
- .getAs();
+ auto Cond =
+ SVB.evalBinOp(State, BO_GE, RetVal,
+SVB.makeZeroVal(IsRetLongTy ? C.getASTContext().LongTy
+: C.getASTContext().IntTy),
+SVB.getConditionType())
+ .getAs();
if (!Cond)
return;
StateNotFailed = StateNotFailed->assume(*Cond, true);
@@ -1078,7 +1084,9 @@ void StreamChecker::evalFtell(const FnDescription *Desc,
const CallEvent ,
return;
ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1,
C.getASTContext().LongTy));
+ CE, C.getLocationContext(),
+ SVB.makeIntVal(-1, IsRetLongTy ? C.getASTContext().LongTy
+ : C.getASTContext().IntTy));
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
diff --git a/clang/test/Analysis/stream-errno.c
b/clang/test/Analysis/stream-errno.c
index bf0a61db2424f9..62befbc78e5a75 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -216,9 +216,10 @@ void check_fileno(void) {
int N = fileno(F);
if (N == -1) {
clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
-if (errno) {} // no-warning
-fclose(F);
-return;
+if (errno) {}// no-warning
+ } else {
+clang_analyzer_eval(N >= 0); // expected-warning{{TRUE}}
}
if (errno) {} // expected-warning{{An undefined value may be read from
'errno'}}
+ fclose(F);
}
diff --git a/clang/test/Analysis/stream-noopen.c
b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..198b5dc45e35da 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -129,6 +129,18 @@ void check_ftell(FILE *F) {
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void check_fileno(FILE *F) {
+ int Ret = fileno(F);
+ clang_analyzer_eval(F != NULL);// expected-warning{{TRUE}}
+ if (!(Ret >= 0)) {
+clang_analyzer_eval(Ret == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {}// no-warning
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(ferror(F));// expected-warning{{UNKNOWN}}
+}
+
void test_rewind(FILE *F) {
errno = 0;
rewind(F);
``
https://github.com/llvm/llvm-project/pull/76207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Ben Shi (benshi001)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/76207.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+19-11)
- (modified) clang/test/Analysis/stream-errno.c (+4-3)
- (modified) clang/test/Analysis/stream-noopen.c (+12)
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 254b36ed03968d..80ac910060720f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,7 +265,11 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -342,8 +345,8 @@ class StreamChecker : public Checker();
ProgramStateRef StateNotFailed =
State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
-SVB.makeZeroVal(C.getASTContext().LongTy),
-SVB.getConditionType())
- .getAs();
+ auto Cond =
+ SVB.evalBinOp(State, BO_GE, RetVal,
+SVB.makeZeroVal(IsRetLongTy ? C.getASTContext().LongTy
+: C.getASTContext().IntTy),
+SVB.getConditionType())
+ .getAs();
if (!Cond)
return;
StateNotFailed = StateNotFailed->assume(*Cond, true);
@@ -1078,7 +1084,9 @@ void StreamChecker::evalFtell(const FnDescription *Desc,
const CallEvent ,
return;
ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1,
C.getASTContext().LongTy));
+ CE, C.getLocationContext(),
+ SVB.makeIntVal(-1, IsRetLongTy ? C.getASTContext().LongTy
+ : C.getASTContext().IntTy));
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
diff --git a/clang/test/Analysis/stream-errno.c
b/clang/test/Analysis/stream-errno.c
index bf0a61db2424f9..62befbc78e5a75 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -216,9 +216,10 @@ void check_fileno(void) {
int N = fileno(F);
if (N == -1) {
clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
-if (errno) {} // no-warning
-fclose(F);
-return;
+if (errno) {}// no-warning
+ } else {
+clang_analyzer_eval(N >= 0); // expected-warning{{TRUE}}
}
if (errno) {} // expected-warning{{An undefined value may be read from
'errno'}}
+ fclose(F);
}
diff --git a/clang/test/Analysis/stream-noopen.c
b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..198b5dc45e35da 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -129,6 +129,18 @@ void check_ftell(FILE *F) {
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void check_fileno(FILE *F) {
+ int Ret = fileno(F);
+ clang_analyzer_eval(F != NULL);// expected-warning{{TRUE}}
+ if (!(Ret >= 0)) {
+clang_analyzer_eval(Ret == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {}// no-warning
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(ferror(F));// expected-warning{{UNKNOWN}}
+}
+
void test_rewind(FILE *F) {
errno = 0;
rewind(F);
``
https://github.com/llvm/llvm-project/pull/76207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of `fileno` in the StreamChecker (PR #76207)
https://github.com/benshi001 created
https://github.com/llvm/llvm-project/pull/76207
None
>From a7fed7e081981b1c7c6c41dd72f0bc0736260754 Mon Sep 17 00:00:00 2001
From: Ben Shi
Date: Fri, 22 Dec 2023 14:47:48 +0800
Subject: [PATCH] [clang][analyzer] Improve modeling of `fileno` in the
StreamChecker
---
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 30 ---
clang/test/Analysis/stream-errno.c| 7 +++--
clang/test/Analysis/stream-noopen.c | 12
3 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 254b36ed03968d..80ac910060720f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -265,7 +265,11 @@ class StreamChecker : public Checker FnTestDescriptions = {
@@ -342,8 +345,8 @@ class StreamChecker : public Checker();
ProgramStateRef StateNotFailed =
State->BindExpr(CE, C.getLocationContext(), RetVal);
- auto Cond = SVB.evalBinOp(State, BO_GE, RetVal,
-SVB.makeZeroVal(C.getASTContext().LongTy),
-SVB.getConditionType())
- .getAs();
+ auto Cond =
+ SVB.evalBinOp(State, BO_GE, RetVal,
+SVB.makeZeroVal(IsRetLongTy ? C.getASTContext().LongTy
+: C.getASTContext().IntTy),
+SVB.getConditionType())
+ .getAs();
if (!Cond)
return;
StateNotFailed = StateNotFailed->assume(*Cond, true);
@@ -1078,7 +1084,9 @@ void StreamChecker::evalFtell(const FnDescription *Desc,
const CallEvent ,
return;
ProgramStateRef StateFailed = State->BindExpr(
- CE, C.getLocationContext(), SVB.makeIntVal(-1,
C.getASTContext().LongTy));
+ CE, C.getLocationContext(),
+ SVB.makeIntVal(-1, IsRetLongTy ? C.getASTContext().LongTy
+ : C.getASTContext().IntTy));
// This function does not affect the stream state.
// Still we add success and failure state with the appropriate return value.
diff --git a/clang/test/Analysis/stream-errno.c
b/clang/test/Analysis/stream-errno.c
index bf0a61db2424f9..62befbc78e5a75 100644
--- a/clang/test/Analysis/stream-errno.c
+++ b/clang/test/Analysis/stream-errno.c
@@ -216,9 +216,10 @@ void check_fileno(void) {
int N = fileno(F);
if (N == -1) {
clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
-if (errno) {} // no-warning
-fclose(F);
-return;
+if (errno) {}// no-warning
+ } else {
+clang_analyzer_eval(N >= 0); // expected-warning{{TRUE}}
}
if (errno) {} // expected-warning{{An undefined value may be read from
'errno'}}
+ fclose(F);
}
diff --git a/clang/test/Analysis/stream-noopen.c
b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..198b5dc45e35da 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -129,6 +129,18 @@ void check_ftell(FILE *F) {
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void check_fileno(FILE *F) {
+ int Ret = fileno(F);
+ clang_analyzer_eval(F != NULL);// expected-warning{{TRUE}}
+ if (!(Ret >= 0)) {
+clang_analyzer_eval(Ret == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+if (errno) {}// no-warning
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(ferror(F));// expected-warning{{UNKNOWN}}
+}
+
void test_rewind(FILE *F) {
errno = 0;
rewind(F);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
