[clang] 1e270be - [-Wunsafe-buffer-usage] Add fix-its for function parameters using the `span` strategy

via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 9 15:55:06 PDT 2023


Author: ziqingluo-90
Date: 2023-06-09T15:44:38-07:00
New Revision: 1e270be0886c3a770e7a967679552a02dfc1dca9

URL: https://github.com/llvm/llvm-project/commit/1e270be0886c3a770e7a967679552a02dfc1dca9
DIFF: https://github.com/llvm/llvm-project/commit/1e270be0886c3a770e7a967679552a02dfc1dca9.diff

LOG: [-Wunsafe-buffer-usage] Add fix-its for function parameters using the `span` strategy

Generate fix-its for function parameters that are raw pointers used
unsafely.  Currently, the analyzer fixes one parameter at a time.

Fix-its for a function parameter includes:

- Fix the parameter declaration of the definition, result in a new
  overload of the function. We call the function with the original
  signature the old overload.
- For any other existing declaration of the old overload, mark it with
  the [[unsafe_buffer_usage]] attribute and generate a new overload
  declaration next to it.
- Creates a new definition for the old overload, which is simply
  defined by a call to the new overload.

Reviewed by: NoQ (Artem Dergachev), t-rasmud (Rashmi Mudduluru), and
             jkorous (Jan Korous)

Differential revision: https://reviews.llvm.org/D143048

Added: 
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp

Modified: 
    clang/lib/Analysis/UnsafeBufferUsage.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c9cc4ccbfb5d5..2133cbaa9be8d 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1224,6 +1224,12 @@ UPCAddressofArraySubscriptGadget::getFixits(const Strategy &S) const {
   return std::nullopt; // something went wrong, no fix-it
 }
 
+// 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<char> Txt;
@@ -1275,6 +1281,83 @@ static std::optional<StringRef> getExprText(const Expr *E,
   return std::nullopt;
 }
 
+// Returns the literal text in `SourceRange SR`, if `SR` is a valid range.
+static std::optional<StringRef> getRangeText(SourceRange SR,
+                                             const SourceManager &SM,
+                                             const LangOptions &LangOpts) {
+  bool Invalid = false;
+  CharSourceRange CSR = CharSourceRange::getCharRange(SR.getBegin(), SR.getEnd());
+  StringRef Text = Lexer::getSourceText(CSR, SM, LangOpts, &Invalid);
+
+  if (!Invalid)
+    return Text;
+  return std::nullopt;
+}
+
+// Returns the text of the pointee type of `T` from a `VarDecl` of a pointer
+// type. The text is obtained through from `TypeLoc`s.  Since `TypeLoc` does not
+// have source ranges of qualifiers ( The `QualifiedTypeLoc` looks hacky too me
+// :( ), `Qualifiers` of the pointee type is returned separately through the
+// output parameter `QualifiersToAppend`.
+static std::optional<std::string>
+getPointeeTypeText(const ParmVarDecl *VD, const SourceManager &SM,
+                   const LangOptions &LangOpts,
+                   std::optional<Qualifiers> *QualifiersToAppend) {
+  QualType Ty = VD->getType();
+  QualType PteTy;
+
+  assert(Ty->isPointerType() && !Ty->isFunctionPointerType() &&
+         "Expecting a VarDecl of type of pointer to object type");
+  PteTy = Ty->getPointeeType();
+  if (PteTy->hasUnnamedOrLocalType())
+    // If the pointee type is unnamed, we can't refer to it
+    return std::nullopt;
+
+  TypeLoc TyLoc = VD->getTypeSourceInfo()->getTypeLoc();
+  TypeLoc PteTyLoc = TyLoc.getUnqualifiedLoc().getNextTypeLoc();
+  SourceLocation VDNameStartLoc = VD->getLocation();
+
+  if (!SM.isBeforeInTranslationUnit(PteTyLoc.getSourceRange().getEnd(),
+                                    VDNameStartLoc)) {
+    // We only deal with the cases where the source text of the pointee type
+    // appears on the left-hand side of the variable identifier completely,
+    // including the following forms:
+    // `T ident`,
+    // `T ident[]`, where `T` is any type.
+    // Examples of excluded cases are `T (*ident)[]` or `T ident[][n]`.
+    return std::nullopt;
+  }
+  if (PteTy.hasQualifiers()) {
+    // TypeLoc does not provide source ranges for qualifiers (it says it's
+    // intentional but seems fishy to me), so we cannot get the full text
+    // `PteTy` via source ranges.
+    *QualifiersToAppend = PteTy.getQualifiers();
+  }
+
+  // Note that TypeLoc.getEndLoc() returns the begin location of the last token:
+  SourceRange CSR{
+      PteTyLoc.getBeginLoc(),
+      Lexer::getLocForEndOfToken(PteTyLoc.getEndLoc(), 0, SM, LangOpts)};
+
+  return getRangeText(CSR, SM, LangOpts)->str();
+}
+
+// Returns the text of the name (with qualifiers) of a `FunctionDecl`.
+static std::optional<StringRef> getFunNameText(const FunctionDecl *FD,
+                                               const SourceManager &SM,
+                                               const LangOptions &LangOpts) {
+  SourceLocation BeginLoc = FD->getQualifier()
+                                ? FD->getQualifierLoc().getBeginLoc()
+                                : FD->getNameInfo().getBeginLoc();
+  // Note that `FD->getNameInfo().getEndLoc()` returns the begin location of the
+  // last token:
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+      FD->getNameInfo().getEndLoc(), 0, SM, LangOpts);
+  SourceRange NameRange{BeginLoc, EndLoc};
+
+  return getRangeText(NameRange, SM, LangOpts);
+}
+
 std::optional<FixItList>
 DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
   const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
@@ -1609,6 +1692,252 @@ static FixItList fixVarDeclWithSpan(const VarDecl *D, const ASTContext &Ctx,
   return FixIts;
 }
 
+static bool hasConflictingOverload(const FunctionDecl *FD) {
+  return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult();
+}
+
+// Returns the text representation of clang::unsafe_buffer_usage attribute.
+static std::string getUnsafeBufferUsageAttributeText() {
+  static const char *const RawAttr = "[[clang::unsafe_buffer_usage]]";
+  std::stringstream SS;
+  SS << RawAttr << getEndOfLine().str();
+  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<int>` type, we
+// also add overloads:
+//
+// [[clang::unsafe_buffer_usage]] void f(int *p); // original decl
+// void f(std::span<int> p);                      // added overload decl
+// void f(std::span<int> 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<FixItList>
+createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
+                                const FunctionDecl *FD, const ASTContext &Ctx,
+                                UnsafeBufferUsageHandler &Handler) {
+  // FIXME: need to make this conflict checking better:
+  if (hasConflictingOverload(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, &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
+                       StringRef NewTypeText) -> std::optional<std::string> {
+    std::stringstream SS;
+
+    SS << ";";
+    SS << getEndOfLine().str();
+    // Append: ret-type func-name "("
+    if (auto Prefix = getRangeText(
+            SourceRange(FD->getBeginLoc(), (*FD->param_begin())->getBeginLoc()),
+            SM, LangOpts))
+      SS << Prefix->str();
+    else
+      return std::nullopt; // give up
+    // Append: 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.str();
+        // print parameter name if provided:
+        if (IdentifierInfo * II = Parm->getIdentifier())
+          SS << " " << II->getName().str();
+      } else if (auto ParmTypeText =
+                     getRangeText(Parm->getSourceRange(), SM, LangOpts)) {
+        // print the whole `Parm` without modification:
+        SS << ParmTypeText->str();
+      } else
+        return std::nullopt; // something wrong, give up
+      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 =
+      [&Handler, &SM,
+       &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
+                  StringRef NewTypeText) -> std::optional<std::string> {
+    std::stringstream SS;
+
+    SS << getEndOfLine().str();
+    // Append: attr-name ret-type func-name "(" param-list ")" "{"
+    if (auto FDPrefix = getRangeText(
+            SourceRange(FD->getBeginLoc(), FD->getBody()->getBeginLoc()), SM,
+            LangOpts))
+      SS << getUnsafeBufferUsageAttributeText() << FDPrefix->str() << "{";
+    else
+      return std::nullopt;
+    // Append: "return" func-name "("
+    if (auto FunQualName = getFunNameText(FD, SM, LangOpts))
+      SS << "return " << FunQualName->str() << "(";
+    else
+      return std::nullopt;
+
+    // Append: 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.str() << "(" << Parm->getIdentifier()->getName().str() << ", "
+           << Handler.getUserFillPlaceHolder("size") << ")";
+      else
+        SS << Parm->getIdentifier()->getName().str();
+      if (i != NumParms - 1)
+        SS << ", ";
+    }
+    // finish call and the body
+    SS << ");}" << getEndOfLine().str();
+    // FIXME: 80-char line formatting?
+    return SS.str();
+  };
+
+  FixItList FixIts{};
+  for (FunctionDecl *FReDecl : FD->redecls()) {
+    std::optional<SourceLocation> Loc = getPastLoc(FReDecl, SM, LangOpts);
+
+    if (!Loc)
+      return {};
+    if (FReDecl->isThisDeclarationADefinition()) {
+      assert(FReDecl == FD && "inconsistent function definition");
+      // Inserts a definition with the old signature to the end of
+      // `FReDecl`:
+      if (auto OldOverloadDef =
+              OldOverloadDefCreator(FReDecl, ParmIdx, NewTyText))
+        FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *OldOverloadDef));
+      else
+        return {}; // give up
+    } else {
+      // Adds the unsafe-buffer attribute (if not already there) to `FReDecl`:
+      if (!FReDecl->hasAttr<UnsafeBufferUsageAttr>()) {
+        FixIts.emplace_back(FixItHint::CreateInsertion(
+            FReDecl->getBeginLoc(), getUnsafeBufferUsageAttributeText()));
+      }
+      // Inserts a declaration with the new signature to the end of `FReDecl`:
+      if (auto NewOverloadDecl =
+              NewOverloadSignatureCreator(FReDecl, ParmIdx, NewTyText))
+        FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *NewOverloadDecl));
+      else
+        return {};
+    }
+  }
+  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
+// `createOverloadsForFixedParams`).
+static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
+                                  UnsafeBufferUsageHandler &Handler) {
+  if (PVD->hasDefaultArg())
+    // FIXME: generate fix-its for default values:
+    return {};
+  assert(PVD->getType()->isPointerType());
+  auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
+
+  if (!FD)
+    return {};
+
+  std::optional<Qualifiers> PteTyQualifiers = std::nullopt;
+  std::optional<std::string> PteTyText = getPointeeTypeText(
+      PVD, Ctx.getSourceManager(), Ctx.getLangOpts(), &PteTyQualifiers);
+
+  if (!PteTyText)
+    return {};
+
+  std::optional<StringRef> PVDNameText = PVD->getIdentifier()->getName();
+
+  if (!PVDNameText)
+    return {};
+
+  std::string SpanOpen = "std::span<";
+  std::string SpanClose = ">";
+  std::string SpanTyText;
+  std::stringstream SS;
+
+  SS << SpanOpen << *PteTyText;
+  // Append qualifiers to span element type:
+  if (PteTyQualifiers)
+    SS << " " << PteTyQualifiers->getAsString();
+  SS << SpanClose;
+  // Save the Span type text:
+  SpanTyText = SS.str();
+  // Append qualifiers to the type of the parameter:
+  if (PVD->getType().hasQualifiers())
+    SS << " " << PVD->getType().getQualifiers().getAsString();
+  // Append parameter's name:
+  SS << " " << PVDNameText->str();
+
+  FixItList Fixes;
+  unsigned ParmIdx = 0;
+
+  Fixes.push_back(
+      FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str()));
+  for (auto *ParmIter : FD->parameters()) {
+    if (PVD == ParmIter)
+      break;
+    ParmIdx++;
+  }
+  if (ParmIdx < FD->getNumParams())
+    if (auto OverloadFix = createOverloadsForFixedParams(ParmIdx, SpanTyText,
+                                                           FD, Ctx, Handler)) {
+      Fixes.append(*OverloadFix);
+      return Fixes;
+    }
+  return {};
+}
+
 static FixItList fixVariableWithSpan(const VarDecl *VD,
                                      const DeclUseTracker &Tracker,
                                      const ASTContext &Ctx,
@@ -1628,14 +1957,44 @@ static FixItList fixVariableWithSpan(const VarDecl *VD,
   return fixVarDeclWithSpan(VD, Ctx, Handler.getUserFillPlaceHolder());
 }
 
-static FixItList fixVariable(const VarDecl *VD, Strategy::Kind K,
-                             const DeclUseTracker &Tracker,
-                             const ASTContext &Ctx,
-                             UnsafeBufferUsageHandler &Handler) {
+// 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,
+            UnsafeBufferUsageHandler &Handler) {
+  if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
+    auto *FD = dyn_cast<clang::FunctionDecl>(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<CXXMethodDecl>(FD) ||
+        // skip when the function body is a try-block
+        (FD->hasBody() && isa<CXXTryStmt>(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<ParmVarDecl>(VD))
+        return fixParamWithSpan(PVD, Ctx, Handler);
+
+      if (VD->isLocalVarDecl())
+        return fixVariableWithSpan(VD, Tracker, Ctx, Handler);
+    }
     return {};
   }
   case Strategy::Kind::Iterator:
@@ -1676,14 +2035,14 @@ static bool impossibleToFixForVar(const FixableGadgetSets &FixablesForUnsafeVars
 
 static std::map<const VarDecl *, FixItList>
 getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
-        const DeclUseTracker &Tracker, const ASTContext &Ctx,
-        UnsafeBufferUsageHandler &Handler,
-        const DefMapTy &VarGrpMap) {
+	  const ASTContext &Ctx,
+          /* The function decl under analysis */ const Decl *D,
+    const DeclUseTracker &Tracker, UnsafeBufferUsageHandler &Handler,
+	  const DefMapTy &VarGrpMap) {
   std::map<const VarDecl *, FixItList> FixItsForVariable;
   for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
-    const Strategy::Kind ReplacementTypeForVD = S.lookup(VD);
     FixItsForVariable[VD] =
-        fixVariable(VD, ReplacementTypeForVD, Tracker, Ctx, Handler);
+        fixVariable(VD, S.lookup(VD), D, Tracker, Ctx, Handler);
     // If we fail to produce Fix-It for the declaration we have to skip the
     // variable entirely.
     if (FixItsForVariable[VD].empty()) {
@@ -1752,8 +2111,8 @@ getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
         
         FixItList GroupFix;
         if (FixItsForVariable.find(Var) == FixItsForVariable.end()) {
-          GroupFix = fixVariable(Var, ReplacementTypeForVD, Tracker,
-                                           Var->getASTContext(), Handler);
+          GroupFix = fixVariable(Var, ReplacementTypeForVD, D,
+                                 Tracker, Var->getASTContext(), Handler);
         } else {
           GroupFix = FixItsForVariable[Var];
         }
@@ -1812,8 +2171,9 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
   // Filter out non-local vars and vars with unclaimed DeclRefExpr-s.
   for (auto it = FixablesForAllVars.byVar.cbegin();
        it != FixablesForAllVars.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<ParmVarDecl>(it->first)) ||
+          Tracker.hasUnclaimedUses(it->first)) {
       it = FixablesForAllVars.byVar.erase(it);
     } else {
       ++it;
@@ -1909,8 +2269,10 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
   }
 
   Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
-  FixItsForVariableGroup = getFixIts(FixablesForAllVars, NaiveStrategy, Tracker,
-                                D->getASTContext(), Handler, VariableGroupsMap);
+
+  FixItsForVariableGroup =
+      getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D,
+                Tracker, Handler, VariableGroupsMap);
 
   // FIXME Detect overlapping FixIts.
 

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
new file mode 100644
index 0000000000000..c6b095031e0e3
--- /dev/null
+++ b/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  -fsafe-buffer-usage-suggestions -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits  -fsafe-buffer-usage-suggestions %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}}
+}

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-overload.cpp
new file mode 100644
index 0000000000000..3bc3f4e333499
--- /dev/null
+++ b/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 -fsafe-buffer-usage-suggestions -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<int> p"
+    int tmp;
+    tmp = p[5];
+  }
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid foo(int *p) {return foo(std::span<int>(p, <# size #>));}\n"
+
+  // Similarly, `NS::bar` is distinct from `bar`:
+  void bar(int *p);
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid bar(std::span<int> 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<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS::bar(int *p) {return NS::bar(std::span<int>(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<int> p"
+      int tmp;
+      tmp = p[5];
+    }
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:6}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid alpha(int *p) {return alpha(std::span<int>(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

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span-qualified-names.cpp
new file mode 100644
index 0000000000000..5b68fc531d3a6
--- /dev/null
+++ b/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 -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
+
+namespace NS1 {
+  void foo(int *);
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+  // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:18-[[@LINE-2]]:18}:";\nvoid foo(std::span<int>)"
+  namespace NS2 {
+    void foo(int *);
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+    // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid foo(std::span<int>)"
+    namespace NS3 {
+      void foo(int *);
+      // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:7-[[@LINE-1]]:7}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+      // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:22-[[@LINE-2]]:22}:";\nvoid foo(std::span<int>)"
+    }
+  }
+
+  typedef int MyType;
+}
+
+void NS1::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::foo(int *p) {return NS1::foo(std::span<int>(p, <# size #>));}\n"
+
+void NS1::NS2::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:20-[[@LINE-1]]:26}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::NS2::foo(int *p) {return NS1::NS2::foo(std::span<int>(p, <# size #>));}\n"
+
+void NS1::NS2::NS3::foo(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:31}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid NS1::NS2::NS3::foo(int *p) {return NS1::NS2::NS3::foo(std::span<int>(p, <# size #>));}\n"
+
+
+void f(NS1::MyType * x) {
+  // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:8-[[@LINE-1]]:23}:"std::span<NS1::MyType> x"
+  NS1::MyType tmp;
+  tmp = x[5];
+}
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid f(NS1::MyType * x) {return f(std::span<NS1::MyType>(x, <# size #>));}\n"

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
new file mode 100644
index 0000000000000..bb9deecaefec4
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions -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}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:";\nvoid simple(std::span<int> p)"
+
+#else
+
+void simple(int *);
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid simple(std::span<int>)"
+
+void simple(int *p) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:13-[[@LINE-1]]:19}:"std::span<int> p"
+  int tmp;
+  tmp = p[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid simple(int *p) {return simple(std::span<int>(p, <# size #>));}\n"
+
+
+void twoParms(int *p, int * q) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:15-[[@LINE-1]]:21}:"std::span<int> p"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:23-[[@LINE-2]]:30}:"std::span<int> q"
+  int tmp;
+  tmp = p[5] + q[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid twoParms(int *p, int * q) {return twoParms(std::span<int>(p, <# size #>), q);}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:2-[[@LINE-2]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid twoParms(int *p, int * q) {return twoParms(p, std::span<int>(q, <# size #>));}\n"
+
+void ptrToConst(const int * x) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span<int const> x"
+  int tmp = x[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid ptrToConst(const int * x) {return ptrToConst(std::span<int const>(x, <# size #>));}\n"
+
+// The followings test cases where multiple FileIDs maybe involved
+// when the analyzer loads characters from source files.
+
+#define FUN_NAME(x) _##x##_
+
+// The analyzer reads `void FUNNAME(macro_defined_name)(` from the
+// source file.  The MACRO and this source file have 
diff erent
+// FileIDs.
+void FUN_NAME(macro_defined_name)(int * x) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:35-[[@LINE-1]]:42}:"std::span<int> x"
+  int tmp = x[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid FUN_NAME(macro_defined_name)(int * x) {return FUN_NAME(macro_defined_name)(std::span<int>(x, <# size #>));}\n"
+
+
+// The followings test various type specifiers
+namespace {
+  void simpleSpecifier(unsigned long long int *p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:24-[[@LINE-1]]:49}:"std::span<unsigned long long int> p"
+    auto tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid simpleSpecifier(unsigned long long int *p) {return simpleSpecifier(std::span<unsigned long long int>(p, <# size #>));}\n"
+
+  void attrParm([[maybe_unused]] int * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:34-[[@LINE-1]]:41}:"std::span<int> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid attrParm({{\[}}{{\[}}maybe_unused{{\]}}{{\]}} int * p) {return attrParm(std::span<int>(p, <# size #>));}\n"
+
+  using T = unsigned long long int;
+
+  void usingTypenameSpecifier(T * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:31-[[@LINE-1]]:36}:"std::span<T> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid usingTypenameSpecifier(T * p) {return usingTypenameSpecifier(std::span<T>(p, <# size #>));}\n"
+
+  typedef unsigned long long int T2;
+
+  void typedefSpecifier(T2 * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:31}:"std::span<T2> p"
+    int tmp = p[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid typedefSpecifier(T2 * p) {return typedefSpecifier(std::span<T2>(p, <# size #>));}\n"
+
+  class SomeClass {
+  } C;
+
+  void classTypeSpecifier(const class SomeClass * p) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:27-[[@LINE-1]]:52}:"std::span<class SomeClass const> p"
+    if (++p) {}
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid classTypeSpecifier(const class SomeClass * p) {return classTypeSpecifier(std::span<class SomeClass const>(p, <# size #>));}\n"
+
+  struct {
+    // anon
+  } ANON_S;
+
+  struct MyStruct {
+    // namned
+  } NAMED_S;
+
+  // FIXME: `decltype(ANON_S)` represents an unnamed type but it can
+  // be referred as "`decltype(ANON_S)`", so the analysis should
+  // fix-it.
+  void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,
+                         decltype(NAMED_S) ** rr) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:26-[[@LINE-2]]:41}:"std::span<decltype(C)> p"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:65-[[@LINE-3]]:86}:"std::span<decltype(NAMED_S)> r"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:26-[[@LINE-3]]:49}:"std::span<decltype(NAMED_S) *> rr"
+    if (++p) {}
+    if (++q) {}
+    if (++r) {}
+    if (++rr) {}
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span<decltype(C)>(p, <# size #>), q, r, rr);}\n
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, std::span<decltype(NAMED_S)>(r, <# size #>), rr);}\n"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:4-[[@LINE-3]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, r, std::span<decltype(NAMED_S) *>(rr, <# size #>));}\n"
+
+#define MACRO_TYPE(T) long T
+
+  void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:18-[[@LINE-1]]:46}:"std::span<unsigned MACRO_TYPE(int)> p"
+    // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:48-[[@LINE-2]]:77}:"std::span<unsigned MACRO_TYPE(long)> q"
+    int tmp = p[5];
+    tmp = q[5];
+  }
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span<unsigned MACRO_TYPE(int)>(p, <# size #>), q);}\n"
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(p, std::span<unsigned MACRO_TYPE(long)>(q, <# size #>));}\n"
+}
+
+// The followings test various declarators:
+void decayedArray(int a[]) {
+  // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:19-[[@LINE-1]]:26}:"std::span<int> a"
+  int tmp;
+  tmp = a[5];
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid decayedArray(int a[]) {return decayedArray(std::span<int>(a, <# size #>));}\n"
+
+void decayedArrayOfArray(int a[10][10]) {
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+  if (++a){}
+}
+
+void complexDeclarator(int * (*a[10])[10]) {
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+  if (++a){}
+}
+
+// Make sure we do not generate fixes for the following cases:
+
+#define MACRO_NAME MyName
+
+void macroIdentifier(int *MACRO_NAME) { // The fix-it ends with a macro. It will be discarded due to overlap with macros.
+  // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+  if (++MyName){}
+}
+
+#endif

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp
new file mode 100644
index 0000000000000..bc918a7b53e9c
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fcxx-exceptions -fsafe-buffer-usage-suggestions -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fcxx-exceptions -fdiagnostics-parseable-fixits -fsafe-buffer-usage-suggestions %s 2>&1 | FileCheck %s
+
+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<int> const x"
+  int tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid const_ptr(int * const x) {return const_ptr(std::span<int>(x, <# size #>));}\n"
+
+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-DAG: fix-it:{{.*}}:{[[@LINE-1]]:25-[[@LINE-1]]:44}:"std::span<int const> const x"
+  int tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid const_ptr_to_const(const int * const x) {return const_ptr_to_const(std::span<int const>(x, <# size #>));}\n"
+
+typedef struct {int x;} NAMED_UNNAMED_STRUCT; // an unnamed struct type named by a typedef
+typedef struct {int x;} * PTR_TO_ANON;        // pointer to an unnamed struct
+typedef NAMED_UNNAMED_STRUCT * PTR_TO_NAMED;  // pointer to a named type
+
+// We can fix a pointer to a named type
+void namedPointeeType(NAMED_UNNAMED_STRUCT * p) {  // expected-warning{{'p' is an unsafe pointer used for buffer access}}\ expected-note{{change type of 'p' to 'std::span' to preserve bounds information}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:23-[[@LINE-1]]:47}:"std::span<NAMED_UNNAMED_STRUCT> p"
+  if (++p) {  // expected-note{{used in pointer arithmetic here}}
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+  }
+}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}}\nvoid namedPointeeType(NAMED_UNNAMED_STRUCT * p) {return namedPointeeType(std::span<NAMED_UNNAMED_STRUCT>(p, <# size #>));}\n"
+
+// We CANNOT fix a pointer to an unnamed type
+// CHECK-NOT: fix-it:
+void unnamedPointeeType(PTR_TO_ANON p) {  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+  if (++p) {  // expected-note{{used in pointer arithmetic here}}
+  }
+}
+
+// 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<typename T>
+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<int>' 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 at -1{{'a' is an unsafe pointer used for buffer access}}
+// expected-warning at -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 at -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 at -1{{'x' is an unsafe pointer used for buffer access}}
+  int tmp;
+  tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+

diff  --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index 1f469a07da6a2..b6f16756bda68 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -36,7 +36,7 @@ void testIncrement(char *p) { // expected-warning{{'p' is an unsafe pointer used
 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 at -1{{'p' is an unsafe pointer used for buffer access}}
 // expected-warning at -2{{'pp' is an unsafe pointer used for buffer access}}
   foo(p[1],             // expected-note{{used in buffer access here}}
@@ -104,6 +104,12 @@ void testArraySubscriptsWithAuto(int *p, int **pp) {
   foo(ap4[1]);    // expected-note{{used in buffer access here}}
 }
 
+void testUnevaluatedContext(int * p) {// no-warning
+  foo(sizeof(p[1]),             // no-warning
+      sizeof(decltype(p[1])));  // no-warning
+}
+
+// expected-note at +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 at -1{{'p' is an unsafe pointer used for buffer access}}
   // expected-warning at -2{{'q' is an unsafe pointer used for buffer access}}
@@ -319,7 +325,7 @@ template<typename T> void fArr(T t[]) {
 
 template void fArr<int>(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 at -1{{'t' is an unsafe pointer used for buffer access}}
   return t[1]; // expected-note{{used in buffer access here}}
 }


        


More information about the cfe-commits mailing list