[clang] de88d7d - [-Wunsafe-buffer-usage] Fix a bug in "warning libc functions (#101583)"
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 6 12:58:13 PDT 2024
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) {
More information about the cfe-commits
mailing list