https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/108308
>From 3c8830a0e69922faf4fad190ba0b2e01a3392e62 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <ziq...@udel.edu> Date: Tue, 6 Aug 2024 17:54:23 -0700 Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Reduce false positives with constant arrays in libc warnings For `snprintf(a, sizeof a, ...)`, the first two arguments form a safe pattern if `a` is a constant array. In such a case, this commit will suppress the warning. (rdar://117182250) --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 27 ++++++++++++++++++- ...arn-unsafe-buffer-usage-libc-functions.cpp | 14 +++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 21d4368151eb47..3451ba7cc34901 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -668,7 +668,7 @@ AST_MATCHER(FunctionDecl, isPredefinedUnsafeLibcFunc) { auto *II = Node.getIdentifier(); - if (!II) + if (!II) return false; StringRef Name = LibcFunNamePrefixSuffixParser().matchName( @@ -833,9 +833,16 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg, // // For the first two arguments: `ptr` and `size`, they are safe if in the // following patterns: +// +// Pattern 1: // ptr := DRE.data(); // size:= DRE.size()/DRE.size_bytes() // And DRE is a hardened container or view. +// +// Pattern 2: +// ptr := Constant-Array-DRE; +// size:= any expression that has compile-time constant value equivalent to +// sizeof (Constant-Array-DRE) AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) { const FunctionDecl *FD = Node.getDirectCallee(); @@ -856,6 +863,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) { !Size->getType()->isIntegerType()) return false; // not an snprintf call + // Pattern 1: static StringRef SizedObjs[] = {"span", "array", "vector", "basic_string_view", "basic_string"}; Buf = Buf->IgnoreParenImpCasts(); @@ -886,6 +894,23 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) { SizedObj) return false; // It is in fact safe } + + // Pattern 2: + if (auto *DRE = dyn_cast<DeclRefExpr>(Buf->IgnoreParenImpCasts())) { + ASTContext &Ctx = Finder->getASTContext(); + + if (auto *CAT = Ctx.getAsConstantArrayType(DRE->getType())) { + Expr::EvalResult ER; + // The array element type must be compatible with `char` otherwise an + // explicit cast will be needed, which will make this check unreachable. + // Therefore, the array extent is same as its' bytewise size. + if (Size->EvaluateAsConstantExpr(ER, Ctx)) { + APSInt EVal = ER.Val.getInt(); // Size must have integer type + + return APSInt::compareValues(EVal, APSInt(CAT->getSize(), true)) != 0; + } + } + } return true; // ptr and size are not in safe pattern } } // namespace libc_func_matchers 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 a3716073609f6f..8e777a7a51c994 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp @@ -83,6 +83,12 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) { sscanf(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf' is unsafe}} sscanf_s(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf_s' is unsafe}} fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, p); // expected-warning{{function 'fprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + + char a[10], b[11]; + int c[10]; + + snprintf(a, sizeof(b), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}} + snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}} fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn printf("%s%d", "hello", *p); // no warn snprintf(s.data(), s.size_bytes(), "%s%d", "hello", *p); // no warn @@ -93,13 +99,19 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) { __asan_printf();// a printf but no argument, so no warn } -void v(std::string s1, int *p) { +void safe_examples(std::string s1, int *p) { snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn fprintf((FILE*)0, s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn + + char a[10]; + + snprintf(a, sizeof a, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn + snprintf(a, sizeof(decltype(a)), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn + snprintf(a, 10, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn } >From 009842978e5c8503451c89591cd1e2a344023351 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <ziq...@udel.edu> Date: Wed, 11 Sep 2024 17:07:37 -0700 Subject: [PATCH 2/2] fix code format --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 3451ba7cc34901..a00b166a8b4f93 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -668,7 +668,7 @@ AST_MATCHER(FunctionDecl, isPredefinedUnsafeLibcFunc) { auto *II = Node.getIdentifier(); - if (!II) + if (!II) return false; StringRef Name = LibcFunNamePrefixSuffixParser().matchName( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits