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_...@apple.com> Date: Fri, 20 Sep 2024 16:27:09 -0700 Subject: [PATCH 1/3] [-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_...@apple.com> Date: Fri, 20 Sep 2024 18:53:28 -0700 Subject: [PATCH 2/3] 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_...@apple.com> Date: Fri, 20 Sep 2024 19:33:45 -0700 Subject: [PATCH 3/3] 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits