[clang] Don't use the AST to display backend diagnostics

We keep a map from function name to source location so we don't have to
do it via looking up a source location from the AST. However, since
function names can be long, we actually use a hash of the function name
as the key.

Additionally, we can't rely on Clang's printing of function names via
the AST, so we just demangle the name instead.

This is necessary to implement
https://lists.llvm.org/pipermail/cfe-dev/2021-September/068930.html.

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D110665

diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 35e0d26..eacb7e4 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -26,7 +26,7 @@ def err_fe_linking_module : Error<"cannot link module '%0': %1">, DefaultFatal;
 def warn_fe_linking_module : Warning<"linking module '%0': %1">, InGroup<LinkerWarnings>;
 def note_fe_linking_module : Note<"linking module '%0': %1">;
 
-def warn_fe_frame_larger_than : Warning<"stack frame size (%0) exceeds limit (%1) in %q2">,
+def warn_fe_frame_larger_than : Warning<"stack frame size (%0) exceeds limit (%1) in '%2'">,
     BackendInfo, InGroup<BackendFrameLargerThan>;
 def warn_fe_backend_frame_larger_than: Warning<"%0">,
     BackendInfo, InGroup<BackendFrameLargerThan>;

diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 6e1d7f3..9fb5f29 100644 (file)
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -25,6 +25,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/Demangle/Demangle.h"
@@ -126,6 +127,17 @@ namespace clang {
 
     SmallVector<LinkModule, 4> LinkModules;
 
+    // A map from mangled names to their function's source location, used for
+    // backend diagnostics as the Clang AST may be unavailable. We actually use
+    // the mangled name's hash as the key because mangled names can be very
+    // long and take up lots of space. Using a hash can cause name collision,
+    // but that is rare and the consequences are pointing to a wrong source
+    // location which is not severe. This is a vector instead of an actual map
+    // because we optimize for time building this map rather than time
+    // retrieving an entry, as backend diagnostics are uncommon.
+    std::vector<std::pair<llvm::hash_code, FullSourceLoc>>
+        ManglingFullSourceLocs;
+
     // This is here so that the diagnostic printer knows the module a diagnostic
     // refers to.
     llvm::Module *CurLinkModule = nullptr;
@@ -330,6 +342,15 @@ namespace clang {
       if (LinkInModules())
         return;
 
+      for (auto &F : getModule()->functions()) {
+        if (const Decl *FD = Gen->GetDeclForMangledName(F.getName())) {
+          auto Loc = FD->getASTContext().getFullLoc(FD->getLocation());
+          // TODO: use a fast content hash when available.
+          auto NameHash = llvm::hash_value(F.getName());
+          ManglingFullSourceLocs.push_back(std::make_pair(NameHash, Loc));
+        }
+      }
+
       EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef());
 
       EmitBackendOutput(Diags, HeaderSearchOpts, CodeGenOpts, TargetOpts,
@@ -376,6 +397,8 @@ namespace clang {
                                 bool &BadDebugInfo, StringRef &Filename,
                                 unsigned &Line, unsigned &Column) const;
 
+    Optional<FullSourceLoc> getFunctionSourceLocation(const Function &F) const;
+
     void DiagnosticHandlerImpl(const llvm::DiagnosticInfo &DI);
     /// Specialized handler for InlineAsm diagnostic.
     /// \return True if the diagnostic has been successfully reported, false
@@ -569,17 +592,16 @@ BackendConsumer::StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D) {
     // We do not know how to format other severities.
     return false;
 
-  if (const Decl *ND = Gen->GetDeclForMangledName(D.getFunction().getName())) {
-    // FIXME: Shouldn't need to truncate to uint32_t
-    Diags.Report(ND->getASTContext().getFullLoc(ND->getLocation()),
-                 diag::warn_fe_frame_larger_than)
-        << static_cast<uint32_t>(D.getStackSize())
-        << static_cast<uint32_t>(D.getStackLimit())
-        << Decl::castToDeclContext(ND);
-    return true;
-  }
+  auto Loc = getFunctionSourceLocation(D.getFunction());
+  if (!Loc)
+    return false;
 
-  return false;
+  // FIXME: Shouldn't need to truncate to uint32_t
+  Diags.Report(*Loc, diag::warn_fe_frame_larger_than)
+      << static_cast<uint32_t>(D.getStackSize())
+      << static_cast<uint32_t>(D.getStackLimit())
+      << llvm::demangle(D.getFunction().getName().str());
+  return true;
 }
 
 const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
@@ -608,9 +630,10 @@ const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
   // function definition. We use the definition's right brace to differentiate
   // from diagnostics that genuinely relate to the function itself.
   FullSourceLoc Loc(DILoc, SourceMgr);
-  if (Loc.isInvalid())
-    if (const Decl *FD = Gen->GetDeclForMangledName(D.getFunction().getName()))
-      Loc = FD->getASTContext().getFullLoc(FD->getLocation());
+  if (Loc.isInvalid()) {
+    if (auto MaybeLoc = getFunctionSourceLocation(D.getFunction()))
+      Loc = *MaybeLoc;
+  }
 
   if (DILoc.isInvalid() && D.isLocationAvailable())
     // If we were not able to translate the file:line:col information
@@ -623,6 +646,16 @@ const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
   return Loc;
 }
 
+Optional<FullSourceLoc>
+BackendConsumer::getFunctionSourceLocation(const Function &F) const {
+  auto Hash = llvm::hash_value(F.getName());
+  for (const auto &Pair : ManglingFullSourceLocs) {
+    if (Pair.first == Hash)
+      return Pair.second;
+  }
+  return Optional<FullSourceLoc>();
+}
+
 void BackendConsumer::UnsupportedDiagHandler(
     const llvm::DiagnosticInfoUnsupported &D) {
   // We only support warnings or errors.

diff --git a/clang/test/Frontend/backend-diagnostic.c b/clang/test/Frontend/backend-diagnostic.c
index 695158c..e5dc4a6 100644 (file)
--- a/clang/test/Frontend/backend-diagnostic.c
+++ b/clang/test/Frontend/backend-diagnostic.c
@@ -15,9 +15,9 @@
 
 extern void doIt(char *);
 
-// REGULAR: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in function 'stackSizeWarning'
-// PROMOTE: error: stack frame size ([[#]]) exceeds limit ([[#]]) in function 'stackSizeWarning'
-// IGNORE-NOT: stack frame size ([[#]]) exceeds limit ([[#]]) in function 'stackSizeWarning'
+// REGULAR: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
+// PROMOTE: error: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
+// IGNORE-NOT: stack frame size ([[#]]) exceeds limit ([[#]]) in 'stackSizeWarning'
 void stackSizeWarning() {
   char buffer[80];
   doIt(buffer);

diff --git a/clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp b/clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
index aabadc6..0eca44f 100644 (file)
--- a/clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
+++ b/clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
@@ -12,7 +12,7 @@ namespace frameSizeThunkWarning {
     virtual void f();
   };
 
-  // CHECK: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in function 'frameSizeThunkWarning::B::f'
+  // CHECK: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in 'frameSizeThunkWarning::B::f()'
   // CHECK: warning: stack frame size ([[#]]) exceeds limit ([[#]]) in function '_ZTv0_n12_N21frameSizeThunkWarning1B1fEv'
   void B::f() {
     volatile int x = 0; // Ensure there is stack usage.

diff --git a/clang/test/Misc/backend-stack-frame-diagnostics.cpp b/clang/test/Misc/backend-stack-frame-diagnostics.cpp
index f0ceac0..2059ab5 100644 (file)
--- a/clang/test/Misc/backend-stack-frame-diagnostics.cpp
+++ b/clang/test/Misc/backend-stack-frame-diagnostics.cpp
@@ -26,7 +26,7 @@ void frameSizeWarning(int, int) {}
 
 void frameSizeWarning();
 
-void frameSizeWarning() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in function 'frameSizeWarning'}}
+void frameSizeWarning() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in 'frameSizeWarning()'}}
   char buffer[80];
   doIt(buffer);
 }
@@ -45,7 +45,7 @@ void frameSizeWarningIgnored() {
 
 void frameSizeLocalClassWarning() {
   struct S {
-    S() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in function 'frameSizeLocalClassWarning()::S::S'}}
+    S() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in 'frameSizeLocalClassWarning()::S::S()'}}
       char buffer[80];
       doIt(buffer);
     }
@@ -55,7 +55,7 @@ void frameSizeLocalClassWarning() {
 
 void frameSizeLambdaWarning() {
   auto fn =
-      []() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in lambda expression}}
+      []() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in 'frameSizeLambdaWarning()::$_0::operator()() const'}}
     char buffer[80];
     doIt(buffer);
   };
@@ -64,7 +64,7 @@ void frameSizeLambdaWarning() {
 
 void frameSizeBlocksWarning() {
   auto fn =
-      ^() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in block literal}}
+      ^() { // expected-warning-re {{stack frame size ({{[0-9]+}}) exceeds limit ({{[0-9]+}}) in 'invocation function for block in frameSizeBlocksWarning()'}}
     char buffer[80];
     doIt(buffer);
   };