diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 769c6d9ebefaa..7734e56d1376d 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -2152,15 +2152,18 @@ UPCPreIncrementGadget::getFixits(const FixitStrategy &S) const { // 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`. +// In some cases (e. g. constant size array) the initializer should remain +// unchanged and the function returns empty list. In case the function can't +// provide the right fixit it will return nullopt. // // FIXME: Support multi-level pointers // // Parameters: // `Init` a pointer to the initializer expression // `Ctx` a reference to the ASTContext -static FixItList +static std::optional FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, - const StringRef UserFillPlaceHolder) { + const StringRef UserFillPlaceHolder) { const SourceManager &SM = Ctx.getSourceManager(); const LangOptions &LangOpts = Ctx.getLangOpts(); @@ -2176,11 +2179,11 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, std::optional InitLocation = getEndCharLoc(Init, SM, LangOpts); if (!InitLocation) - return {}; + return std::nullopt; SourceRange SR(Init->getBeginLoc(), *InitLocation); - return {FixItHint::CreateRemoval(SR)}; + return FixItList{FixItHint::CreateRemoval(SR)}; } FixItList FixIts{}; @@ -2199,7 +2202,7 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, if (!Ext->HasSideEffects(Ctx)) { std::optional ExtentString = getExprText(Ext, SM, LangOpts); if (!ExtentString) - return {}; + return std::nullopt; ExtentText = *ExtentString; } } else if (!CxxNew->isArray()) @@ -2208,10 +2211,10 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, 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()); + // std::span has a single parameter constructor for initialization with + // constant size array. The size is auto-deduced as the constructor is a + // function template. The correct fixit is empty - no changes should happen. + return FixItList{}; } else { // In cases `Init` is of the form `&Var` after stripping of implicit // casts, where `&` is the built-in operator, the extent is 1. @@ -2227,7 +2230,7 @@ FixVarInitializerWithSpan(const Expr *Init, ASTContext &Ctx, std::optional LocPassInit = getPastLoc(Init, SM, LangOpts); if (!LocPassInit) - return {}; + return std::nullopt; StrBuffer.append(", "); StrBuffer.append(ExtentText); @@ -2317,12 +2320,12 @@ static FixItList fixLocalVarDeclWithSpan(const VarDecl *D, ASTContext &Ctx, } // Fix the initializer if it exists: if (const Expr *Init = D->getInit()) { - FixItList InitFixIts = + std::optional InitFixIts = FixVarInitializerWithSpan(Init, Ctx, UserFillPlaceHolder); - if (InitFixIts.empty()) + if (!InitFixIts) return {}; - FixIts.insert(FixIts.end(), std::make_move_iterator(InitFixIts.begin()), - std::make_move_iterator(InitFixIts.end())); + FixIts.insert(FixIts.end(), std::make_move_iterator(InitFixIts->begin()), + std::make_move_iterator(InitFixIts->end())); // If the declaration has the form `T *ident = init`, we want to replace // `T *ident = ` with `std::span ident`: EndLocForReplacement = Init->getBeginLoc().getLocWithOffset(-1); diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-array-inits-ptr.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-array-inits-ptr.cpp new file mode 100644 index 0000000000000..2044056cf6eea --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-array-inits-ptr.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +void safe_array_initing_safe_ptr(unsigned idx) { + int buffer[10]; + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: + int* ptr = buffer; + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: +} + +void safe_array_initing_unsafe_ptr(unsigned idx) { + int buffer[123321123]; + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: + int* ptr = buffer; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:13}:"std::span ptr" + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:{{.*}}123321123 + ptr[idx + 1] = 0; +} 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 index 114ceaad56e45..ca5dab15a0dff 100644 --- 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 @@ -52,12 +52,8 @@ void local_variable_qualifiers_specifiers() { int a[10]; const int * p = a; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:18}:"std::span p" - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:19}:"{" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:20}:", 10}" const int * const q = a; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:24}:"std::span const q" - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:25-[[@LINE-2]]:25}:"{" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:", 10}" int tmp; tmp = p[5]; tmp = q[5]; @@ -88,12 +84,8 @@ void local_ptr_to_array() { 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 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 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];