llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ziqing Luo (ziqingluo-90) <details> <summary>Changes</summary> 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) This pattern will effectively remove a lot of false positives in our downstream projects. --- Full diff: https://github.com/llvm/llvm-project/pull/108308.diff 2 Files Affected: - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+26-1) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp (+13-1) ``````````diff 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 } `````````` </details> https://github.com/llvm/llvm-project/pull/108308 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
