Skip to content

Commit 6879391

Browse files
JDevlieghereJDevlieghere
committed
[lldb] Replace Host::SystemLog with Debugger::Report{Error,Warning}
As it exists today, Host::SystemLog is used exclusively for error reporting. With the introduction of diagnostic events, we have a better way of reporting those. Instead of printing directly to stderr, these messages now get printed to the debugger's error stream (when using the default event handler). Alternatively, if someone is listening for these events, they can decide how to display them, for example in the context of an IDE such as Xcode. This change also means we no longer write these messages to the system log on Darwin. As far as I know, nobody is relying on this, but I think this is something we could add to the diagnostic event mechanism. Differential revision: https://reviews.llvm.org/D128480
1 parent 2faacf6 commit 6879391

File tree

13 files changed

+98
-184
lines changed

13 files changed

+98
-184
lines changed

‎lldb/include/lldb/Host/Host.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,6 @@ class Host {
8686
StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
8787
lldb::pid_t pid);
8888

89-
enum SystemLogType { eSystemLogWarning, eSystemLogError };
90-
91-
static void SystemLog(SystemLogType type, const char *format, ...)
92-
__attribute__((format(printf, 2, 3)));
93-
94-
static void SystemLog(SystemLogType type, const char *format, va_list args);
95-
9689
/// Get the process ID for the calling process.
9790
///
9891
/// \return

‎lldb/source/Core/Module.cpp

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,27 +1107,6 @@ void Module::GetDescription(llvm::raw_ostream &s,
11071107
s << llvm::formatv("({0})", object_name);
11081108
}
11091109

1110-
void Module::ReportError(const char *format, ...) {
1111-
if (format && format[0]) {
1112-
StreamString strm;
1113-
strm.PutCString("error: ");
1114-
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
1115-
strm.PutChar(' ');
1116-
va_list args;
1117-
va_start(args, format);
1118-
strm.PrintfVarArg(format, args);
1119-
va_end(args);
1120-
1121-
const int format_len = strlen(format);
1122-
if (format_len > 0) {
1123-
const char last_char = format[format_len - 1];
1124-
if (last_char != '\n' && last_char != '\r')
1125-
strm.EOL();
1126-
}
1127-
Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
1128-
}
1129-
}
1130-
11311110
bool Module::FileHasChanged() const {
11321111
// We have provided the DataBuffer for this module to avoid accessing the
11331112
// filesystem. We never want to reload those files.
@@ -1170,7 +1149,7 @@ void Module::ReportErrorIfModifyDetected(const char *format, ...) {
11701149
m_first_file_changed_log = true;
11711150
if (format) {
11721151
StreamString strm;
1173-
strm.PutCString("error: the object file ");
1152+
strm.PutCString("the object file ");
11741153
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
11751154
strm.PutCString(" has been modified\n");
11761155

@@ -1186,17 +1165,31 @@ void Module::ReportErrorIfModifyDetected(const char *format, ...) {
11861165
strm.EOL();
11871166
}
11881167
strm.PutCString("The debug session should be aborted as the original "
1189-
"debug information has been overwritten.\n");
1190-
Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData());
1168+
"debug information has been overwritten.");
1169+
Debugger::ReportError(std::string(strm.GetString()));
11911170
}
11921171
}
11931172
}
11941173
}
11951174

1175+
void Module::ReportError(const char *format, ...) {
1176+
if (format && format[0]) {
1177+
StreamString strm;
1178+
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
1179+
strm.PutChar(' ');
1180+
1181+
va_list args;
1182+
va_start(args, format);
1183+
strm.PrintfVarArg(format, args);
1184+
va_end(args);
1185+
1186+
Debugger::ReportError(std::string(strm.GetString()));
1187+
}
1188+
}
1189+
11961190
void Module::ReportWarning(const char *format, ...) {
11971191
if (format && format[0]) {
11981192
StreamString strm;
1199-
strm.PutCString("warning: ");
12001193
GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
12011194
strm.PutChar(' ');
12021195

@@ -1205,13 +1198,7 @@ void Module::ReportWarning(const char *format, ...) {
12051198
strm.PrintfVarArg(format, args);
12061199
va_end(args);
12071200

1208-
const int format_len = strlen(format);
1209-
if (format_len > 0) {
1210-
const char last_char = format[format_len - 1];
1211-
if (last_char != '\n' && last_char != '\r')
1212-
strm.EOL();
1213-
}
1214-
Host::SystemLog(Host::eSystemLogWarning, "%s", strm.GetData());
1201+
Debugger::ReportWarning(std::string(strm.GetString()));
12151202
}
12161203
}
12171204

‎lldb/source/Host/common/Host.cpp

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -219,32 +219,6 @@ MonitorChildProcessThreadFunction(::pid_t pid,
219219

220220
#endif // #if !defined (__APPLE__) && !defined (_WIN32)
221221

222-
#if !defined(__APPLE__)
223-
224-
void Host::SystemLog(SystemLogType type, const char *format, va_list args) {
225-
vfprintf(stderr, format, args);
226-
}
227-
228-
#endif
229-
230-
void Host::SystemLog(SystemLogType type, const char *format, ...) {
231-
{
232-
va_list args;
233-
va_start(args, format);
234-
SystemLog(type, format, args);
235-
va_end(args);
236-
}
237-
238-
Log *log = GetLog(LLDBLog::Host);
239-
if (log && log->GetVerbose()) {
240-
// Log to log channel. This allows testcases to grep for log output.
241-
va_list args;
242-
va_start(args, format);
243-
log->VAPrintf(format, args);
244-
va_end(args);
245-
}
246-
}
247-
248222
lldb::pid_t Host::GetCurrentProcessID() { return ::getpid(); }
249223

250224
#ifndef _WIN32

‎lldb/source/Host/macosx/objcxx/Host.mm

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,40 +1491,3 @@ static bool ShouldLaunchUsingXPC(ProcessLaunchInfo &launch_info) {
14911491
}
14921492
return HostThread();
14931493
}
1494-
1495-
//----------------------------------------------------------------------
1496-
// Log to both stderr and to ASL Logging when running on MacOSX.
1497-
//----------------------------------------------------------------------
1498-
void Host::SystemLog(SystemLogType type, const char *format, va_list args) {
1499-
if (format && format[0]) {
1500-
static aslmsg g_aslmsg = NULL;
1501-
if (g_aslmsg == NULL) {
1502-
g_aslmsg = ::asl_new(ASL_TYPE_MSG);
1503-
char asl_key_sender[PATH_MAX];
1504-
snprintf(asl_key_sender, sizeof(asl_key_sender),
1505-
"com.apple.LLDB.framework");
1506-
::asl_set(g_aslmsg, ASL_KEY_SENDER, asl_key_sender);
1507-
}
1508-
1509-
// Copy the va_list so we can log this message twice
1510-
va_list copy_args;
1511-
va_copy(copy_args, args);
1512-
// Log to stderr
1513-
::vfprintf(stderr, format, copy_args);
1514-
va_end(copy_args);
1515-
1516-
int asl_level;
1517-
switch (type) {
1518-
case eSystemLogError:
1519-
asl_level = ASL_LEVEL_ERR;
1520-
break;
1521-
1522-
case eSystemLogWarning:
1523-
asl_level = ASL_LEVEL_WARNING;
1524-
break;
1525-
}
1526-
1527-
// Log to ASL
1528-
::asl_vlog(NULL, g_aslmsg, asl_level, format, args);
1529-
}
1530-
}

‎lldb/source/Interpreter/Options.cpp

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -224,21 +224,25 @@ Option *Options::GetLongOptions() {
224224
option_seen.find(short_opt);
225225
StreamString strm;
226226
if (defs[i].HasShortOption())
227-
Host::SystemLog(Host::eSystemLogError,
228-
"option[%u] --%s has a short option -%c that "
229-
"conflicts with option[%u] --%s, short option won't "
230-
"be used for --%s\n",
231-
(int)i, defs[i].long_option, short_opt, pos->second,
232-
m_getopt_table[pos->second].definition->long_option,
233-
defs[i].long_option);
227+
Debugger::ReportError(
228+
llvm::formatv(
229+
"option[{0}] --{1} has a short option -{2} that "
230+
"conflicts with option[{3}] --{4}, short option won't "
231+
"be used for --{5}",
232+
i, defs[i].long_option, short_opt, pos->second,
233+
m_getopt_table[pos->second].definition->long_option,
234+
defs[i].long_option)
235+
.str());
234236
else
235-
Host::SystemLog(Host::eSystemLogError,
236-
"option[%u] --%s has a short option 0x%x that "
237-
"conflicts with option[%u] --%s, short option won't "
238-
"be used for --%s\n",
239-
(int)i, defs[i].long_option, short_opt, pos->second,
240-
m_getopt_table[pos->second].definition->long_option,
241-
defs[i].long_option);
237+
Debugger::ReportError(
238+
llvm::formatv(
239+
"option[{0}] --{1} has a short option {2:x} that "
240+
"conflicts with option[{3}] --{4}, short option won't "
241+
"be used for --{5}n",
242+
(int)i, defs[i].long_option, short_opt, pos->second,
243+
m_getopt_table[pos->second].definition->long_option,
244+
defs[i].long_option)
245+
.str());
242246
}
243247
}
244248

‎lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -337,11 +337,11 @@ bool DynamicLoaderDarwin::UnloadModuleSections(Module *module,
337337
section_sp, old_section_load_addr))
338338
changed = true;
339339
} else {
340-
Host::SystemLog(Host::eSystemLogWarning,
341-
"warning: unable to find and unload segment named "
342-
"'%s' in '%s' in macosx dynamic loader plug-in.\n",
343-
info.segments[i].name.AsCString("<invalid>"),
344-
image_object_file->GetFileSpec().GetPath().c_str());
340+
Debugger::ReportWarning(
341+
llvm::formatv("unable to find and unload segment named "
342+
"'{0}' in '{1}' in macosx dynamic loader plug-in",
343+
info.segments[i].name.AsCString("<invalid>"),
344+
image_object_file->GetFileSpec().GetPath()));
345345
}
346346
}
347347
}

‎lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,10 +1932,10 @@ class MachSymtabSectionInfo {
19321932
if (first_section_sp)
19331933
filename = first_section_sp->GetObjectFile()->GetFileSpec().GetPath();
19341934

1935-
Host::SystemLog(Host::eSystemLogError,
1936-
"error: unable to find section %d for a symbol in "
1937-
"%s, corrupt file?\n",
1938-
n_sect, filename.c_str());
1935+
Debugger::ReportError(
1936+
llvm::formatv("unable to find section {0} for a symbol in "
1937+
"{1}, corrupt file?",
1938+
n_sect, filename));
19391939
}
19401940
}
19411941
if (m_section_infos[n_sect].vm_range.Contains(file_addr)) {
@@ -2804,12 +2804,11 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
28042804
// No symbol should be NULL, even the symbols with no
28052805
// string values should have an offset zero which
28062806
// points to an empty C-string
2807-
Host::SystemLog(
2808-
Host::eSystemLogError,
2809-
"error: DSC unmapped local symbol[%u] has invalid "
2810-
"string table offset 0x%x in %s, ignoring symbol\n",
2807+
Debugger::ReportError(llvm::formatv(
2808+
"DSC unmapped local symbol[{0}] has invalid "
2809+
"string table offset {1:x} in {2}, ignoring symbol",
28112810
nlist_index, nlist.n_strx,
2812-
module_sp->GetFileSpec().GetPath().c_str());
2811+
module_sp->GetFileSpec().GetPath());
28132812
continue;
28142813
}
28152814
if (symbol_name[0] == '\0')
@@ -3730,11 +3729,10 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
37303729
if (symbol_name == nullptr) {
37313730
// No symbol should be NULL, even the symbols with no string values
37323731
// should have an offset zero which points to an empty C-string
3733-
Host::SystemLog(Host::eSystemLogError,
3734-
"error: symbol[%u] has invalid string table offset "
3735-
"0x%x in %s, ignoring symbol\n",
3736-
nlist_idx, nlist.n_strx,
3737-
module_sp->GetFileSpec().GetPath().c_str());
3732+
Debugger::ReportError(llvm::formatv(
3733+
"symbol[{0}] has invalid string table offset {1:x} in {2}, "
3734+
"ignoring symbol",
3735+
nlist_idx, nlist.n_strx, module_sp->GetFileSpec().GetPath()));
37383736
return true;
37393737
}
37403738
if (symbol_name[0] == '\0')

‎lldb/source/Symbol/CompactUnwindInfo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "lldb/Symbol/CompactUnwindInfo.h"
10+
#include "lldb/Core/Debugger.h"
1011
#include "lldb/Core/Module.h"
1112
#include "lldb/Core/Section.h"
1213
#include "lldb/Symbol/ObjectFile.h"
@@ -317,9 +318,8 @@ void CompactUnwindInfo::ScanIndex(const ProcessSP &process_sp) {
317318
m_unwindinfo_data.GetByteSize() ||
318319
indexSectionOffset > m_unwindinfo_data.GetByteSize() ||
319320
offset > m_unwindinfo_data.GetByteSize()) {
320-
Host::SystemLog(Host::eSystemLogError, "error: Invalid offset "
321-
"encountered in compact unwind "
322-
"info, skipping\n");
321+
Debugger::ReportError(
322+
"Invalid offset encountered in compact unwind info, skipping");
323323
// don't trust anything from this compact_unwind section if it looks
324324
// blatantly invalid data in the header.
325325
m_indexes_computed = eLazyBoolNo;

‎lldb/source/Symbol/DWARFCallFrameInfo.cpp

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "lldb/Symbol/DWARFCallFrameInfo.h"
10+
#include "lldb/Core/Debugger.h"
1011
#include "lldb/Core/Module.h"
1112
#include "lldb/Core/Section.h"
1213
#include "lldb/Core/dwarf.h"
@@ -268,9 +269,9 @@ DWARFCallFrameInfo::ParseCIE(const dw_offset_t cie_offset) {
268269
cie_sp->ptr_encoding = DW_EH_PE_absptr; // default
269270
cie_sp->version = m_cfi_data.GetU8(&offset);
270271
if (cie_sp->version > CFI_VERSION4) {
271-
Host::SystemLog(Host::eSystemLogError,
272-
"CIE parse error: CFI version %d is not supported\n",
273-
cie_sp->version);
272+
Debugger::ReportError(
273+
llvm::formatv("CIE parse error: CFI version {0} is not supported",
274+
cie_sp->version));
274275
return nullptr;
275276
}
276277

@@ -287,10 +288,10 @@ DWARFCallFrameInfo::ParseCIE(const dw_offset_t cie_offset) {
287288

288289
if (i == CFI_AUG_MAX_SIZE &&
289290
cie_sp->augmentation[CFI_AUG_MAX_SIZE - 1] != '\0') {
290-
Host::SystemLog(Host::eSystemLogError,
291-
"CIE parse error: CIE augmentation string was too large "
292-
"for the fixed sized buffer of %d bytes.\n",
293-
CFI_AUG_MAX_SIZE);
291+
Debugger::ReportError(llvm::formatv(
292+
"CIE parse error: CIE augmentation string was too large "
293+
"for the fixed sized buffer of {0} bytes.",
294+
CFI_AUG_MAX_SIZE));
294295
return nullptr;
295296
}
296297

@@ -451,10 +452,9 @@ void DWARFCallFrameInfo::GetFDEIndex() {
451452
}
452453

453454
if (next_entry > m_cfi_data.GetByteSize() + 1) {
454-
Host::SystemLog(Host::eSystemLogError, "error: Invalid fde/cie next "
455-
"entry offset of 0x%x found in "
456-
"cie/fde at 0x%x\n",
457-
next_entry, current_entry);
455+
Debugger::ReportError(llvm::formatv("Invalid fde/cie next entry offset "
456+
"of {0:x} found in cie/fde at {1:x}",
457+
next_entry, current_entry));
458458
// Don't trust anything in this eh_frame section if we find blatantly
459459
// invalid data.
460460
m_fde_index.Clear();
@@ -484,10 +484,9 @@ void DWARFCallFrameInfo::GetFDEIndex() {
484484
cie_offset = cie_id;
485485

486486
if (cie_offset > m_cfi_data.GetByteSize()) {
487-
Host::SystemLog(Host::eSystemLogError,
488-
"error: Invalid cie offset of 0x%x "
489-
"found in cie/fde at 0x%x\n",
490-
cie_offset, current_entry);
487+
Debugger::ReportError(llvm::formatv("Invalid cie offset of {0:x} "
488+
"found in cie/fde at {1:x}",
489+
cie_offset, current_entry));
491490
// Don't trust anything in this eh_frame section if we find blatantly
492491
// invalid data.
493492
m_fde_index.Clear();
@@ -513,10 +512,9 @@ void DWARFCallFrameInfo::GetFDEIndex() {
513512
FDEEntryMap::Entry fde(addr, length, current_entry);
514513
m_fde_index.Append(fde);
515514
} else {
516-
Host::SystemLog(Host::eSystemLogError, "error: unable to find CIE at "
517-
"0x%8.8x for cie_id = 0x%8.8x for "
518-
"entry at 0x%8.8x.\n",
519-
cie_offset, cie_id, current_entry);
515+
Debugger::ReportError(llvm::formatv(
516+
"unable to find CIE at {0:x} for cie_id = {1:x} for entry at {2:x}.",
517+
cie_offset, cie_id, current_entry));
520518
}
521519
offset = next_entry;
522520
}

0 commit comments

Comments
 (0)