Skip to content

Commit

Permalink
[clang][analyzer] Add missing stream related functions to StdLibraryF…
Browse files Browse the repository at this point in the history 
…unctionsChecker. (llvm#76979)

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.
  • Loading branch information
@balazske
balazske authored Feb 8, 2024
1 parent 49ee2ff commit b85fe40
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 64 deletions.
79 changes: 68 additions & 11 deletions clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -2231,6 +2234,63 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(0))));

std::optional<QualType> Off_tTy = lookupTy("off_t");
std::optional<RangeInt> Off_tMax = getMaxValue(Off_tTy);

// 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))},
ErrnoIrrelevant, 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({ArgumentCondition(0, WithinRange, Range(0, UCharRangeMax)),
ReturnValueCondition(BO_EQ, ArgNo(0))},
ErrnoMustNotBeChecked, GenericSuccessMsg)
.Case({ArgumentCondition(0, OutOfRange, Range(0, UCharRangeMax)),
ReturnValueCondition(WithinRange, Range(0, UCharRangeMax))},
ErrnoMustNotBeChecked, GenericSuccessMsg)
.Case({ReturnValueCondition(WithinRange, SingleValue(EOFv))},
ErrnoNEZeroIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(1))));

// 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)}, ErrnoIrrelevant, GenericFailureMsg)
.ArgConstraint(NotNull(ArgNo(0)))
.ArgConstraint(ArgumentCondition(1, WithinRange, Range(0, IntMax)))
.ArgConstraint(
BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1)))
.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 ungetc(int c, FILE *stream);
addToFunctionSummaryMap(
"ungetc", Signature(ArgTypes{IntTy, FilePtrTy}, RetType{IntTy}),
Expand All @@ -2250,9 +2310,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
0, WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}}))
.ArgConstraint(NotNull(ArgNo(1))));

std::optional<QualType> Off_tTy = lookupTy("off_t");
std::optional<RangeInt> Off_tMax = getMaxValue(Off_tTy);

// 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.
Expand Down
15 changes: 14 additions & 1 deletion clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ typedef unsigned long int pthread_t;
typedef unsigned long time_t;
typedef unsigned long clockid_t;
typedef __INT64_TYPE__ off64_t;
typedef __INT64_TYPE__ fpos_t;

typedef struct {
int a;
Expand Down Expand Up @@ -42,9 +43,22 @@ FILE *fopen(const char *restrict pathname, const char *restrict mode);
FILE *tmpfile(void);
FILE *freopen(const char *restrict pathname, const char *restrict mode,
FILE *restrict stream);
FILE *fdopen(int fd, const char *mode);
int fclose(FILE *stream);
int putc(int c, FILE *stream);
int fputc(int c, FILE *stream);
char *fgets(char *restrict s, int n, FILE *restrict stream);
int fputs(const char *restrict s, FILE *restrict stream);
int fseek(FILE *stream, long offset, int whence);
int fgetpos(FILE *restrict stream, fpos_t *restrict pos);
int fsetpos(FILE *stream, const fpos_t *pos);
int fflush(FILE *stream);
long ftell(FILE *stream);
int fileno(FILE *stream);
void rewind(FILE *stream);
void clearerr(FILE *stream);
int feof(FILE *stream);
int ferror(FILE *stream);
long a64l(const char *str64);
char *l64a(long value);
int open(const char *path, int oflag, ...);
Expand Down Expand Up @@ -100,7 +114,6 @@ int pclose(FILE *stream);
int close(int fildes);
long fpathconf(int fildes, int name);
long pathconf(const char *path, int name);
FILE *fdopen(int fd, const char *mode);
void rewinddir(DIR *dir);
void seekdir(DIR *dirp, long loc);
int rand_r(unsigned int *seedp);
Expand Down
16 changes: 14 additions & 2 deletions clang/test/Analysis/std-c-library-functions-POSIX.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,22 @@
// CHECK: Loaded summary for: FILE *popen(const char *command, const char *type)
// CHECK: Loaded summary for: int fclose(FILE *stream)
// CHECK: Loaded summary for: int pclose(FILE *stream)
// CHECK: Loaded summary for: int getc(FILE *)
// CHECK: Loaded summary for: int fgetc(FILE *)
// CHECK: Loaded summary for: int putc(int c, FILE *stream)
// CHECK: Loaded summary for: int fputc(int c, FILE *stream)
// CHECK: Loaded summary for: char *fgets(char *restrict s, int n, FILE *restrict stream)
// CHECK: Loaded summary for: int fputs(const char *restrict s, FILE *restrict stream)
// CHECK: Loaded summary for: int fseek(FILE *stream, long offset, int whence)
// CHECK: Loaded summary for: int fseeko(FILE *stream, off_t offset, int whence)
// CHECK: Loaded summary for: off_t ftello(FILE *stream)
// CHECK: Loaded summary for: int fgetpos(FILE *restrict stream, fpos_t *restrict pos)
// CHECK: Loaded summary for: int fsetpos(FILE *stream, const fpos_t *pos)
// CHECK: Loaded summary for: int fflush(FILE *stream)
// CHECK: Loaded summary for: long ftell(FILE *stream)
// CHECK: Loaded summary for: int fileno(FILE *stream)
// CHECK: Loaded summary for: void rewind(FILE *stream)
// CHECK: Loaded summary for: void clearerr(FILE *stream)
// CHECK: Loaded summary for: int feof(FILE *stream)
// CHECK: Loaded summary for: int ferror(FILE *stream)
// CHECK: Loaded summary for: long a64l(const char *str64)
// CHECK: Loaded summary for: char *l64a(long value)
// CHECK: Loaded summary for: int open(const char *path, int oflag, ...)
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Analysis/std-c-library-functions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"

Expand Down
26 changes: 0 additions & 26 deletions clang/test/Analysis/stream-error.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,32 +491,6 @@ void error_ftello(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.
Expand Down
120 changes: 99 additions & 21 deletions clang/test/Analysis/stream-noopen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,95 @@ 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}}
// expected-warning@-1 {{FALSE}}
}
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}}
// expected-warning@-1 {{FALSE}}
}
clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}

char Buf1[10];
Ret = fgets(Buf1, 11, F); // expected-warning {{The 1st argument to 'fgets' is a buffer with size 10}}
}

void test_fgets_bufsize(FILE *F) {
char Buf[10];
fgets(Buf, 11, F); // expected-warning {{The 1st argument to 'fgets' is a buffer with size 10}}
}

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_ungetc(FILE *F) {
int Ret = ungetc('X', F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
if (Ret == 'X') {
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_ungetc_EOF(FILE *F, int C) {
int Ret = ungetc(EOF, F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
Ret = ungetc(C, F);
if (Ret == EOF) {
clang_analyzer_eval(C == EOF); // expected-warning {{TRUE}}
// expected-warning@-1{{FALSE}}
}
}

void test_fclose(FILE *F) {
int Ret = fclose(F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
Expand Down Expand Up @@ -138,28 +227,17 @@ void test_rewind(FILE *F) {
rewind(F);
}

void test_ungetc(FILE *F) {
int Ret = ungetc('X', F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
if (Ret == 'X') {
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_ungetc_EOF(FILE *F, int C) {
int Ret = ungetc(EOF, F);
clang_analyzer_eval(F != NULL); // expected-warning {{TRUE}}
clang_analyzer_eval(Ret == EOF); // expected-warning {{TRUE}}
clang_analyzer_eval(errno != 0); // expected-warning {{TRUE}}
Ret = ungetc(C, 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(C == EOF); // expected-warning {{TRUE}}
// expected-warning@-1{{FALSE}}
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}}
}
}

Expand Down
25 changes: 24 additions & 1 deletion clang/test/Analysis/stream.c
View file
Original file line number Diff line number Diff line change
@@ -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}}
Expand Down Expand Up @@ -316,3 +318,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);
}

0 comments on commit b85fe40

Please sign in to comment.