Author: Alejandro Álvarez Ayllón Date: 2024-03-06T12:10:07+01:00 New Revision: 239312e49b2fdf35e23d75197a621ee6087743ee
URL: https://github.com/llvm/llvm-project/commit/239312e49b2fdf35e23d75197a621ee6087743ee DIFF: https://github.com/llvm/llvm-project/commit/239312e49b2fdf35e23d75197a621ee6087743ee.diff LOG: Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf" (#83281) `va_list` is a platform-specific type. On some, it is a struct instead of a pointer to a struct, so `lookupFn` was ignoring calls to `vfprintf` and `vfscanf`. `stream.c` now runs in four different platforms to make sure the logic works across targets. Added: Modified: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h clang/test/Analysis/Inputs/system-header-simulator-for-valist.h clang/test/Analysis/Inputs/system-header-simulator.h clang/test/Analysis/stream-invalidate.c clang/test/Analysis/stream.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 0208f94e1b5a2a..2ec47bf55df76b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -318,18 +318,30 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, {{{"fgets"}, 3}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, false), 2}}, + {{{"getc"}, 1}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + std::bind(&StreamChecker::evalFgetx, _1, _2, _3, _4, true), 0}}, {{{"fputc"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}}, {{{"fputs"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, false), 1}}, + {{{"putc"}, 2}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + std::bind(&StreamChecker::evalFputx, _1, _2, _3, _4, true), 1}}, {{{"fprintf"}}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}}, + {{{"vfprintf"}, 3}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), + std::bind(&StreamChecker::evalFprintf, _1, _2, _3, _4), 0}}, {{{"fscanf"}}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}}, + {{{"vfscanf"}, 3}, + {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, true), + std::bind(&StreamChecker::evalFscanf, _1, _2, _3, _4), 0}}, {{{"ungetc"}, 2}, {std::bind(&StreamChecker::preReadWrite, _1, _2, _3, _4, false), std::bind(&StreamChecker::evalUngetc, _1, _2, _3, _4), 1}}, @@ -389,6 +401,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, mutable int SeekCurVal = 1; /// Expanded value of SEEK_END, 2 if not found. mutable int SeekEndVal = 2; + /// The built-in va_list type is platform-specific + mutable QualType VaListType; void evalFopen(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const; @@ -518,7 +532,8 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, return nullptr; for (auto *P : Call.parameters()) { QualType T = P->getType(); - if (!T->isIntegralOrEnumerationType() && !T->isPointerType()) + if (!T->isIntegralOrEnumerationType() && !T->isPointerType() && + T.getCanonicalType() != VaListType) return nullptr; } @@ -557,6 +572,10 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, SeekCurVal = *OptInt; } + void initVaListType(CheckerContext &C) const { + VaListType = C.getASTContext().getBuiltinVaListType().getCanonicalType(); + } + /// Searches for the ExplodedNode where the file descriptor was acquired for /// StreamSym. static const ExplodedNode *getAcquisitionSite(const ExplodedNode *N, @@ -705,6 +724,7 @@ static ProgramStateRef escapeArgs(ProgramStateRef State, CheckerContext &C, void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { initMacroValues(C); + initVaListType(C); const FnDescription *Desc = lookupFn(Call); if (!Desc || !Desc->PreFn) @@ -1085,10 +1105,13 @@ void StreamChecker::evalFscanf(const FnDescription *Desc, const CallEvent &Call, if (!StateNotFailed) return; - SmallVector<unsigned int> EscArgs; - for (auto EscArg : llvm::seq(2u, Call.getNumArgs())) - EscArgs.push_back(EscArg); - StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs); + if (auto const *Callee = Call.getCalleeIdentifier(); + !Callee || !Callee->getName().equals("vfscanf")) { + SmallVector<unsigned int> EscArgs; + for (auto EscArg : llvm::seq(2u, Call.getNumArgs())) + EscArgs.push_back(EscArg); + StateNotFailed = escapeArgs(StateNotFailed, C, Call, EscArgs); + } if (StateNotFailed) C.addTransition(StateNotFailed); diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h index 098a2208fecbe9..c26d3582149120 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h @@ -5,7 +5,7 @@ // suppressed. #pragma clang system_header -typedef struct __sFILE { +typedef struct _FILE { unsigned char *_p; } FILE; FILE *fopen(const char *restrict, const char *restrict) __asm("_" "fopen" ); diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h index 7299b61353d460..720944abb8ad47 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h @@ -10,6 +10,8 @@ #define restrict /*restrict*/ #endif +typedef struct _FILE FILE; + typedef __builtin_va_list va_list; #define va_start(ap, param) __builtin_va_start(ap, param) @@ -21,6 +23,10 @@ int vprintf (const char *restrict format, va_list arg); int vsprintf (char *restrict s, const char *restrict format, va_list arg); +int vfprintf(FILE *stream, const char *format, va_list ap); + +int vfscanf(FILE *stream, const char *format, va_list ap); + int some_library_function(int n, va_list arg); // No warning from system header. diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h index 15986984802c0e..8fd51449ecc0a4 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator.h +++ b/clang/test/Analysis/Inputs/system-header-simulator.h @@ -73,6 +73,9 @@ int ferror(FILE *stream); int fileno(FILE *stream); int fflush(FILE *stream); + +int getc(FILE *stream); + size_t strlen(const char *); char *strcpy(char *restrict, const char *restrict); diff --git a/clang/test/Analysis/stream-invalidate.c b/clang/test/Analysis/stream-invalidate.c index 6745d11a2fe701..5046a356d0583d 100644 --- a/clang/test/Analysis/stream-invalidate.c +++ b/clang/test/Analysis/stream-invalidate.c @@ -4,6 +4,7 @@ // RUN: -analyzer-checker=debug.ExprInspection #include "Inputs/system-header-simulator.h" +#include "Inputs/system-header-simulator-for-valist.h" void clang_analyzer_eval(int); void clang_analyzer_dump(int); @@ -145,3 +146,44 @@ void test_fgetpos() { fclose(F); } + +void test_fprintf() { + FILE *F1 = tmpfile(); + if (!F1) + return; + + unsigned a = 42; + char *output = "HELLO"; + int r = fprintf(F1, "%s\t%u\n", output, a); + // fprintf does not invalidate any of its input + // 69 is ascii for 'E' + clang_analyzer_dump(a); // expected-warning {{42 S32b}} + clang_analyzer_dump(output[1]); // expected-warning {{69 S32b}} + fclose(F1); +} + +int test_vfscanf_inner(const char *fmt, ...) { + FILE *F1 = tmpfile(); + if (!F1) + return EOF; + + va_list ap; + va_start(ap, fmt); + + int r = vfscanf(F1, fmt, ap); + + fclose(F1); + va_end(ap); + return r; +} + +void test_vfscanf() { + int i = 42; + int j = 43; + int r = test_vfscanf_inner("%d", &i); + if (r != EOF) { + // i gets invalidated by the call to test_vfscanf_inner, not by vfscanf. + clang_analyzer_dump(i); // expected-warning {{conj_$}} + clang_analyzer_dump(j); // expected-warning {{43 S32b}} + } +} diff --git a/clang/test/Analysis/stream.c b/clang/test/Analysis/stream.c index 378c9154f8f6a8..7ba27740a93796 100644 --- a/clang/test/Analysis/stream.c +++ b/clang/test/Analysis/stream.c @@ -1,6 +1,10 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -triple=x86_64-pc-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -triple=armv8-none-linux-eabi -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -triple=aarch64-linux-gnu -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -triple=hexagon -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s #include "Inputs/system-header-simulator.h" +#include "Inputs/system-header-simulator-for-valist.h" void clang_analyzer_eval(int); @@ -65,12 +69,24 @@ void check_fseek(void) { fclose(fp); } +void check_fseeko(void) { + FILE *fp = tmpfile(); + fseeko(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}} + fclose(fp); +} + void check_ftell(void) { FILE *fp = tmpfile(); ftell(fp); // expected-warning {{Stream pointer might be NULL}} fclose(fp); } +void check_ftello(void) { + FILE *fp = tmpfile(); + ftello(fp); // expected-warning {{Stream pointer might be NULL}} + fclose(fp); +} + void check_rewind(void) { FILE *fp = tmpfile(); rewind(fp); // expected-warning {{Stream pointer might be NULL}} @@ -129,6 +145,18 @@ void f_dopen(int fd) { fclose(F); } +void f_vfprintf(int fd, va_list args) { + FILE *F = fdopen(fd, "r"); + vfprintf(F, "%d", args); // expected-warning {{Stream pointer might be NULL}} + fclose(F); +} + +void f_vfscanf(int fd, va_list args) { + FILE *F = fdopen(fd, "r"); + vfscanf(F, "%u", args); // expected-warning {{Stream pointer might be NULL}} + fclose(F); +} + void f_seek(void) { FILE *p = fopen("foo", "r"); if (!p) @@ -138,6 +166,15 @@ void f_seek(void) { fclose(p); } +void f_seeko(void) { + FILE *p = fopen("foo", "r"); + if (!p) + return; + fseeko(p, 1, SEEK_SET); // no-warning + fseeko(p, 1, 3); // expected-warning {{The whence argument to fseek() should be SEEK_SET, SEEK_END, or SEEK_CUR}} + fclose(p); +} + void f_double_close(void) { FILE *p = fopen("foo", "r"); if (!p) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
