Skip to content

Commit a9419bc

Browse files
Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)"
This reverts commit 570bc5d
1 parent 680c780 commit a9419bc

File tree

6 files changed

+119
-7
lines changed

6 files changed

+119
-7
lines changed

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -348,18 +348,30 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
348348
{{{"fgets"}, 3},
349349
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
350350
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}},
351+
{{{"getc"}, 1},
352+
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
353+
std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}},
351354
{{{"fputc"}, 2},
352355
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
353356
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
354357
{{{"fputs"}, 2},
355358
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
356359
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}},
360+
{{{"putc"}, 2},
361+
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
362+
std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}},
357363
{{{"fprintf"}},
358364
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
359365
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
366+
{{{"vfprintf"}, 3},
367+
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
368+
std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}},
360369
{{{"fscanf"}},
361370
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
362371
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
372+
{{{"vfscanf"}, 3},
373+
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true),
374+
std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}},
363375
{{{"ungetc"}, 2},
364376
{std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false),
365377
std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}},
@@ -419,6 +431,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
419431
mutable int SeekCurVal = 1;
420432
/// Expanded value of SEEK_END, 2 if not found.
421433
mutable int SeekEndVal = 2;
434+
/// The built-in va_list type is platform-specific
435+
mutable QualType VaListType;
422436

423437
void evalFopen(const FnDescription *Desc, const CallEvent &Call,
424438
CheckerContext &C) const;
@@ -548,7 +562,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
548562
return nullptr;
549563
for (auto *P : Call.parameters()) {
550564
QualType T = P->getType();
551-
if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
565+
if (!T->isIntegralOrEnumerationType() && !T->isPointerType() &&
566+
T.getCanonicalType() != VaListType)
552567
return nullptr;
553568
}
554569

@@ -600,6 +615,11 @@ class StreamChecker : public Checker<check::PreCall, eval::Call,
600615
SeekCurVal = *OptInt;
601616
}
602617

618+
void initVaListType(CheckerContext &C) const {
619+
VaListType =
620+
C.getASTContext().getBuiltinVaListType().getCanonicalType();
621+
}
622+
603623
/// Searches for the ExplodedNode where the file descriptor was acquired for
604624
/// StreamSym.
605625
static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N,
@@ -652,6 +672,7 @@ static ProgramStateRef escapeArgs(ProgramStateRef State, CheckerContext &C,
652672
void StreamChecker::checkPreCall(const CallEvent &Call,
653673
CheckerContext &C) const {
654674
initMacroValues(C);
675+
initVaListType(C);
655676

656677
const FnDescription *Desc = lookupFn(Call);
657678
if (!Desc || !Desc->PreFn)
@@ -1038,10 +1059,13 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call,
10381059
if (!StateNotFailed)
10391060
return;
10401061

1041-
SmallVector<unsigned int> EscArgs;
1042-
for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
1043-
EscArgs.push_back(EscArg);
1044-
StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
1062+
if (auto const *Callee = Call.getCalleeIdentifier();
1063+
!Callee || !Callee->getName().equals("vfscanf")) {
1064+
SmallVector<unsigned int> EscArgs;
1065+
for (auto EscArg : llvm::seq(2u, Call.getNumArgs()))
1066+
EscArgs.push_back(EscArg);
1067+
StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs);
1068+
}
10451069

10461070
if (StateNotFailed)
10471071
C.addTransition(StateNotFailed);

clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// suppressed.
66
#pragma clang system_header
77

8-
typedef struct __sFILE {
8+
typedef struct _FILE {
99
unsigned char *_p;
1010
} FILE;
1111
FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" );

clang/test/Analysis/Inputs/system-header-simulator-for-valist.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
#define restrict /*restrict*/
1111
#endif
1212

13+
typedef struct _FILE FILE;
14+
1315
typedef __builtin_va_list va_list;
1416

1517
#define va_start(ap, param) __builtin_va_start(ap, param)
@@ -21,6 +23,10 @@ int vprintf (const char *restrict format, va_list arg);
2123

2224
int vsprintf (char *restrict s, const char *restrict format, va_list arg);
2325

26+
int vfprintf(FILE *stream, const char *format, va_list ap);
27+
28+
int vfscanf(FILE *stream, const char *format, va_list ap);
29+
2430
int some_library_function(int n, va_list arg);
2531

2632
// No warning from system header.

clang/test/Analysis/Inputs/system-header-simulator.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ int ferror(FILE *stream);
7373
int fileno(FILE *stream);
7474
int fflush(FILE *stream);
7575

76+
77+
int getc(FILE *stream);
78+
7679
size_t strlen(const char *);
7780

7881
char *strcpy(char *restrict, const char *restrict);

clang/test/Analysis/stream-invalidate.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// RUN: -analyzer-checker=debug.ExprInspection
55

66
#include "Inputs/system-header-simulator.h"
7+
#include "Inputs/system-header-simulator-for-valist.h"
78

89
void clang_analyzer_eval(int);
910
void clang_analyzer_dump(int);
@@ -145,3 +146,44 @@ void test_fgetpos() {
145146

146147
fclose(F);
147148
}
149+
150+
void test_fprintf() {
151+
FILE *F1 = tmpfile();
152+
if (!F1)
153+
return;
154+
155+
unsigned a = 42;
156+
char *output = "HELLO";
157+
int r = fprintf(F1, "%s\t%u\n", output, a);
158+
// fprintf does not invalidate any of its input
159+
// 69 is ascii for 'E'
160+
clang_analyzer_dump(a); // expected-warning {{42 S32b}}
161+
clang_analyzer_dump(output[1]); // expected-warning {{69 S32b}}
162+
fclose(F1);
163+
}
164+
165+
int test_vfscanf_inner(const char *fmt, ...) {
166+
FILE *F1 = tmpfile();
167+
if (!F1)
168+
return EOF;
169+
170+
va_list ap;
171+
va_start(ap, fmt);
172+
173+
int r = vfscanf(F1, fmt, ap);
174+
175+
fclose(F1);
176+
va_end(ap);
177+
return r;
178+
}
179+
180+
void test_vfscanf() {
181+
int i = 42;
182+
int j = 43;
183+
int r = test_vfscanf_inner("%d", &i);
184+
if (r != EOF) {
185+
// i gets invalidated by the call to test_vfscanf_inner, not by vfscanf.
186+
clang_analyzer_dump(i); // expected-warning {{conj_$}}
187+
clang_analyzer_dump(j); // expected-warning {{43 S32b}}
188+
}
189+
}

clang/test/Analysis/stream.c

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
1+
// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
2+
// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
3+
// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
4+
// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
25

36
#include "Inputs/system-header-simulator.h"
7+
#include "Inputs/system-header-simulator-for-valist.h"
48

59
void clang_analyzer_eval(int);
610

@@ -65,12 +69,24 @@ void check_fseek(void) {
6569
fclose(fp);
6670
}
6771

72+
void check_fseeko(void) {
73+
FILE *fp = tmpfile();
74+
fseeko(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
75+
fclose(fp);
76+
}
77+
6878
void check_ftell(void) {
6979
FILE *fp = tmpfile();
7080
ftell(fp); // expected-warning {{Stream pointer might be NULL}}
7181
fclose(fp);
7282
}
7383

84+
void check_ftello(void) {
85+
FILE *fp = tmpfile();
86+
ftello(fp); // expected-warning {{Stream pointer might be NULL}}
87+
fclose(fp);
88+
}
89+
7490
void check_rewind(void) {
7591
FILE *fp = tmpfile();
7692
rewind(fp); // expected-warning {{Stream pointer might be NULL}}
@@ -129,6 +145,18 @@ void f_dopen(int fd) {
129145
fclose(F);
130146
}
131147

148+
void f_vfprintf(int fd, va_list args) {
149+
FILE *F = fdopen(fd, "r");
150+
vfprintf(F, "%d", args); // expected-warning {{Stream pointer might be NULL}}
151+
fclose(F);
152+
}
153+
154+
void f_vfscanf(int fd, va_list args) {
155+
FILE *F = fdopen(fd, "r");
156+
vfscanf(F, "%u", args); // expected-warning {{Stream pointer might be NULL}}
157+
fclose(F);
158+
}
159+
132160
void f_seek(void) {
133161
FILE *p = fopen("foo", "r");
134162
if (!p)
@@ -138,6 +166,15 @@ void f_seek(void) {
138166
fclose(p);
139167
}
140168

169+
void f_seeko(void) {
170+
FILE *p = fopen("foo", "r");
171+
if (!p)
172+
return;
173+
fseeko(p, 1, SEEK_SET); // no-warning
174+
fseeko(p, 1, 3); // expected-warning {{The whence argument to fseek() should be SEEK_SET, SEEK_END, or SEEK_CUR}}
175+
fclose(p);
176+
}
177+
141178
void f_double_close(void) {
142179
FILE *p = fopen("foo", "r");
143180
if (!p)

0 commit comments

Comments
 (0)