balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a project: clang.
Recognization of function names is done now with the CallDescription
class instead of using IdentifierInfo. This means function name and
argument count is compared too.
A new check for filtering not global-C-functions was added.
Test was updated.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D67706
Files:
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream.c
Index: clang/test/Analysis/stream.c
===================================================================
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,6 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
typedef __typeof__(sizeof(int)) size_t;
+typedef __typeof__(sizeof(int)) fpos_t;
typedef struct _IO_FILE FILE;
#define SEEK_SET 0 /* Seek from beginning of file. */
#define SEEK_CUR 1 /* Seek from current position. */
@@ -9,36 +10,93 @@
extern FILE *tmpfile(void);
extern int fclose(FILE *fp);
extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
extern int fseek (FILE *__stream, long int __off, int __whence);
extern long int ftell (FILE *__stream);
extern void rewind (FILE *__stream);
+extern int fgetpos(FILE *stream, fpos_t *pos);
+extern int fsetpos(FILE *stream, const fpos_t *pos);
+extern void clearerr(FILE *stream);
+extern int feof(FILE *stream);
+extern int ferror(FILE *stream);
+extern int fileno(FILE *stream);
-void f1(void) {
- FILE *p = fopen("foo", "r");
- char buf[1024];
- fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
- fclose(p);
+void check_fread() {
+ FILE *fp = tmpfile();
+ fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
}
-void f2(void) {
- FILE *p = fopen("foo", "r");
- fseek(p, 1, SEEK_SET); // expected-warning {{Stream pointer might be NULL}}
- fclose(p);
+void check_fwrite() {
+ FILE *fp = tmpfile();
+ fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
}
-void f3(void) {
- FILE *p = fopen("foo", "r");
- ftell(p); // expected-warning {{Stream pointer might be NULL}}
- fclose(p);
+void check_fseek() {
+ FILE *fp = tmpfile();
+ fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_ftell() {
+ FILE *fp = tmpfile();
+ ftell(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_rewind() {
+ FILE *fp = tmpfile();
+ rewind(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_fgetpos() {
+ FILE *fp = tmpfile();
+ fpos_t pos;
+ fgetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_fsetpos() {
+ FILE *fp = tmpfile();
+ fpos_t pos;
+ fsetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_clearerr() {
+ FILE *fp = tmpfile();
+ clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_feof() {
+ FILE *fp = tmpfile();
+ feof(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void check_ferror() {
+ FILE *fp = tmpfile();
+ ferror(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
}
-void f4(void) {
+void check_fileno() {
+ FILE *fp = tmpfile();
+ fileno(fp); // expected-warning {{Stream pointer might be NULL}}
+ fclose(fp);
+}
+
+void f_open(void) {
FILE *p = fopen("foo", "r");
- rewind(p); // expected-warning {{Stream pointer might be NULL}}
+ char buf[1024];
+ fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
fclose(p);
}
-void f5(void) {
+void f_seek(void) {
FILE *p = fopen("foo", "r");
if (!p)
return;
@@ -47,26 +105,20 @@
fclose(p);
}
-void f6(void) {
+void f_double_close(void) {
FILE *p = fopen("foo", "r");
fclose(p);
fclose(p); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
}
-void f7(void) {
- FILE *p = tmpfile();
- ftell(p); // expected-warning {{Stream pointer might be NULL}}
- fclose(p);
-}
-
-void f8(int c) {
+void f_leak(int c) {
FILE *p = fopen("foo.c", "r");
if(c)
return; // expected-warning {{Opened File never closed. Potential Resource leak}}
fclose(p);
}
-FILE *f9(void) {
+FILE *f_null_checked(void) {
FILE *p = fopen("foo.c", "r");
if (p)
return p; // no-warning
@@ -82,4 +134,3 @@
void pr8081(FILE *stream, long offset, int whence) {
fseek(stream, offset, whence);
}
-
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -57,20 +57,19 @@
class StreamChecker : public Checker<eval::Call,
check::DeadSymbols > {
- mutable IdentifierInfo *II_fopen, *II_tmpfile, *II_fclose, *II_fread,
- *II_fwrite,
- *II_fseek, *II_ftell, *II_rewind, *II_fgetpos, *II_fsetpos,
- *II_clearerr, *II_feof, *II_ferror, *II_fileno;
+ CallDescription CD_fopen, CD_tmpfile, CD_fclose, CD_fread, CD_fwrite,
+ CD_fseek, CD_ftell, CD_rewind, CD_fgetpos, CD_fsetpos, CD_clearerr,
+ CD_feof, CD_ferror, CD_fileno;
mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
BT_doubleclose, BT_ResourceLeak;
public:
StreamChecker()
- : II_fopen(nullptr), II_tmpfile(nullptr), II_fclose(nullptr),
- II_fread(nullptr), II_fwrite(nullptr), II_fseek(nullptr),
- II_ftell(nullptr), II_rewind(nullptr), II_fgetpos(nullptr),
- II_fsetpos(nullptr), II_clearerr(nullptr), II_feof(nullptr),
- II_ferror(nullptr), II_fileno(nullptr) {}
+ : CD_fopen("fopen"), CD_tmpfile("tmpfile"), CD_fclose("fclose", 1),
+ CD_fread("fread", 4), CD_fwrite("fwrite", 4), CD_fseek("fseek", 3),
+ CD_ftell("ftell", 1), CD_rewind("rewind", 1), CD_fgetpos("fgetpos", 2),
+ CD_fsetpos("fsetpos", 2), CD_clearerr("clearerr", 1),
+ CD_feof("feof", 1), CD_ferror("ferror", 1), CD_fileno("fileno", 1) {}
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -105,6 +104,9 @@
bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+ return false;
+
const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
if (!FD || FD->getKind() != Decl::Function)
return false;
@@ -113,89 +115,59 @@
if (!CE)
return false;
- ASTContext &Ctx = C.getASTContext();
- if (!II_fopen)
- II_fopen = &Ctx.Idents.get("fopen");
- if (!II_tmpfile)
- II_tmpfile = &Ctx.Idents.get("tmpfile");
- if (!II_fclose)
- II_fclose = &Ctx.Idents.get("fclose");
- if (!II_fread)
- II_fread = &Ctx.Idents.get("fread");
- if (!II_fwrite)
- II_fwrite = &Ctx.Idents.get("fwrite");
- if (!II_fseek)
- II_fseek = &Ctx.Idents.get("fseek");
- if (!II_ftell)
- II_ftell = &Ctx.Idents.get("ftell");
- if (!II_rewind)
- II_rewind = &Ctx.Idents.get("rewind");
- if (!II_fgetpos)
- II_fgetpos = &Ctx.Idents.get("fgetpos");
- if (!II_fsetpos)
- II_fsetpos = &Ctx.Idents.get("fsetpos");
- if (!II_clearerr)
- II_clearerr = &Ctx.Idents.get("clearerr");
- if (!II_feof)
- II_feof = &Ctx.Idents.get("feof");
- if (!II_ferror)
- II_ferror = &Ctx.Idents.get("ferror");
- if (!II_fileno)
- II_fileno = &Ctx.Idents.get("fileno");
-
- if (FD->getIdentifier() == II_fopen) {
+ if (Call.isCalled(CD_fopen)) {
Fopen(C, CE);
return true;
}
- if (FD->getIdentifier() == II_tmpfile) {
+ if (Call.isCalled(CD_tmpfile)) {
Tmpfile(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fclose) {
+ if (Call.isCalled(CD_fclose)) {
Fclose(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fread) {
+ if (Call.isCalled(CD_fread)) {
Fread(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fwrite) {
+ if (Call.isCalled(CD_fwrite)) {
Fwrite(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fseek) {
+ if (Call.isCalled(CD_fseek)) {
Fseek(C, CE);
return true;
}
- if (FD->getIdentifier() == II_ftell) {
+ if (Call.isCalled(CD_ftell)) {
Ftell(C, CE);
return true;
}
- if (FD->getIdentifier() == II_rewind) {
+ if (Call.isCalled(CD_rewind)) {
Rewind(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fgetpos) {
+ if (Call.isCalled(CD_fgetpos)) {
Fgetpos(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fsetpos) {
+ if (Call.isCalled(CD_fsetpos)) {
Fsetpos(C, CE);
return true;
}
- if (FD->getIdentifier() == II_clearerr) {
+ if (Call.isCalled(CD_clearerr)) {
Clearerr(C, CE);
return true;
}
- if (FD->getIdentifier() == II_feof) {
+ if (Call.isCalled(CD_feof)) {
Feof(C, CE);
return true;
}
- if (FD->getIdentifier() == II_ferror) {
+ if (Call.isCalled(CD_ferror)) {
Ferror(C, CE);
return true;
}
- if (FD->getIdentifier() == II_fileno) {
+ if (Call.isCalled(CD_fileno)) {
Fileno(C, CE);
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
