aeubanks updated this revision to Diff 376024.
aeubanks added a comment.
add comments (including why it's a vector and not an actual map)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110665/new/
https://reviews.llvm.org/D110665
Files:
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
Index: clang/test/Misc/backend-stack-frame-diagnostics.cpp
===================================================================
--- clang/test/Misc/backend-stack-frame-diagnostics.cpp
+++ clang/test/Misc/backend-stack-frame-diagnostics.cpp
@@ -26,7 +26,7 @@
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 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 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 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);
};
Index: clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
===================================================================
--- clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
+++ clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
@@ -12,7 +12,7 @@
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.
Index: clang/test/Frontend/backend-diagnostic.c
===================================================================
--- clang/test/Frontend/backend-diagnostic.c
+++ 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);
Index: clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ 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 @@
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
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.
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -26,7 +26,7 @@
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>;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
