[clang] [-Wunsafe-buffer-usage] Fix a bug and suppress libc warnings for C files (PR #109496)
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 20:57:34 PDT 2024
https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/109496
>From e7f7f82b25eaae86623ac8f47731892b3b629d7d Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 20 Sep 2024 16:27:09 -0700
Subject: [PATCH 1/4] [-Wunsafe-buffer-usage] Fix a bug and suppress libc
warnings for C files
- Fix a bug in UnsafeBufferUsage.cpp related to casting to PointerType
- Suppress -Wunsafe-buffer-usage-in-libc-call for C files
(rdar://117182250)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 8 +++++---
clang/lib/Sema/AnalysisBasedWarnings.cpp | 4 +++-
.../warn-unsafe-buffer-usage-no-libc-functions-in-c.c | 9 +++++++++
3 files changed, 17 insertions(+), 4 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a16762244b1766..110a121e71a7d2 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -250,7 +250,9 @@ AST_MATCHER_P(Stmt, ignoreUnsafeBufferInContainer,
AST_MATCHER_P(Stmt, ignoreUnsafeLibcCall, const UnsafeBufferUsageHandler *,
Handler) {
- return Handler->ignoreUnsafeBufferInLibcCall(Node.getBeginLoc());
+ if (Finder->getASTContext().getLangOpts().CPlusPlus)
+ return Handler->ignoreUnsafeBufferInLibcCall(Node.getBeginLoc());
+ return true; /* Only warn about libc calls for C++ */
}
AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
@@ -789,7 +791,7 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
if (!FristParmTy->isPointerType())
return false; // possibly some user-defined printf function
- QualType FirstPteTy = (cast<PointerType>(FristParmTy))->getPointeeType();
+ QualType FirstPteTy = FristParmTy->getAs<PointerType>()->getPointeeType();
if (!Ctx.getFILEType()
.isNull() && //`FILE *` must be in the context if it is fprintf
@@ -865,7 +867,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
if (!FirstParmTy->isPointerType())
return false; // Not an snprint
- QualType FirstPteTy = cast<PointerType>(FirstParmTy)->getPointeeType();
+ QualType FirstPteTy = FirstParmTy->getAs<PointerType>()->getPointeeType();
const Expr *Buf = Node.getArg(0), *Size = Node.getArg(1);
if (FirstPteTy.isConstQualified() || !Buf->getType()->isPointerType() ||
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 117b2c8bc57935..7e0b929abea683 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2549,6 +2549,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
// UnsafeBufferUsage analysis settings.
+ bool IsCXXLang = S.getLangOpts().CPlusPlus;
bool UnsafeBufferUsageCanEmitSuggestions = S.getLangOpts().CPlusPlus20;
bool UnsafeBufferUsageShouldEmitSuggestions = // Should != Can.
UnsafeBufferUsageCanEmitSuggestions &&
@@ -2581,7 +2582,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
!Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation()) ||
!Diags.isIgnored(diag::warn_unsafe_buffer_usage_in_container,
SourceLocation()) ||
- !Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation())) {
+ (!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation()) &&
+ IsCXXLang)) {
CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);
}
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
new file mode 100644
index 00000000000000..e305c3e140dff9
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s
+
+void* memcpy(void *dst,const void *src, unsigned long size);
+
+void f(int *p, int *q) {
+
+ memcpy(p, q, 10); // no libc warn in C
+ ++p[5]; // expected-warning{{unsafe buffer access}}
+}
>From 42664f03f16a99aa17bb33f473a01db73af796f5 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 20 Sep 2024 18:53:28 -0700
Subject: [PATCH 2/4] Add tests and fix a typo
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 6 +++---
.../warn-unsafe-buffer-usage-libc-functions.cpp | 10 ++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 110a121e71a7d2..6c1979179711b9 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -786,12 +786,12 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
return false; // possibly some user-defined printf function
ASTContext &Ctx = Finder->getASTContext();
- QualType FristParmTy = FD->getParamDecl(0)->getType();
+ QualType FirstParmTy = FD->getParamDecl(0)->getType();
- if (!FristParmTy->isPointerType())
+ if (!FirstParmTy->isPointerType())
return false; // possibly some user-defined printf function
- QualType FirstPteTy = FristParmTy->getAs<PointerType>()->getPointeeType();
+ QualType FirstPteTy = FirstParmTy->getAs<PointerType>()->getPointeeType();
if (!Ctx.getFILEType()
.isNull() && //`FILE *` must be in the context if it is fprintf
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 25b6d8e9e22dc2..6c77364c880e70 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
@@ -56,6 +56,11 @@ namespace std {
}
void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
+ typedef FILE * _Nullable aligned_file_ptr_t __attribute__((align_value(64)));
+ typedef char * _Nullable aligned_char_ptr_t __attribute__((align_value(64)));
+ aligned_file_ptr_t fp;
+ aligned_char_ptr_t cp;
+
memcpy(); // expected-warning{{function 'memcpy' is unsafe}}
std::memcpy(); // expected-warning{{function 'memcpy' is unsafe}}
__builtin_memcpy(p, q, 64); // expected-warning{{function '__builtin_memcpy' is unsafe}}
@@ -71,9 +76,11 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
printf("%s%d", // expected-warning{{function 'printf' is unsafe}}
p, // expected-note{{string argument is not guaranteed to be null-terminated}} note attached to the unsafe argument
*p);
+ printf(cp, p, *p); // expected-warning{{function 'printf' is unsafe}} // expected-note{{string argument is not guaranteed to be null-terminated}}
sprintf(q, "%s%d", "hello", *p); // expected-warning{{function 'sprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
swprintf(q, "%s%d", "hello", *p); // expected-warning{{function 'swprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
snprintf(q, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
+ snprintf(cp, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snwprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snwprintf_s( // expected-warning{{function 'snwprintf_s' is unsafe}}
@@ -84,8 +91,10 @@ 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}}
+ fprintf(fp, "%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}}
wprintf(L"hello %s", p); // expected-warning{{function 'wprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+
char a[10], b[11];
int c[10];
std::wstring WS;
@@ -93,6 +102,7 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
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
+ fprintf(fp, "%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
snprintf(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn
>From 9e0e31b603c9ef7ea9502dc937dc7171e3060fcc Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 20 Sep 2024 19:33:45 -0700
Subject: [PATCH 3/4] address comments
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++--
clang/lib/Sema/AnalysisBasedWarnings.cpp | 3 +--
.../test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp | 2 ++
.../SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c | 3 ++-
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6c1979179711b9..97f1c4f16b8f4c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -791,7 +791,7 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
if (!FirstParmTy->isPointerType())
return false; // possibly some user-defined printf function
- QualType FirstPteTy = FirstParmTy->getAs<PointerType>()->getPointeeType();
+ QualType FirstPteTy = FirstParmTy->castAs<PointerType>()->getPointeeType();
if (!Ctx.getFILEType()
.isNull() && //`FILE *` must be in the context if it is fprintf
@@ -867,7 +867,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
if (!FirstParmTy->isPointerType())
return false; // Not an snprint
- QualType FirstPteTy = FirstParmTy->getAs<PointerType>()->getPointeeType();
+ QualType FirstPteTy = FirstParmTy->castAs<PointerType>()->getPointeeType();
const Expr *Buf = Node.getArg(0), *Size = Node.getArg(1);
if (FirstPteTy.isConstQualified() || !Buf->getType()->isPointerType() ||
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 7e0b929abea683..6496a33b8f5a50 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2549,7 +2549,6 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
// UnsafeBufferUsage analysis settings.
- bool IsCXXLang = S.getLangOpts().CPlusPlus;
bool UnsafeBufferUsageCanEmitSuggestions = S.getLangOpts().CPlusPlus20;
bool UnsafeBufferUsageShouldEmitSuggestions = // Should != Can.
UnsafeBufferUsageCanEmitSuggestions &&
@@ -2583,7 +2582,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
!Diags.isIgnored(diag::warn_unsafe_buffer_usage_in_container,
SourceLocation()) ||
(!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation()) &&
- IsCXXLang)) {
+ S.getLangOpts().CPlusPlus /* only warn about libc calls in C++ */)) {
CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);
}
}
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 6c77364c880e70..a7c19bcac16078 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
// RUN: -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: -verify %s -x objective-c++
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \
// RUN: -verify %s
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
index e305c3e140dff9..4c3cc90daf5aeb 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s
+// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x c
+// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x objective-c
void* memcpy(void *dst,const void *src, unsigned long size);
>From 1797521ff88291474ba59d5bb348ffc8ce6a3a10 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 20 Sep 2024 20:57:02 -0700
Subject: [PATCH 4/4] Change memcpy to __asan_memcpy to avoid libc function
redeclaration
---
.../SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
index 4c3cc90daf5aeb..01d5a1a521318e 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-libc-functions-in-c.c
@@ -1,10 +1,10 @@
// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x c
// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x objective-c
-void* memcpy(void *dst,const void *src, unsigned long size);
+void* __asan_memcpy(void *dst,const void *src, unsigned long size);
void f(int *p, int *q) {
- memcpy(p, q, 10); // no libc warn in C
+ __asan_memcpy(p, q, 10); // no libc warn in C
++p[5]; // expected-warning{{unsafe buffer access}}
}
More information about the cfe-commits
mailing list