[clang] 2568286 - [clang] Don't use the AST to display backend diagnostics
Arthur Eubanks via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 4 14:20:12 PDT 2021
Author: Arthur Eubanks
Date: 2021-10-04T14:14:32-07:00
New Revision: 2568286892310877f24313c8e1626e415f9ae406
URL: https://github.com/llvm/llvm-project/commit/2568286892310877f24313c8e1626e415f9ae406
DIFF: https://github.com/llvm/llvm-project/commit/2568286892310877f24313c8e1626e415f9ae406.diff
LOG: [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
Added:
Modified:
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/lib/CodeGen/CodeGenAction.cpp
clang/test/Frontend/backend-diagnostic.c
clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
clang/test/Misc/backend-stack-frame-diagnostics.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 35e0d2649f5ee..eacb7e4de0ead 100644
--- 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 6e1d7f320673c..9fb5f29c94ab5 100644
--- 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
diff erentiate
// 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 695158cdd186e..e5dc4a6bacac4 100644
--- 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 aabadc61ecf00..0eca44f49f27f 100644
--- 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 f0ceac00ea357..2059ab5a5bce1 100644
--- 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);
};
More information about the cfe-commits
mailing list