mirror of
https://github.com/llvm/llvm-project.git
synced 2025-04-27 04:06:07 +00:00
Revert "[-Wunsafe-buffer-usage] Generate fix-it for local variable declarations"
This reverts commit a29e67614c3b7018287e5f68c57bba7618aa880e.
This commit is contained in:
parent
97fbbd657f
commit
622be09c81
@ -37,15 +37,6 @@ public:
|
||||
/// 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
|
||||
|
@ -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
|
||||
|
@ -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.
|
||||
|
@ -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 @@ public:
|
||||
/// 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 @@ public:
|
||||
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) \
|
||||
x ## Gadget::matcher().bind(#x),
|
||||
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
|
||||
// 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) \
|
||||
#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")
|
||||
)))
|
||||
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")
|
||||
))
|
||||
// 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 different 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.
|
||||
|
||||
|
@ -2200,18 +2200,13 @@ public:
|
||||
// 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
|
||||
|
@ -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
|
||||
}
|
@ -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@-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}}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user