diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index c865b2e8bdb37..e8a93164302a4 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -31,7 +31,10 @@ class VariableGroupsManager { /// together in one step. /// /// `Var` must be a variable that needs fix (so it must be in a group). - virtual VarGrpRef getGroupOfVar(const VarDecl *Var) const =0; + /// `HasParm` is an optional argument that will be set to true if the set of + /// variables, where `Var` is in, contains parameters. + virtual VarGrpRef getGroupOfVar(const VarDecl *Var, + bool *HasParm = nullptr) const =0; }; /// The interface that lets the caller handle unsafe buffer usage analysis @@ -62,11 +65,14 @@ class UnsafeBufferUsageHandler { bool IsRelatedToDecl) = 0; /// Invoked when a fix is suggested against a variable. This function groups - /// all variables that must be fixed together (i.e their types must be changed to the - /// same target type to prevent type mismatches) into a single fixit. + /// all variables that must be fixed together (i.e their types must be changed + /// to the same target type to prevent type mismatches) into a single fixit. + /// + /// `D` is the declaration of the callable under analysis that owns `Variable` + /// and all of its group mates. virtual void handleUnsafeVariableGroup(const VarDecl *Variable, const VariableGroupsManager &VarGrpMgr, - FixItList &&Fixes) = 0; + FixItList &&Fixes, const Decl *D) = 0; #ifndef NDEBUG public: diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 0ac4df8edb242..9613cca63193f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11916,6 +11916,9 @@ def note_unsafe_buffer_operation : Note< "used%select{| in pointer arithmetic| in buffer access}0 here">; def note_unsafe_buffer_variable_fixit_group : Note< "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">; +def note_unsafe_buffer_variable_fixit_together : Note< + "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information" + "%select{|, and change %2 to safe types to make function %4 bounds-safe}3">; def note_safe_buffer_usage_suggestions_disabled : Note< "pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">; #ifndef NDEBUG diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index df89ab4363808..348585ef16e96 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1434,6 +1434,22 @@ static bool hasUnsupportedSpecifiers(const VarDecl *VD, AttrRangeOverlapping; } +// Returns the `SourceRange` of `D`. The reason why this function exists is +// that `D->getSourceRange()` may return a range where the end location is the +// starting location of the last token. The end location of the source range +// returned by this function is the last location of the last token. +static SourceRange getSourceRangeToTokenEnd(const Decl *D, + const SourceManager &SM, + LangOptions LangOpts) { + SourceLocation Begin = D->getBeginLoc(); + SourceLocation + End = // `D->getEndLoc` should always return the starting location of the + // last token, so we should get the end of the token + Lexer::getLocForEndOfToken(D->getEndLoc(), 0, SM, LangOpts); + + return SourceRange(Begin, End); +} + // Returns the text of the pointee type of `T` from a `VarDecl` of a pointer // type. The text is obtained through from `TypeLoc`s. Since `TypeLoc` does not // have source ranges of qualifiers ( The `QualifiedTypeLoc` looks hacky too me @@ -1528,6 +1544,21 @@ static std::optional getFunNameText(const FunctionDecl *FD, return getRangeText(NameRange, SM, LangOpts); } +// Returns the text representing a `std::span` type where the element type is +// represented by `EltTyText`. +// +// Note the optional parameter `Qualifiers`: one needs to pass qualifiers +// explicitly if the element type needs to be qualified. +static std::string +getSpanTypeText(StringRef EltTyText, + std::optional Quals = std::nullopt) { + const char *const SpanOpen = "std::span<"; + + if (Quals) + return SpanOpen + EltTyText.str() + ' ' + Quals->getAsString() + '>'; + return SpanOpen + EltTyText.str() + '>'; +} + std::optional DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const { const VarDecl *VD = dyn_cast(BaseDeclRefExpr->getDecl()); @@ -1916,11 +1947,11 @@ static bool hasConflictingOverload(const FunctionDecl *FD) { return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult(); } -// For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new -// type, this function produces fix-its to make the change self-contained. Let -// 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity -// defined by the `FunDecl` after the change to the parameter. Fix-its produced -// by this function are +// For a `FunctionDecl`, whose `ParmVarDecl`s are being changed to have new +// types, this function produces fix-its to make the change self-contained. Let +// 'F' be the entity defined by the original `FunctionDecl` and "NewF" be the +// entity defined by the `FunctionDecl` after the change to the parameters. +// Fix-its produced by this function are // 1. Add the `[[clang::unsafe_buffer_usage]]` attribute to each declaration // of 'F'; // 2. Create a declaration of "NewF" next to each declaration of `F`; @@ -1953,10 +1984,11 @@ static bool hasConflictingOverload(const FunctionDecl *FD) { // [[clang::unsafe_buffer_usage]] // #endif // -// `NewTypeText` is the string representation of the new type, to which the -// parameter indexed by `ParmIdx` is being changed. +// `ParmsMask` is an array of size of `FD->getNumParams()` such +// that `ParmsMask[i]` is true iff the `i`-th parameter will be fixed with some +// strategy. static std::optional -createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, +createOverloadsForFixedParams(const std::vector &ParmsMask, const Strategy &S, const FunctionDecl *FD, const ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) { // FIXME: need to make this conflict checking better: @@ -1965,13 +1997,30 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, const SourceManager &SM = Ctx.getSourceManager(); const LangOptions &LangOpts = Ctx.getLangOpts(); + const unsigned NumParms = FD->getNumParams(); + std::vector NewTysTexts(NumParms); + + for (unsigned i = 0; i < NumParms; i++) { + if (!ParmsMask[i]) + continue; + + std::optional PteTyQuals = std::nullopt; + std::optional PteTyText = + getPointeeTypeText(FD->getParamDecl(i), SM, LangOpts, &PteTyQuals); + + if (!PteTyText) + // something wrong in obtaining the text of the pointee type, give up + return std::nullopt; + // FIXME: whether we should create std::span type depends on the Strategy. + NewTysTexts[i] = getSpanTypeText(*PteTyText, PteTyQuals); + } // FIXME Respect indentation of the original code. // A lambda that creates the text representation of a function declaration - // with the new type signature: + // with the new type signatures: const auto NewOverloadSignatureCreator = - [&SM, &LangOpts](const FunctionDecl *FD, unsigned ParmIdx, - StringRef NewTypeText) -> std::optional { + [&SM, &LangOpts, &NewTysTexts, + &ParmsMask](const FunctionDecl *FD) -> std::optional { std::stringstream SS; SS << ";"; @@ -1991,13 +2040,16 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, if (Parm->isImplicit()) continue; - if (i == ParmIdx) { - SS << NewTypeText.str(); + if (ParmsMask[i]) { + // This `i`-th parameter will be fixed with `NewTysTexts[i]` being its + // new type: + SS << NewTysTexts[i]; // print parameter name if provided: - if (IdentifierInfo * II = Parm->getIdentifier()) - SS << " " << II->getName().str(); - } else if (auto ParmTypeText = - getRangeText(Parm->getSourceRange(), SM, LangOpts)) { + if (IdentifierInfo *II = Parm->getIdentifier()) + SS << ' ' << II->getName().str(); + } else if (auto ParmTypeText = getRangeText( + getSourceRangeToTokenEnd(Parm, SM, LangOpts), + SM, LangOpts)) { // print the whole `Parm` without modification: SS << ParmTypeText->str(); } else @@ -2012,9 +2064,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, // A lambda that creates the text representation of a function definition with // the original signature: const auto OldOverloadDefCreator = - [&SM, &Handler, - &LangOpts](const FunctionDecl *FD, unsigned ParmIdx, - StringRef NewTypeText) -> std::optional { + [&Handler, &SM, &LangOpts, &NewTysTexts, + &ParmsMask](const FunctionDecl *FD) -> std::optional { std::stringstream SS; SS << getEndOfLine().str(); @@ -2044,10 +2095,10 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, if (!Parm->getIdentifier()) // If a parameter of a function definition has no name: return std::nullopt; - if (i == ParmIdx) + if (ParmsMask[i]) // This is our spanified paramter! - SS << NewTypeText.str() << "(" << Parm->getIdentifier()->getName().str() << ", " - << getUserFillPlaceHolder("size") << ")"; + SS << NewTysTexts[i] << "(" << Parm->getIdentifier()->getName().str() + << ", " << getUserFillPlaceHolder("size") << ")"; else SS << Parm->getIdentifier()->getName().str(); if (i != NumParms - 1) @@ -2069,8 +2120,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, assert(FReDecl == FD && "inconsistent function definition"); // Inserts a definition with the old signature to the end of // `FReDecl`: - if (auto OldOverloadDef = - OldOverloadDefCreator(FReDecl, ParmIdx, NewTyText)) + if (auto OldOverloadDef = OldOverloadDefCreator(FReDecl)) FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *OldOverloadDef)); else return {}; // give up @@ -2082,8 +2132,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, FReDecl->getBeginLoc(), " "))); } // Inserts a declaration with the new signature to the end of `FReDecl`: - if (auto NewOverloadDecl = - NewOverloadSignatureCreator(FReDecl, ParmIdx, NewTyText)) + if (auto NewOverloadDecl = NewOverloadSignatureCreator(FReDecl)) FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *NewOverloadDecl)); else return {}; @@ -2092,9 +2141,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, return FixIts; } -// To fix a `ParmVarDecl` to be of `std::span` type. In addition, creating a -// new overload of the function so that the change is self-contained (see -// `createOverloadsForFixedParams`). +// To fix a `ParmVarDecl` to be of `std::span` type. static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) { if (hasUnsupportedSpecifiers(PVD, Ctx.getSourceManager())) { @@ -2107,49 +2154,37 @@ static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx, return {}; } - assert(PVD->getType()->isPointerType()); - auto *FD = dyn_cast(PVD->getDeclContext()); + std::optional PteTyQualifiers = std::nullopt; + std::optional PteTyText = getPointeeTypeText( + PVD, Ctx.getSourceManager(), Ctx.getLangOpts(), &PteTyQualifiers); - if (!FD) { - DEBUG_NOTE_DECL_FAIL(PVD, " : invalid func decl"); + if (!PteTyText) { + DEBUG_NOTE_DECL_FAIL(PVD, " : invalid pointee type"); + return {}; + } + + std::optional PVDNameText = PVD->getIdentifier()->getName(); + + if (!PVDNameText) { + DEBUG_NOTE_DECL_FAIL(PVD, " : invalid identifier name"); return {}; } std::stringstream SS; std::optional SpanTyText = createSpanTypeForVarDecl(PVD, Ctx); - std::optional ParmIdentText; - if (!SpanTyText) - return {}; - SS << *SpanTyText; + if (PteTyQualifiers) + // Append qualifiers if they exist: + SS << getSpanTypeText(*PteTyText, PteTyQualifiers); + else + SS << getSpanTypeText(*PteTyText); // Append qualifiers to the type of the parameter: if (PVD->getType().hasQualifiers()) - SS << " " << PVD->getType().getQualifiers().getAsString(); - ParmIdentText = - getVarDeclIdentifierText(PVD, Ctx.getSourceManager(), Ctx.getLangOpts()); - if (!ParmIdentText) - return {}; + SS << ' ' << PVD->getType().getQualifiers().getAsString(); // Append parameter's name: - SS << " " << ParmIdentText->str(); - - FixItList Fixes; - unsigned ParmIdx = 0; - - Fixes.push_back( - FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str())); - for (auto *ParmIter : FD->parameters()) { - if (PVD == ParmIter) - break; - ParmIdx++; - } - if (ParmIdx < FD->getNumParams()) - if (auto OverloadFix = createOverloadsForFixedParams(ParmIdx, *SpanTyText, - FD, Ctx, Handler)) { - Fixes.append(*OverloadFix); - return Fixes; - } - DEBUG_NOTE_DECL_FAIL(PVD, " : invalid number of parameters"); - return {}; + SS << ' ' << PVDNameText->str(); + // Add replacement fix-it: + return {FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str())}; } static FixItList fixVariableWithSpan(const VarDecl *VD, @@ -2244,6 +2279,12 @@ static bool overlapWithMacro(const FixItList &FixIts) { }); } +// Returns true iff `VD` is a parameter of the declaration `D`: +static bool isParameterOf(const VarDecl *VD, const Decl *D) { + return isa(VD) && + VD->getDeclContext() == dyn_cast(D); +} + // Erases variables in `FixItsForVariable`, if such a variable has an unfixable // group mate. A variable `v` is unfixable iff `FixItsForVariable` does not // contain `v`. @@ -2257,8 +2298,7 @@ static void eraseVarsForUnfixableGroupMates( VarGrpRef Grp = VarGrpMgr.getGroupOfVar(VD); if (llvm::any_of(Grp, [&FixItsForVariable](const VarDecl *GrpMember) -> bool { - return FixItsForVariable.find(GrpMember) == - FixItsForVariable.end(); + return !FixItsForVariable.count(GrpMember); })) { // At least one group member cannot be fixed, so we have to erase the // whole group: @@ -2270,6 +2310,45 @@ static void eraseVarsForUnfixableGroupMates( FixItsForVariable.erase(VarToErase); } +// Returns the fix-its that create bounds-safe function overloads for the +// function `D`, if `D`'s parameters will be changed to safe-types through +// fix-its in `FixItsForVariable`. +// +// NOTE: In case `D`'s parameters will be changed but bounds-safe function +// overloads cannot created, the whole group that contains the parameters will +// be erased from `FixItsForVariable`. +static FixItList createFunctionOverloadsForParms( + std::map &FixItsForVariable /* mutable */, + const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD, + const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) { + FixItList FixItsSharedByParms{}; + std::vector ParmsNeedFixMask(FD->getNumParams(), false); + const VarDecl *FirstParmNeedsFix = nullptr; + + for (unsigned i = 0; i < FD->getNumParams(); i++) + if (FixItsForVariable.count(FD->getParamDecl(i))) { + ParmsNeedFixMask[i] = true; + FirstParmNeedsFix = FD->getParamDecl(i); + } + if (FirstParmNeedsFix) { + // In case at least one parameter needs to be fixed: + std::optional OverloadFixes = + createOverloadsForFixedParams(ParmsNeedFixMask, S, FD, Ctx, Handler); + + if (OverloadFixes) { + FixItsSharedByParms.append(*OverloadFixes); + } else { + // Something wrong in generating `OverloadFixes`, need to remove the + // whole group, where parameters are in, from `FixItsForVariable` (Note + // that all parameters should be in the same group): + for (auto *Member : VarGrpMgr.getGroupOfVar(FirstParmNeedsFix)) + FixItsForVariable.erase(Member); + } + } + return FixItsSharedByParms; +} + +// Constructs self-contained fix-its for each variable in `FixablesForAllVars`. static std::map getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S, ASTContext &Ctx, @@ -2323,21 +2402,36 @@ getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S, // `FixItsForVariable` iff it can be fixed and all its group mates can be // fixed. + // Fix-its of bounds-safe overloads of `D` are shared by parameters of `D`. + // That is, when fixing multiple parameters in one step, these fix-its will + // be applied only once (instead of being applied per parameter). + FixItList FixItsSharedByParms{}; + + if (auto *FD = dyn_cast(D)) + FixItsSharedByParms = createFunctionOverloadsForParms( + FixItsForVariable, VarGrpMgr, FD, S, Ctx, Handler); + // The map that maps each variable `v` to fix-its for the whole group where // `v` is in: std::map FinalFixItsForVariable{ FixItsForVariable}; for (auto &[Var, Ignore] : FixItsForVariable) { - const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Var); + bool AnyParm = false; + const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Var, &AnyParm); for (const VarDecl *GrpMate : VarGroupForVD) { if (Var == GrpMate) continue; if (FixItsForVariable.count(GrpMate)) - FinalFixItsForVariable[Var].insert(FinalFixItsForVariable[Var].end(), - FixItsForVariable[GrpMate].begin(), - FixItsForVariable[GrpMate].end()); + FinalFixItsForVariable[Var].append(FixItsForVariable[GrpMate]); + } + if (AnyParm) { + // This assertion should never fail. Otherwise we have a bug. + assert(!FixItsSharedByParms.empty() && + "Should not try to fix a parameter that does not belong to a " + "FunctionDecl"); + FinalFixItsForVariable[Var].append(FixItsSharedByParms); } } // Fix-its that will be applied in one step shall NOT: @@ -2367,17 +2461,30 @@ getNaiveStrategy(const llvm::SmallVectorImpl &UnsafeVars) { class VariableGroupsManagerImpl : public VariableGroupsManager { const std::vector Groups; const std::map &VarGrpMap; + const llvm::SetVector &GrpsUnionForParms; public: VariableGroupsManagerImpl( const std::vector &Groups, - const std::map &VarGrpMap) - : Groups(Groups), VarGrpMap(VarGrpMap) {} + const std::map &VarGrpMap, + const llvm::SetVector &GrpsUnionForParms) + : Groups(Groups), VarGrpMap(VarGrpMap), + GrpsUnionForParms(GrpsUnionForParms) {} + + VarGrpRef getGroupOfVar(const VarDecl *Var, bool *HasParm) const override { + if (GrpsUnionForParms.contains(Var)) { + if (HasParm) + *HasParm = true; + return GrpsUnionForParms.getArrayRef(); + } + if (HasParm) + *HasParm = false; + + auto It = VarGrpMap.find(Var); - VarGrpRef getGroupOfVar(const VarDecl *Var) const override { - auto I = VarGrpMap.find(Var); - assert(I != VarGrpMap.end()); - return Groups[I->second]; + if (It == VarGrpMap.end()) + return std::nullopt; + return Groups[It->second]; } }; @@ -2563,6 +2670,9 @@ void clang::checkUnsafeBufferUsage(const Decl *D, // variables belong to. Group indexes refer to the elements in `Groups`. // `VarGrpMap` is complete in that every variable that needs fix is in it. std::map VarGrpMap; + // The union group over the ones in "Groups" that contain parameters of `D`: + llvm::SetVector + GrpsUnionForParms; // these variables need to be fixed in one step // Group Connected Components for Unsafe Vars // (Dependencies based on pointer assignments) @@ -2585,11 +2695,17 @@ void clang::checkUnsafeBufferUsage(const Decl *D, } } } + + bool HasParm = false; unsigned GrpIdx = Groups.size() - 1; for (const VarDecl *V : VarGroup) { VarGrpMap[V] = GrpIdx; + if (!HasParm && isParameterOf(V, D)) + HasParm = true; } + if (HasParm) + GrpsUnionForParms.insert(VarGroup.begin(), VarGroup.end()); } } @@ -2614,7 +2730,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D, for (auto I = FixablesForAllVars.byVar.begin(); I != FixablesForAllVars.byVar.end();) { // Note `VisitedVars` contain all the variables in the graph: - if (VisitedVars.find((*I).first) == VisitedVars.end()) { + if (!VisitedVars.count((*I).first)) { // no such var in graph: I = FixablesForAllVars.byVar.erase(I); } else @@ -2622,11 +2738,14 @@ void clang::checkUnsafeBufferUsage(const Decl *D, } Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars); - VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap); + VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms); - FixItsForVariableGroup = - getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D, - Tracker, Handler, VarGrpMgr); + if (isa(D)) + // The only case where `D` is not a `NamedDecl` is when `D` is a + // `BlockDecl`. Let's not fix variables in blocks for now + FixItsForVariableGroup = + getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D, + Tracker, Handler, VarGrpMgr); for (const auto &G : UnsafeOps.noVar) { Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false); @@ -2637,7 +2756,8 @@ void clang::checkUnsafeBufferUsage(const Decl *D, Handler.handleUnsafeVariableGroup(VD, VarGrpMgr, FixItsIt != FixItsForVariableGroup.end() ? std::move(FixItsIt->second) - : FixItList{}); + : FixItList{}, + D); for (const auto &G : WarningGadgets) { Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true); } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index addaca4db6d8b..77bb560eb6288 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2267,21 +2267,30 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { void handleUnsafeVariableGroup(const VarDecl *Variable, const VariableGroupsManager &VarGrpMgr, - FixItList &&Fixes) override { + FixItList &&Fixes, const Decl *D) 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(); if (!Fixes.empty()) { - const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable); + assert(isa(D) && + "Fix-its are generated only for `NamedDecl`s"); + const NamedDecl *ND = cast(D); + bool BriefMsg = false; + // If the variable group involves parameters, the diagnostic message will + // NOT explain how the variables are grouped as the reason is non-trivial + // and irrelavant to users' experience: + const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable, &BriefMsg); unsigned FixItStrategy = 0; // For now we only have 'std::span' strategy - const auto &FD = S.Diag(Variable->getLocation(), - diag::note_unsafe_buffer_variable_fixit_group); + const auto &FD = + S.Diag(Variable->getLocation(), + BriefMsg ? diag::note_unsafe_buffer_variable_fixit_together + : diag::note_unsafe_buffer_variable_fixit_group); FD << Variable << FixItStrategy; FD << listVariableGroupAsString(Variable, VarGroupForVD) - << (VarGroupForVD.size() > 1); + << (VarGroupForVD.size() > 1) << ND; for (const auto &F : Fixes) { FD << F; } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp new file mode 100644 index 0000000000000..b3210e93c98fa --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp @@ -0,0 +1,317 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fblocks -fsafe-buffer-usage-suggestions -verify %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fblocks -fsafe-buffer-usage-suggestions \ +// RUN: %s 2>&1 | FileCheck %s + +// FIXME: what about possible diagnostic message non-determinism? + +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]: +void parmsNoFix(int *p, int *q) { + int * a = new int[10]; + int * b = new int[10]; //expected-warning{{'b' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'b' to 'std::span' to preserve bounds information}} + + a = p; + a = q; + b[5] = 5; // expected-note{{used in buffer access here}} +} + +// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:21-[[@LINE+2]]:27}:"std::span p" +// CHECK: fix-it:{{.*}}:{[[@LINE+14]]:2-[[@LINE+14]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void parmsSingleton(int *p) {return parmsSingleton(std::span(p, <# size #>));}\n" +void parmsSingleton(int *p) { //expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information}} + // CHECK: fix-it:{{.*}}:{[[@LINE+3]]:3-[[@LINE+3]]:12}:"std::span a" + // CHECK: fix-it:{{.*}}:{[[@LINE+2]]:13-[[@LINE+2]]:13}:"{" + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:24-[[@LINE+1]]:24}:", 10}" + int * a = new int[10]; + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span b" + int * b; //expected-warning{{'b' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'b' to 'std::span' to preserve bounds information, and change 'a' to 'std::span' to propagate bounds information between them}} + + b = a; + b[5] = 5; // expected-note{{used in buffer access here}} + p[5] = 5; // expected-note{{used in buffer access here}} +} + + +// Parameters other than `r` all will be fixed +// CHECK: fix-it:{{.*}}:{[[@LINE+15]]:24-[[@LINE+15]]:30}:"std::span p" +// CHECK fix-it:{{.*}}:{[[@LINE+14]]:32-[[@LINE+14]]:39}:"std::span q" +// CHECK: fix-it:{{.*}}:{[[@LINE+13]]:41-[[@LINE+13]]:48}:"std::span a" +// CHECK: fix-it:{{.*}}:{[[@LINE+23]]:2-[[@LINE+23]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span(p, <# size #>), std::span(q, <# size #>), std::span(a, <# size #>), r);}\n" + +// repeat 2 more times as each of the 3 fixing parameters generates the set of fix-its above. + +// CHECK: fix-it:{{.*}}:{[[@LINE+8]]:24-[[@LINE+8]]:30}:"std::span p" +// CHECK fix-it:{{.*}}:{[[@LINE+7]]:32-[[@LINE+7]]:39}:"std::span q" +// CHECK: fix-it:{{.*}}:{[[@LINE+6]]:41-[[@LINE+6]]:48}:"std::span a" +// CHECK: fix-it:{{.*}}:{[[@LINE+16]]:2-[[@LINE+16]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span(p, <# size #>), std::span(q, <# size #>), std::span(a, <# size #>), r);}\n" +// CHECK: fix-it:{{.*}}:{[[@LINE+4]]:24-[[@LINE+4]]:30}:"std::span p" +// CHECK fix-it:{{.*}}:{[[@LINE+3]]:32-[[@LINE+3]]:39}:"std::span q" +// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:41-[[@LINE+2]]:48}:"std::span a" +// CHECK: fix-it:{{.*}}:{[[@LINE+12]]:2-[[@LINE+12]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span(p, <# size #>), std::span(q, <# size #>), std::span(a, <# size #>), r);}\n" +void * multiParmAllFix(int *p, int **q, int a[], int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'a' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'a' to safe types to make function 'multiParmAllFix' bounds-safe}} \ + expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'a' to safe types to make function 'multiParmAllFix' bounds-safe}} \ + expected-note{{change type of 'a' to 'std::span' to preserve bounds information, and change 'p' and 'q' to safe types to make function 'multiParmAllFix' bounds-safe}} + int tmp; + + tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = a[5]; // expected-note{{used in buffer access here}} + if (++q) {} // expected-note{{used in pointer arithmetic here}} + return r; +} + +void * multiParmAllFix(int *p, int **q, int a[], int * r); +// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} " +// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:58-[[@LINE-2]]:58}:";\nvoid * multiParmAllFix(std::span p, std::span q, std::span a, int * r)" + +// Fixing local variables implicates fixing parameters +void multiParmLocalAllFix(int *p, int * r) { + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:28-[[@LINE-1]]:34}:"std::span p" + // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:36-[[@LINE-2]]:43}:"std::span r" + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span x" + int * x; // expected-warning{{'x' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'x' to 'std::span' to preserve bounds information, and change 'p', 'z', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}} + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span z" + int * z; // expected-warning{{'z' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'z' to 'std::span' to preserve bounds information, and change 'x', 'p', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}} + int * y; + + x = p; + y = x; + // `x` needs to be fixed so does the pointer assigned to `x`, i.e.,`p` + x[5] = 5; // expected-note{{used in buffer access here}} + z = r; + // `z` needs to be fixed so does the pointer assigned to `z`, i.e.,`r` + z[5] = 5; // expected-note{{used in buffer access here}} + // Since `p` and `r` are parameters need to be fixed together, + // fixing `x` involves fixing all `p`, `r` and `z`. Similar for + // fixing `z`. +} +// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void multiParmLocalAllFix(int *p, int * r) {return multiParmLocalAllFix(std::span(p, <# size #>), std::span(r, <# size #>));}\n" + + +// Fixing parameters implicates fixing local variables +// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:29-[[@LINE+2]]:35}:"std::span p" +// CHECK: fix-it:{{.*}}:{[[@LINE+1]]:37-[[@LINE+1]]:44}:"std::span r" +void multiParmLocalAllFix2(int *p, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'x', 'r', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'x', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}} + int * x = new int[10]; + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span x" + // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{" + // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}" + int * z = new int[10]; + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span z" + // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{" + // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}" + int * y; + + p = x; + y = x; + p[5] = 5; // expected-note{{used in buffer access here}} + r = z; + r[5] = 5; // expected-note{{used in buffer access here}} +} +// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void multiParmLocalAllFix2(int *p, int * r) {return multiParmLocalAllFix2(std::span(p, <# size #>), std::span(r, <# size #>));}\n" + + +// No fix emitted for any of the parameter since parameter `r` cannot be fixed +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]] +void noneParmFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + r++; // expected-note{{used in pointer arithmetic here}} + tmp = r[5]; // expected-note{{used in buffer access here}} +} +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]] + +// To show what if the `r` in `noneParmFix` can be fixed: +void noneParmFix_control(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'r' to safe types to make function 'noneParmFix_control' bounds-safe}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'r' to safe types to make function 'noneParmFix_control' bounds-safe}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p' and 'q' to safe types to make function 'noneParmFix_control' bounds-safe}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + if (++r) {} // expected-note{{used in pointer arithmetic here}} + tmp = r[5]; // expected-note{{used in buffer access here}} +} + + +// No fix emitted for any of the parameter since local variable `l` cannot be fixed. +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]] +void noneParmOrLocalFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + // `l` and `r` must be fixed together while all parameters must be fixed together as well: + int * l; l = r; // expected-warning{{'l' is an unsafe pointer used for buffer access}} + + l++; // expected-note{{used in pointer arithmetic here}} +} +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]] + +// To show what if the `l` can be fixed in `noneParmOrLocalFix`: +void noneParmOrLocalFix_control(int * p, int * q, int * r) {// \ + expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q', 'r', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'r', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'q', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'q', and 'r' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} + l = r; + if (++l){}; // expected-note{{used in pointer arithmetic here}} +} + + +// No fix emitted for any of the parameter since local variable `l` cannot be fixed. +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]] +void noneParmOrLocalFix2(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + + int * a; a = r; + int * b; b = a; + int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}} + + // `a, b, l` and parameters must be fixed together but `l` can't be fixed: + l++; // expected-note{{used in pointer arithmetic here}} +} +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]] + +// To show what if the `l` can be fixed in `noneParmOrLocalFix2`: +void noneParmOrLocalFix2_control(int * p, int * q, int * r) { // \ + expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'q', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + + int * a; a = r; + int * b; b = a; + int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'q', 'r', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} + + l = b; + if(++l){} // expected-note{{used in pointer arithmetic here}} +} + +// No fix emitted for any of the parameter since local variable `l` cannot be fixed +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]] +void noneParmOrLocalFix3(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + + int * a; a = r; + int * b; b = a; + int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}} + + l++; // expected-note{{used in pointer arithmetic here}} + + int * x; x = p; // expected-warning{{'x' is an unsafe pointer used for buffer access}} + tmp = x[5]; // expected-note{{used in buffer access here}} +} +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]] + +void noneParmOrLocalFix3_control(int * p, int * q, int * r) { // \ + expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'x', 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'x', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'x', 'q', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + + int * a; a = r; + int * b; b = a; + int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'x', 'q', 'r', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} + + l = b; + if (++l){}; // expected-note{{used in pointer arithmetic here}} + + int * x; // expected-warning{{'x' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'x' to 'std::span' to preserve bounds information, and change 'p', 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} + x = p; + tmp = x[5]; // expected-note{{used in buffer access here}} +} + + +// No fix emitted for any of the parameter but some local variables will get fixed +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]] +void noneParmSomeLocalFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \ + expected-warning{{'q' is an unsafe pointer used for buffer access}} \ + expected-warning{{'r' is an unsafe pointer used for buffer access}} + int tmp = p[5]; // expected-note{{used in buffer access here}} + tmp = q[5]; // expected-note{{used in buffer access here}} + tmp = r[5]; // expected-note{{used in buffer access here}} + + int * a; a = r; + int * b; b = a; + int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}} + + l++; // expected-note{{used in pointer arithmetic here}} + + //`x` and `y` can be fixed + int * x = new int[10]; + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span x" + // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{" + // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}" + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span y" + int * y; // expected-warning{{'y' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'y' to 'std::span' to preserve bounds information, and change 'x' to 'std::span' to propagate bounds information between them}} + y = x; + tmp = y[5]; // expected-note{{used in buffer access here}} +} +// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]] + +// Test that other parameters of (lambdas and blocks) do not interfere +// the grouping of variables of the function. +// CHECK: fix-it:{{.*}}:{[[@LINE+3]]:30-[[@LINE+3]]:37}:"std::span p" +// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:39-[[@LINE+2]]:46}:"std::span q" +// CHECK: fix-it:{{.*}}:{[[@LINE+20]]:2-[[@LINE+20]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void parmsFromLambdaAndBlock(int * p, int * q) {return parmsFromLambdaAndBlock(std::span(p, <# size #>), std::span(q, <# size #>));}\n" +void parmsFromLambdaAndBlock(int * p, int * q) { + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span a" + int * a; // expected-warning{{'a' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'a' to 'std::span' to preserve bounds information, and change 'p', 'b', and 'q' to safe types to make function 'parmsFromLambdaAndBlock' bounds-safe}} + // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span b" + int * b; // expected-warning{{'b' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'b' to 'std::span' to preserve bounds information, and change 'a', 'p', and 'q' to safe types to make function 'parmsFromLambdaAndBlock' bounds-safe}} + auto Lamb = [](int * x) -> void { // expected-warning{{'x' is an unsafe pointer used for buffer access}} + x[5] = 5; // expected-note{{used in buffer access here}} + }; + + void (^Blk)(int*) = ^(int * y) { // expected-warning{{'y' is an unsafe pointer used for buffer access}} + y[5] = 5; // expected-note{{used in buffer access here}} + }; + + a = p; + b = q; + a[5] = 5; // expected-note{{used in buffer access here}} + b[5] = 5; // expected-note{{used in buffer access here}} +} diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp index 4d3fac80b804e..9a54b13c90342 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp @@ -29,8 +29,7 @@ void twoParms(int *p, int * q) { int tmp; tmp = p[5] + q[5]; } -// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(std::span(p, <# size #>), q);}\n" -// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:2-[[@LINE-2]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(p, std::span(q, <# size #>));}\n" +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(std::span(p, <# size #>), std::span(q, <# size #>));}\n" void ptrToConst(const int * x) { // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span x" @@ -100,22 +99,33 @@ namespace { // namned } NAMED_S; + // FIXME: `decltype(ANON_S)` represents an unnamed type but it can // be referred as "`decltype(ANON_S)`", so the analysis should // fix-it. - void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r, - decltype(NAMED_S) ** rr) { - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:26-[[@LINE-2]]:41}:"std::span p" - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:65-[[@LINE-3]]:86}:"std::span r" - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:26-[[@LINE-3]]:49}:"std::span rr" + // As parameter `q` cannot be fixed, fixes to parameters are all being given up. + void decltypeSpecifierAnon(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r, + decltype(NAMED_S) ** rr) { + // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:30-[[@LINE-2]]:45}:"std::span p" + // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:47-[[@LINE-3]]:67}:"std::span q" + // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:69-[[@LINE-4]]:90}:"std::span r" + // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:53}:"std::span rr" if (++p) {} if (++q) {} if (++r) {} if (++rr) {} } - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span(p, <# size #>), q, r, rr);}\n - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, std::span(r, <# size #>), rr);}\n" - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:4-[[@LINE-3]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, r, std::span(rr, <# size #>));}\n" + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifierAnon(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n decltype(NAMED_S) ** rr) {return decltypeSpecifierAnon(std::span(p, <# size #>), std::span(q, <# size #>), std::span(r, <# size #>), std::span(rr, <# size #>));}\n + + void decltypeSpecifier(decltype(C) * p, decltype(NAMED_S) * r, decltype(NAMED_S) ** rr) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:26-[[@LINE-1]]:41}:"std::span p" + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:43-[[@LINE-2]]:64}:"std::span r" + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:66-[[@LINE-3]]:89}:"std::span rr" + if (++p) {} + if (++r) {} + if (++rr) {} + } + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(NAMED_S) * r, decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span(p, <# size #>), std::span(r, <# size #>), std::span(rr, <# size #>));}\n #define MACRO_TYPE(T) long T @@ -125,8 +135,7 @@ namespace { int tmp = p[5]; tmp = q[5]; } - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span(p, <# size #>), q);}\n" - // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(p, std::span(q, <# size #>));}\n" + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span(p, <# size #>), std::span(q, <# size #>));}\n" } // The followings test various declarators: diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp index 06c852c2fab41..c5f0a9ef92937 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -36,7 +36,7 @@ void testIncrement(char *p) { // expected-warning{{'p' is an unsafe pointer used void * voidPtrCall(void); char * charPtrCall(void); -void testArraySubscripts(int *p, int **pp) { // expected-note{{change type of 'pp' to 'std::span' to preserve bounds information}} +void testArraySubscripts(int *p, int **pp) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'pp' is an unsafe pointer used for buffer access}} foo(p[1], // expected-note{{used in buffer access here}} @@ -97,7 +97,6 @@ void testUnevaluatedContext(int * p) {// no-warning sizeof(decltype(p[1]))); // no-warning } -// expected-note@+1{{change type of 'a' to 'std::span' to preserve bounds information}} void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}