JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, aprantl, mib. Herald added a project: All. JDevlieghere requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1.
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. By switching over, we print these error messages to the debugger's error stream (when using the default event handler) or report them to an IDE such as Xcode (if they have subscribed to these events). It also means we nog longer write the messages to the system log on Darwin, but as far as I know, nobody is relying on this functionality today. If this is deemed important externally, I can add it again when I add the system logging functionality again in the context of D128321 <https://reviews.llvm.org/D128321>. https://reviews.llvm.org/D128480 Files: 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
Index: lldb/tools/lldb-server/lldb-platform.cpp =================================================================== --- lldb/tools/lldb-server/lldb-platform.cpp +++ lldb/tools/lldb-server/lldb-platform.cpp @@ -79,8 +79,7 @@ // 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"); + fprintf(stderr, "SIGHUP received, exiting lldb-server...\n"); abort(); break; } Index: lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py =================================================================== --- lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py +++ lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py @@ -30,12 +30,15 @@ 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: Index: lldb/source/Symbol/SymbolContext.cpp =================================================================== --- lldb/source/Symbol/SymbolContext.cpp +++ 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 @@ 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 Index: lldb/source/Symbol/Function.cpp =================================================================== --- lldb/source/Symbol/Function.cpp +++ 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 @@ 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); } Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp =================================================================== --- lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ 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 @@ 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 @@ 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 @@ } 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 @@ 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 @@ 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; } Index: lldb/source/Symbol/CompactUnwindInfo.cpp =================================================================== --- lldb/source/Symbol/CompactUnwindInfo.cpp +++ 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 @@ 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; Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp =================================================================== --- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -1932,10 +1932,10 @@ 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("error: 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 @@ // 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 @@ 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') Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp =================================================================== --- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -337,11 +337,11 @@ 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())); } } } Index: lldb/source/Interpreter/Options.cpp =================================================================== --- lldb/source/Interpreter/Options.cpp +++ lldb/source/Interpreter/Options.cpp @@ -224,21 +224,25 @@ 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()); } } Index: lldb/source/Host/macosx/objcxx/Host.mm =================================================================== --- lldb/source/Host/macosx/objcxx/Host.mm +++ lldb/source/Host/macosx/objcxx/Host.mm @@ -1491,40 +1491,3 @@ } 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); - } -} Index: lldb/source/Host/common/Host.cpp =================================================================== --- lldb/source/Host/common/Host.cpp +++ lldb/source/Host/common/Host.cpp @@ -219,32 +219,6 @@ #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 Index: lldb/source/Core/Module.cpp =================================================================== --- lldb/source/Core/Module.cpp +++ lldb/source/Core/Module.cpp @@ -1107,27 +1107,6 @@ 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 @@ 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,32 @@ 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; + strm.PutCString("error: "); + 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 +1199,7 @@ 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())); } } Index: lldb/include/lldb/Host/Host.h =================================================================== --- lldb/include/lldb/Host/Host.h +++ lldb/include/lldb/Host/Host.h @@ -86,13 +86,6 @@ 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
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits