[clang] 33f6161 - [-Wunsafe-buffer-usage] Group parameter fix-its
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 21 12:45:42 PDT 2023
Author: Ziqing Luo
Date: 2023-09-21T12:45:30-07:00
New Revision: 33f6161d9eaaa7c5a01ee8e50dec440d3052d34a
URL: https://github.com/llvm/llvm-project/commit/33f6161d9eaaa7c5a01ee8e50dec440d3052d34a
DIFF: https://github.com/llvm/llvm-project/commit/33f6161d9eaaa7c5a01ee8e50dec440d3052d34a.diff
LOG: [-Wunsafe-buffer-usage] Group parameter fix-its
For a function `F` whose parameters need to be fixed, we group fix-its
of F's parameters together so that either all of the parameters get
fixed or none of them gets fixed.
Reviewed by: NoQ (Artem Dergachev), t-rasmud (Rashmi Mudduluru), jkorous (Jan Korous)
Differential revision: https://reviews.llvm.org/D153059
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index c865b2e8bdb3794..e8a93164302a47d 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 0ac4df8edb242f6..9613cca63193ffd 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 df89ab436380857..348585ef16e967b 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<StringRef> 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<Qualifiers> Quals = std::nullopt) {
+ const char *const SpanOpen = "std::span<";
+
+ if (Quals)
+ return SpanOpen + EltTyText.str() + ' ' + Quals->getAsString() + '>';
+ return SpanOpen + EltTyText.str() + '>';
+}
+
std::optional<FixItList>
DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
const VarDecl *VD = dyn_cast<VarDecl>(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<FixItList>
-createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
+createOverloadsForFixedParams(const std::vector<bool> &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<std::string> NewTysTexts(NumParms);
+
+ for (unsigned i = 0; i < NumParms; i++) {
+ if (!ParmsMask[i])
+ continue;
+
+ std::optional<Qualifiers> PteTyQuals = std::nullopt;
+ std::optional<std::string> 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<std::string> {
+ [&SM, &LangOpts, &NewTysTexts,
+ &ParmsMask](const FunctionDecl *FD) -> std::optional<std::string> {
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<std::string> {
+ [&Handler, &SM, &LangOpts, &NewTysTexts,
+ &ParmsMask](const FunctionDecl *FD) -> std::optional<std::string> {
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<FunctionDecl>(PVD->getDeclContext());
+ std::optional<Qualifiers> PteTyQualifiers = std::nullopt;
+ std::optional<std::string> 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<StringRef> PVDNameText = PVD->getIdentifier()->getName();
+
+ if (!PVDNameText) {
+ DEBUG_NOTE_DECL_FAIL(PVD, " : invalid identifier name");
return {};
}
std::stringstream SS;
std::optional<std::string> SpanTyText = createSpanTypeForVarDecl(PVD, Ctx);
- std::optional<StringRef> 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<ParmVarDecl>(VD) &&
+ VD->getDeclContext() == dyn_cast<DeclContext>(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<const VarDecl *, FixItList> &FixItsForVariable /* mutable */,
+ const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD,
+ const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) {
+ FixItList FixItsSharedByParms{};
+ std::vector<bool> 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<FixItList> 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<const VarDecl *, FixItList>
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<FunctionDecl>(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<const VarDecl *, FixItList> 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<const VarDecl *> &UnsafeVars) {
class VariableGroupsManagerImpl : public VariableGroupsManager {
const std::vector<VarGrpTy> Groups;
const std::map<const VarDecl *, unsigned> &VarGrpMap;
+ const llvm::SetVector<const VarDecl *> &GrpsUnionForParms;
public:
VariableGroupsManagerImpl(
const std::vector<VarGrpTy> &Groups,
- const std::map<const VarDecl *, unsigned> &VarGrpMap)
- : Groups(Groups), VarGrpMap(VarGrpMap) {}
+ const std::map<const VarDecl *, unsigned> &VarGrpMap,
+ const llvm::SetVector<const VarDecl *> &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<const VarDecl *, unsigned> VarGrpMap;
+ // The union group over the ones in "Groups" that contain parameters of `D`:
+ llvm::SetVector<const VarDecl *>
+ 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<NamedDecl>(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 addaca4db6d8b2a..77bb560eb6288f7 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<NamedDecl>(D) &&
+ "Fix-its are generated only for `NamedDecl`s");
+ const NamedDecl *ND = cast<NamedDecl>(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 000000000000000..b3210e93c98fa5d
--- /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<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+14]]:2-[[@LINE+14]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void parmsSingleton(int *p) {return parmsSingleton(std::span<int>(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<int> 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<int> 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<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+14]]:32-[[@LINE+14]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+13]]:41-[[@LINE+13]]:48}:"std::span<int> 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<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(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<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+7]]:32-[[@LINE+7]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+6]]:41-[[@LINE+6]]:48}:"std::span<int> 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<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(a, <# size #>), r);}\n"
+// CHECK: fix-it:{{.*}}:{[[@LINE+4]]:24-[[@LINE+4]]:30}:"std::span<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+3]]:32-[[@LINE+3]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:41-[[@LINE+2]]:48}:"std::span<int> 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<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(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<int> p, std::span<int *> q, std::span<int> 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<int> p"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:36-[[@LINE-2]]:43}:"std::span<int> r"
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> 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<int> 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<int>(p, <# size #>), std::span<int>(r, <# size #>));}\n"
+
+
+// Fixing parameters implicates fixing local variables
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:29-[[@LINE+2]]:35}:"std::span<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+1]]:37-[[@LINE+1]]:44}:"std::span<int> 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<int> 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<int> 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<int>(p, <# size #>), std::span<int>(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<int> 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<int> 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<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:39-[[@LINE+2]]:46}:"std::span<int> 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<int>(p, <# size #>), std::span<int>(q, <# size #>));}\n"
+void parmsFromLambdaAndBlock(int * p, int * q) {
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> 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<int> 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 4d3fac80b804e35..9a54b13c903429c 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<int>(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<int>(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<int>(p, <# size #>), std::span<int>(q, <# size #>));}\n"
void ptrToConst(const int * x) {
// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span<int const> 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<decltype(C)> p"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:65-[[@LINE-3]]:86}:"std::span<decltype(NAMED_S)> r"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:26-[[@LINE-3]]:49}:"std::span<decltype(NAMED_S) *> 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<decltype(C)> p"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:47-[[@LINE-3]]:67}:"std::span<decltype(ANON_S)> q"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:69-[[@LINE-4]]:90}:"std::span<decltype(NAMED_S)> r"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-4]]:30-[[@LINE-4]]:53}:"std::span<decltype(NAMED_S) *> 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<decltype(C)>(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<decltype(NAMED_S)>(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<decltype(NAMED_S) *>(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<decltype(C)>(p, <# size #>), std::span<decltype(ANON_S)>(q, <# size #>), std::span<decltype(NAMED_S)>(r, <# size #>), std::span<decltype(NAMED_S) *>(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<decltype(C)> p"
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:43-[[@LINE-2]]:64}:"std::span<decltype(NAMED_S)> r"
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:66-[[@LINE-3]]:89}:"std::span<decltype(NAMED_S) *> 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<decltype(C)>(p, <# size #>), std::span<decltype(NAMED_S)>(r, <# size #>), std::span<decltype(NAMED_S) *>(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<unsigned MACRO_TYPE(int)>(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<unsigned MACRO_TYPE(long)>(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<unsigned MACRO_TYPE(int)>(p, <# size #>), std::span<unsigned MACRO_TYPE(long)>(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 06c852c2fab41bc..c5f0a9ef929371b 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 at -1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning at -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 at +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 at -1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning at -2{{'q' is an unsafe pointer used for buffer access}}
More information about the cfe-commits
mailing list