Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -54,7 +54,7 @@ // This function invokes the analysis and allows the caller to react to it // through the handler class. void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, - bool EmitFixits); + bool EmitSuggestions, bool EmitFixits); namespace internal { // Tests if any two `FixItHint`s in `FixIts` conflict. Two `FixItHint`s Index: clang/include/clang/Basic/DiagnosticOptions.def =================================================================== --- clang/include/clang/Basic/DiagnosticOptions.def +++ clang/include/clang/Basic/DiagnosticOptions.def @@ -95,6 +95,8 @@ /// Column limit for formatting message diagnostics, or 0 if unused. VALUE_DIAGOPT(MessageLength, 32, 0) +DIAGOPT(ShowSafeBufferUsageSuggestions, 1, 0) + #undef DIAGOPT #undef ENUM_DIAGOPT #undef VALUE_DIAGOPT Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11789,6 +11789,8 @@ "used%select{| in pointer arithmetic| in buffer access}0 here">; def note_unsafe_buffer_variable_fixit : Note< "change type of '%0' to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information">; +def note_safe_buffer_usage_suggestions_disabled : Note< + "pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">; def err_loongarch_builtin_requires_la32 : Error< "this builtin requires target: loongarch32">; Index: clang/include/clang/Driver/Options.td =================================================================== --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -1560,6 +1560,11 @@ Group, Flags<[CC1Option]>, HelpText<"Print a template comparison tree for differing templates">, MarshallingInfoFlag>; +defm safe_buffer_usage_suggestions : BoolFOption<"safe-buffer-usage-suggestions", + DiagnosticOpts<"ShowSafeBufferUsageSuggestions">, DefaultFalse, + PosFlag, + NegFlag>; def fdiscard_value_names : Flag<["-"], "fdiscard-value-names">, Group, HelpText<"Discard value names in LLVM IR">, Flags<[NoXarchOption]>; def fno_discard_value_names : Flag<["-"], "fno-discard-value-names">, Group, Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -611,7 +611,8 @@ /// Scan the function and return a list of gadgets found with provided kits. static std::tuple -findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) { +findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler, + bool EmitSuggestions) { struct GadgetFinderCallback : MatchFinder::MatchCallback { FixableGadgetList FixableGadgets; @@ -665,33 +666,37 @@ // clang-format off M.addMatcher( stmt(forEveryDescendant( - eachOf( - // A `FixableGadget` matcher and a `WarningGadget` matcher should not disable - // each other (they could if they were put in the same `anyOf` group). - // We also should make sure no two `FixableGadget` (resp. `WarningGadget`) matchers - // match for the same node, so that we can group them - // in one `anyOf` group (for better performance via short-circuiting). stmt(anyOf( -#define FIXABLE_GADGET(x) \ - x ## Gadget::matcher().bind(#x), -#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" - // Also match DeclStmts because we'll need them when fixing - // their underlying VarDecls that otherwise don't have - // any backreferences to DeclStmts. - declStmt().bind("any_ds") - )), - stmt(anyOf( - // Add Gadget::matcher() for every gadget in the registry. #define WARNING_GADGET(x) \ - allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)), + allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)), #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" - // In parallel, match all DeclRefExprs so that to find out - // whether there are any uncovered by gadgets. - declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre") - ))) - )), - &CB + unless(stmt()) // Avoid a hanging comma. + )) + )), &CB ); + + if (EmitSuggestions) { + M.addMatcher( + stmt(forEveryDescendant( + stmt(anyOf( + // It's ok for a FixableGadget to match a node that was previously + // matched by a WarningGadget. +#define FIXABLE_GADGET(x) \ + x ## Gadget::matcher().bind(#x), +#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" + // Add Gadget::matcher() for every gadget in the registry. + // In parallel, match all DeclRefExprs so that to find out + // whether there are any uncovered by gadgets. + declRefExpr(anyOf(hasPointerType(), hasArrayType()), + to(varDecl())).bind("any_dre"), + // Also match DeclStmts because we'll need them when fixing + // their underlying VarDecls that otherwise don't have + // any backreferences to DeclStmts. + declStmt().bind("any_ds") + )) + )), &CB + ); + } // clang-format on M.match(*D->getBody(), D->getASTContext()); @@ -1131,17 +1136,30 @@ void clang::checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, + bool EmitSuggestions, bool EmitFixits) { assert(D && D->getBody()); + assert(!(EmitFixits && !EmitSuggestions) && + "Unsafe buffer usage fixits requested without suggestions!"); WarningGadgetSets UnsafeOps; FixableGadgetSets FixablesForUnsafeVars; DeclUseTracker Tracker; { - auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D, Handler); - UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); - FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); + auto [FixableGadgets, WarningGadgets, TrackerRes] = + findGadgets(D, Handler, EmitSuggestions); + if (EmitSuggestions) { + UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); + FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); + } else { + // Don't group anything by variable if we aren't suggesting fixes. + UnsafeOps.noVar.reserve(WarningGadgets.size()); + for (std::unique_ptr &G : WarningGadgets) { + UnsafeOps.noVar.push_back(std::move(G)); + } + WarningGadgets.clear(); + } Tracker = std::move(TrackerRes); } Index: clang/lib/Driver/ToolChains/Clang.cpp =================================================================== --- clang/lib/Driver/ToolChains/Clang.cpp +++ clang/lib/Driver/ToolChains/Clang.cpp @@ -7021,6 +7021,9 @@ A->claim(); } + Args.addOptInFlag(CmdArgs, options::OPT_fsafe_buffer_usage_suggestions, + options::OPT_fno_safe_buffer_usage_suggestions); + // Setup statistics file output. SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D); if (!StatsFile.empty()) { Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2156,9 +2156,11 @@ namespace { class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { Sema &S; + bool SuggestSuggestions; public: - UnsafeBufferUsageReporter(Sema &S) : S(S) {} + UnsafeBufferUsageReporter(Sema &S, bool SuggestSuggestions) + : S(S), SuggestSuggestions(SuggestSuggestions) {} void handleUnsafeOperation(const Stmt *Operation, bool IsRelatedToDecl) override { @@ -2197,15 +2199,23 @@ Loc = Operation->getBeginLoc(); Range = Operation->getSourceRange(); } - if (IsRelatedToDecl) + if (IsRelatedToDecl) { + assert(!SuggestSuggestions && + "Variables blamed for unsafe buffer usage without suggestions!"); S.Diag(Loc, diag::note_unsafe_buffer_operation) << MsgParam << Range; - else + } else { S.Diag(Loc, diag::warn_unsafe_buffer_operation) << MsgParam << Range; + if (SuggestSuggestions) { + S.Diag(Loc, diag::note_safe_buffer_usage_suggestions_disabled); + } + } } // FIXME: rename to handleUnsafeVariable void handleFixableVariable(const VarDecl *Variable, FixItList &&Fixes) override { + assert(!SuggestSuggestions && + "Unsafe buffer usage fixits displayed without suggestions!"); S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable) << Variable << (Variable->getType()->isPointerType() ? 0 : 1) << Variable->getSourceRange(); @@ -2521,11 +2531,12 @@ // Emit unsafe buffer usage warnings and fixits. if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, D->getBeginLoc()) || !Diags.isIgnored(diag::warn_unsafe_buffer_variable, D->getBeginLoc())) { - UnsafeBufferUsageReporter R(S); - checkUnsafeBufferUsage( - D, R, - /*EmitFixits=*/S.getDiagnostics().getDiagnosticOptions().ShowFixits && - S.getLangOpts().CPlusPlus20); + DiagnosticOptions &DiagOpts = S.getDiagnostics().getDiagnosticOptions(); + bool EmitSuggestions = DiagOpts.ShowSafeBufferUsageSuggestions && + S.getLangOpts().CPlusPlus20; + bool EmitFixits = EmitSuggestions && DiagOpts.ShowFixits; + UnsafeBufferUsageReporter R(S, /*SuggestSuggestions=*/!EmitSuggestions); + checkUnsafeBufferUsage(D, R, EmitSuggestions, EmitFixits); } // If none of the previous checks caused a CFG build, trigger one here Index: clang/test/SemaCXX/unsafe-buffer-usage-diag-type.cpp =================================================================== --- clang/test/SemaCXX/unsafe-buffer-usage-diag-type.cpp +++ clang/test/SemaCXX/unsafe-buffer-usage-diag-type.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions -verify %s namespace localVar { void testRefersPtrLocalVarDecl(int i) { Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-assign-to-array-subscr-on-ptr.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-assign-to-array-subscr-on-ptr.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-assign-to-array-subscr-on-ptr.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s // TODO cases where we don't want fixits Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp @@ -1,4 +1,7 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits -fsyntax-only \ +// RUN: %s 2>&1 | FileCheck %s // TODO test we don't mess up vertical whitespace // TODO test different whitespaces Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s typedef int * Int_ptr_t; typedef int Int_t; Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions -verify %s [[clang::unsafe_buffer_usage]] void deprecatedFunction3(); Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s void basic(int * x) { int tmp; Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -Wno-unused-value -verify %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -Wno-unused-value -verify %s void basic(int * x) { // expected-warning{{'x' is an unsafe pointer used for buffer access}} int *p1 = new int[10]; // not to warn Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-suggestions-flag.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-suggestions-flag.cpp @@ -0,0 +1,54 @@ +// Test the -cc1 flag. There's no -fno-... option in -cc1 invocations, +// just the positive option. + +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify=OFF %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify=ON %s \ +// RUN: -fsafe-buffer-usage-suggestions + +// Test driver flags. Now there's both -f... and -fno-... to worry about. + +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -Xclang -verify=OFF %s +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=ON %s +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fno-safe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=OFF %s + +// In case of driver flags, last flag takes precedence. + +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fno-safe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=OFF %s +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fno-safe-buffer-usage-suggestions \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=ON %s + +// Passing through -Xclang. + +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -Xclang -fsafe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=ON %s + +// -Xclang flags take precedence over driver flags. + +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -Xclang -fsafe-buffer-usage-suggestions \ +// RUN: -fno-safe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=ON %s +// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fno-safe-buffer-usage-suggestions \ +// RUN: -Xclang -fsafe-buffer-usage-suggestions \ +// RUN: -Xclang -verify=ON %s + +void foo(int *x) { // \ + // ON-warning{{'x' is an unsafe pointer used for buffer access}} + // FIXME: Better "OFF" warning? + x[5] = 10; // \ + // ON-note {{used in buffer access here}} \ + // OFF-warning{{unsafe buffer access}} \ + // OFF-note {{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}} +} Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fblocks -include %s -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fblocks -include %s -verify %s // RUN: %clang -x c++ -fsyntax-only -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s // RUN: %clang_cc1 -std=c++11 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s