Author: Jonas Devlieghere Date: 2022-06-24T09:46:26-07:00 New Revision: 6879391908cab68e8d43a0097fa8c9cd3b86eff9
URL: https://github.com/llvm/llvm-project/commit/6879391908cab68e8d43a0097fa8c9cd3b86eff9 DIFF: https://github.com/llvm/llvm-project/commit/6879391908cab68e8d43a0097fa8c9cd3b86eff9.diff LOG: [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 Added: Modified: lldb/include/lldb/Host/Host.h lldb/source/Core/Module.cpp lldb/source/Host/common/Host.cpp lldb/source/Host/macosx/objcxx/Host.mm lldb/source/Interpreter/Options.cpp lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Symbol/CompactUnwindInfo.cpp lldb/source/Symbol/DWARFCallFrameInfo.cpp lldb/source/Symbol/Function.cpp lldb/source/Symbol/SymbolContext.cpp lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py lldb/tools/lldb-server/lldb-platform.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h index 3b49b5e393507..b0097a1c1c420 100644 --- a/lldb/include/lldb/Host/Host.h +++ b/lldb/include/lldb/Host/Host.h @@ -86,13 +86,6 @@ class Host { StartMonitoringChildProcess(const MonitorChildProcessCallback &callback, lldb::pid_t pid); - enum SystemLogType { eSystemLogWarning, eSystemLogError }; - - static void SystemLog(SystemLogType type, const char *format, ...) - __attribute__((format(printf, 2, 3))); - - static void SystemLog(SystemLogType type, const char *format, va_list args); - /// Get the process ID for the calling process. /// /// \return diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 7160c2386efd6..41c21e1dc326b 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1107,27 +1107,6 @@ void Module::GetDescription(llvm::raw_ostream &s, s << llvm::formatv("({0})", object_name); } -void Module::ReportError(const char *format, ...) { - if (format && format[0]) { - StreamString strm; - strm.PutCString("error: "); - GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief); - strm.PutChar(' '); - va_list args; - va_start(args, format); - strm.PrintfVarArg(format, args); - va_end(args); - - const int format_len = strlen(format); - if (format_len > 0) { - const char last_char = format[format_len - 1]; - if (last_char != '\n' && last_char != '\r') - strm.EOL(); - } - Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData()); - } -} - bool Module::FileHasChanged() const { // We have provided the DataBuffer for this module to avoid accessing the // filesystem. We never want to reload those files. @@ -1170,7 +1149,7 @@ void Module::ReportErrorIfModifyDetected(const char *format, ...) { m_first_file_changed_log = true; if (format) { StreamString strm; - strm.PutCString("error: the object file "); + strm.PutCString("the object file "); GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull); strm.PutCString(" has been modified\n"); @@ -1186,17 +1165,31 @@ void Module::ReportErrorIfModifyDetected(const char *format, ...) { strm.EOL(); } strm.PutCString("The debug session should be aborted as the original " - "debug information has been overwritten.\n"); - Host::SystemLog(Host::eSystemLogError, "%s", strm.GetData()); + "debug information has been overwritten."); + Debugger::ReportError(std::string(strm.GetString())); } } } } +void Module::ReportError(const char *format, ...) { + if (format && format[0]) { + StreamString strm; + GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief); + strm.PutChar(' '); + + va_list args; + va_start(args, format); + strm.PrintfVarArg(format, args); + va_end(args); + + Debugger::ReportError(std::string(strm.GetString())); + } +} + void Module::ReportWarning(const char *format, ...) { if (format && format[0]) { StreamString strm; - strm.PutCString("warning: "); GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull); strm.PutChar(' '); @@ -1205,13 +1198,7 @@ void Module::ReportWarning(const char *format, ...) { strm.PrintfVarArg(format, args); va_end(args); - const int format_len = strlen(format); - if (format_len > 0) { - const char last_char = format[format_len - 1]; - if (last_char != '\n' && last_char != '\r') - strm.EOL(); - } - Host::SystemLog(Host::eSystemLogWarning, "%s", strm.GetData()); + Debugger::ReportWarning(std::string(strm.GetString())); } } diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp index e8c5ece9a4f6c..331042590ee92 100644 --- a/lldb/source/Host/common/Host.cpp +++ b/lldb/source/Host/common/Host.cpp @@ -219,32 +219,6 @@ MonitorChildProcessThreadFunction(::pid_t pid, #endif // #if !defined (__APPLE__) && !defined (_WIN32) -#if !defined(__APPLE__) - -void Host::SystemLog(SystemLogType type, const char *format, va_list args) { - vfprintf(stderr, format, args); -} - -#endif - -void Host::SystemLog(SystemLogType type, const char *format, ...) { - { - va_list args; - va_start(args, format); - SystemLog(type, format, args); - va_end(args); - } - - Log *log = GetLog(LLDBLog::Host); - if (log && log->GetVerbose()) { - // Log to log channel. This allows testcases to grep for log output. - va_list args; - va_start(args, format); - log->VAPrintf(format, args); - va_end(args); - } -} - lldb::pid_t Host::GetCurrentProcessID() { return ::getpid(); } #ifndef _WIN32 diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm index 7c490f922128d..4c369e9ec8a17 100644 --- a/lldb/source/Host/macosx/objcxx/Host.mm +++ b/lldb/source/Host/macosx/objcxx/Host.mm @@ -1491,40 +1491,3 @@ static bool ShouldLaunchUsingXPC(ProcessLaunchInfo &launch_info) { } return HostThread(); } - -//---------------------------------------------------------------------- -// Log to both stderr and to ASL Logging when running on MacOSX. -//---------------------------------------------------------------------- -void Host::SystemLog(SystemLogType type, const char *format, va_list args) { - if (format && format[0]) { - static aslmsg g_aslmsg = NULL; - if (g_aslmsg == NULL) { - g_aslmsg = ::asl_new(ASL_TYPE_MSG); - char asl_key_sender[PATH_MAX]; - snprintf(asl_key_sender, sizeof(asl_key_sender), - "com.apple.LLDB.framework"); - ::asl_set(g_aslmsg, ASL_KEY_SENDER, asl_key_sender); - } - - // Copy the va_list so we can log this message twice - va_list copy_args; - va_copy(copy_args, args); - // Log to stderr - ::vfprintf(stderr, format, copy_args); - va_end(copy_args); - - int asl_level; - switch (type) { - case eSystemLogError: - asl_level = ASL_LEVEL_ERR; - break; - - case eSystemLogWarning: - asl_level = ASL_LEVEL_WARNING; - break; - } - - // Log to ASL - ::asl_vlog(NULL, g_aslmsg, asl_level, format, args); - } -} diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 1278f2ad5d7bb..26d9d2a17867b 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -224,21 +224,25 @@ Option *Options::GetLongOptions() { option_seen.find(short_opt); StreamString strm; if (defs[i].HasShortOption()) - Host::SystemLog(Host::eSystemLogError, - "option[%u] --%s has a short option -%c that " - "conflicts with option[%u] --%s, short option won't " - "be used for --%s\n", - (int)i, defs[i].long_option, short_opt, pos->second, - m_getopt_table[pos->second].definition->long_option, - defs[i].long_option); + Debugger::ReportError( + llvm::formatv( + "option[{0}] --{1} has a short option -{2} that " + "conflicts with option[{3}] --{4}, short option won't " + "be used for --{5}", + i, defs[i].long_option, short_opt, pos->second, + m_getopt_table[pos->second].definition->long_option, + defs[i].long_option) + .str()); else - Host::SystemLog(Host::eSystemLogError, - "option[%u] --%s has a short option 0x%x that " - "conflicts with option[%u] --%s, short option won't " - "be used for --%s\n", - (int)i, defs[i].long_option, short_opt, pos->second, - m_getopt_table[pos->second].definition->long_option, - defs[i].long_option); + Debugger::ReportError( + llvm::formatv( + "option[{0}] --{1} has a short option {2:x} that " + "conflicts with option[{3}] --{4}, short option won't " + "be used for --{5}n", + (int)i, defs[i].long_option, short_opt, pos->second, + m_getopt_table[pos->second].definition->long_option, + defs[i].long_option) + .str()); } } diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp index 043504be510aa..0063760e9583d 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -337,11 +337,11 @@ bool DynamicLoaderDarwin::UnloadModuleSections(Module *module, section_sp, old_section_load_addr)) changed = true; } else { - Host::SystemLog(Host::eSystemLogWarning, - "warning: unable to find and unload segment named " - "'%s' in '%s' in macosx dynamic loader plug-in.\n", - info.segments[i].name.AsCString("<invalid>"), - image_object_file->GetFileSpec().GetPath().c_str()); + Debugger::ReportWarning( + llvm::formatv("unable to find and unload segment named " + "'{0}' in '{1}' in macosx dynamic loader plug-in", + info.segments[i].name.AsCString("<invalid>"), + image_object_file->GetFileSpec().GetPath())); } } } diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 0155ff8901b8a..fdebfcd388ff9 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -1932,10 +1932,10 @@ class MachSymtabSectionInfo { if (first_section_sp) filename = first_section_sp->GetObjectFile()->GetFileSpec().GetPath(); - Host::SystemLog(Host::eSystemLogError, - "error: unable to find section %d for a symbol in " - "%s, corrupt file?\n", - n_sect, filename.c_str()); + Debugger::ReportError( + llvm::formatv("unable to find section {0} for a symbol in " + "{1}, corrupt file?", + n_sect, filename)); } } if (m_section_infos[n_sect].vm_range.Contains(file_addr)) { @@ -2804,12 +2804,11 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) { // No symbol should be NULL, even the symbols with no // string values should have an offset zero which // points to an empty C-string - Host::SystemLog( - Host::eSystemLogError, - "error: DSC unmapped local symbol[%u] has invalid " - "string table offset 0x%x in %s, ignoring symbol\n", + Debugger::ReportError(llvm::formatv( + "DSC unmapped local symbol[{0}] has invalid " + "string table offset {1:x} in {2}, ignoring symbol", nlist_index, nlist.n_strx, - module_sp->GetFileSpec().GetPath().c_str()); + module_sp->GetFileSpec().GetPath()); continue; } if (symbol_name[0] == '\0') @@ -3730,11 +3729,10 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) { if (symbol_name == nullptr) { // No symbol should be NULL, even the symbols with no string values // should have an offset zero which points to an empty C-string - Host::SystemLog(Host::eSystemLogError, - "error: symbol[%u] has invalid string table offset " - "0x%x in %s, ignoring symbol\n", - nlist_idx, nlist.n_strx, - module_sp->GetFileSpec().GetPath().c_str()); + Debugger::ReportError(llvm::formatv( + "symbol[{0}] has invalid string table offset {1:x} in {2}, " + "ignoring symbol", + nlist_idx, nlist.n_strx, module_sp->GetFileSpec().GetPath())); return true; } if (symbol_name[0] == '\0') diff --git a/lldb/source/Symbol/CompactUnwindInfo.cpp b/lldb/source/Symbol/CompactUnwindInfo.cpp index c6855d739a3c2..ce597523c061a 100644 --- a/lldb/source/Symbol/CompactUnwindInfo.cpp +++ b/lldb/source/Symbol/CompactUnwindInfo.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Symbol/CompactUnwindInfo.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" #include "lldb/Core/Section.h" #include "lldb/Symbol/ObjectFile.h" @@ -317,9 +318,8 @@ void CompactUnwindInfo::ScanIndex(const ProcessSP &process_sp) { m_unwindinfo_data.GetByteSize() || indexSectionOffset > m_unwindinfo_data.GetByteSize() || offset > m_unwindinfo_data.GetByteSize()) { - Host::SystemLog(Host::eSystemLogError, "error: Invalid offset " - "encountered in compact unwind " - "info, skipping\n"); + Debugger::ReportError( + "Invalid offset encountered in compact unwind info, skipping"); // don't trust anything from this compact_unwind section if it looks // blatantly invalid data in the header. m_indexes_computed = eLazyBoolNo; diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index 2a9c52c26eab5..4e51bd411a756 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Symbol/DWARFCallFrameInfo.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" #include "lldb/Core/Section.h" #include "lldb/Core/dwarf.h" @@ -268,9 +269,9 @@ DWARFCallFrameInfo::ParseCIE(const dw_offset_t cie_offset) { cie_sp->ptr_encoding = DW_EH_PE_absptr; // default cie_sp->version = m_cfi_data.GetU8(&offset); if (cie_sp->version > CFI_VERSION4) { - Host::SystemLog(Host::eSystemLogError, - "CIE parse error: CFI version %d is not supported\n", - cie_sp->version); + Debugger::ReportError( + llvm::formatv("CIE parse error: CFI version {0} is not supported", + cie_sp->version)); return nullptr; } @@ -287,10 +288,10 @@ DWARFCallFrameInfo::ParseCIE(const dw_offset_t cie_offset) { if (i == CFI_AUG_MAX_SIZE && cie_sp->augmentation[CFI_AUG_MAX_SIZE - 1] != '\0') { - Host::SystemLog(Host::eSystemLogError, - "CIE parse error: CIE augmentation string was too large " - "for the fixed sized buffer of %d bytes.\n", - CFI_AUG_MAX_SIZE); + Debugger::ReportError(llvm::formatv( + "CIE parse error: CIE augmentation string was too large " + "for the fixed sized buffer of {0} bytes.", + CFI_AUG_MAX_SIZE)); return nullptr; } @@ -451,10 +452,9 @@ void DWARFCallFrameInfo::GetFDEIndex() { } if (next_entry > m_cfi_data.GetByteSize() + 1) { - Host::SystemLog(Host::eSystemLogError, "error: Invalid fde/cie next " - "entry offset of 0x%x found in " - "cie/fde at 0x%x\n", - next_entry, current_entry); + Debugger::ReportError(llvm::formatv("Invalid fde/cie next entry offset " + "of {0:x} found in cie/fde at {1:x}", + next_entry, current_entry)); // Don't trust anything in this eh_frame section if we find blatantly // invalid data. m_fde_index.Clear(); @@ -484,10 +484,9 @@ void DWARFCallFrameInfo::GetFDEIndex() { cie_offset = cie_id; if (cie_offset > m_cfi_data.GetByteSize()) { - Host::SystemLog(Host::eSystemLogError, - "error: Invalid cie offset of 0x%x " - "found in cie/fde at 0x%x\n", - cie_offset, current_entry); + Debugger::ReportError(llvm::formatv("Invalid cie offset of {0:x} " + "found in cie/fde at {1:x}", + cie_offset, current_entry)); // Don't trust anything in this eh_frame section if we find blatantly // invalid data. m_fde_index.Clear(); @@ -513,10 +512,9 @@ void DWARFCallFrameInfo::GetFDEIndex() { FDEEntryMap::Entry fde(addr, length, current_entry); m_fde_index.Append(fde); } else { - Host::SystemLog(Host::eSystemLogError, "error: unable to find CIE at " - "0x%8.8x for cie_id = 0x%8.8x for " - "entry at 0x%8.8x.\n", - cie_offset, cie_id, current_entry); + Debugger::ReportError(llvm::formatv( + "unable to find CIE at {0:x} for cie_id = {1:x} for entry at {2:x}.", + cie_offset, cie_id, current_entry)); } offset = next_entry; } diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp index fbbd05cc2480f..648a12524aed1 100644 --- a/lldb/source/Symbol/Function.cpp +++ b/lldb/source/Symbol/Function.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Symbol/Function.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Disassembler.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleList.h" @@ -348,12 +349,9 @@ Block &Function::GetBlock(bool can_create) { if (module_sp) { module_sp->GetSymbolFile()->ParseBlocksRecursive(*this); } else { - Host::SystemLog(Host::eSystemLogError, - "error: unable to find module " - "shared pointer for function '%s' " - "in %s\n", - GetName().GetCString(), - m_comp_unit->GetPrimaryFile().GetPath().c_str()); + Debugger::ReportError(llvm::formatv( + "unable to find module shared pointer for function '{0}' in {1}", + GetName().GetCString(), m_comp_unit->GetPrimaryFile().GetPath())); } m_block.SetBlockInfoHasBeenParsed(true, true); } diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp index 33999fabca434..a10db0755d03a 100644 --- a/lldb/source/Symbol/SymbolContext.cpp +++ b/lldb/source/Symbol/SymbolContext.cpp @@ -8,6 +8,7 @@ #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Host/Host.h" @@ -494,20 +495,16 @@ bool SymbolContext::GetParentOfInlinedScope(const Address &curr_frame_pc, objfile = symbol_file->GetObjectFile(); } if (objfile) { - Host::SystemLog( - Host::eSystemLogWarning, - "warning: inlined block 0x%8.8" PRIx64 - " doesn't have a range that contains file address 0x%" PRIx64 - " in %s\n", + Debugger::ReportWarning(llvm::formatv( + "inlined block {0:x} doesn't have a range that contains file " + "address {1:x} in {2}", curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress(), - objfile->GetFileSpec().GetPath().c_str()); + objfile->GetFileSpec().GetPath())); } else { - Host::SystemLog( - Host::eSystemLogWarning, - "warning: inlined block 0x%8.8" PRIx64 - " doesn't have a range that contains file address 0x%" PRIx64 - "\n", - curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress()); + Debugger::ReportWarning(llvm::formatv( + "inlined block {0:x} doesn't have a range that contains file " + "address {1:x}", + curr_inlined_block->GetID(), curr_frame_pc.GetFileAddress())); } } #endif diff --git a/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py b/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py index 1c9e8be53bbd8..13512620857a6 100644 --- a/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py +++ b/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py @@ -30,12 +30,15 @@ def test_expr(self): self.assertTrue(os.path.isdir(mod_cache), "module cache exists") logfile = self.getBuildArtifact("host.log") - self.runCmd("log enable -v -f %s lldb host" % logfile) - target, _, _, _ = lldbutil.run_to_source_breakpoint( - self, "break here", lldb.SBFileSpec("main.m")) - target.GetModuleAtIndex(0).FindTypes('my_int') + with open(logfile, 'w') as f: + sbf = lldb.SBFile(f.fileno(), 'w', False) + status = self.dbg.SetErrorFile(sbf) + self.assertSuccess(status) + + target, _, _, _ = lldbutil.run_to_source_breakpoint( + self, "break here", lldb.SBFileSpec("main.m")) + target.GetModuleAtIndex(0).FindTypes('my_int') - found = False with open(logfile, 'r') as f: for line in f: if "hash mismatch" in line and "Foo" in line: diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 9e07f4c8debdc..1a969eec8db85 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -77,10 +77,9 @@ static void signal_handler(int signo) { case SIGHUP: // Use SIGINT first, if that does not work, use SIGHUP as a last resort. // And we should not call exit() here because it results in the global - // destructors - // to be invoked and wreaking havoc on the threads still running. - Host::SystemLog(Host::eSystemLogWarning, - "SIGHUP received, exiting lldb-server...\n"); + // destructors to be invoked and wreaking havoc on the threads still + // running. + llvm::errs() << "SIGHUP received, exiting lldb-server...\n"; abort(); break; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
