[clang] 644ac2a - [-Wunsafe-buffer-usage] Introduce std::array fixits (#80084)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 12 15:52:24 PST 2024
Author: jkorous-apple
Date: 2024-02-12T15:52:20-08:00
New Revision: 644ac2a018c9bf83c9ba256074e552ad7f1fe941
URL: https://github.com/llvm/llvm-project/commit/644ac2a018c9bf83c9ba256074e552ad7f1fe941
DIFF: https://github.com/llvm/llvm-project/commit/644ac2a018c9bf83c9ba256074e552ad7f1fe941.diff
LOG: [-Wunsafe-buffer-usage] Introduce std::array fixits (#80084)
Array subscript on a const size array is not bounds-checked. The idiomatic
replacement is std::array which is bounds-safe in hardened mode of libc++.
This commit extends the fixit-producing machine to consider std::array as a
transformation target type and teaches it to handle the array subscript on const
size arrays with a trivial (empty) fixit.
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.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-debug.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 aca1ad998822c5..5d16dcc824c50c 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -42,6 +42,43 @@ class VariableGroupsManager {
virtual VarGrpRef getGroupOfParms() const =0;
};
+// FixitStrategy is a map from variables to the way we plan to emit fixes for
+// these variables. It is figured out gradually by trying
diff erent fixes
+// for
diff erent variables depending on gadgets in which these variables
+// participate.
+class FixitStrategy {
+public:
+ enum class Kind {
+ Wontfix, // We don't plan to emit a fixit for this variable.
+ Span, // We recommend replacing the variable with std::span.
+ Iterator, // We recommend replacing the variable with std::span::iterator.
+ Array, // We recommend replacing the variable with std::array.
+ Vector // We recommend replacing the variable with std::vector.
+ };
+
+private:
+ using MapTy = llvm::DenseMap<const VarDecl *, Kind>;
+
+ MapTy Map;
+
+public:
+ FixitStrategy() = default;
+ FixitStrategy(const FixitStrategy &) = delete; // Let's avoid copies.
+ FixitStrategy &operator=(const FixitStrategy &) = delete;
+ FixitStrategy(FixitStrategy &&) = default;
+ FixitStrategy &operator=(FixitStrategy &&) = default;
+
+ void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
+
+ Kind lookup(const VarDecl *VD) const {
+ auto I = Map.find(VD);
+ if (I == Map.end())
+ return Kind::Wontfix;
+
+ return I->second;
+ }
+};
+
/// The interface that lets the caller handle unsafe buffer usage analysis
/// results by overriding this class's handle... methods.
class UnsafeBufferUsageHandler {
@@ -75,9 +112,11 @@ class UnsafeBufferUsageHandler {
///
/// `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, const Decl *D) = 0;
+ virtual void
+ handleUnsafeVariableGroup(const VarDecl *Variable,
+ const VariableGroupsManager &VarGrpMgr,
+ FixItList &&Fixes, const Decl *D,
+ const FixitStrategy &VarTargetTypes) = 0;
#ifndef NDEBUG
public:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 83b89d1449f420..754733a6c5fffd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12128,9 +12128,9 @@ def warn_unsafe_buffer_operation : Warning<
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">;
+ "change type of %0 to '%select{std::span' to preserve bounds information|std::array' to label it for hardening|std::span::iterator' to preserve bounds information}1%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"
+ "change type of %0 to '%select{std::span' to preserve bounds information|std::array' to label it for hardening|std::span::iterator' to preserve bounds information}1"
"%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">;
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a6dcf16b928e26..3c2a6fd81b1d8f 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -12,10 +12,14 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
#include <memory>
#include <optional>
#include <queue>
@@ -407,9 +411,6 @@ using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
// Convenience typedef.
using FixItList = SmallVector<FixItHint, 4>;
-
-// Defined below.
-class Strategy;
} // namespace
namespace {
@@ -486,7 +487,7 @@ class FixableGadget : public Gadget {
/// Returns a fixit that would fix the current gadget according to
/// the current strategy. Returns std::nullopt if the fix cannot be produced;
/// returns an empty list if no fixes are necessary.
- virtual std::optional<FixItList> getFixits(const Strategy &) const {
+ virtual std::optional<FixItList> getFixits(const FixitStrategy &) const {
return std::nullopt;
}
@@ -737,7 +738,8 @@ class PointerInitGadget : public FixableGadget {
return stmt(PtrInitStmt);
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override {
// FIXME: This needs to be the entire DeclStmt, assuming that this method
@@ -789,7 +791,8 @@ class PointerAssignmentGadget : public FixableGadget {
return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override {
// FIXME: This should be the binary operator, assuming that this method
@@ -892,7 +895,8 @@ class ULCArraySubscriptGadget : public FixableGadget {
return expr(isInUnspecifiedLvalueContext(Target));
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return Node; }
@@ -932,7 +936,8 @@ class UPCStandalonePointerGadget : public FixableGadget {
return stmt(isInUnspecifiedPointerContext(target));
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return Node; }
@@ -976,7 +981,8 @@ class PointerDereferenceGadget : public FixableGadget {
virtual const Stmt *getBaseStmt() const final { return Op; }
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
};
// Represents expressions of the form `&DRE[any]` in the Unspecified Pointer
@@ -1009,7 +1015,8 @@ class UPCAddressofArraySubscriptGadget : public FixableGadget {
.bind(UPCAddressofArraySubscriptTag)))));
}
- virtual std::optional<FixItList> getFixits(const Strategy &) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &) const override;
virtual const Stmt *getBaseStmt() const override { return Node; }
@@ -1088,46 +1095,6 @@ class DeclUseTracker {
};
} // namespace
-namespace {
-// Strategy is a map from variables to the way we plan to emit fixes for
-// these variables. It is figured out gradually by trying
diff erent fixes
-// for
diff erent variables depending on gadgets in which these variables
-// participate.
-class Strategy {
-public:
- enum class Kind {
- Wontfix, // We don't plan to emit a fixit for this variable.
- Span, // We recommend replacing the variable with std::span.
- Iterator, // We recommend replacing the variable with std::span::iterator.
- Array, // We recommend replacing the variable with std::array.
- Vector // We recommend replacing the variable with std::vector.
- };
-
-private:
- using MapTy = llvm::DenseMap<const VarDecl *, Kind>;
-
- MapTy Map;
-
-public:
- Strategy() = default;
- Strategy(const Strategy &) = delete; // Let's avoid copies.
- Strategy &operator=(const Strategy &) = delete;
- Strategy(Strategy &&) = default;
- Strategy &operator=(Strategy &&) = default;
-
- void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
-
- Kind lookup(const VarDecl *VD) const {
- auto I = Map.find(VD);
- if (I == Map.end())
- return Kind::Wontfix;
-
- return I->second;
- }
-};
-} // namespace
-
-
// Representing a pointer type expression of the form `++Ptr` in an Unspecified
// Pointer Context (UPC):
class UPCPreIncrementGadget : public FixableGadget {
@@ -1159,7 +1126,8 @@ class UPCPreIncrementGadget : public FixableGadget {
).bind(UPCPreIncrementTag)))));
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return Node; }
@@ -1204,7 +1172,8 @@ class UUCAddAssignGadget : public FixableGadget {
// clang-format on
}
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return Node; }
@@ -1254,7 +1223,8 @@ class DerefSimplePtrArithFixableGadget : public FixableGadget {
// clang-format on
}
- virtual std::optional<FixItList> getFixits(const Strategy &s) const final;
+ virtual std::optional<FixItList>
+ getFixits(const FixitStrategy &s) const final;
// TODO remove this method from FixableGadget interface
virtual const Stmt *getBaseStmt() const final { return nullptr; }
@@ -1464,38 +1434,40 @@ bool clang::internal::anyConflict(const SmallVectorImpl<FixItHint> &FixIts,
}
std::optional<FixItList>
-PointerAssignmentGadget::getFixits(const Strategy &S) const {
+PointerAssignmentGadget::getFixits(const FixitStrategy &S) const {
const auto *LeftVD = cast<VarDecl>(PtrLHS->getDecl());
const auto *RightVD = cast<VarDecl>(PtrRHS->getDecl());
switch (S.lookup(LeftVD)) {
- case Strategy::Kind::Span:
- if (S.lookup(RightVD) == Strategy::Kind::Span)
- return FixItList{};
- return std::nullopt;
- case Strategy::Kind::Wontfix:
- return std::nullopt;
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
- llvm_unreachable("unsupported strategies for FixableGadgets");
+ case FixitStrategy::Kind::Span:
+ if (S.lookup(RightVD) == FixitStrategy::Kind::Span)
+ return FixItList{};
+ return std::nullopt;
+ case FixitStrategy::Kind::Wontfix:
+ return std::nullopt;
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Array:
+ return std::nullopt;
+ case FixitStrategy::Kind::Vector:
+ llvm_unreachable("unsupported strategies for FixableGadgets");
}
return std::nullopt;
}
std::optional<FixItList>
-PointerInitGadget::getFixits(const Strategy &S) const {
+PointerInitGadget::getFixits(const FixitStrategy &S) const {
const auto *LeftVD = PtrInitLHS;
const auto *RightVD = cast<VarDecl>(PtrInitRHS->getDecl());
switch (S.lookup(LeftVD)) {
- case Strategy::Kind::Span:
- if (S.lookup(RightVD) == Strategy::Kind::Span)
- return FixItList{};
- return std::nullopt;
- case Strategy::Kind::Wontfix:
- return std::nullopt;
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
+ case FixitStrategy::Kind::Span:
+ if (S.lookup(RightVD) == FixitStrategy::Kind::Span)
+ return FixItList{};
+ return std::nullopt;
+ case FixitStrategy::Kind::Wontfix:
+ return std::nullopt;
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Array:
+ return std::nullopt;
+ case FixitStrategy::Kind::Vector:
llvm_unreachable("unsupported strategies for FixableGadgets");
}
return std::nullopt;
@@ -1512,12 +1484,12 @@ static bool isNonNegativeIntegerExpr(const Expr *Expr, const VarDecl *VD,
}
std::optional<FixItList>
-ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
+ULCArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
if (const auto *DRE =
dyn_cast<DeclRefExpr>(Node->getBase()->IgnoreImpCasts()))
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
switch (S.lookup(VD)) {
- case Strategy::Kind::Span: {
+ case FixitStrategy::Kind::Span: {
// If the index has a negative constant value, we give up as no valid
// fix-it can be generated:
@@ -1528,10 +1500,11 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
// no-op is a good fix-it, otherwise
return FixItList{};
}
- case Strategy::Kind::Wontfix:
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
+ case FixitStrategy::Kind::Array:
+ return FixItList{};
+ case FixitStrategy::Kind::Wontfix:
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Vector:
llvm_unreachable("unsupported strategies for FixableGadgets");
}
}
@@ -1542,17 +1515,18 @@ static std::optional<FixItList> // forward declaration
fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node);
std::optional<FixItList>
-UPCAddressofArraySubscriptGadget::getFixits(const Strategy &S) const {
+UPCAddressofArraySubscriptGadget::getFixits(const FixitStrategy &S) const {
auto DREs = getClaimedVarUseSites();
const auto *VD = cast<VarDecl>(DREs.front()->getDecl());
switch (S.lookup(VD)) {
- case Strategy::Kind::Span:
+ case FixitStrategy::Kind::Span:
return fixUPCAddressofArraySubscriptWithSpan(Node);
- case Strategy::Kind::Wontfix:
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
+ case FixitStrategy::Kind::Wontfix:
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Array:
+ return std::nullopt;
+ case FixitStrategy::Kind::Vector:
llvm_unreachable("unsupported strategies for FixableGadgets");
}
return std::nullopt; // something went wrong, no fix-it
@@ -1803,10 +1777,10 @@ getSpanTypeText(StringRef EltTyText,
}
std::optional<FixItList>
-DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
+DerefSimplePtrArithFixableGadget::getFixits(const FixitStrategy &s) const {
const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
- if (VD && s.lookup(VD) == Strategy::Kind::Span) {
+ if (VD && s.lookup(VD) == FixitStrategy::Kind::Span) {
ASTContext &Ctx = VD->getASTContext();
// std::span can't represent elements before its begin()
if (auto ConstVal = Offset->getIntegerConstantExpr(Ctx))
@@ -1866,10 +1840,10 @@ DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
}
std::optional<FixItList>
-PointerDereferenceGadget::getFixits(const Strategy &S) const {
+PointerDereferenceGadget::getFixits(const FixitStrategy &S) const {
const VarDecl *VD = cast<VarDecl>(BaseDeclRefExpr->getDecl());
switch (S.lookup(VD)) {
- case Strategy::Kind::Span: {
+ case FixitStrategy::Kind::Span: {
ASTContext &Ctx = VD->getASTContext();
SourceManager &SM = Ctx.getSourceManager();
// Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0]
@@ -1884,11 +1858,12 @@ PointerDereferenceGadget::getFixits(const Strategy &S) const {
}
break;
}
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
- llvm_unreachable("Strategy not implemented yet!");
- case Strategy::Kind::Wontfix:
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Array:
+ return std::nullopt;
+ case FixitStrategy::Kind::Vector:
+ llvm_unreachable("FixitStrategy not implemented yet!");
+ case FixitStrategy::Kind::Wontfix:
llvm_unreachable("Invalid strategy!");
}
@@ -1897,28 +1872,28 @@ PointerDereferenceGadget::getFixits(const Strategy &S) const {
// Generates fix-its replacing an expression of the form UPC(DRE) with
// `DRE.data()`
-std::optional<FixItList> UPCStandalonePointerGadget::getFixits(const Strategy &S)
- const {
+std::optional<FixItList>
+UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const {
const auto VD = cast<VarDecl>(Node->getDecl());
switch (S.lookup(VD)) {
- case Strategy::Kind::Span: {
- ASTContext &Ctx = VD->getASTContext();
- SourceManager &SM = Ctx.getSourceManager();
- // Inserts the .data() after the DRE
- std::optional<SourceLocation> EndOfOperand =
- getPastLoc(Node, SM, Ctx.getLangOpts());
-
- if (EndOfOperand)
- return FixItList{{FixItHint::CreateInsertion(
- *EndOfOperand, ".data()")}};
- // FIXME: Points inside a macro expansion.
- break;
- }
- case Strategy::Kind::Wontfix:
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
- llvm_unreachable("unsupported strategies for FixableGadgets");
+ case FixitStrategy::Kind::Span: {
+ ASTContext &Ctx = VD->getASTContext();
+ SourceManager &SM = Ctx.getSourceManager();
+ // Inserts the .data() after the DRE
+ std::optional<SourceLocation> EndOfOperand =
+ getPastLoc(Node, SM, Ctx.getLangOpts());
+
+ if (EndOfOperand)
+ return FixItList{{FixItHint::CreateInsertion(*EndOfOperand, ".data()")}};
+ // FIXME: Points inside a macro expansion.
+ break;
+ }
+ case FixitStrategy::Kind::Wontfix:
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Array:
+ return std::nullopt;
+ case FixitStrategy::Kind::Vector:
+ llvm_unreachable("unsupported strategies for FixableGadgets");
}
return std::nullopt;
@@ -1962,14 +1937,14 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
}
std::optional<FixItList>
-UUCAddAssignGadget::getFixits(const Strategy &S) const {
+UUCAddAssignGadget::getFixits(const FixitStrategy &S) const {
DeclUseList DREs = getClaimedVarUseSites();
if (DREs.size() != 1)
return std::nullopt; // In cases of `Ptr += n` where `Ptr` is not a DRE, we
// give up
if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
- if (S.lookup(VD) == Strategy::Kind::Span) {
+ if (S.lookup(VD) == FixitStrategy::Kind::Span) {
FixItList Fixes;
const Stmt *AddAssignNode = getBaseStmt();
@@ -2003,14 +1978,15 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const {
return std::nullopt; // Not in the cases that we can handle for now, give up.
}
-std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const {
+std::optional<FixItList>
+UPCPreIncrementGadget::getFixits(const FixitStrategy &S) const {
DeclUseList DREs = getClaimedVarUseSites();
if (DREs.size() != 1)
return std::nullopt; // In cases of `++Ptr` where `Ptr` is not a DRE, we
// give up
if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
- if (S.lookup(VD) == Strategy::Kind::Span) {
+ if (S.lookup(VD) == FixitStrategy::Kind::Span) {
FixItList Fixes;
std::stringstream SS;
const Stmt *PreIncNode = getBaseStmt();
@@ -2033,7 +2009,6 @@ std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) con
return std::nullopt; // Not in the cases that we can handle for now, give up.
}
-
// For a non-null initializer `Init` of `T *` type, this function returns
// `FixItHint`s producing a list initializer `{Init, S}` as a part of a fix-it
// to output stream.
@@ -2261,7 +2236,7 @@ static bool hasConflictingOverload(const FunctionDecl *FD) {
// }
//
static std::optional<FixItList>
-createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
+createOverloadsForFixedParams(const FixitStrategy &S, const FunctionDecl *FD,
const ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
// FIXME: need to make this conflict checking better:
@@ -2278,9 +2253,9 @@ createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
for (unsigned i = 0; i < NumParms; i++) {
const ParmVarDecl *PVD = FD->getParamDecl(i);
- if (S.lookup(PVD) == Strategy::Kind::Wontfix)
+ if (S.lookup(PVD) == FixitStrategy::Kind::Wontfix)
continue;
- if (S.lookup(PVD) != Strategy::Kind::Span)
+ if (S.lookup(PVD) != FixitStrategy::Kind::Span)
// Not supported, not suppose to happen:
return std::nullopt;
@@ -2291,7 +2266,8 @@ createOverloadsForFixedParams(const Strategy &S, const FunctionDecl *FD,
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.
+ // FIXME: whether we should create std::span type depends on the
+ // FixitStrategy.
NewTysTexts[i] = getSpanTypeText(*PteTyText, PteTyQuals);
ParmsMask[i] = true;
AtLeastOneParmToFix = true;
@@ -2495,10 +2471,103 @@ static FixItList fixVariableWithSpan(const VarDecl *VD,
return fixLocalVarDeclWithSpan(VD, Ctx, getUserFillPlaceHolder(), Handler);
}
+static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
+ UnsafeBufferUsageHandler &Handler) {
+ FixItList FixIts{};
+
+ // Note: the code below expects the declaration to not use any type sugar like
+ // typedef.
+ if (auto CAT = dyn_cast<clang::ConstantArrayType>(D->getType())) {
+ const QualType &ArrayEltT = CAT->getElementType();
+ assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
+ // FIXME: support multi-dimensional arrays
+ if (isa<clang::ArrayType>(ArrayEltT.getCanonicalType()))
+ return {};
+
+ const SourceLocation IdentifierLoc = getVarDeclIdentifierLoc(D);
+
+ // Get the spelling of the element type as written in the source file
+ // (including macros, etc.).
+ auto MaybeElemTypeTxt =
+ getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(),
+ Ctx.getLangOpts());
+ if (!MaybeElemTypeTxt)
+ return {};
+ const llvm::StringRef ElemTypeTxt = MaybeElemTypeTxt->trim();
+
+ // Find the '[' token.
+ std::optional<Token> NextTok = Lexer::findNextToken(
+ IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
+ while (NextTok && !NextTok->is(tok::l_square) &&
+ NextTok->getLocation() <= D->getSourceRange().getEnd())
+ NextTok = Lexer::findNextToken(NextTok->getLocation(),
+ Ctx.getSourceManager(), Ctx.getLangOpts());
+ if (!NextTok)
+ return {};
+ const SourceLocation LSqBracketLoc = NextTok->getLocation();
+
+ // Get the spelling of the array size as written in the source file
+ // (including macros, etc.).
+ auto MaybeArraySizeTxt = getRangeText(
+ {LSqBracketLoc.getLocWithOffset(1), D->getTypeSpecEndLoc()},
+ Ctx.getSourceManager(), Ctx.getLangOpts());
+ if (!MaybeArraySizeTxt)
+ return {};
+ const llvm::StringRef ArraySizeTxt = MaybeArraySizeTxt->trim();
+ if (ArraySizeTxt.empty()) {
+ // FIXME: Support array size getting determined from the initializer.
+ // Examples:
+ // int arr1[] = {0, 1, 2};
+ // int arr2{3, 4, 5};
+ // We might be able to preserve the non-specified size with `auto` and
+ // `std::to_array`:
+ // auto arr1 = std::to_array<int>({0, 1, 2});
+ return {};
+ }
+
+ std::optional<StringRef> IdentText =
+ getVarDeclIdentifierText(D, Ctx.getSourceManager(), Ctx.getLangOpts());
+
+ if (!IdentText) {
+ DEBUG_NOTE_DECL_FAIL(D, " : failed to locate the identifier");
+ return {};
+ }
+
+ SmallString<32> Replacement;
+ raw_svector_ostream OS(Replacement);
+ OS << "std::array<" << ElemTypeTxt << ", " << ArraySizeTxt << "> "
+ << IdentText->str();
+
+ FixIts.push_back(FixItHint::CreateReplacement(
+ SourceRange{D->getBeginLoc(), D->getTypeSpecEndLoc()}, OS.str()));
+ }
+
+ return FixIts;
+}
+
+static FixItList fixVariableWithArray(const VarDecl *VD,
+ const DeclUseTracker &Tracker,
+ const ASTContext &Ctx,
+ UnsafeBufferUsageHandler &Handler) {
+ const DeclStmt *DS = Tracker.lookupDecl(VD);
+ assert(DS && "Fixing non-local variables not implemented yet!");
+ if (!DS->isSingleDecl()) {
+ // FIXME: to support handling multiple `VarDecl`s in a single `DeclStmt`
+ return {};
+ }
+ // Currently DS is an unused variable but we'll need it when
+ // non-single decls are implemented, where the pointee type name
+ // and the '*' are spread around the place.
+ (void)DS;
+
+ // FIXME: handle cases where DS has multiple declarations
+ return fixVarDeclWithArray(VD, Ctx, Handler);
+}
+
// TODO: we should be consistent to use `std::nullopt` to represent no-fix due
// to any unexpected problem.
static FixItList
-fixVariable(const VarDecl *VD, Strategy::Kind K,
+fixVariable(const VarDecl *VD, FixitStrategy::Kind K,
/* The function decl under analysis */ const Decl *D,
const DeclUseTracker &Tracker, ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
@@ -2529,7 +2598,7 @@ fixVariable(const VarDecl *VD, Strategy::Kind K,
}
switch (K) {
- case Strategy::Kind::Span: {
+ case FixitStrategy::Kind::Span: {
if (VD->getType()->isPointerType()) {
if (const auto *PVD = dyn_cast<ParmVarDecl>(VD))
return fixParamWithSpan(PVD, Ctx, Handler);
@@ -2540,11 +2609,18 @@ fixVariable(const VarDecl *VD, Strategy::Kind K,
DEBUG_NOTE_DECL_FAIL(VD, " : not a pointer");
return {};
}
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
- llvm_unreachable("Strategy not implemented yet!");
- case Strategy::Kind::Wontfix:
+ case FixitStrategy::Kind::Array: {
+ if (VD->isLocalVarDecl() &&
+ isa<clang::ConstantArrayType>(VD->getType().getCanonicalType()))
+ return fixVariableWithArray(VD, Tracker, Ctx, Handler);
+
+ DEBUG_NOTE_DECL_FAIL(VD, " : not a local const-size array");
+ return {};
+ }
+ case FixitStrategy::Kind::Iterator:
+ case FixitStrategy::Kind::Vector:
+ llvm_unreachable("FixitStrategy not implemented yet!");
+ case FixitStrategy::Kind::Wontfix:
llvm_unreachable("Invalid strategy!");
}
llvm_unreachable("Unknown strategy!");
@@ -2605,7 +2681,8 @@ static void eraseVarsForUnfixableGroupMates(
static FixItList createFunctionOverloadsForParms(
std::map<const VarDecl *, FixItList> &FixItsForVariable /* mutable */,
const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD,
- const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) {
+ const FixitStrategy &S, ASTContext &Ctx,
+ UnsafeBufferUsageHandler &Handler) {
FixItList FixItsSharedByParms{};
std::optional<FixItList> OverloadFixes =
@@ -2625,8 +2702,8 @@ static FixItList createFunctionOverloadsForParms(
// Constructs self-contained fix-its for each variable in `FixablesForAllVars`.
static std::map<const VarDecl *, FixItList>
-getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
- ASTContext &Ctx,
+getFixIts(FixableGadgetSets &FixablesForAllVars, const FixitStrategy &S,
+ ASTContext &Ctx,
/* The function decl under analysis */ const Decl *D,
const DeclUseTracker &Tracker, UnsafeBufferUsageHandler &Handler,
const VariableGroupsManager &VarGrpMgr) {
@@ -2724,11 +2801,14 @@ getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
}
template <typename VarDeclIterTy>
-static Strategy
+static FixitStrategy
getNaiveStrategy(llvm::iterator_range<VarDeclIterTy> UnsafeVars) {
- Strategy S;
+ FixitStrategy S;
for (const VarDecl *VD : UnsafeVars) {
- S.set(VD, Strategy::Kind::Span);
+ if (isa<ConstantArrayType>(VD->getType().getCanonicalType()))
+ S.set(VD, FixitStrategy::Kind::Array);
+ else
+ S.set(VD, FixitStrategy::Kind::Span);
}
return S;
}
@@ -3034,7 +3114,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
// We assign strategies to variables that are 1) in the graph and 2) can be
// fixed. Other variables have the default "Won't fix" strategy.
- Strategy NaiveStrategy = getNaiveStrategy(llvm::make_filter_range(
+ FixitStrategy NaiveStrategy = getNaiveStrategy(llvm::make_filter_range(
VisitedVars, [&FixablesForAllVars](const VarDecl *V) {
// If a warned variable has no "Fixable", it is considered unfixable:
return FixablesForAllVars.byVar.count(V);
@@ -3057,9 +3137,9 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
auto FixItsIt = FixItsForVariableGroup.find(VD);
Handler.handleUnsafeVariableGroup(VD, VarGrpMgr,
FixItsIt != FixItsForVariableGroup.end()
- ? std::move(FixItsIt->second)
- : FixItList{},
- D);
+ ? std::move(FixItsIt->second)
+ : FixItList{},
+ D, NaiveStrategy);
for (const auto &G : WarningGadgets) {
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true,
D->getASTContext());
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 78b9f324e13906..8239ba49429d3c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2297,7 +2297,8 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
- FixItList &&Fixes, const Decl *D) override {
+ FixItList &&Fixes, const Decl *D,
+ const FixitStrategy &VarTargetTypes) override {
assert(!SuggestSuggestions &&
"Unsafe buffer usage fixits displayed without suggestions!");
S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
@@ -2312,7 +2313,18 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
// 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
+ unsigned FixItStrategy = 0;
+ switch (VarTargetTypes.lookup(Variable)) {
+ case clang::FixitStrategy::Kind::Span:
+ FixItStrategy = 0;
+ break;
+ case clang::FixitStrategy::Kind::Array:
+ FixItStrategy = 1;
+ break;
+ default:
+ assert(false && "We support only std::span and std::array");
+ };
+
const auto &FD =
S.Diag(Variable->getLocation(),
BriefMsg ? diag::note_unsafe_buffer_variable_fixit_together
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
new file mode 100644
index 00000000000000..90c11b1be95c25
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: -fsafe-buffer-usage-suggestions \
+// RUN: -verify %s
+
+// CHECK-NOT: [-Wunsafe-buffer-usage]
+
+
+void foo(unsigned idx) {
+ int buffer[10]; // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'buffer' to 'std::array' to label it for hardening}}
+ buffer[idx] = 0; // expected-note{{used in buffer access here}}
+}
+
+int global_buffer[10]; // expected-warning{{'global_buffer' is an unsafe buffer that does not perform bounds checks}}
+void foo2(unsigned idx) {
+ global_buffer[idx] = 0; // expected-note{{used in buffer access here}}
+}
+
+struct Foo {
+ int member_buffer[10];
+};
+void foo2(Foo& f, unsigned idx) {
+ f.member_buffer[idx] = 0; // expected-warning{{unsafe buffer access}}
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
index 5fff0854d45467..a5b578b98d4e5b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
@@ -32,15 +32,6 @@ void foo() {
// debug-note{{safe buffers debug: gadget 'ULCArraySubscript' refused to produce a fix}}
}
-void failed_decl() {
- int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}} \
- // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : not a pointer}}
-
- for (int i = 0; i < 10; i++) {
- a[i] = i; // expected-note{{used in buffer access here}}
- }
-}
-
void failed_multiple_decl() {
int *a = new int[4], b; // expected-warning{{'a' is an unsafe pointer used for buffer access}} \
// debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : multiple VarDecls}}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
new file mode 100644
index 00000000000000..3adfc324dfbe30
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp
@@ -0,0 +1,228 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN: -fsafe-buffer-usage-suggestions \
+// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+typedef int * Int_ptr_t;
+typedef int Int_t;
+
+void simple(unsigned idx) {
+ int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+ buffer[idx] = 0;
+}
+
+void array2d(unsigned idx) {
+ int buffer[10][10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ buffer[idx][idx] = 0;
+}
+
+void array2d_vla(unsigned sz, unsigned idx) {
+ int buffer1[10][sz];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ int buffer2[sz][10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ buffer1[idx][idx] = 0;
+ buffer2[idx][idx] = 0;
+}
+
+void array2d_assign_from_elem(unsigned idx) {
+ int buffer[10][10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ int a = buffer[idx][idx];
+}
+
+void array2d_use(int *);
+void array2d_call(unsigned idx) {
+ int buffer[10][10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ array2d_use(buffer[idx]);
+}
+void array2d_call_vla(unsigned sz, unsigned idx) {
+ int buffer[10][sz];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ array2d_use(buffer[idx]);
+}
+
+void array2d_typedef(unsigned idx) {
+ typedef int ten_ints_t[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ ten_ints_t buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+ buffer[idx][idx] = 0;
+}
+
+void whitespace_in_declaration(unsigned idx) {
+ int buffer_w [ 10 ];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:35}:"std::array<int, 10> buffer_w"
+ buffer_w[idx] = 0;
+}
+
+void comments_in_declaration(unsigned idx) {
+ int /* [A] */ buffer_w /* [B] */ [ /* [C] */ 10 /* [D] */ ] ;
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:69}:"std::array<int /* [A] */, /* [C] */ 10 /* [D] */> buffer_w"
+ buffer_w[idx] = 0;
+}
+
+void initializer(unsigned idx) {
+ int buffer[3] = {0};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::array<int, 3> buffer"
+
+ int buffer2[3] = {0, 1, 2};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 3> buffer2"
+
+ buffer[idx] = 0;
+ buffer2[idx] = 0;
+}
+
+void auto_size(unsigned idx) {
+ int buffer[] = {0, 1, 2};
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+ buffer[idx] = 0;
+}
+
+void universal_initialization(unsigned idx) {
+ int buffer[] {0, 1, 2};
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+ buffer[idx] = 0;
+}
+
+void multi_decl1(unsigned idx) {
+ int a, buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+ buffer[idx] = 0;
+}
+
+void multi_decl2(unsigned idx) {
+ int buffer[10], b;
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+// FIXME: implement support
+
+ buffer[idx] = 0;
+}
+
+void local_array_ptr_to_const(unsigned idx, const int*& a) {
+ const int * buffer[10] = {a};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:25}:"std::array<const int *, 10> buffer"
+ a = buffer[idx];
+}
+
+void local_array_const_ptr(unsigned idx, int*& a) {
+ int * const buffer[10] = {a};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:25}:"std::array<int * const, 10> buffer"
+
+ a = buffer[idx];
+}
+
+void local_array_const_ptr_via_typedef(unsigned idx, int*& a) {
+ typedef int * const my_const_ptr;
+ my_const_ptr buffer[10] = {a};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:26}:"std::array<my_const_ptr, 10> buffer"
+
+ a = buffer[idx];
+}
+
+void local_array_const_ptr_to_const(unsigned idx, const int*& a) {
+ const int * const buffer[10] = {a};
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:31}:"std::array<const int * const, 10> buffer"
+
+ a = buffer[idx];
+
+}
+
+template<typename T>
+void unsupported_local_array_in_template(unsigned idx) {
+ T buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+ buffer[idx] = 0;
+}
+// Instantiate the template function to force its analysis.
+template void unsupported_local_array_in_template<int>(unsigned);
+
+typedef unsigned int my_uint;
+void typedef_as_elem_type(unsigned idx) {
+ my_uint buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:21}:"std::array<my_uint, 10> buffer"
+ buffer[idx] = 0;
+}
+
+void decltype_as_elem_type(unsigned idx) {
+ int a;
+ decltype(a) buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:25}:"std::array<decltype(a), 10> buffer"
+ buffer[idx] = 0;
+}
+
+void macro_as_elem_type(unsigned idx) {
+#define MY_INT int
+ MY_INT buffer[10];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+// FIXME: implement support
+
+ buffer[idx] = 0;
+#undef MY_INT
+}
+
+void macro_as_identifier(unsigned idx) {
+#define MY_BUFFER buffer
+ int MY_BUFFER[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:20}:"std::array<int, 10> MY_BUFFER"
+ MY_BUFFER[idx] = 0;
+#undef MY_BUFFER
+}
+
+void macro_as_size(unsigned idx) {
+#define MY_TEN 10
+ int buffer[MY_TEN];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:21}:"std::array<int, MY_TEN> buffer"
+ buffer[idx] = 0;
+#undef MY_TEN
+}
+
+typedef unsigned int my_array[42];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+void typedef_as_array_type(unsigned idx) {
+ my_array buffer;
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+ buffer[idx] = 0;
+}
+
+void decltype_as_array_type(unsigned idx) {
+ int buffer[42];
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+ decltype(buffer) buffer2;
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:.*-[[@LINE-1]]:.*}
+ buffer2[idx] = 0;
+}
+
+void constant_as_size(unsigned idx) {
+ const unsigned my_const = 10;
+ int buffer[my_const];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:23}:"std::array<int, my_const> buffer"
+ buffer[idx] = 0;
+}
+
+void subscript_negative() {
+ int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+
+ // For constant-size arrays any negative index will lead to buffer underflow.
+ // std::array::operator[] has unsigned parameter so the value will be casted to unsigned.
+ // This will very likely be buffer overflow but hardened std::array catch these at runtime.
+ buffer[-5] = 0;
+}
+
+void subscript_signed(int signed_idx) {
+ int buffer[10];
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::array<int, 10> buffer"
+
+ // For constant-size arrays any negative index will lead to buffer underflow.
+ // std::array::operator[] has unsigned parameter so the value will be casted to unsigned.
+ // This will very likely be buffer overflow but hardened std::array catches these at runtime.
+ buffer[signed_idx] = 0;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index c5f0a9ef929371..67cdf252d6a8b6 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -61,6 +61,7 @@ void testArraySubscripts(int *p, int **pp) {
);
int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'a' to 'std::array' to label it for hardening}}
int b[10][10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
foo(a[1], 1[a], // expected-note2{{used in buffer access here}}
@@ -174,6 +175,7 @@ auto file_scope_lambda = [](int *ptr) {
void testLambdaCapture() {
int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
int b[10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'b' to 'std::array' to label it for hardening}}
int c[10];
auto Lam1 = [a]() {
@@ -191,7 +193,9 @@ void testLambdaCapture() {
void testLambdaImplicitCapture() {
int a[10]; // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'a' to 'std::array' to label it for hardening}}
int b[10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'b' to 'std::array' to label it for hardening}}
auto Lam1 = [=]() {
return a[1]; // expected-note{{used in buffer access here}}
@@ -344,6 +348,7 @@ template<typename T> void fArr(T t[]) {
// expected-warning at -1{{'t' is an unsafe pointer used for buffer access}}
foo(t[1]); // expected-note{{used in buffer access here}}
T ar[8]; // expected-warning{{'ar' is an unsafe buffer that does not perform bounds checks}}
+ // expected-note at -1{{change type of 'ar' to 'std::array' to label it for hardening}}
foo(ar[5]); // expected-note{{used in buffer access here}}
}
More information about the cfe-commits
mailing list