Re-land "5b012bf5ab5fcb840fe7f6c8664b8981ce6f24f3"

Removed dependency on `clangSema` from UnsafeBufferAnalysis.
llvm · Jul 15, 2023 · a07a6f6 · a07a6f6
1 parent da822ce
commit a07a6f6
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 54 deletions.
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -46,14 +46,9 @@ class UnsafeBufferUsageHandler {
   /// Returns a reference to the `Preprocessor`:
   virtual bool isSafeBufferOptOut(const SourceLocation &Loc) const = 0;
 
-  /// Returns the text indicating that the user needs to provide input there:
   virtual std::string
-  getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") const {
-    std::string s = std::string("<# ");
-    s += HintTextToUser;
-    s += " #>";
-    return s;
-  }
+  getUnsafeBufferUsageAttributeTextAt(SourceLocation Loc,
+                                      StringRef WSSuffix = "") const = 0;
 };
 
 // This function invokes the analysis and allows the caller to react to it

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1290,6 +1290,14 @@ static StringRef getEndOfLine() {
   return EOL;
 }
 
+// Returns the text indicating that the user needs to provide input there:
+std::string getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") {
+  std::string s = std::string("<# ");
+  s += HintTextToUser;
+  s += " #>";
+  return s;
+}
+
 // Return the text representation of the given `APInt Val`:
 static std::string getAPIntText(APInt Val) {
   SmallVector<char> Txt;
@@ -1755,14 +1763,6 @@ 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
@@ -1809,7 +1809,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
   // 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.
@@ -1859,7 +1859,7 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
   // A lambda that creates the text representation of a function definition with
   // the original signature:
   const auto OldOverloadDefCreator =
-      [&Handler, &SM,
+      [&SM, &Handler,
        &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
                   StringRef NewTypeText) -> std::optional<std::string> {
     std::stringstream SS;
@@ -1869,7 +1869,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
     if (auto FDPrefix = getRangeText(
             SourceRange(FD->getBeginLoc(), FD->getBody()->getBeginLoc()), SM,
             LangOpts))
-      SS << getUnsafeBufferUsageAttributeText() << FDPrefix->str() << "{";
+      SS << Handler.getUnsafeBufferUsageAttributeTextAt(FD->getBeginLoc(), " ")
+         << FDPrefix->str() << "{";
     else
       return std::nullopt;
     // Append: "return" func-name "("
@@ -1889,8 +1890,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
              "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") << ")";
+        SS << NewTypeText.str() << "(" << Parm->getIdentifier()->getName().str() << ", "
+           << getUserFillPlaceHolder("size") << ")";
       else
         SS << Parm->getIdentifier()->getName().str();
       if (i != NumParms - 1)
@@ -1921,7 +1922,8 @@ createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
       // Adds the unsafe-buffer attribute (if not already there) to `FReDecl`:
       if (!FReDecl->hasAttr<UnsafeBufferUsageAttr>()) {
         FixIts.emplace_back(FixItHint::CreateInsertion(
-            FReDecl->getBeginLoc(), getUnsafeBufferUsageAttributeText()));
+            FReDecl->getBeginLoc(), Handler.getUnsafeBufferUsageAttributeTextAt(
+                                        FReDecl->getBeginLoc(), " ")));
       }
       // Inserts a declaration with the new signature to the end of `FReDecl`:
       if (auto NewOverloadDecl =
@@ -2013,7 +2015,7 @@ static FixItList fixVariableWithSpan(const VarDecl *VD,
   (void)DS;
 
   // FIXME: handle cases where DS has multiple declarations
-  return fixVarDeclWithSpan(VD, Ctx, Handler.getUserFillPlaceHolder());
+  return fixVarDeclWithSpan(VD, Ctx, getUserFillPlaceHolder());
 }
 
 // TODO: we should be consistent to use `std::nullopt` to represent no-fix due

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2281,6 +2281,31 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
   bool isSafeBufferOptOut(const SourceLocation &Loc) const override {
     return S.PP.isSafeBufferOptOut(S.getSourceManager(), Loc);
   }
+
+  // Returns the text representation of clang::unsafe_buffer_usage attribute.
+  // `WSSuffix` holds customized "white-space"s, e.g., newline or whilespace
+  // characters.
+  std::string
+  getUnsafeBufferUsageAttributeTextAt(SourceLocation Loc,
+                                      StringRef WSSuffix = "") const override {
+    Preprocessor &PP = S.getPreprocessor();
+    TokenValue ClangUnsafeBufferUsageTokens[] = {
+        tok::l_square,
+        tok::l_square,
+        PP.getIdentifierInfo("clang"),
+        tok::coloncolon,
+        PP.getIdentifierInfo("unsafe_buffer_usage"),
+        tok::r_square,
+        tok::r_square};
+
+    StringRef MacroName;
+
+    // The returned macro (it returns) is guaranteed not to be function-like:
+    MacroName = PP.getLastMacroWithSpelling(Loc, ClangUnsafeBufferUsageTokens);
+    if (MacroName.empty())
+      MacroName = "[[clang::unsafe_buffer_usage]]";
+    return MacroName.str() + WSSuffix.str();
+  }
 };
 } // namespace
 

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-attributes-spelling.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-attributes-spelling.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits  -fsafe-buffer-usage-suggestions -DCMD_UNSAFE_ATTR=[[clang::unsafe_buffer_usage]] %s 2>&1 | FileCheck %s
+
+
+// no need to check fix-its for definition in this test ...
+void foo(int *p) {
+  int tmp = p[5];
+}
+
+// Will use the macro defined from the command line:
+void foo(int *);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"CMD_UNSAFE_ATTR "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:";\nvoid foo(std::span<int>)"
+
+
+#undef CMD_UNSAFE_ATTR
+void foo(int *);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:";\nvoid foo(std::span<int>)"
+
+
+#define UNSAFE_ATTR [[clang::unsafe_buffer_usage]]
+
+void foo(int *);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"UNSAFE_ATTR "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:";\nvoid foo(std::span<int>)"
+
+#undef UNSAFE_ATTR
+
+#if __has_cpp_attribute(clang::unsafe_buffer_usage)
+#define UNSAFE_ATTR [[clang::unsafe_buffer_usage]]
+#endif
+
+void foo(int *);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"UNSAFE_ATTR "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:";\nvoid foo(std::span<int>)"
+
+#undef UNSAFE_ATTR
+
+#if __has_cpp_attribute(clang::unsafe_buffer_usage)
+// we don't know how to use this macro
+#define UNSAFE_ATTR(x) [[clang::unsafe_buffer_usage]]
+#endif
+
+void foo(int *);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:";\nvoid foo(std::span<int>)"
+
+#undef UNSAFE_ATTR
+
+#define UNSAFE_ATTR_1 [[clang::unsafe_buffer_usage]]
+#define UNSAFE_ATTR_2 [[clang::unsafe_buffer_usage]]
+#define UNSAFE_ATTR_3 [[clang::unsafe_buffer_usage]]
+
+// Should use the last defined macro (i.e., UNSAFE_ATTR_3) for
+// `[[clang::unsafe_buffer_usage]]`
+void foo(int *p);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"UNSAFE_ATTR_3 "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:17-[[@LINE-2]]:17}:";\nvoid foo(std::span<int> p)"
+
+
+#define WRONG_ATTR_1 [clang::unsafe_buffer_usage]]
+#define WRONG_ATTR_2 [[clang::unsafe_buffer_usage]
+#define WRONG_ATTR_3 [[clang::unsafe_buffer_usag]]
+
+// The last defined macro for
+// `[[clang::unsafe_buffer_usage]]` is still UNSAFE_ATTR_3
+void foo(int *p);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"UNSAFE_ATTR_3 "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:17-[[@LINE-2]]:17}:";\nvoid foo(std::span<int> p)"
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
@@ -46,11 +46,11 @@ namespace NS {
     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"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void 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-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:19}:";\nvoid bar(std::span<int> p)"
 } // end of namespace NS
 
@@ -60,7 +60,7 @@ void NS::bar(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"
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void NS::bar(int *p) {return NS::bar(std::span<int>(p, <# size #>));}\n"
 
 namespace NESTED {
   void alpha(int);
@@ -74,7 +74,7 @@ namespace NESTED {
       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"
+    // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:6-[[@LINE-1]]:6}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void alpha(int *p) {return alpha(std::span<int>(p, <# size #>));}\n"
   }
 }
 

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
@@ -2,15 +2,15 @@
 
 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-1]]:3-[[@LINE-1]]:3}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
   // 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-1]]:5-[[@LINE-1]]:5}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
     // 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-1]]:7-[[@LINE-1]]:7}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
       // CHECK-DAG: fix-it:"{{.*}}:{[[@LINE-2]]:22-[[@LINE-2]]:22}:";\nvoid foo(std::span<int>)"
     }
   }
@@ -23,26 +23,26 @@ void NS1::foo(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"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void 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"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void 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"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void 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"
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void f(NS1::MyType * x) {return f(std::span<NS1::MyType>(x, <# size #>));}\n"