[clang] 622be09 - Revert "[-Wunsafe-buffer-usage] Generate fix-it for local variable declarations"
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 7 14:48:17 PST 2023
Author: Ziqing Luo
Date: 2023-02-07T14:47:43-08:00
New Revision: 622be09c815266632e204eaf1c7a35f050220459
URL: https://github.com/llvm/llvm-project/commit/622be09c815266632e204eaf1c7a35f050220459
DIFF: https://github.com/llvm/llvm-project/commit/622be09c815266632e204eaf1c7a35f050220459.diff
LOG: Revert "[-Wunsafe-buffer-usage] Generate fix-it for local variable declarations"
This reverts commit a29e67614c3b7018287e5f68c57bba7618aa880e.
Added:
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Removed:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 04c9fdea444f8..e3f87cd0f3660 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -37,15 +37,6 @@ class UnsafeBufferUsageHandler {
/// Invoked when a fix is suggested against a variable.
virtual void handleFixableVariable(const VarDecl *Variable,
FixItList &&List) = 0;
-
- /// Returns the text indicating that the user needs to provide input there:
- virtual std::string
- getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") {
- std::string s = std::string("<# ");
- s += HintTextToUser;
- s += " #>";
- return s;
- }
};
// This function invokes the analysis and allows the caller to react to it
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 75657d8d9a584..78889da32b3b4 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -30,7 +30,6 @@ WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
-FIXABLE_GADGET(ULCArraySubscript)
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9b9ec23a2f21f..bc70dbf4a624d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11790,8 +11790,6 @@ def warn_unsafe_buffer_operation : Warning<
InGroup<UnsafeBufferUsage>, DefaultIgnore;
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
-def note_unsafe_buffer_variable_fixit : Note<
- "change type of '%0' to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information">;
def err_loongarch_builtin_requires_la32 : Error<
"this builtin requires target: loongarch32">;
} // end of sema component.
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7741d6cd4157f..331479a80f6cc 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -9,7 +9,6 @@
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
#include "llvm/ADT/SmallVector.h"
#include <memory>
#include <optional>
@@ -116,19 +115,6 @@ AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All);
return Visitor.findMatch(DynTypedNode::create(Node));
}
-
-AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
- return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
-}
-
-// Returns a matcher that matches any expression 'e' such that `innerMatcher`
-// matches 'e' and 'e' is in an Unspecified Lvalue Context.
-static internal::Matcher<Stmt>
-isInUnspecifiedLvalueContext(internal::Matcher<Expr> innerMatcher) {
- return implicitCastExpr(hasCastKind(CastKind::CK_LValueToRValue),
- castSubExpr(innerMatcher));
- // FIXME: add assignmentTo context...
-}
} // namespace clang::ast_matchers
namespace {
@@ -296,7 +282,7 @@ class DecrementGadget : public WarningGadget {
/// Array subscript expressions on raw pointers as if they're arrays. Unsafe as
/// it doesn't have any bounds checks for the array.
class ArraySubscriptGadget : public WarningGadget {
- static constexpr const char *const ArraySubscrTag = "ArraySubscript";
+ static constexpr const char *const ArraySubscrTag = "arraySubscr";
const ArraySubscriptExpr *ASE;
public:
@@ -407,48 +393,6 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
return {};
}
};
-
-
-// Represents expressions of the form `DRE[*]` in the Unspecified Lvalue
-// Context (see `isInUnspecifiedLvalueContext`).
-// Note here `[]` is the built-in subscript operator.
-class ULCArraySubscriptGadget : public FixableGadget {
-private:
- static constexpr const char *const ULCArraySubscriptTag = "ArraySubscriptUnderULC";
- const ArraySubscriptExpr *Node;
-
-public:
- ULCArraySubscriptGadget(const MatchFinder::MatchResult &Result)
- : FixableGadget(Kind::ULCArraySubscript),
- Node(Result.Nodes.getNodeAs<ArraySubscriptExpr>(ULCArraySubscriptTag)) {
- assert(Node != nullptr && "Expecting a non-null matching result");
- }
-
- static bool classof(const Gadget *G) {
- return G->getKind() == Kind::ULCArraySubscript;
- }
-
- static Matcher matcher() {
- auto ArrayOrPtr = anyOf(hasPointerType(), hasArrayType());
- auto BaseIsArrayOrPtrDRE =
- hasBase(ignoringParenImpCasts(declRefExpr(ArrayOrPtr)));
- auto Target =
- arraySubscriptExpr(BaseIsArrayOrPtrDRE).bind(ULCArraySubscriptTag);
-
- return expr(isInUnspecifiedLvalueContext(Target));
- }
-
- virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
-
- virtual DeclUseList getClaimedVarUseSites() const override {
- if (const auto *DRE = dyn_cast<DeclRefExpr>(Node->getBase()->IgnoreImpCasts())) {
- return {DRE};
- }
- return {};
- }
-};
} // namespace
namespace {
@@ -602,30 +546,26 @@ static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker> findGadg
// clang-format off
M.addMatcher(
stmt(forEveryDescendant(
- eachOf(
- // A `FixableGadget` matcher and a `WarningGadget` matcher should not disable
- // each other (they could if they were put in the same `anyOf` group).
- // We also should make sure no two `FixableGadget` (resp. `WarningGadget`) matchers
- // match for the same node, so that we can group them
- // in one `anyOf` group (for better performance via short-circuiting).
stmt(anyOf(
-#define FIXABLE_GADGET(x) \
+ // Add Gadget::matcher() for every gadget in the registry.
+#define GADGET(x) \
x ## Gadget::matcher().bind(#x),
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
+ // In parallel, match all DeclRefExprs so that to find out
+ // whether there are any uncovered by gadgets.
+ declRefExpr(anyOf(hasPointerType(), hasArrayType()),
+ to(varDecl())).bind("any_dre"),
// Also match DeclStmts because we'll need them when fixing
// their underlying VarDecls that otherwise don't have
// any backreferences to DeclStmts.
declStmt().bind("any_ds")
- )),
- stmt(anyOf(
- // Add Gadget::matcher() for every gadget in the registry.
-#define WARNING_GADGET(x) \
- x ## Gadget::matcher().bind(#x),
-#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
- // In parallel, match all DeclRefExprs so that to find out
- // whether there are any uncovered by gadgets.
- declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre")
- )))
+ ))
+ // FIXME: Idiomatically there should be a forCallable(equalsNode(D))
+ // here, to make sure that the statement actually belongs to the
+ // function and not to a nested function. However, forCallable uses
+ // ParentMap which can't be used before the AST is fully constructed.
+ // The original problem doesn't sound like it needs ParentMap though,
+ // maybe there's a more direct solution?
)),
&CB
);
@@ -694,241 +634,11 @@ groupFixablesByVar(FixableGadgetList &&AllFixableOperations) {
return FixablesForUnsafeVars;
}
-std::optional<FixItList>
-ULCArraySubscriptGadget::getFixits(const Strategy &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: {
- // If the index has a negative constant value, we give up as no valid
- // fix-it can be generated:
- const ASTContext &Ctx = // FIXME: we need ASTContext to be passed in!
- VD->getASTContext();
- if (auto ConstVal = Node->getIdx()->getIntegerConstantExpr(Ctx)) {
- if (ConstVal->isNegative())
- return std::nullopt;
- } else if (!Node->getIdx()->getType()->isUnsignedIntegerType())
- return std::nullopt;
- // 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:
- llvm_unreachable("unsupported strategies for FixableGadgets");
- }
- }
- return std::nullopt;
-}
-
-// Return the text representation of the given `APInt Val`:
-static std::string getAPIntText(APInt Val) {
- SmallVector<char> Txt;
- Val.toString(Txt, 10, true);
- // APInt::toString does not add '\0' to the end of the string for us:
- Txt.push_back('\0');
- return Txt.data();
-}
-
-// Return the source location of the last character of the AST `Node`.
-template <typename NodeTy>
-static SourceLocation getEndCharLoc(const NodeTy *Node, const SourceManager &SM,
- const LangOptions &LangOpts) {
- return Lexer::getLocForEndOfToken(Node->getEndLoc(), 1, SM, LangOpts);
-}
-
-// Return the source location just past the last character of the AST `Node`.
-template <typename NodeTy>
-static SourceLocation getPastLoc(const NodeTy *Node, const SourceManager &SM,
- const LangOptions &LangOpts) {
- return Lexer::getLocForEndOfToken(Node->getEndLoc(), 0, SM, LangOpts);
-}
-
-// Return text representation of an `Expr`.
-static StringRef getExprText(const Expr *E, const SourceManager &SM,
- const LangOptions &LangOpts) {
- SourceLocation LastCharLoc = getPastLoc(E, SM, LangOpts);
-
- return Lexer::getSourceText(
- CharSourceRange::getCharRange(E->getBeginLoc(), LastCharLoc), SM,
- LangOpts);
-}
-
-// 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.
-// In many cases, this function cannot figure out the actual extent `S`. It
-// then will use a place holder to replace `S` to ask users to fill `S` in. The
-// initializer shall be used to initialize a variable of type `std::span<T>`.
-//
-// FIXME: Support multi-level pointers
-//
-// Parameters:
-// `Init` a pointer to the initializer expression
-// `Ctx` a reference to the ASTContext
-static FixItList
-populateInitializerFixItWithSpan(const Expr *Init, const ASTContext &Ctx,
- const StringRef UserFillPlaceHolder) {
- FixItList FixIts{};
- const SourceManager &SM = Ctx.getSourceManager();
- const LangOptions &LangOpts = Ctx.getLangOpts();
- std::string ExtentText = UserFillPlaceHolder.data();
- StringRef One = "1";
-
- // Insert `{` before `Init`:
- FixIts.push_back(FixItHint::CreateInsertion(Init->getBeginLoc(), "{"));
- // Try to get the data extent. Break into
diff erent cases:
- if (auto CxxNew = dyn_cast<CXXNewExpr>(Init->IgnoreImpCasts())) {
- // In cases `Init` is `new T[n]` and there is no explicit cast over
- // `Init`, we know that `Init` must evaluates to a pointer to `n` objects
- // of `T`. So the extent is `n` unless `n` has side effects. Similar but
- // simpler for the case where `Init` is `new T`.
- if (const Expr *Ext = CxxNew->getArraySize().value_or(nullptr)) {
- if (!Ext->HasSideEffects(Ctx))
- ExtentText = getExprText(Ext, SM, LangOpts);
- } else if (!CxxNew->isArray())
- // Although the initializer is not allocating a buffer, the pointer
- // variable could still be used in buffer access operations.
- ExtentText = One;
- } else if (const auto *CArrTy = Ctx.getAsConstantArrayType(
- Init->IgnoreImpCasts()->getType())) {
- // In cases `Init` is of an array type after stripping off implicit casts,
- // the extent is the array size. Note that if the array size is not a
- // constant, we cannot use it as the extent.
- ExtentText = getAPIntText(CArrTy->getSize());
- } else {
- // In cases `Init` is of the form `&Var` after stripping of implicit
- // casts, where `&` is the built-in operator, the extent is 1.
- if (auto AddrOfExpr = dyn_cast<UnaryOperator>(Init->IgnoreImpCasts()))
- if (AddrOfExpr->getOpcode() == UnaryOperatorKind::UO_AddrOf &&
- isa_and_present<DeclRefExpr>(AddrOfExpr->getSubExpr()))
- ExtentText = One;
- // TODO: we can handle more cases, e.g., `&a[0]`, `&a`, `std::addressof`, and explicit casting, etc.
- // etc.
- }
-
- SmallString<32> StrBuffer{};
- SourceLocation LocPassInit = getPastLoc(Init, SM, LangOpts);
-
- StrBuffer.append(", ");
- StrBuffer.append(ExtentText);
- StrBuffer.append("}");
- FixIts.push_back(FixItHint::CreateInsertion(LocPassInit, StrBuffer.str()));
- return FixIts;
-}
-
-// For a `VarDecl` of the form `T * var (= Init)?`, this
-// function generates a fix-it for the declaration, which re-declares `var` to
-// be of `span<T>` type and transforms the initializer, if present, to a span
-// constructor---`span<T> var {Init, Extent}`, where `Extent` may need the user
-// to fill in.
-//
-// FIXME: support Multi-level pointers
-//
-// Parameters:
-// `D` a pointer the variable declaration node
-// `Ctx` a reference to the ASTContext
-// Returns:
-// the generated fix-it
-static FixItList fixVarDeclWithSpan(const VarDecl *D, const ASTContext &Ctx,
- const StringRef UserFillPlaceHolder) {
- const QualType &SpanEltT = D->getType()->getPointeeType();
- assert(!SpanEltT.isNull() && "Trying to fix a non-pointer type variable!");
-
- FixItList FixIts{};
- SourceLocation
- ReplacementLastLoc; // the inclusive end location of the replacement
- const SourceManager &SM = Ctx.getSourceManager();
-
- if (const Expr *Init = D->getInit()) {
- FixItList InitFixIts =
- populateInitializerFixItWithSpan(Init, Ctx, UserFillPlaceHolder);
-
- if (InitFixIts.empty())
- return {}; // Something wrong with fixing initializer, give up
- // The loc right before the initializer:
- ReplacementLastLoc = Init->getBeginLoc().getLocWithOffset(-1);
- FixIts.insert(FixIts.end(), std::make_move_iterator(InitFixIts.begin()),
- std::make_move_iterator(InitFixIts.end()));
- } else
- ReplacementLastLoc = getEndCharLoc(D, SM, Ctx.getLangOpts());
-
- // Producing fix-it for variable declaration---make `D` to be of span type:
- SmallString<32> Replacement;
- raw_svector_ostream OS(Replacement);
-
- OS << "std::span<" << SpanEltT.getAsString() << "> " << D->getName();
- FixIts.push_back(FixItHint::CreateReplacement(
- SourceRange{D->getBeginLoc(), ReplacementLastLoc}, OS.str()));
- return FixIts;
-}
-
-static FixItList fixVariableWithSpan(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 fixVarDeclWithSpan(VD, Ctx, Handler.getUserFillPlaceHolder());
-}
-
-static FixItList fixVariable(const VarDecl *VD, Strategy::Kind K,
- const DeclUseTracker &Tracker,
- const ASTContext &Ctx,
- UnsafeBufferUsageHandler &Handler) {
- switch (K) {
- case Strategy::Kind::Span: {
- if (VD->getType()->isPointerType() && VD->isLocalVarDecl())
- return fixVariableWithSpan(VD, Tracker, Ctx, Handler);
- return {};
- }
- case Strategy::Kind::Iterator:
- case Strategy::Kind::Array:
- case Strategy::Kind::Vector:
- llvm_unreachable("Strategy not implemented yet!");
- case Strategy::Kind::Wontfix:
- llvm_unreachable("Invalid strategy!");
- }
-}
-
-// Returns true iff there exists a `FixItHint` 'h' in `FixIts` such that the
-// `RemoveRange` of 'h' overlaps with a macro use.
-static bool overlapWithMacro(const FixItList &FixIts) {
- // FIXME: For now we only check if the range (or the first token) is (part of)
- // a macro expansion. Ideally, we want to check for all tokens in the range.
- return llvm::any_of(FixIts, [](const FixItHint &Hint) {
- auto BeginLoc = Hint.RemoveRange.getBegin();
- if (BeginLoc.isMacroID())
- // If the range (or the first token) is (part of) a macro expansion:
- return true;
- return false;
- });
-}
-
static std::map<const VarDecl *, FixItList>
-getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
- const DeclUseTracker &Tracker, const ASTContext &Ctx,
- UnsafeBufferUsageHandler &Handler) {
+getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S) {
std::map<const VarDecl *, FixItList> FixItsForVariable;
for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
- FixItsForVariable[VD] = fixVariable(VD, S.lookup(VD), Tracker, Ctx, Handler);
- // If we fail to produce Fix-It for the declaration we have to skip the
- // variable entirely.
- if (FixItsForVariable[VD].empty()) {
- FixItsForVariable.erase(VD);
- continue;
- }
+ // TODO fixVariable - fixit for the variable itself
bool ImpossibleToFix = false;
llvm::SmallVector<FixItHint, 16> FixItsForVD;
for (const auto &F : Fixables) {
@@ -947,10 +657,6 @@ getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
else
FixItsForVariable[VD].insert(FixItsForVariable[VD].end(),
FixItsForVD.begin(), FixItsForVD.end());
- // Fix-it shall not overlap with macros or/and templates:
- if (overlapWithMacro(FixItsForVariable[VD])) {
- FixItsForVariable.erase(VD);
- }
}
return FixItsForVariable;
}
@@ -996,8 +702,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
std::map<const VarDecl *, FixItList> FixItsForVariable =
- getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker,
- D->getASTContext(), Handler);
+ getFixIts(FixablesForUnsafeVars, NaiveStrategy);
// FIXME Detect overlapping FixIts.
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index c1969a0f85d7f..b6dce0808705e 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2200,18 +2200,13 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
// FIXME: rename to handleUnsafeVariable
void handleFixableVariable(const VarDecl *Variable,
FixItList &&Fixes) override {
- S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
- << Variable << (Variable->getType()->isPointerType() ? 0 : 1)
- << Variable->getSourceRange();
- if (!Fixes.empty()) {
- unsigned FixItStrategy = 0; // For now we only has 'std::span' strategy
- const auto &FD = S.Diag(Variable->getLocation(),
- diag::note_unsafe_buffer_variable_fixit);
-
- FD << Variable->getName() << FixItStrategy;
- for (const auto &F : Fixes)
- FD << F;
- }
+ const auto &D =
+ S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable);
+ D << Variable;
+ D << (Variable->getType()->isPointerType() ? 0 : 1);
+ D << Variable->getSourceRange();
+ for (const auto &F : Fixes)
+ D << F;
}
};
} // namespace
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
deleted file mode 100644
index 4f0e0fa31a431..0000000000000
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
+++ /dev/null
@@ -1,192 +0,0 @@
-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-typedef int * Int_ptr_t;
-typedef int Int_t;
-
-void local_array_subscript_simple() {
- int tmp;
- int *p = new int[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
- const int *q = new int[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:17}:"std::span<const int> q"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:29-[[@LINE-3]]:29}:", 10}"
- tmp = p[5];
- tmp = q[5];
-
- Int_ptr_t x = new int[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::span<int> x"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:28-[[@LINE-3]]:28}:", 10}"
- Int_ptr_t y = new int;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::span<int> y"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 1}"
- Int_t * z = new int[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:14}:"std::span<Int_t> z"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:", 10}"
- Int_t * w = new Int_t[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:14}:"std::span<Int_t> w"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:28-[[@LINE-3]]:28}:", 10}"
-
- tmp = x[5];
- tmp = y[5]; // y[5] will crash after being span
- tmp = z[5];
- tmp = w[5];
-}
-
-void local_array_subscript_auto() {
- int tmp;
- auto p = new int[10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
- tmp = p[5];
-}
-
-void local_array_subscript_variable_extent() {
- int n = 10;
- int tmp;
- int *p = new int[n];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", n}"
- // If the extent expression does not have a constant value, we cannot fill the extent for users...
- int *q = new int[n++];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", <# placeholder #>}"
- tmp = p[5];
- tmp = q[5];
-}
-
-
-void local_ptr_to_array() {
- int tmp;
- int n = 10;
- int a[10];
- int b[n]; // If the extent expression does not have a constant value, we cannot fill the extent for users...
- int *p = a;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", 10}"
- int *q = b;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
- // No way to know if `n` is ever mutated since `int b[n];`, so no way to figure out the extent
- tmp = p[5];
- tmp = q[5];
-}
-
-void local_ptr_addrof_init() {
- int var;
- int * q = &var;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> q"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:", 1}"
- // This expression involves unsafe buffer accesses, which will crash
- // at runtime after applying the fix-it,
- var = q[5];
-}
-
-void decl_without_init() {
- int tmp;
- int * p;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> p"
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]
- Int_ptr_t q;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:14}:"std::span<int> q"
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]
- tmp = p[5];
- tmp = q[5];
-}
-
-// Explicit casts are required in the following cases. No way to
-// figure out span extent for them automatically.
-void explict_cast() {
- int tmp;
- int * p = (int*) new int[10][10];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:35-[[@LINE-3]]:35}:", <# placeholder #>}"
- tmp = p[5];
-
- int a;
- char * q = (char *)&a;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:13}:"std::span<char> q"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", <# placeholder #>}"
- tmp = (int) q[5];
-
- void * r = &a;
- char * s = (char *) r;
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:13}:"std::span<char> s"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"{"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", <# placeholder #>}"
- tmp = (int) s[5];
-}
-
-void unsupported_multi_decl(int * x) {
- int * p = x, * q = new int[10];
- // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]
- *p = q[5];
-}
-
-void unsupported_fixit_overlapping_macro(int * x) {
- int tmp;
- // In the case below, a tentative fix-it replaces `MY_INT * p =` with `std::span<MY_INT> p `.
- // It overlaps with the use of the macro `MY_INT`. The fix-it is
- // discarded then.
-#define MY_INT int
- MY_INT * p = new int[10];
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
- tmp = p[5];
-
-#define MY_VAR(name) int * name
- MY_VAR(q) = new int[10];
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
- tmp = q[5];
-
- // In cases where fix-its do not change the original code where
- // macros are used, those fix-its will be emitted. For example,
- // fixits are inserted before and after `new MY_INT[MY_TEN]` below.
-#define MY_TEN 10
- int * g = new MY_INT[MY_TEN];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> g"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:31-[[@LINE-3]]:31}:", MY_TEN}"
- tmp = g[5];
-
-#undef MY_INT
-#undef MY_VAR
-#undef MY_TEN
-}
-
-void unsupported_subscript_negative(int i, unsigned j, unsigned long k) {
- int tmp;
- int * p = new int[10];
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
-
- tmp = p[-1]; // If `p` is made a span, this `[]` operation is wrong,
- // so no fix-it emitted.
-
- int * q = new int[10];
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
-
- tmp = q[5];
- tmp = q[i]; // If `q` is made a span, this `[]` operation may be
- // wrong as we do not know if `i` is non-negative, so
- // no fix-it emitted.
-
- int * r = new int[10];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> r"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
-
- tmp = r[j] + r[k]; // both `j` and `k` are unsigned so they must be non-negative
- tmp = r[(unsigned int)-1]; // a cast-to-unsigned-expression is also non-negative
-}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index ef78a7336defc..a8ee6f58d140a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -80,24 +80,20 @@ void testArraySubscripts(int *p, int **pp) {
void testArraySubscriptsWithAuto(int *p, int **pp) {
int a[10];
- auto ap1 = a; // expected-warning{{'ap1' is an unsafe pointer used for buffer access}} \
- expected-note{{change type of 'ap1' to 'std::span' to preserve bounds information}}
+ auto ap1 = a; // expected-warning{{'ap1' is an unsafe pointer used for buffer access}}
- foo(ap1[1]); // expected-note{{used in buffer access here}}
+ foo(ap1[1]); // expected-note{{used in buffer access here}}
- auto ap2 = p; // expected-warning{{'ap2' is an unsafe pointer used for buffer access}} \
- expected-note{{change type of 'ap2' to 'std::span' to preserve bounds information}}
+ auto ap2 = p; // expected-warning{{'ap2' is an unsafe pointer used for buffer access}}
foo(ap2[1]); // expected-note{{used in buffer access here}}
- auto ap3 = pp; // expected-warning{{'ap3' is an unsafe pointer used for buffer access}} \
- expected-note{{change type of 'ap3' to 'std::span' to preserve bounds information}}
+ auto ap3 = pp; // expected-warning{{'ap3' is an unsafe pointer used for buffer access}}
foo(ap3[1][1]); // expected-note{{used in buffer access here}}
// expected-warning at -1{{unsafe buffer access}}
- auto ap4 = *pp; // expected-warning{{'ap4' is an unsafe pointer used for buffer access}} \
- expected-note{{change type of 'ap4' to 'std::span' to preserve bounds information}}
+ auto ap4 = *pp; // expected-warning{{'ap4' is an unsafe pointer used for buffer access}}
foo(ap4[1]); // expected-note{{used in buffer access here}}
}
@@ -359,8 +355,7 @@ void testMultiLineDeclStmt(int * p) {
auto
- ap1 = p; // expected-warning{{'ap1' is an unsafe pointer used for buffer access}} \
- expected-note{{change type of 'ap1' to 'std::span' to preserve bounds information}}
+ ap1 = p; // expected-warning{{'ap1' is an unsafe pointer used for buffer access}}
foo(ap1[1]); // expected-note{{used in buffer access here}}
}
More information about the cfe-commits
mailing list