Skip to content

Commit 2a69280

Browse files
committed
More aggressively deduplicate global warnings based on contents. (llvm#112801)
I've been getting complaints from users being spammed by -gmodules missing file warnings going out of control because each object file depends on an entire DAG of PCM files that usually are all missing at once. To reduce this problem, this patch does two things: 1. Module now maintains a DenseMap<hash, once> that is used to display each warning only once, based on its actual text. 2. The PCM warning itself is reworded to include less details, such as the DIE offset, which is only useful to LLDB developers, who can get this from the dwarf log if they need it. Because the detail is omitted the hashing from (1) deduplicates the warnings. rdar://138144624 (cherry picked from commit 697a455) Conflicts: lldb/include/lldb/Core/Module.h
1 parent 0657c77 commit 2a69280

File tree

4 files changed

+61
-24
lines changed

4 files changed

+61
-24
lines changed

lldb/include/lldb/Core/Module.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include "llvm/ADT/DenseSet.h"
3232
#include "llvm/ADT/STLFunctionalExtras.h"
33+
#include "llvm/ADT/StableHashing.h"
3334
#include "llvm/ADT/StringRef.h"
3435
#include <optional>
3536
#include "llvm/Support/Chrono.h"
@@ -1076,11 +1077,14 @@ class Module : public std::enable_shared_from_this<Module>,
10761077
/// time for the symbol tables can be aggregated here.
10771078
StatsDuration m_symtab_index_time;
10781079

1079-
std::once_flag m_optimization_warning;
1080-
std::once_flag m_language_warning;
10811080
#ifdef LLDB_ENABLE_SWIFT
10821081
std::once_flag m_toolchain_mismatch_warning;
10831082
#endif
1083+
/// A set of hashes of all warnings and errors, to avoid reporting them
1084+
/// multiple times to the same Debugger.
1085+
llvm::DenseMap<llvm::stable_hash, std::unique_ptr<std::once_flag>>
1086+
m_shown_diagnostics;
1087+
std::recursive_mutex m_diagnostic_mutex;
10841088

10851089
void SymbolIndicesToSymbolContextList(Symtab *symtab,
10861090
std::vector<uint32_t> &symbol_indexes,
@@ -1112,6 +1116,7 @@ class Module : public std::enable_shared_from_this<Module>,
11121116
LazyBool m_is_swift_cxx_interop_enabled = eLazyBoolCalculate;
11131117
LazyBool m_is_embedded_swift = eLazyBoolCalculate;
11141118
#endif
1119+
std::once_flag *GetDiagnosticOnceFlag(llvm::StringRef msg);
11151120
};
11161121

11171122
} // namespace lldb_private

lldb/source/Core/Module.cpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,8 +1127,8 @@ void Module::ReportWarningOptimization(
11271127
ss << file_name
11281128
<< " was compiled with optimization - stepping may behave "
11291129
"oddly; variables may not be available.";
1130-
Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
1131-
&m_optimization_warning);
1130+
llvm::StringRef msg = ss.GetString();
1131+
Debugger::ReportWarning(msg.str(), debugger_id, GetDiagnosticOnceFlag(msg));
11321132
}
11331133

11341134
void Module::ReportWarningUnsupportedLanguage(
@@ -1138,8 +1138,8 @@ void Module::ReportWarningUnsupportedLanguage(
11381138
<< Language::GetNameForLanguageType(language)
11391139
<< "\". "
11401140
"Inspection of frame variables will be limited.";
1141-
Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
1142-
&m_language_warning);
1141+
llvm::StringRef msg = ss.GetString();
1142+
Debugger::ReportWarning(msg.str(), debugger_id, GetDiagnosticOnceFlag(msg));
11431143
}
11441144

11451145
#ifdef LLDB_ENABLE_SWIFT
@@ -1257,20 +1257,29 @@ void Module::ReportErrorIfModifyDetected(
12571257
}
12581258
}
12591259

1260+
std::once_flag *Module::GetDiagnosticOnceFlag(llvm::StringRef msg) {
1261+
std::lock_guard<std::recursive_mutex> guard(m_diagnostic_mutex);
1262+
auto &once_ptr = m_shown_diagnostics[llvm::stable_hash_name(msg)];
1263+
if (!once_ptr)
1264+
once_ptr = std::make_unique<std::once_flag>();
1265+
return once_ptr.get();
1266+
}
1267+
12601268
void Module::ReportError(const llvm::formatv_object_base &payload) {
12611269
StreamString strm;
12621270
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
1263-
strm.PutChar(' ');
1264-
strm.PutCString(payload.str());
1265-
Debugger::ReportError(strm.GetString().str());
1271+
std::string msg = payload.str();
1272+
strm << ' ' << msg;
1273+
Debugger::ReportError(strm.GetString().str(), {}, GetDiagnosticOnceFlag(msg));
12661274
}
12671275

12681276
void Module::ReportWarning(const llvm::formatv_object_base &payload) {
12691277
StreamString strm;
12701278
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
1271-
strm.PutChar(' ');
1272-
strm.PutCString(payload.str());
1273-
Debugger::ReportWarning(std::string(strm.GetString()));
1279+
std::string msg = payload.str();
1280+
strm << ' ' << msg;
1281+
Debugger::ReportWarning(strm.GetString().str(), {},
1282+
GetDiagnosticOnceFlag(msg));
12741283
}
12751284

12761285
void Module::LogMessage(Log *log, const llvm::formatv_object_base &payload) {

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,13 +2088,15 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
20882088
Status error = ModuleList::GetSharedModule(dwo_module_spec, module_sp,
20892089
nullptr, nullptr, nullptr);
20902090
if (!module_sp) {
2091+
// ReportWarning also rate-limits based on the warning string,
2092+
// but in a -gmodules build, each object file has a similar DAG
2093+
// of module dependencies that would all be listed here.
20912094
GetObjectFile()->GetModule()->ReportWarning(
2092-
"{0:x16}: unable to locate module needed for external types: "
2093-
"{1}\nerror: {2}\nDebugging will be degraded due to missing "
2094-
"types. Rebuilding the project will regenerate the needed "
2095-
"module files.",
2096-
die.GetOffset(), dwo_module_spec.GetFileSpec().GetPath().c_str(),
2097-
error.AsCString("unknown error"));
2095+
"{0}", error.AsCString("unknown error"));
2096+
GetObjectFile()->GetModule()->ReportWarning(
2097+
"Unable to locate module needed for external types.\n"
2098+
"Debugging will be degraded due to missing types. Rebuilding the "
2099+
"project will regenerate the needed module files.");
20982100
continue;
20992101
}
21002102

@@ -2114,12 +2116,11 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
21142116

21152117
if (dwo_id != dwo_dwo_id) {
21162118
GetObjectFile()->GetModule()->ReportWarning(
2117-
"{0:x16}: Module {1} is out-of-date (hash mismatch). Type "
2118-
"information "
2119-
"from this module may be incomplete or inconsistent with the rest of "
2120-
"the program. Rebuilding the project will regenerate the needed "
2121-
"module files.",
2122-
die.GetOffset(), dwo_module_spec.GetFileSpec().GetPath().c_str());
2119+
"Module {0} is out-of-date (hash mismatch).\n"
2120+
"Type information from this module may be incomplete or inconsistent "
2121+
"with the rest of the program. Rebuilding the project will "
2122+
"regenerate the needed module files.",
2123+
dwo_module_spec.GetFileSpec().GetPath());
21232124
}
21242125
}
21252126
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# REQUIRES: system-darwin
2+
# Test the rate-limiting of module not found warnings.
3+
# RUN: rm -rf %t
4+
# RUN: mkdir -p %t
5+
6+
# RUN: echo 'module "C" { header "c.h" }' >%t/module.modulemap
7+
# RUN: echo 'struct c {};' >>%t/c.h
8+
# RUN: echo '@import C;' >%t/a.m
9+
# RUN: echo 'struct a { struct c c; } a;' >>%t/a.m
10+
# RUN: echo '@import C;' >%t/b.m
11+
# RUN: echo 'struct b { struct c c; } b;' >>%t/b.m
12+
# RUN: echo 'int main() {}' >>%t/b.m
13+
14+
# RUN: %clang_host -fmodules -Xclang -fmodules-cache-path=%t/cache -I%t -g -gmodules %t/a.m -o %t/a.o -c
15+
# RUN: %clang_host -fmodules -Xclang -fmodules-cache-path=%t/cache -I%t -g -gmodules %t/b.m -o %t/b.o -c
16+
# RUN: %clang_host %t/a.o %t/b.o -o %t/a.out
17+
# RUN: rm -rf %t/cache
18+
# RUN: %lldb %t/a.out -o "b main" -o run -o "p a" -o "p b" -o q 2>&1 | FileCheck %s
19+
# CHECK: {{[ab]}}.o{{.*}}/cache/{{.*}}/C-{{.*}}.pcm' does not exist
20+
# CHECK-NOT: /cache/{{.*}}/C-{.*}.pcm' does not exist
21+
# CHECK: {{[ab]}}.o{{.*}}/cache/{{.*}}/C-{{.*}}.pcm' does not exist
22+
# CHECK-NOT: /cache/{{.*}}/C-{.*}.pcm' does not exist

0 commit comments

Comments
 (0)