[lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

[JDevlieghere]

Summary

Fix incorrect uses of formatv specifiers in LLDB_LOG. Unlike Python, arguments must be numbered. All these log statements take llvm:Errors so use the LLDB_LOG_ERROR macro instead.

[bulbazord]

These uses of LLDB_LOG are taking errors, consuming them while turning them into strings, and logging them. Unless I'm missing something, I think it would make more sense to convert these to use LLDB_LOG_ERROR.

lldb/source/Core/Debugger.cpp

1917–1919

Same here.

[bulbazord]

lldb/source/Core/Debugger.cpp

1917–1919

Sorry, ignore this comment, I deleted a previous inline comment and forgot to delete this one.

[JDevlieghere]

In D154532#4474534, @bulbazord wrote:

These uses of LLDB_LOG are taking errors, consuming them while turning them into strings, and logging them. Unless I'm missing something, I think it would make more sense to convert these to use LLDB_LOG_ERROR.

Good point, I blindly fixed the format specifiers without lookin what was being passed in.

[fdeazeve]

In https://reviews.llvm.org/D154532, the string argument comes after the error. Are these interchangeable?

[fdeazeve]

Ah you updated the review as I typed the question! LGTM

[bulbazord]

The 2nd argument to LLDB_LOG_ERROR is the error itself, so these should be something like LLDB_LOG_ERROR(GetLog(whatever), std::move(err), "message: {0}"); or something to this effect.

[bulbazord]

I see you updated it while I typed it up. LGTM.

lldb/source/API/SBTarget.cpp

bool SBTarget::FindBreakpointsByName(const char *name,
   LLDB_INSTRUMENT_VA(this, name, bkpts);
 
   TargetSP target_sp(GetSP());
   if (target_sp) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     llvm::Expected<std::vector<BreakpointSP>> expected_vector =
         target_sp->GetBreakpointList().FindBreakpointsByName(name);
     if (!expected_vector) {
-      LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
-               llvm::toString(expected_vector.takeError()));
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), expected_vector.takeError(),
+                     "invalid breakpoint name: {0}");
       return false;
     }
     for (BreakpointSP bkpt_sp : *expected_vector) {
       bkpts.AppendByID(bkpt_sp->GetID());
     }
   }
   return true;
 }
const char *SBTarget::GetTriple() {
   // strings put the string into the string pool once and the strings never
   // comes out
   ConstString const_triple(triple.c_str());
   return const_triple.GetCString();
 }
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
   TargetSP target_sp(GetSP());
   if (!target_sp)
     return nullptr;
 
   std::string abi_name(target_sp->GetABIName().str());
   ConstString const_name(abi_name.c_str());
   return const_name.GetCString();
 }

lldb/source/Core/Debugger.cpp

if (!m_event_handler_thread.IsJoinable()) {
     // Use larger 8MB stack for this thread
     llvm::Expected<HostThread> event_handler_thread =
         ThreadLauncher::LaunchThread(
             thread_name, [this] { return DefaultEventHandler(); },
             g_debugger_event_thread_stack_bytes);
 
     if (event_handler_thread) {
       m_event_handler_thread = *event_handler_thread;
     } else {
-      LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-               llvm::toString(event_handler_thread.takeError()));
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Host), event_handler_thread.takeError(),
+                     "failed to launch host thread: {0}");
     }
 
     // Make sure DefaultEventHandler() is running and listening to events
     // before we return from this function. We are only listening for events of
     // type eBroadcastBitEventThreadIsListening so we don't need to check the
     // event, we just need to wait an infinite amount of time for it (nullptr
     // timeout as the first parameter)
     lldb::EventSP event_sp;

 bool Debugger::StartIOHandlerThread() {
   if (!m_io_handler_thread.IsJoinable()) {
     llvm::Expected<HostThread> io_handler_thread = ThreadLauncher::LaunchThread(
         "lldb.debugger.io-handler", [this] { return IOHandlerThread(); },
         8 * 1024 * 1024); // Use larger 8MB stack for this thread
     if (io_handler_thread) {
       m_io_handler_thread = *io_handler_thread;
     } else {
-      LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-               llvm::toString(io_handler_thread.takeError()));
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Host), io_handler_thread.takeError(),
+                     "failed to launch host thread: {0}");
     }
   }
   return m_io_handler_thread.IsJoinable();
 }
 
 void Debugger::StopIOHandlerThread() {
   if (m_io_handler_thread.IsJoinable()) {
     GetInputFile().Close();

lldb/source/Core/ThreadedCommunication.cpp

bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
   auto maybe_thread = ThreadLauncher::LaunchThread(
       thread_name, [this] { return ReadThread(); });
   if (maybe_thread) {
     m_read_thread = *maybe_thread;
   } else {
     if (error_ptr)
       *error_ptr = Status(maybe_thread.takeError());
     else {
-      LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-               llvm::toString(maybe_thread.takeError()));
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+                     "failed to launch host thread: {0}");
     }
   }
 
   if (!m_read_thread.IsJoinable())
     m_read_thread_enabled = false;
 
   return m_read_thread_enabled;
 }

lldb/source/Host/common/ProcessLaunchInfo.cpp

void ProcessLaunchInfo::NoOpMonitorCallback(lldb::pid_t pid, int signal,
   LLDB_LOG(log, "pid = {0}, signal = {1}, status = {2}", pid, signal, status);
 }
 
 bool ProcessLaunchInfo::MonitorProcess() const {
   if (m_monitor_callback && ProcessIDIsValid()) {
     llvm::Expected<HostThread> maybe_thread =
         Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
     if (!maybe_thread)
-      LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-               llvm::toString(maybe_thread.takeError()));
+      LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+                     "failed to launch host thread: {0}");
     return true;
   }
   return false;
 }
 
 void ProcessLaunchInfo::SetDetachOnError(bool enable) {
   if (enable)
     m_flags.Set(lldb::eLaunchFlagDetachOnError);

lldb/source/Target/Process.cpp

bool Process::StartPrivateStateThread(bool is_secondary_thread) {
   llvm::Expected<HostThread> private_state_thread =
       ThreadLauncher::LaunchThread(
           thread_name,
           [this, is_secondary_thread] {
             return RunPrivateStateThread(is_secondary_thread);
           },
           8 * 1024 * 1024);
   if (!private_state_thread) {
-    LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-             llvm::toString(private_state_thread.takeError()));
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Host), private_state_thread.takeError(),
+                   "failed to launch host thread: {0}");
     return false;
   }
 
   assert(private_state_thread->IsJoinable());
   m_private_state_thread = *private_state_thread;
   ResumePrivateStateThread();
   return true;
 }