Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -19,6 +19,8 @@ namespace clang { + class Sema; + /// The interface that lets the caller handle unsafe buffer usage analysis /// results by overriding this class's handle... methods. class UnsafeBufferUsageHandler { @@ -54,6 +56,7 @@ // This function invokes the analysis and allows the caller to react to it // through the handler class. void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, + Sema &S, bool EmitFixits); namespace internal { Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -10,11 +10,14 @@ #include "clang/AST/Decl.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Sema/Lookup.h" #include "llvm/ADT/SmallVector.h" #include #include +#include using namespace llvm; using namespace clang; @@ -869,6 +872,12 @@ return std::nullopt; } +// FIXME: this function should be customizable through format +static StringRef getEndOfLine() { + static const char *const EOL = "\n"; + return EOL; +} + // Return the text representation of the given `APInt Val`: static std::string getAPIntText(APInt Val) { SmallVector Txt; @@ -1094,6 +1103,221 @@ return FixIts; } +static bool hasConflictingOverload(Sema &S, const FunctionDecl *FD) { + return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult(); +} + +// Returns the text representation of clang::unsafe_buffer_usage attribute. +// (FIXME:) The text could vary depending on whether macros over the attribute +// are defined. +static std::string getUnsafeBufferUsageAttributeText() { + std::stringstream SS; + + SS << "#if __has_cpp_attribute(clang::unsafe_buffer_usage)" + << getEndOfLine().data() << "[[clang::unsafe_buffer_usage]]" + << getEndOfLine().data() << "#endif" << getEndOfLine().data(); + return SS.str(); +} + +// Returns the name of `FD` including qualifiers if `FD` is declared with qualifiers. +static std::string getFunctionQualifiedName(const FunctionDecl *FD, const SourceManager &SM) { + std::stringstream SS; + + if (FD->getQualifier()) { + SS << std::string(SM.getCharacterData(FD->getQualifierLoc().getBeginLoc()), + SM.getCharacterData(FD->getQualifierLoc().getEndLoc())); + SS << "::" << FD->getNameAsString(); + } else { + SS << FD->getNameAsString(); + } + return SS.str(); +} + +// For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new +// type, this function produces fix-its to make the change self-contained. Let +// 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity +// defined by the `FunDecl` after the change to the parameter. Fix-its produced +// by this function are +// 1. Add the `[[clang::unsafe_buffer_usage]]` attribute to each declaration +// of 'F'; +// 2. Create a declaration of "NewF" next to each declaration of `F`; +// 3. Create a definition of "F" (as its' original definition is now belongs +// to "NewF") next to its original definition. The body of the creating +// definition calls to "NewF". +// +// Example: +// +// void f(int *p); // original declaration +// void f(int *p) { // original definition +// p[5]; +// } +// +// To change the parameter `p` to be of `std::span` type, we +// also add overloads: +// +// [[clang::unsafe_buffer_usage]] void f(int *p); // original decl +// void f(std::span p); // added overload decl +// void f(std::span p) { // original def where param is changed +// p[5]; +// } +// [[clang::unsafe_buffer_usage]] void f(int *p) { // added def +// return f(std::span(p, <# size #>)); +// } +// +// The actual fix-its may contain more details, e.g., the attribute may be guard +// by a macro +// #if __has_cpp_attribute(clang::unsafe_buffer_usage) +// [[clang::unsafe_buffer_usage]] +// #endif +// +// `NewTypeText` is the string representation of the new type, to which the +// parameter indexed by `ParmIdx` is being changed. +static std::optional +creatingOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText, + const FunctionDecl *FD, const ASTContext &Ctx, + Sema &S, UnsafeBufferUsageHandler &Handler) { + // FIXME: need to make this conflict checking better: + if (hasConflictingOverload(S, FD)) + return std::nullopt; + + const SourceManager &SM = Ctx.getSourceManager(); + const LangOptions &LangOpts = Ctx.getLangOpts(); + // FIXME Respect indentation of the original code. + + // A lambda that creates the text representation of a function declaration + // with the new type signature: + const auto NewOverloadSignatureCreator = + [&SM](const FunctionDecl *FD, unsigned ParmIdx, + StringRef NewTypeText) -> std::string { + std::stringstream SS; + + SS << ";"; + SS << getEndOfLine().data(); + // Appending: ret-type func-name "(" + SS << std::string(SM.getCharacterData(FD->getBeginLoc()), + SM.getCharacterData((*FD->param_begin())->getBeginLoc())); + + // Appending: parameter-type-list + const unsigned NumParms = FD->getNumParams(); + + for (unsigned i = 0; i < NumParms; i++) { + const ParmVarDecl *Parm = FD->getParamDecl(i); + + if (Parm->isImplicit()) + continue; + if (i == ParmIdx) + SS << NewTypeText.data(); + else + SS << Parm->getType().getAsString(); + if (Parm->getIdentifier()) + SS << " " << Parm->getNameAsString(); + if (i != NumParms - 1) + SS << ", "; + } + SS << ")"; + return SS.str(); + }; + + // A lambda that creates the text representation of a function definition with + // the original signature: + const auto OldOverloadDefCreator = + [&SM, &Handler](const FunctionDecl *FD, unsigned ParmIdx, + StringRef NewTypeText) -> std::string { + std::stringstream SS; + + SS << getEndOfLine().data(); + // Appending: attr-name ret-type func-name "(" param-list ")" "{" + SS << getUnsafeBufferUsageAttributeText() + << std::string(SM.getCharacterData(FD->getBeginLoc()), + SM.getCharacterData(FD->getBody()->getBeginLoc())) + << "{"; + // Appending: "return " func-name "(" + SS << "return " << getFunctionQualifiedName(FD, SM) << "("; + + // Appending: arg-list + const unsigned NumParms = FD->getNumParams(); + for (unsigned i = 0; i < NumParms; i++) { + const ParmVarDecl *Parm = FD->getParamDecl(i); + + if (Parm->isImplicit()) + continue; + assert(Parm->getIdentifier() && + "A parameter of a function definition has no name"); + if (i == ParmIdx) + // This is our spanified paramter! + SS << NewTypeText.data() << "(" << Parm->getName().str() << ", " + << Handler.getUserFillPlaceHolder("size") << ")"; + else + SS << Parm->getName().str(); + if (i != NumParms - 1) + SS << ", "; + } + // finish call and the body + SS << ");}" << getEndOfLine().data(); + // FIXME: 80-char line formatting? + return SS.str(); + }; + + FixItList FixIts{}; + for (FunctionDecl *FReDecl : FD->redecls()) { + if (FReDecl->isThisDeclarationADefinition()) { + assert(FReDecl == FD && "inconsistent function definition"); + // Inserts a definition with the old signature to the end of + // `FReDecl`: + FixIts.emplace_back(FixItHint::CreateInsertion( + getPastLoc(FReDecl, SM, LangOpts), + OldOverloadDefCreator(FReDecl, ParmIdx, NewTyText))); + } else { + // Adds the unsafe-buffer attribute (if not already there) to `FReDecl`: + if (!FReDecl->hasAttr()) { + FixIts.emplace_back(FixItHint::CreateInsertion( + FReDecl->getBeginLoc(), getUnsafeBufferUsageAttributeText())); + } + // Inserts a declaration with the new signature to the end of `FReDecl`: + FixIts.emplace_back(FixItHint::CreateInsertion( + getPastLoc(FReDecl, SM, LangOpts), + NewOverloadSignatureCreator(FReDecl, ParmIdx, NewTyText))); + } + } + return FixIts; +} + +// To fix a `ParmVarDecl` to be of `std::span` type. In addition, creating a +// new overload of the function so that the change is self-contained (see +// `creatingOverloadsForFixedParams`). +static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx, + Sema &S, UnsafeBufferUsageHandler &Handler) { + if (PVD->hasDefaultArg()) + // FIXME: generate fix-its for default values: + return {}; + assert(PVD->getType()->isPointerType()); + auto *FD = dyn_cast(PVD->getDeclContext()); + + if (!FD) + return {}; + + std::string NewTyText = + "std::span<" + PVD->getType()->getPointeeType().getAsString() + ">"; + std::string NewDeclText = NewTyText + " " + PVD->getName().str(); + FixItList Fixes; + unsigned ParmIdx = 0; + + Fixes.push_back( + FixItHint::CreateReplacement(PVD->getSourceRange(), NewDeclText)); + for (auto ParmIter : FD->parameters()) { + if (PVD == ParmIter) + break; + ParmIdx++; + } + if (ParmIdx < FD->getNumParams()) + if (auto OverloadFix = creatingOverloadsForFixedParams( + ParmIdx, NewTyText, FD, Ctx, S, Handler)) { + Fixes.append(*OverloadFix); + return Fixes; + } + return {}; +} + static FixItList fixVariableWithSpan(const VarDecl *VD, const DeclUseTracker &Tracker, const ASTContext &Ctx, @@ -1113,14 +1337,44 @@ return fixVarDeclWithSpan(VD, Ctx, Handler.getUserFillPlaceHolder()); } +// TODO: we should be consistent to use `std::nullopt` to represent no-fix due +// to any unexpected problem. static FixItList fixVariable(const VarDecl *VD, Strategy::Kind K, + /* The function decl under analysis */ const Decl *D, const DeclUseTracker &Tracker, - const ASTContext &Ctx, + const ASTContext &Ctx, Sema &S, UnsafeBufferUsageHandler &Handler) { + if (const auto *PVD = dyn_cast(VD)) { + auto *FD = dyn_cast(PVD->getDeclContext()); + if (!FD || FD != D) + // `FD != D` means that `PVD` belongs to a function that is not being + // analyzed currently. Thus `FD` may not be complete. + return {}; + + // TODO If function has a try block we can't change params unless we check + // also its catch block for their use. + // FIXME We might support static class methods, some select methods, + // operators and possibly lamdas. + if (FD->isMain() || FD->isConstexpr() || + FD->getTemplatedKind() != FunctionDecl::TemplatedKind::TK_NonTemplate || + FD->isVariadic() || + // also covers call-operator of lamdas + isa(FD) || + // skip when the function body is a try-block + (FD->hasBody() && isa(FD->getBody())) || + FD->isOverloadedOperator()) + return {}; // TODO test all these cases + } + switch (K) { case Strategy::Kind::Span: { - if (VD->getType()->isPointerType() && VD->isLocalVarDecl()) - return fixVariableWithSpan(VD, Tracker, Ctx, Handler); + if (VD->getType()->isPointerType()) { + if (const auto *PVD = dyn_cast(VD)) + return fixParamWithSpan(PVD, Ctx, S, Handler); + + if (VD->isLocalVarDecl()) + return fixVariableWithSpan(VD, Tracker, Ctx, Handler); + } return {}; } case Strategy::Kind::Iterator: @@ -1149,12 +1403,15 @@ static std::map getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S, - const DeclUseTracker &Tracker, const ASTContext &Ctx, + /* The function decl under analysis */ const Decl *D, + const DeclUseTracker &Tracker, Sema &Sema, UnsafeBufferUsageHandler &Handler) { + const ASTContext& Ctx = D->getASTContext(); std::map FixItsForVariable; for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) { + const Strategy::Kind ReplacementTypeForVD = S.lookup(VD); FixItsForVariable[VD] = - fixVariable(VD, S.lookup(VD), Tracker, Ctx, Handler); + fixVariable(VD, ReplacementTypeForVD, D, Tracker, Ctx, Sema, Handler); // If we fail to produce Fix-It for the declaration we have to skip the // variable entirely. if (FixItsForVariable[VD].empty()) { @@ -1202,6 +1459,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, + Sema & S, bool EmitFixits) { assert(D && D->getBody()); @@ -1222,8 +1480,9 @@ // Filter out non-local vars and vars with unclaimed DeclRefExpr-s. for (auto it = FixablesForUnsafeVars.byVar.cbegin(); it != FixablesForUnsafeVars.byVar.cend();) { - // FIXME: Support ParmVarDecl as well. - if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) { + // FIXME: need to deal with global variables later + if ((!it->first->isLocalVarDecl() && !isa(it->first)) || + Tracker.hasUnclaimedUses(it->first)) { it = FixablesForUnsafeVars.byVar.erase(it); } else { ++it; @@ -1235,8 +1494,8 @@ UnsafeVars.push_back(VD); Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars); - FixItsForVariable = getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker, - D->getASTContext(), Handler); + FixItsForVariable = + getFixIts(FixablesForUnsafeVars, NaiveStrategy, D, Tracker, S, Handler); // FIXME Detect overlapping FixIts. } Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2349,7 +2349,7 @@ if (ThisNodeHasBody) { UnsafeBufferUsageReporter R(S); - checkUnsafeBufferUsage(Node, R, EmitFixits); + checkUnsafeBufferUsage(Node, R, S, EmitFixits); } } // Continue to traverse descendants: Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// We do not fix parameters of the `main` function + +// CHECK-NOT: fix-it: + +// main function +int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe pointer used for buffer access}} + char tmp; + tmp = argv[5][5]; // expected-note{{used in buffer access here}} \ + expected-warning{{unsafe buffer access}} +} Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp @@ -0,0 +1,112 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -include %s %s 2>&1 | FileCheck %s + +// We cannot deal with overload conflicts for now so NO fix-it to +// function parameters will be emitted if there are overloads for that +// function. + +#ifndef INCLUDE_ME +#define INCLUDE_ME + +void baz(); + +#else + + +void foo(int *p, int * q); + +void foo(int *p); + +void foo(int *p) { + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: + int tmp; + tmp = p[5]; +} + +// an overload declaration of `bar(int *)` appears after it +void bar(int *p) { + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: + int tmp; + tmp = p[5]; +} + +void bar(); + +// an overload declaration of `baz(int)` appears is included +void baz(int *p) { + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]: + int tmp; + tmp = p[5]; +} + +namespace NS { + // `NS::foo` is a distinct function from `foo`, so it has no + // overload and shall be fixed. + void foo(int *p) { + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:18}:"std::span p" + int tmp; + tmp = p[5]; + } + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid foo(int *p) {return foo(std::span(p, <# size #>));}\n" + + // Similarly, `NS::bar` is distinct from `bar`: + void bar(int *p); + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:3}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid bar(std::span p)" +} // end of namespace NS + +// This is the implementation of `NS::bar`, which shall be fixed. +void NS::bar(int *p) { + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:20}:"std::span p" + int tmp; + tmp = p[5]; +} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid NS::bar(int *p) {return NS::bar(std::span(p, <# size #>));}\n" + +namespace NESTED { + void alpha(int); + void beta(int *, int *); + + namespace INNER { + // `NESTED::INNER::alpha` is distinct from `NESTED::alpha`, so it + // has no overload and shall be fixed. + void alpha(int *p) { + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:22}:"std::span p" + int tmp; + tmp = p[5]; + } + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:6}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid alpha(int *p) {return alpha(std::span(p, <# size #>));}\n" + } +} + +namespace NESTED { + // There is an `NESTED::beta(int *, int *)` declared above, so this + // unsafe function will not be fixed. + void beta(int *p) { + // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]: + int tmp; + tmp = p[5]; + } + + namespace INNER { + void delta(int); // No fix for `NESTED::INNER::delta` + void delta(int*); + } +} + +// There is an `NESTED::beta(int *)` declared above, so this unsafe +// function will not be fixed. +void NESTED::beta(int *p, int *q) { + // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]: + int tmp; + tmp = p[5]; + tmp = q[5]; +} + +void NESTED::INNER::delta(int * p) { + // CHECK-NOT: fix-it:"{{.*}}":[[@LINE-1]]: + int tmp; + tmp = p[5]; +} + + +#endif Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp @@ -0,0 +1,48 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +namespace NS1 { + void foo(int *); + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" + // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:18-[[@LINE-2]]:18}:";\nvoid foo(std::span)" + namespace NS2 { + void foo(int *); + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" + // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid foo(std::span)" + namespace NS3 { + void foo(int *); + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:7-[[@LINE-1]]:7}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" + // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:22-[[@LINE-2]]:22}:";\nvoid foo(std::span)" + } + } + + typedef int MyType; +} + +void NS1::foo(int *p) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span p" + int tmp; + tmp = p[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid NS1::foo(int *p) {return NS1::foo(std::span(p, <# size #>));}\n" + +void NS1::NS2::foo(int *p) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:20-[[@LINE-1]]:26}:"std::span p" + int tmp; + tmp = p[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid NS1::NS2::foo(int *p) {return NS1::NS2::foo(std::span(p, <# size #>));}\n" + +void NS1::NS2::NS3::foo(int *p) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:31}:"std::span p" + int tmp; + tmp = p[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid NS1::NS2::NS3::foo(int *p) {return NS1::NS2::NS3::foo(std::span(p, <# size #>));}\n" + + +void f(NS1::MyType * x) { + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:8-[[@LINE-1]]:23}:"std::span x" + NS1::MyType tmp; + tmp = x[5]; +} +// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid f(NS1::MyType * x) {return f(std::span(x, <# size #>));}\n" Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -include %s %s 2>&1 | FileCheck %s + +// TODO test if there's not a single character in the file after a decl or def + +#ifndef INCLUDE_ME +#define INCLUDE_ME + +void simple(int *p); +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid simple(std::span p)" + +#else + +void simple(int *); +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\n" +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid simple(std::span)" + +void simple(int *p) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:13-[[@LINE-1]]:19}:"std::span p" + int tmp; + tmp = p[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid simple(int *p) {return simple(std::span(p, <# size #>));}\n" + + +void twoParms(int *p, int * q) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span p" + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:23-[[@LINE-2]]:30}:"std::span q" + int tmp; + tmp = p[5] + q[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid twoParms(int *p, int * q) {return twoParms(std::span(p, <# size #>), q);}\n" +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:2-[[@LINE-2]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid twoParms(int *p, int * q) {return twoParms(p, std::span(q, <# size #>));}\n" + +void decayedArrayParm(int arr[]) { + // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:23-[[@LINE-1]]:32}:"std::span arr" + int tmp; + tmp = arr[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid decayedArrayParm(int arr[]) {return decayedArrayParm(std::span(arr, <# size #>));}\n" + + +void ptrToConst(const int * x) { + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span x" + int tmp = x[5]; +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid ptrToConst(const int * x) {return ptrToConst(std::span(x, <# size #>));}\n" + +#endif Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp @@ -0,0 +1,116 @@ +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// We handle cv-qualifiers poorly: + +void const_ptr(int * const x) { // expected-warning{{'x' is an unsafe pointer used for buffer access}} expected-note{{change type of 'x' to 'std::span' to preserve bounds information}} + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:29}:"std::span x" + // FIXME: the fix-it above is incorrect + int tmp = x[5]; // expected-note{{used in buffer access here}} +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid const_ptr(int * const x) {return const_ptr(std::span(x, <# size #>));}\n" +// FIXME: the fix-it above is incorrect + +void const_ptr_to_const(const int * const x) {// expected-warning{{'x' is an unsafe pointer used for buffer access}} expected-note{{change type of 'x' to 'std::span' to preserve bounds information}} + // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:44}:"std::span x" + // FIXME: the fix-it above is incorrect + int tmp = x[5]; // expected-note{{used in buffer access here}} +} +// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n#if __has_cpp_attribute(clang::unsafe_buffer_usage)\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n#endif\nvoid const_ptr_to_const(const int * const x) {return const_ptr_to_const(std::span(x, <# size #>));}\n" +// FIXME: the fix-it above is incorrect + +// We do not fix parameters participating unsafe operations for the +// following functions/methods or function-like expressions: + +// CHECK-NOT: fix-it: +class A { + // constructor & descructor + A(int * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + int tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} + } + + // class member methods + void foo(int *p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + int tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} + } + + // overload operator + int operator+(int * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + int tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} + return tmp; + } +}; + +// lambdas +void foo() { + auto Lamb = [&](int *p) // expected-warning{{'p' is an unsafe pointer used for buffer access}} + -> int { + int tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} + return tmp; + }; +} + +// template +template +void template_foo(T * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + T tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} +} + +void instantiate_template_foo() { + int * p; + template_foo(p); // FIXME expected note {{in instantiation of function template specialization 'template_foo' requested here}} +} + +// variadic function +void vararg_foo(int * p...) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + int tmp; + tmp = p[5]; // expected-note{{used in buffer access here}} +} + +// constexpr functions +constexpr int constexpr_foo(int * p) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} + return p[5]; // expected-note{{used in buffer access here}} +} + +// function body is a try-block +void fn_with_try_block(int* p) // expected-warning{{'p' is an unsafe pointer used for buffer access}} + try { + int tmp; + + if (p == nullptr) + throw 42; + tmp = p[5]; // expected-note{{used in buffer access here}} + } + catch (int) { + *p = 0; + } + +// The following two unsupported cases are not specific to +// parm-fixits. Adding them here in case they get forgotten. +void isArrayDecayToPointerUPC(int a[][10], int (*b)[10]) { +// expected-warning@-1{{'a' is an unsafe pointer used for buffer access}} +// expected-warning@-2{{'b' is an unsafe pointer used for buffer access}} + int tmp; + + tmp = a[5][5] + b[5][5]; // expected-warning2{{unsafe buffer access}} expected-note2{{used in buffer access here}} +} + +// parameter having default values: +void parmWithDefaultValue(int * x = 0) { + // expected-warning@-1{{'x' is an unsafe pointer used for buffer access}} + int tmp; + tmp = x[5]; // expected-note{{used in buffer access here}} +} + +void parmWithDefaultValueDecl(int * x = 0); + +void parmWithDefaultValueDecl(int * x) { + // expected-warning@-1{{'x' is an unsafe pointer used for buffer access}} + int tmp; + tmp = x[5]; // expected-note{{used in buffer access here}} +} Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -34,7 +34,7 @@ void * voidPtrCall(void); char * charPtrCall(void); -void testArraySubscripts(int *p, int **pp) { +void testArraySubscripts(int *p, int **pp) { // expected-note{{change type of 'pp' to 'std::span' to preserve bounds information}} // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'pp' is an unsafe pointer used for buffer access}} foo(p[1], // expected-note{{used in buffer access here}} @@ -108,6 +108,7 @@ sizeof(decltype(p[1]))); // expected-note{{used in buffer access here}} } +// expected-note@+1{{change type of 'a' to 'std::span' to preserve bounds information}} void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'q' is an unsafe pointer used for buffer access}} @@ -323,7 +324,7 @@ template void fArr(int t[]); // FIXME: expected note {{in instantiation of}} -int testReturn(int t[]) { +int testReturn(int t[]) {// expected-note{{change type of 't' to 'std::span' to preserve bounds information}} // expected-warning@-1{{'t' is an unsafe pointer used for buffer access}} return t[1]; // expected-note{{used in buffer access here}} }