Author: ziqingluo-90 Date: 2024-09-06T12:57:47-07:00 New Revision: de88d7db7b77141297fbb5638ee1e18d1fba53b8
URL: https://github.com/llvm/llvm-project/commit/de88d7db7b77141297fbb5638ee1e18d1fba53b8 DIFF: https://github.com/llvm/llvm-project/commit/de88d7db7b77141297fbb5638ee1e18d1fba53b8.diff LOG: [-Wunsafe-buffer-usage] Fix a bug in "warning libc functions (#101583)" The commit d7dd2c468fecae871ba67e891a3519c758c94b63 crashes for such an example: ``` void printf() { printf(); } ``` Because it assumes `printf` must have arguments. This commit fixes this issue. (rdar://117182250) Added: Modified: clang/lib/Analysis/UnsafeBufferUsage.cpp clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index ec76eae8b60776..21d4368151eb47 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -14,6 +14,7 @@ #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" +#include "clang/AST/Type.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/SourceLocation.h" @@ -763,32 +764,27 @@ AST_MATCHER(FunctionDecl, isNormalPrintfFunc) { AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg, clang::ast_matchers::internal::Matcher<Expr>, UnsafeStringArgMatcher) { - // Determine what printf it is: - const Expr *FirstArg = Node.getArg(0); - ASTContext &Ctx = Finder->getASTContext(); + // Determine what printf it is by examining formal parameters: + const FunctionDecl *FD = Node.getDirectCallee(); - if (isa<StringLiteral>(FirstArg->IgnoreParenImpCasts())) { - // It is a printf/kprintf. And, the format is a string literal: - bool isKprintf = false; - const Expr *UnsafeArg; + assert(FD && "It should have been checked that FD is non-null."); - if (auto *Callee = Node.getDirectCallee()) - if (auto *II = Callee->getIdentifier()) - isKprintf = II->getName() == "kprintf"; - if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 0, Ctx, isKprintf)) - return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder); - return false; - } + unsigned NumParms = FD->getNumParams(); + + if (NumParms < 1) + return false; // possibly some user-defined printf function - QualType PtrTy = FirstArg->getType(); + ASTContext &Ctx = Finder->getASTContext(); + QualType FristParmTy = FD->getParamDecl(0)->getType(); - assert(PtrTy->isPointerType()); + if (!FristParmTy->isPointerType()) + return false; // possibly some user-defined printf function - QualType PteTy = (cast<PointerType>(PtrTy))->getPointeeType(); + QualType FirstPteTy = (cast<PointerType>(FristParmTy))->getPointeeType(); - if (!Ctx.getFILEType().isNull() /* If `FILE *` is not ever in the ASTContext, - there can't be any file pointer then */ - && PteTy.getCanonicalType() == Ctx.getFILEType().getCanonicalType()) { + if (!Ctx.getFILEType() + .isNull() && //`FILE *` must be in the context if it is fprintf + FirstPteTy.getCanonicalType() == Ctx.getFILEType().getCanonicalType()) { // It is a fprintf: const Expr *UnsafeArg; @@ -797,17 +793,32 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg, return false; } - const Expr *SecondArg = Node.getArg(1); - - if (SecondArg->getType()->isIntegerType()) { - // It is a snprintf: + if (FirstPteTy.isConstQualified()) { + // If the first parameter is a `const char *`, it is a printf/kprintf: + bool isKprintf = false; const Expr *UnsafeArg; - if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false)) + if (auto *II = FD->getIdentifier()) + isKprintf = II->getName() == "kprintf"; + if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 0, Ctx, isKprintf)) return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder); return false; } - // It is printf but the format string is passed by pointer. The only thing we + + if (NumParms > 2) { + QualType SecondParmTy = FD->getParamDecl(1)->getType(); + + if (!FirstPteTy.isConstQualified() && SecondParmTy->isIntegerType()) { + // If the first parameter type is non-const qualified `char *` and the + // second is an integer, it is a snprintf: + const Expr *UnsafeArg; + + if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false)) + return UnsafeStringArgMatcher.matches(*UnsafeArg, Finder, Builder); + return false; + } + } + // We don't really recognize this "normal" printf, the only thing we // can do is to require all pointers to be null-terminated: for (auto Arg : Node.arguments()) if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) @@ -826,12 +837,23 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg, // size:= DRE.size()/DRE.size_bytes() // And DRE is a hardened container or view. AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) { - if (Node.getNumArgs() < 3) - return false; // not an snprintf call + const FunctionDecl *FD = Node.getDirectCallee(); + + assert(FD && "It should have been checked that FD is non-null."); + + if (FD->getNumParams() < 3) + return false; // Not an snprint + + QualType FirstParmTy = FD->getParamDecl(0)->getType(); + + if (!FirstParmTy->isPointerType()) + return false; // Not an snprint + QualType FirstPteTy = cast<PointerType>(FirstParmTy)->getPointeeType(); const Expr *Buf = Node.getArg(0), *Size = Node.getArg(1); - if (!Buf->getType()->isPointerType() || !Size->getType()->isIntegerType()) + if (FirstPteTy.isConstQualified() || !Buf->getType()->isPointerType() || + !Size->getType()->isIntegerType()) return false; // not an snprintf call static StringRef SizedObjs[] = {"span", "array", "vector", diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp index 0438f71b1c7922..a3716073609f6f 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp @@ -20,6 +20,7 @@ int snwprintf_s( char* buffer, unsigned buf_size, const char* format, ... ); int vsnprintf( char* buffer, unsigned buf_size, const char* format, ... ); int sscanf_s(const char * buffer, const char * format, ...); int sscanf(const char * buffer, const char * format, ... ); +int __asan_printf(); namespace std { template< class InputIt, class OutputIt > @@ -64,7 +65,6 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) { strcpy_s(); // expected-warning{{function 'strcpy_s' is unsafe}} wcscpy_s(); // expected-warning{{function 'wcscpy_s' is unsafe}} - /* Test printfs */ fprintf((FILE*)p, "%s%d", p, *p); // expected-warning{{function 'fprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} printf("%s%d", // expected-warning{{function 'printf' is unsafe}} @@ -90,6 +90,7 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) { snwprintf(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn snwprintf_s(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn strlen("hello");// no warn + __asan_printf();// a printf but no argument, so no warn } void v(std::string s1, int *p) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
