[clang] [clang][analyzer] Add missing stream related functions to StdCLibraryFunctionsChecker. (PR #76979)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 4 09:23:16 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balázs Kéri (balazske)
<details>
<summary>Changes</summary>
Some stream functions were recently added to `StreamChecker` that were not modeled by `StdCLibraryFunctionsChecker`. To ensure consistency these functions are added to the other checker too.
Some of the related tests are re-organized.
---
Full diff: https://github.com/llvm/llvm-project/pull/76979.diff
6 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (+71-17)
- (modified) clang/test/Analysis/std-c-library-functions.c (+2-2)
- (modified) clang/test/Analysis/stream-error.c (-26)
- (modified) clang/test/Analysis/stream-noopen.c (+68)
- (modified) clang/test/Analysis/stream-note.c (+1)
- (modified) clang/test/Analysis/stream.c (+24-1)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 20068653d530a3..f4bf68c3147fd1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -2023,13 +2023,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
{{EOFv, EOFv}, {0, UCharRangeMax}},
"an unsigned char value or EOF")));
- // The getc() family of functions that returns either a char or an EOF.
- addToFunctionSummaryMap(
- {"getc", "fgetc"}, Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
- Summary(NoEvalCall)
- .Case({ReturnValueCondition(WithinRange,
- {{EOFv, EOFv}, {0, UCharRangeMax}})},
- ErrnoIrrelevant));
addToFunctionSummaryMap(
"getchar", Signature(ArgTypes{}, RetType{IntTy}),
Summary(NoEvalCall)
@@ -2139,7 +2132,17 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
std::move(GetenvSummary));
}
- if (ModelPOSIX) {
+ if (!ModelPOSIX) {
+ // Without POSIX use of 'errno' is not specified (in these cases).
+ // Add these functions without 'errno' checks.
+ addToFunctionSummaryMap(
+ {"getc", "fgetc"}, Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(WithinRange,
+ {{EOFv, EOFv}, {0, UCharRangeMax}})},
+ ErrnoIrrelevant)
+ .ArgConstraint(NotNull(ArgNo(0))));
+ } else {
const auto ReturnsZeroOrMinusOne =
ConstraintSet{ReturnValueCondition(WithinRange, Range(-1, 0))};
const auto ReturnsZero =
@@ -2192,6 +2195,16 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.ArgConstraint(NotNull(ArgNo(1)))
.ArgConstraint(NotNull(ArgNo(2))));
+ // FILE *fdopen(int fd, const char *mode);
+ addToFunctionSummaryMap(
+ "fdopen",
+ Signature(ArgTypes{IntTy, ConstCharPtrTy}, RetType{FilePtrTy}),
+ Summary(NoEvalCall)
+ .Case({NotNull(Ret)}, ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
+ .ArgConstraint(NotNull(ArgNo(1))));
+
// int fclose(FILE *stream);
addToFunctionSummaryMap(
"fclose", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
@@ -2201,6 +2214,56 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(0))));
+ // int fgetc(FILE *stream);
+ // 'getc' is the same as 'fgetc' but may be a macro
+ addToFunctionSummaryMap(
+ {"getc", "fgetc"}, Signature(ArgTypes{FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(WithinRange, {{0, UCharRangeMax}})},
+ ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv))},
+ ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(NotNull(ArgNo(0))));
+
+ // int fputc(int c, FILE *stream);
+ // 'putc' is the same as 'fputc' but may be a macro
+ addToFunctionSummaryMap(
+ {"putc", "fputc"},
+ Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(BO_EQ, ArgNo(0))},
+ ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv))},
+ ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(NotNull(ArgNo(1)))
+ .ArgConstraint(
+ ArgumentCondition(0, WithinRange, {{0, UCharRangeMax}})));
+
+ // char *fgets(char *restrict s, int n, FILE *restrict stream);
+ addToFunctionSummaryMap(
+ "fgets",
+ Signature(ArgTypes{CharPtrRestrictTy, IntTy, FilePtrRestrictTy},
+ RetType{CharPtrTy}),
+ Summary(NoEvalCall)
+ .Case({ReturnValueCondition(BO_EQ, ArgNo(0))},
+ ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({IsNull(Ret)}, ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(ArgumentCondition(1, WithinRange, Range(0, IntMax)))
+ .ArgConstraint(NotNull(ArgNo(2))));
+
+ // int fputs(const char *restrict s, FILE *restrict stream);
+ addToFunctionSummaryMap(
+ "fputs",
+ Signature(ArgTypes{ConstCharPtrRestrictTy, FilePtrRestrictTy},
+ RetType{IntTy}),
+ Summary(NoEvalCall)
+ .Case(ReturnsNonnegative, ErrnoMustNotBeChecked, GenericSuccessMsg)
+ .Case({ReturnValueCondition(WithinRange, SingleValue(EOFv))},
+ ErrnoNEZeroIrrelevant, GenericFailureMsg)
+ .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(NotNull(ArgNo(1))));
+
// int fseek(FILE *stream, long offset, int whence);
// FIXME: It can be possible to get the 'SEEK_' values (like EOFv) and use
// these for condition of arg 2.
@@ -2800,15 +2863,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
"pathconf", Signature(ArgTypes{ConstCharPtrTy, IntTy}, RetType{LongTy}),
Summary(NoEvalCall).ArgConstraint(NotNull(ArgNo(0))));
- // FILE *fdopen(int fd, const char *mode);
- // FIXME: Improve for errno modeling.
- addToFunctionSummaryMap(
- "fdopen",
- Signature(ArgTypes{IntTy, ConstCharPtrTy}, RetType{FilePtrTy}),
- Summary(NoEvalCall)
- .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))
- .ArgConstraint(NotNull(ArgNo(1))));
-
// void rewinddir(DIR *dir);
addToFunctionSummaryMap(
"rewinddir", Signature(ArgTypes{DirPtrTy}, RetType{VoidTy}),
diff --git a/clang/test/Analysis/std-c-library-functions.c b/clang/test/Analysis/std-c-library-functions.c
index b7eb6b284460e5..e6564e2bae7611 100644
--- a/clang/test/Analysis/std-c-library-functions.c
+++ b/clang/test/Analysis/std-c-library-functions.c
@@ -53,8 +53,6 @@
// CHECK-NEXT: Loaded summary for: int toupper(int)
// CHECK-NEXT: Loaded summary for: int tolower(int)
// CHECK-NEXT: Loaded summary for: int toascii(int)
-// CHECK-NEXT: Loaded summary for: int getc(FILE *)
-// CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
// CHECK-NEXT: Loaded summary for: int getchar(void)
// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
@@ -63,6 +61,8 @@
// CHECK-NEXT: Loaded summary for: ssize_t getline(char **restrict, size_t *restrict, FILE *restrict)
// CHECK-NEXT: Loaded summary for: ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict)
// CHECK-NEXT: Loaded summary for: char *getenv(const char *)
+// CHECK-NEXT: Loaded summary for: int getc(FILE *)
+// CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
#include "Inputs/std-c-library-functions.h"
diff --git a/clang/test/Analysis/stream-error.c b/clang/test/Analysis/stream-error.c
index 13c6684b5840af..15e9840295fba8 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -308,32 +308,6 @@ 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
- fclose(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.
diff --git a/clang/test/Analysis/stream-noopen.c b/clang/test/Analysis/stream-noopen.c
index 2daf640c18a1d4..3c58baa0a841a2 100644
--- a/clang/test/Analysis/stream-noopen.c
+++ b/clang/test/Analysis/stream-noopen.c
@@ -57,6 +57,60 @@ void test_fwrite(FILE *F) {
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
}
+void test_fgetc(FILE *F) {
+ int Ret = fgetc(F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ if (Ret != EOF) {
+ if (errno) {} // expected-warning {{undefined}}
+ } else {
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fputc(FILE *F) {
+ int Ret = fputc('a', F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ if (Ret != EOF) {
+ clang_analyzer_eval(Ret == 'a'); // expected-warning {{TRUE}}
+ if (errno) {} // expected-warning {{undefined}}
+ } else {
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fgets(char *Buf, int N, FILE *F) {
+ char *Ret = fgets(Buf, N, F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ clang_analyzer_eval(Buf != NULL); // expected-warning {{TRUE}}
+ clang_analyzer_eval(N >= 0); // expected-warning {{TRUE}}
+ if (Ret == Buf) {
+ if (errno) {} // expected-warning {{undefined}}
+ } else {
+ clang_analyzer_eval(Ret == 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void test_fputs(char *Buf, FILE *F) {
+ int Ret = fputs(Buf, F);
+ clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
+ clang_analyzer_eval(Buf != NULL); // expected-warning {{TRUE}}
+ if (Ret >= 0) {
+ if (errno) {} // expected-warning {{undefined}}
+ } else {
+ clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
+ clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
+ }
+ clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
void test_fclose(FILE *F) {
int Ret = fclose(F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
@@ -138,6 +192,20 @@ void test_rewind(FILE *F) {
rewind(F);
}
+void test_fflush(FILE *F) {
+ errno = 0;
+ int Ret = fflush(F);
+ clang_analyzer_eval(F != NULL); // expected-warning{{TRUE}}
+ // expected-warning at -1{{FALSE}}
+ if (Ret == EOF) {
+ clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+ } else {
+ clang_analyzer_eval(Ret == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning at -1{{FALSE}}
+ }
+}
+
void test_feof(FILE *F) {
errno = 0;
feof(F);
diff --git a/clang/test/Analysis/stream-note.c b/clang/test/Analysis/stream-note.c
index e412015eb68393..abb4784c078aa8 100644
--- a/clang/test/Analysis/stream-note.c
+++ b/clang/test/Analysis/stream-note.c
@@ -56,6 +56,7 @@ void check_note_freopen(void) {
void check_note_fdopen(int fd) {
FILE *F = fdopen(fd, "r"); // expected-note {{Stream opened here}}
+ // stdargs-note at -1 {{'fdopen' is successful}}
if (!F)
// expected-note at -1 {{'F' is non-null}}
// expected-note at -2 {{Taking false branch}}
diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c
index 060d561c1fe1c2..40a2d9b98754be 100644
--- a/clang/test/Analysis/stream.c
+++ b/clang/test/Analysis/stream.c
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
#include "Inputs/system-header-simulator.h"
+void clang_analyzer_eval(int);
+
void check_fread(void) {
FILE *fp = tmpfile();
fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
@@ -299,3 +301,24 @@ void check_leak_noreturn_2(void) {
} // expected-warning {{Opened stream never closed. Potential resource leak}}
// FIXME: This warning should be placed at the `return` above.
// See https://reviews.llvm.org/D83120 about details.
+
+void 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 fflush_on_open_failed_stream(void) {
+ FILE *F = tmpfile();
+ if (!F) {
+ fflush(F); // no-warning
+ return;
+ }
+ fclose(F);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/76979
More information about the cfe-commits
mailing list