[clang] [-Wunsafe-buffer-usage] Reduce false positives with constant arrays in libc warnings (PR #108308)
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 11 17:08:01 PDT 2024
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 <ziqing at 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 <ziqing at 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(
More information about the cfe-commits
mailing list