Index: clang/lib/Analysis/CMakeLists.txt =================================================================== --- clang/lib/Analysis/CMakeLists.txt +++ clang/lib/Analysis/CMakeLists.txt @@ -39,6 +39,7 @@ clangASTMatchers clangBasic clangLex + clangToolingInclusions DEPENDS omp_gen Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -9,7 +9,10 @@ #include "clang/Analysis/Analyses/UnsafeBufferUsage.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" +#include "clang/Tooling/Inclusions/HeaderIncludes.h" +#include "clang/Tooling/Inclusions/IncludeStyle.h" #include "llvm/ADT/SmallVector.h" #include #include @@ -916,13 +919,31 @@ }); } +static std::string getHeaderFilenameForStrategyKind(Strategy::Kind TargetType) { + switch (TargetType) { + case Strategy::Kind::Span: + return "span"; + case Strategy::Kind::Array: + return "array"; + case Strategy::Kind::Vector: + return "vector"; + case Strategy::Kind::Iterator: + return "span"; + case Strategy::Kind::Wontfix: + assert(false && "Wonfix strategy shouldn't be used to generate Fix-Its"); + }; +} + static std::map getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S, const DeclUseTracker &Tracker, const ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) { + const SourceManager &SM = Ctx.getSourceManager(); std::map FixItsForVariable; for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) { - FixItsForVariable[VD] = fixVariable(VD, S.lookup(VD), Tracker, Ctx, Handler); + const Strategy::Kind ReplacementTypeForVD = S.lookup(VD); + FixItsForVariable[VD] = + fixVariable(VD, ReplacementTypeForVD, Tracker, Ctx, Handler); // If we fail to produce Fix-It for the declaration we have to skip the // variable entirely. if (FixItsForVariable[VD].empty()) { @@ -942,14 +963,50 @@ CorrectFixes.end()); } } - if (ImpossibleToFix) + + if (ImpossibleToFix) { FixItsForVariable.erase(VD); - else - FixItsForVariable[VD].insert(FixItsForVariable[VD].end(), - FixItsForVD.begin(), FixItsForVD.end()); + continue; + } + + FixItsForVariable[VD].insert(FixItsForVariable[VD].end(), + FixItsForVD.begin(), FixItsForVD.end()); + // Fix-it shall not overlap with macros or/and templates: if (overlapWithMacro(FixItsForVariable[VD])) { FixItsForVariable.erase(VD); + continue; + } + + assert(FixItsForVariable[VD].front().RemoveRange.isValid()); + const FileID FileOfVarDeclFixIt = + SM.getFileID(FixItsForVariable[VD].front().RemoveRange.getBegin()); + + const FileEntry *FEOfVarDeclFixIt = + SM.getFileEntryForID(FileOfVarDeclFixIt); + if (!FEOfVarDeclFixIt) { + FixItsForVariable.erase(VD); + continue; + } + std::optional FileContent = + SM.getBufferOrNone(FileOfVarDeclFixIt); + if (!FileContent) { + FixItsForVariable.erase(VD); + continue; + } + + tooling::IncludeStyle InclStyle; // TODO set IncludeStyle + tooling::HeaderIncludes HeaderIncls(FEOfVarDeclFixIt->getName(), + FileContent.value().getBuffer(), + InclStyle); + std::optional Include = HeaderIncls.insert( + getHeaderFilenameForStrategyKind(ReplacementTypeForVD), true, + clang::tooling::IncludeDirective::Include); + if (Include.has_value()) { + SourceLocation HeaderInsertLoc = + SM.getComposedLoc(FileOfVarDeclFixIt, Include->getOffset()); + FixItsForVariable[VD].push_back(FixItHint::CreateInsertion( + HeaderInsertLoc, Include->getReplacementText())); } } return FixItsForVariable; Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-include-header-included-file.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-include-header-included-file.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// CHECK: fix-it:"{{.*}}warn-unsafe-buffer-usage-fixits-include-header.cpp":{5:1-5:1}:"#include \n" + +// The point of this test is to make sure that if we emit Fix-Its for an included file we also add the #include directive to that file (and not necessarily the main file). + +#include "warn-unsafe-buffer-usage-fixits-include-header.cpp" Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-include-header.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-include-header.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// This comment block simulate a file header comment. +// CHECK: fix-it:"{{.*}}warn-unsafe-buffer-usage-fixits-include-header.cpp":{5:1-5:1}:"#include \n" + +void foo() { + /* just some other code */ +} + +void local_array_subscript_simple() { + int tmp; + int *p = new int[10]; + tmp = p[5]; +}