diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index 926a4fce3ae74..8957ab664ed93 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -53,7 +53,8 @@ class UnsafeBufferUsageHandler { // This function invokes the analysis and allows the caller to react to it // through the handler class. -void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler); +void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, + bool EmitFixits); namespace internal { // Tests if any two `FixItHint`s in `FixIts` conflict. Two `FixItHint`s diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index ebf9b352e7bc7..61c2c4e4b52ad 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1008,7 +1008,8 @@ getNaiveStrategy(const llvm::SmallVectorImpl &UnsafeVars) { } void clang::checkUnsafeBufferUsage(const Decl *D, - UnsafeBufferUsageHandler &Handler) { + UnsafeBufferUsageHandler &Handler, + bool EmitFixits) { assert(D && D->getBody()); WarningGadgetSets UnsafeOps; @@ -1016,40 +1017,46 @@ void clang::checkUnsafeBufferUsage(const Decl *D, DeclUseTracker Tracker; { + // FIXME: We could skip even matching Fixables' matchers if EmitFixits == + // false. auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D, Handler); UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets)); FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets)); Tracker = std::move(TrackerRes); } - // Filter out non-local vars and vars with unclaimed DeclRefExpr-s. - for (auto it = FixablesForUnsafeVars.byVar.cbegin(); - it != FixablesForUnsafeVars.byVar.cend();) { - // FIXME: Support ParmVarDecl as well. - if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) { - it = FixablesForUnsafeVars.byVar.erase(it); - } else { - ++it; + std::map FixItsForVariable; + + if (EmitFixits) { + // Filter out non-local vars and vars with unclaimed DeclRefExpr-s. + for (auto it = FixablesForUnsafeVars.byVar.cbegin(); + it != FixablesForUnsafeVars.byVar.cend();) { + // FIXME: Support ParmVarDecl as well. + if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) { + it = FixablesForUnsafeVars.byVar.erase(it); + } else { + ++it; + } } - } - llvm::SmallVector UnsafeVars; - for (const auto &[VD, ignore] : FixablesForUnsafeVars.byVar) - UnsafeVars.push_back(VD); + llvm::SmallVector UnsafeVars; + for (const auto &[VD, ignore] : FixablesForUnsafeVars.byVar) + UnsafeVars.push_back(VD); - Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars); - std::map FixItsForVariable = - getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker, - D->getASTContext(), Handler); + Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars); + FixItsForVariable = getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker, + D->getASTContext(), Handler); - // FIXME Detect overlapping FixIts. + // FIXME Detect overlapping FixIts. + } for (const auto &G : UnsafeOps.noVar) { Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false); } for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) { - auto FixItsIt = FixItsForVariable.find(VD); + auto FixItsIt = + EmitFixits ? FixItsForVariable.find(VD) : FixItsForVariable.end(); Handler.handleFixableVariable(VD, FixItsIt != FixItsForVariable.end() ? std::move(FixItsIt->second) : FixItList{}); diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 1ec6d798e3594..85dc3a7eb9507 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2516,7 +2516,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( 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); + checkUnsafeBufferUsage( + D, R, + /*EmitFixits=*/S.getLangOpts().CPlusPlus20); } // If none of the previous checks caused a CFG build, trigger one here diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-fixits.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-fixits.cpp new file mode 100644 index 0000000000000..b69e20cd4eba3 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-no-fixits.cpp @@ -0,0 +1,82 @@ +// RUN: %clang_cc1 -x c -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// RUN: %clang_cc1 -x c -std=c89 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c -std=gnu89 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c -std=iso9899:1990 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// RUN: %clang_cc1 -x c -std=c17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c -std=gnu17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c -std=iso9899:2017 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c -std=c2x -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// RUN: %clang_cc1 -x c++ -std=c++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c++ -std=gnu++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c++ -std=c++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x c++ -std=gnu++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// RUN: %clang_cc1 -x objective-c++ -std=c++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x objective-c++ -std=gnu++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x objective-c++ -std=c++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -x objective-c++ -std=gnu++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// CHECK-NOT: fix-it: + +typedef int * Int_ptr_t; +typedef int Int_t; + +void local_array_subscript_simple() { + int tmp; + int *p; + const int *q; + tmp = p[5]; + tmp = q[5]; + + Int_ptr_t x; + Int_ptr_t y; + Int_t * z; + Int_t * w; + + tmp = x[5]; + tmp = y[5]; + tmp = z[5]; + tmp = w[5]; +} + +void local_ptr_to_array() { + int tmp; + int n = 10; + int a[10]; + int b[n]; + int *p = a; + int *q = b; + tmp = p[5]; + tmp = q[5]; +} + +void local_ptr_addrof_init() { + int var; + int * q = &var; + var = q[5]; +} + +void decl_without_init() { + int tmp; + int * p; + Int_ptr_t q; + tmp = p[5]; + tmp = q[5]; +} + +void explict_cast() { + int tmp; + int * p; + tmp = p[5]; + + int a; + char * q = (char *)&a; + tmp = (int) q[5]; + + void * r = &a; + char * s = (char *) r; + tmp = (int) s[5]; +}