Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/83...@github.com>
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/83281 >From a9419bc3ffa3d383a9373f0a116795162632dd40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?= <alejandro.alva...@sonarsource.com> Date: Wed, 28 Feb 2024 16:49:35 +0100 Subject: [PATCH 1/2] Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)" This reverts commit 570bc5d291f92e19f6264262b02ddff1a2f2e09b --- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 34 ++++++++++++--- ...ystem-header-simulator-for-simple-stream.h | 2 +- .../system-header-simulator-for-valist.h | 6 +++ .../Analysis/Inputs/system-header-simulator.h | 3 ++ clang/test/Analysis/stream-invalidate.c | 42 +++++++++++++++++++ clang/test/Analysis/stream.c | 39 ++++++++++++++++- 6 files changed, 119 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 65bdc4cac30940..f9928e1325c53d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -348,18 +348,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}}, @@ -419,6 +431,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; @@ -548,7 +562,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; } @@ -600,6 +615,11 @@ 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, @@ -652,6 +672,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) @@ -1038,10 +1059,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) >From 550c55930d1d0bd11a09a78caf5feba063ede33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?= <alejandro.alva...@sonarsource.com> Date: Wed, 28 Feb 2024 16:51:16 +0100 Subject: [PATCH 2/2] Fix formatting --- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index f9928e1325c53d..f2f7a0f83b5c70 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -616,8 +616,7 @@ class StreamChecker : public Checker<check::PreCall, eval::Call, } void initVaListType(CheckerContext &C) const { - VaListType = - C.getASTContext().getBuiltinVaListType().getCanonicalType(); + VaListType = C.getASTContext().getBuiltinVaListType().getCanonicalType(); } /// Searches for the ExplodedNode where the file descriptor was acquired for _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits