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@-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@-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@-1 {{'fdopen' is successful}} if (!F) // expected-note@-1 {{'F' is non-null}} // expected-note@-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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits