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@-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}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
