llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Hans Wennborg (zmodem) <details> <summary>Changes</summary> PR #<!-- -->173096 extended -Wunsafe-buffer-usage-in-libc-call to apply to all functions with the 'format' attribute. This change moves those warnings behind a separate -Wunsafe-buffer-usage-format-attr flag (implicitly enabled by -Wunsafe-buffer-usage), allowing projects to decide whether they want to opt in to this or not. --- Full diff: https://github.com/llvm/llvm-project/pull/175749.diff 6 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h (+2-1) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4) - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+9-3) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+6-1) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp (+17-17) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index ea41eb3becfcf..65dbd3b2ab9d1 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -117,8 +117,9 @@ class UnsafeBufferUsageHandler { /// safe pattern; /// is 3 if string arguments do not guarantee null-termination /// is 4 if the callee takes va_list + /// has bit 3 (0x8) set if the callee is a function with the format attribute /// \param UnsafeArg one of the actual arguments that is unsafe, non-null - /// only when `2 <= PrintfInfo <= 3` + /// only when `2 <= PrintfInfo <= 3 (ignoring the "format attribute" bit)` virtual void handleUnsafeLibcCall(const CallExpr *Call, unsigned PrintfInfo, ASTContext &Ctx, const Expr *UnsafeArg = nullptr) = 0; diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 3764475fbd3df..08ad2fc4b954d 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1773,8 +1773,9 @@ def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">; // Warnings and fixes to support the "safe buffers" programming model. def UnsafeBufferUsageInContainer : DiagGroup<"unsafe-buffer-usage-in-container">; def UnsafeBufferUsageInLibcCall : DiagGroup<"unsafe-buffer-usage-in-libc-call">; +def UnsafeBufferUsageFormatAttr : DiagGroup<"unsafe-buffer-usage-format-attr">; def UnsafeBufferUsageInUniquePtrArrayAccess : DiagGroup<"unsafe-buffer-usage-in-unique-ptr-array-access">; -def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInContainer, UnsafeBufferUsageInLibcCall, UnsafeBufferUsageInUniquePtrArrayAccess]>; +def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInContainer, UnsafeBufferUsageInLibcCall, UnsafeBufferUsageFormatAttr, UnsafeBufferUsageInUniquePtrArrayAccess]>; // Warnings and notes InstallAPI verification. def InstallAPIViolation : DiagGroup<"installapi-violation">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 5cbbc7d130c99..967d54bc44979 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13450,6 +13450,10 @@ def warn_unsafe_buffer_operation : Warning< def warn_unsafe_buffer_libc_call : Warning< "function %0 is unsafe">, InGroup<UnsafeBufferUsageInLibcCall>, DefaultIgnore; +def warn_unsafe_buffer_format_attr_call : Warning< + "formatting function %0 is unsafe">, + InGroup<UnsafeBufferUsageFormatAttr>, DefaultIgnore; + def note_unsafe_buffer_printf_call : Note< "%select{|change to 'snprintf' for explicit bounds checking | buffer pointer and size may not match" "|string argument is not guaranteed to be null-terminated" diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index c7a5988445aa1..12af536053581 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -2098,6 +2098,7 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget { // guarantee null-termination VA_LIST = 4, // one of the `-printf`s function that take va_list, which is // considered unsafe as it is not compile-time check + FORMAT_ATTR = 8, // flag: the callee has the format attribute } WarnedFunKind = OTHERS; UnsafeLibcFunctionCallGadget(const MatchResult &Result) @@ -2282,11 +2283,16 @@ class UnsafeFormatAttributedFunctionCallGadget : public WarningGadget { ASTContext &Ctx) const override { if (UnsafeArg) Handler.handleUnsafeLibcCall( - Call, UnsafeLibcFunctionCallGadget::UnsafeKind::STRING, Ctx, - UnsafeArg); + Call, + UnsafeLibcFunctionCallGadget::UnsafeKind::STRING | + UnsafeLibcFunctionCallGadget::UnsafeKind::FORMAT_ATTR, + Ctx, UnsafeArg); else Handler.handleUnsafeLibcCall( - Call, UnsafeLibcFunctionCallGadget::UnsafeKind::OTHERS, Ctx); + Call, + UnsafeLibcFunctionCallGadget::UnsafeKind::OTHERS | + UnsafeLibcFunctionCallGadget::UnsafeKind::FORMAT_ATTR, + Ctx); } DeclUseList getClaimedVarUseSites() const override { return {}; } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 7b08648080710..df7488c3059e7 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2523,7 +2523,12 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { void handleUnsafeLibcCall(const CallExpr *Call, unsigned PrintfInfo, ASTContext &Ctx, const Expr *UnsafeArg = nullptr) override { - S.Diag(Call->getBeginLoc(), diag::warn_unsafe_buffer_libc_call) + unsigned DiagID = diag::warn_unsafe_buffer_libc_call; + if (PrintfInfo & 0x8) { + DiagID = diag::warn_unsafe_buffer_format_attr_call; + PrintfInfo ^= 0x8; + } + S.Diag(Call->getBeginLoc(), DiagID) << Call->getDirectCallee() // We've checked there is a direct callee << Call->getSourceRange(); if (PrintfInfo > 0) { 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 fe9bc7c809c96..7d6b118154bec 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp @@ -2,10 +2,10 @@ // RUN: -verify %s // RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-gcc-compat\ // RUN: -verify %s -x objective-c++ -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call -Wno-gcc-compat\ -// RUN: -verify %s -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call -Wno-gcc-compat\ -// RUN: -verify %s -DTEST_STD_NS +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \ +// RUN: -Wunsafe-buffer-usage-format-attr -Wno-gcc-compat -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \ +// RUN: -Wunsafe-buffer-usage-format-attr -Wno-gcc-compat -verify %s -DTEST_STD_NS typedef struct {} FILE; typedef unsigned int size_t; @@ -295,7 +295,7 @@ struct FormatAttrTestMember2 { void test_format_attr(char * Str, std::string StdStr) { myprintf("hello", Str); myprintf("hello %s", StdStr.c_str()); - myprintf("hello %s", Str); // expected-warning{{function 'myprintf' is unsafe}} \ + myprintf("hello %s", Str); // expected-warning{{formatting function 'myprintf' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} extern int errno; @@ -304,12 +304,12 @@ void test_format_attr(char * Str, std::string StdStr) { myprintf_2("hello", 0, Str); myprintf_2("hello %s", 0, StdStr.c_str()); - myprintf_2("hello %s", 0, Str); // expected-warning{{function 'myprintf_2' is unsafe}} \ + myprintf_2("hello %s", 0, Str); // expected-warning{{formatting function 'myprintf_2' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} myprintf_3("irrelevant", "hello", 0, Str); myprintf_3("irrelevant", "hello %s", 0, StdStr.c_str()); - myprintf_3("irrelevant", "hello %s", 0, Str); // expected-warning{{function 'myprintf_3' is unsafe}} \ + myprintf_3("irrelevant", "hello %s", 0, Str); // expected-warning{{formatting function 'myprintf_3' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} myscanf("hello %s"); myscanf("hello %s", Str); // expected-warning{{function 'myscanf' is unsafe}} @@ -325,30 +325,30 @@ void test_format_attr(char * Str, std::string StdStr) { Obj.myprintf("hello", Str); Obj.myprintf("hello %s", StdStr.c_str()); - Obj.myprintf("hello %s", Str); // expected-warning{{function 'myprintf' is unsafe}} \ + Obj.myprintf("hello %s", Str); // expected-warning{{formatting function 'myprintf' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} Obj.myprintf_2("hello", 0, Str); Obj.myprintf_2("hello %s", 0, StdStr.c_str()); - Obj.myprintf_2("hello %s", 0, Str); // expected-warning{{function 'myprintf_2' is unsafe}} \ + Obj.myprintf_2("hello %s", 0, Str); // expected-warning{{formatting function 'myprintf_2' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} Obj.myprintf_3("irrelevant", "hello", 0, Str); Obj.myprintf_3("irrelevant", "hello %s", 0, StdStr.c_str()); - Obj.myprintf_3("irrelevant", "hello %s", 0, Str); // expected-warning{{function 'myprintf_3' is unsafe}} \ + Obj.myprintf_3("irrelevant", "hello %s", 0, Str); // expected-warning{{formatting function 'myprintf_3' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} Obj.myscanf("hello %s"); - Obj.myscanf("hello %s", Str); // expected-warning{{function 'myscanf' is unsafe}} + Obj.myscanf("hello %s", Str); // expected-warning{{formatting function 'myscanf' is unsafe}} - Obj.myscanf("hello %d", &X); // expected-warning{{function 'myscanf' is unsafe}} + Obj.myscanf("hello %d", &X); // expected-warning{{formatting function 'myscanf' is unsafe}} - Obj.myprintf_default("irrelevant"); // expected-warning{{function 'myprintf_default' is unsafe}} + Obj.myprintf_default("irrelevant"); // expected-warning{{formatting function 'myprintf_default' is unsafe}} // expected-note@*{{string argument is not guaranteed to be null-terminated}} Obj("hello", Str); Obj("hello %s", StdStr.c_str()); - Obj("hello %s", Str); // expected-warning{{function 'operator()' is unsafe}} \ + Obj("hello %s", Str); // expected-warning{{formatting function 'operator()' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} Obj["hello"]; Obj["hello %s"]; @@ -357,7 +357,7 @@ void test_format_attr(char * Str, std::string StdStr) { Obj2("hello", Str); Obj2("hello %s", StdStr.c_str()); - Obj2("hello %s", Str); // expected-warning{{function 'operator()' is unsafe}} \ + Obj2("hello %s", Str); // expected-warning{{formatting function 'operator()' is unsafe}} \ expected-note{{string argument is not guaranteed to be null-terminated}} } @@ -367,9 +367,9 @@ void myprintf_arg_idx_oob(const char *) __attribute__((__format__ (__printf__, 1 void test_format_attr_invalid_arg_idx(char * Str, std::string StdStr) { myprintf_arg_idx_oob("hello"); - myprintf_arg_idx_oob(Str); // expected-warning{{function 'myprintf_arg_idx_oob' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + myprintf_arg_idx_oob(Str); // expected-warning{{formatting function 'myprintf_arg_idx_oob' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} myprintf_arg_idx_oob(StdStr.c_str()); myprintf("hello"); - myprintf(Str); // expected-warning{{function 'myprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + myprintf(Str); // expected-warning{{formatting function 'myprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} myprintf(StdStr.c_str()); } `````````` </details> https://github.com/llvm/llvm-project/pull/175749 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
