[lldb] Don't use static locals for return value storage in some *AsCString functions

[teemperor]

Summary

Let's just return a std::string to make this safe. formatv seemed overkill for formatting
the return values as they all just append an integer value to a constant string.

[labath]

yes, please

[JDevlieghere]

Nice

lldb/include/lldb/Core/Communication.h

public:
 
   /// Wait for the read thread to process all outstanding data.
   ///
   /// After this function returns, the read thread has processed all data that
   /// has been waiting in the Connection queue.
   ///
   void SynchronizeWithReadThread();
 
-  static const char *ConnectionStatusAsCString(lldb::ConnectionStatus status);
+  static std::string ConnectionStatusAsString(lldb::ConnectionStatus status);
 
   bool GetCloseOnEOF() const { return m_close_on_eof; }
 
   void SetCloseOnEOF(bool b) { m_close_on_eof = b; }
 
   static ConstString &GetStaticBroadcasterClass();
 
   ConstString &GetBroadcasterClass() const override {

lldb/include/lldb/Target/Thread.h

public:
 
   // This sets the stop reason to a "blank" stop reason, so you can call
   // functions on the thread without having the called function run with
   // whatever stop reason you stopped with.
   void SetStopInfoToNothing();
 
   bool ThreadStoppedForAReason();
 
-  static const char *RunModeAsCString(lldb::RunMode mode);
+  static std::string RunModeAsString(lldb::RunMode mode);
 
-  static const char *StopReasonAsCString(lldb::StopReason reason);
+  static std::string StopReasonAsString(lldb::StopReason reason);
 
   virtual const char *GetInfo() { return nullptr; }
 
   /// Retrieve a dictionary of information about this thread
   ///
   /// On Mac OS X systems there may be voucher information.
   /// The top level dictionary returned will have an "activity" key and the
   /// value of the activity is a dictionary.  Keys in that dictionary will

lldb/source/Core/Communication.cpp

while (!done && comm->m_read_thread_enabled) {
     case eConnectionStatusError: // Check GetError() for details
       if (error.GetType() == eErrorTypePOSIX && error.GetError() == EIO) {
         // EIO on a pipe is usually caused by remote shutdown
         disconnect = comm->GetCloseOnEOF();
         done = true;
       }
       if (error.Fail())
         LLDB_LOG(log, "error: {0}, status = {1}", error,
-                 Communication::ConnectionStatusAsCString(status));
+                 Communication::ConnectionStatusAsString(status));
       break;
     case eConnectionStatusInterrupted: // Synchronization signal from
                                        // SynchronizeWithReadThread()
       // The connection returns eConnectionStatusInterrupted only when there is
       // no input pending to be read, so we can signal that.
       comm->BroadcastEvent(eBroadcastBitNoMorePendingInput);
       break;
     case eConnectionStatusNoConnection:   // No connection
     case eConnectionStatusLostConnection: // Lost connection while connected to
                                           // a valid connection
       done = true;
       LLVM_FALLTHROUGH;
     case eConnectionStatusTimedOut: // Request timed out
       if (error.Fail())
         LLDB_LOG(log, "error: {0}, status = {1}", error,
-                 Communication::ConnectionStatusAsCString(status));
+                 Communication::ConnectionStatusAsString(status));
       break;
     }
   }
   log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION);
   if (log)
     LLDB_LOGF(log, "%p Communication::ReadThread () thread exiting...", p);
 
   // Handle threads wishing to synchronize with us.

 }
 
 void Communication::SetConnection(std::unique_ptr<Connection> connection) {
   Disconnect(nullptr);
   StopReadThread(nullptr);
   m_connection_sp = std::move(connection);
 }
 
-const char *
-Communication::ConnectionStatusAsCString(lldb::ConnectionStatus status) {
+std::string
+Communication::ConnectionStatusAsString(lldb::ConnectionStatus status) {
   switch (status) {
   case eConnectionStatusSuccess:
     return "success";
   case eConnectionStatusError:
     return "error";
   case eConnectionStatusTimedOut:
     return "timed out";
   case eConnectionStatusNoConnection:
     return "no connection";
   case eConnectionStatusLostConnection:
     return "lost connection";
   case eConnectionStatusEndOfFile:
     return "end of file";
   case eConnectionStatusInterrupted:
     return "interrupted";
   }
 
-  static char unknown_state_string[64];
-  snprintf(unknown_state_string, sizeof(unknown_state_string),
-           "ConnectionStatus = %i", status);
-  return unknown_state_string;
+  return "@" + std::to_string(status);
 }

lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp

size_t CommunicationKDP::WaitForPacketWithTimeoutMicroSecondsNoLock(
   while (IsConnected() && !timed_out) {
     lldb::ConnectionStatus status = eConnectionStatusNoConnection;
     size_t bytes_read = Read(buffer, sizeof(buffer),
                              timeout_usec == UINT32_MAX
                                  ? Timeout<std::micro>(llvm::None)
                                  : std::chrono::microseconds(timeout_usec),
                              status, &error);
 
-    LLDB_LOGV(log, 
+    LLDB_LOGV(log,
               "Read (buffer, sizeof(buffer), timeout_usec = 0x{0:x}, "
-                  "status = {1}, error = {2}) => bytes_read = {4}",
-                  timeout_usec,
-                  Communication::ConnectionStatusAsCString(status),
-                  error, bytes_read);
+              timeout_usec, Communication::ConnectionStatusAsString(status),
 
     if (bytes_read > 0) {
       if (CheckForPacket(buffer, bytes_read, packet))
         return packet.GetByteSize();
     } else {
       switch (status) {
       case eConnectionStatusInterrupted:
       case eConnectionStatusTimedOut:

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
   bool disconnected = false;
   while (IsConnected() && !timed_out) {
     lldb::ConnectionStatus status = eConnectionStatusNoConnection;
     size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
 
     LLDB_LOGV(log,
               "Read(buffer, sizeof(buffer), timeout = {0}, "
               "status = {1}, error = {2}) => bytes_read = {3}",
-              timeout, Communication::ConnectionStatusAsCString(status), error,
+              timeout, Communication::ConnectionStatusAsString(status), error,
               bytes_read);
 
     if (bytes_read > 0) {
       if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid)
         return PacketResult::Success;
     } else {
       switch (status) {
       case eConnectionStatusTimedOut:

lldb/source/Target/Thread.cpp

bool Thread::SafeToCallFunctions() {
   return true;
 }
 
 lldb::StackFrameSP
 Thread::GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr) {
   return GetStackFrameList()->GetStackFrameSPForStackFramePtr(stack_frame_ptr);
 }
 
-const char *Thread::StopReasonAsCString(lldb::StopReason reason) {
+std::string Thread::StopReasonAsString(lldb::StopReason reason) {
   switch (reason) {
   case eStopReasonInvalid:
     return "invalid";
   case eStopReasonNone:
     return "none";
   case eStopReasonTrace:
     return "trace";
   case eStopReasonBreakpoint:
std::string Thread::StopReasonAsString(lldb::StopReason reason) {
   case eStopReasonPlanComplete:
     return "plan complete";
   case eStopReasonThreadExiting:
     return "thread exiting";
   case eStopReasonInstrumentation:
     return "instrumentation break";
   }
 
-  static char unknown_state_string[64];
-  snprintf(unknown_state_string, sizeof(unknown_state_string),
-           "StopReason = %i", reason);
-  return unknown_state_string;
+  return "StopReason = " + std::to_string(reason);
 }
 
-const char *Thread::RunModeAsCString(lldb::RunMode mode) {
+std::string Thread::RunModeAsString(lldb::RunMode mode) {
   switch (mode) {
   case eOnlyThisThread:
     return "only this thread";
   case eAllThreads:
     return "all threads";
   case eOnlyDuringStepping:
     return "only during stepping";
   }
 
-  static char unknown_state_string[64];
-  snprintf(unknown_state_string, sizeof(unknown_state_string), "RunMode = %i",
-           mode);
-  return unknown_state_string;
+  return "RunMode = " + std::to_string(mode);
 }
 
 size_t Thread::GetStatus(Stream &strm, uint32_t start_frame,
                          uint32_t num_frames, uint32_t num_frames_with_source,
                          bool stop_format, bool only_stacks) {
 
   if (!only_stacks) {
     ExecutionContext exe_ctx(shared_from_this());

lldb/source/Target/ThreadPlanCallFunction.cpp

bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) {
 
   // Check if the breakpoint is one of ours.
 
   StopReason stop_reason;
   if (!m_real_stop_info_sp)
     stop_reason = eStopReasonNone;
   else
     stop_reason = m_real_stop_info_sp->GetStopReason();
-  LLDB_LOGF(log,
-            "ThreadPlanCallFunction::PlanExplainsStop: Got stop reason - %s.",
-            Thread::StopReasonAsCString(stop_reason));
+  LLDB_LOG(log,
+           "ThreadPlanCallFunction::PlanExplainsStop: Got stop reason - {0}.",
+           Thread::StopReasonAsString(stop_reason));
 
   if (stop_reason == eStopReasonBreakpoint && BreakpointsExplainStop())
     return true;
 
   // One more quirk here.  If this event was from Halt interrupting the target,
   // then we should not consider ourselves complete.  Return true to
   // acknowledge the stop.
   if (Process::ProcessEventData::GetInterruptedFromEvent(event_ptr)) {

lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp

if (stop_info_sp) {
     // continue, the PC wouldn't change,
     // and you'd see the breakpoint hit, which would be odd. So the lower
     // levels fake "step onto breakpoint address" and return that as a
     // breakpoint.  So our trace step COULD appear as a breakpoint hit if the
     // next instruction also contained a breakpoint.
     StopReason reason = stop_info_sp->GetStopReason();
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
-    LLDB_LOGF(log, "Step over breakpoint stopped for reason: %s.",
-              Thread::StopReasonAsCString(reason));
+    LLDB_LOG(log, "Step over breakpoint stopped for reason: {0}.",
+             Thread::StopReasonAsString(reason));
 
     switch (reason) {
       case eStopReasonTrace:
       case eStopReasonNone:
         return true;
       case eStopReasonBreakpoint:
       {
         // It's a little surprising that we stop here for a breakpoint hit.