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

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 30 03:18:01 PDT 2020


Author: Raphael Isemann
Date: 2020-07-30T12:17:42+02:00
New Revision: a4a0844248d4a68a866b9c4e18ae89fa49a83ec0

URL: https://github.com/llvm/llvm-project/commit/a4a0844248d4a68a866b9c4e18ae89fa49a83ec0
DIFF: https://github.com/llvm/llvm-project/commit/a4a0844248d4a68a866b9c4e18ae89fa49a83ec0.diff

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

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.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D84505

Added: 
    

Modified: 
    lldb/include/lldb/Core/Communication.h
    lldb/include/lldb/Target/Thread.h
    lldb/source/Core/Communication.cpp
    lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
    lldb/source/Target/Thread.cpp
    lldb/source/Target/ThreadPlanCallFunction.cpp
    lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Communication.h b/lldb/include/lldb/Core/Communication.h
index 6b65974f9522..354c4bbcc283 100644
--- a/lldb/include/lldb/Core/Communication.h
+++ b/lldb/include/lldb/Core/Communication.h
@@ -285,7 +285,7 @@ class Communication : public Broadcaster {
   ///
   void SynchronizeWithReadThread();
 
-  static const char *ConnectionStatusAsCString(lldb::ConnectionStatus status);
+  static std::string ConnectionStatusAsString(lldb::ConnectionStatus status);
 
   bool GetCloseOnEOF() const { return m_close_on_eof; }
 

diff  --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 205a0d965c63..066b8e1845c0 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -253,9 +253,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
 
   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; }
 

diff  --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp
index 859f5be74b43..b50cd0ecab5c 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -339,7 +339,7 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) {
       }
       if (error.Fail())
         LLDB_LOG(log, "error: {0}, status = {1}", error,
-                 Communication::ConnectionStatusAsCString(status));
+                 Communication::ConnectionStatusAsString(status));
       break;
     case eConnectionStatusInterrupted: // Synchronization signal from
                                        // SynchronizeWithReadThread()
@@ -355,7 +355,7 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) {
     case eConnectionStatusTimedOut: // Request timed out
       if (error.Fail())
         LLDB_LOG(log, "error: {0}, status = {1}", error,
-                 Communication::ConnectionStatusAsCString(status));
+                 Communication::ConnectionStatusAsString(status));
       break;
     }
   }
@@ -416,8 +416,8 @@ void Communication::SetConnection(std::unique_ptr<Connection> connection) {
   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";
@@ -435,8 +435,5 @@ Communication::ConnectionStatusAsCString(lldb::ConnectionStatus status) {
     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);
 }

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
index 06190d0c036d..dc283fce8104 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
@@ -194,12 +194,11 @@ size_t CommunicationKDP::WaitForPacketWithTimeoutMicroSecondsNoLock(
                                  : std::chrono::microseconds(timeout_usec),
                              status, &error);
 
-    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);
+    LLDB_LOGV(log,
+              "Read (buffer, sizeof(buffer), timeout_usec = 0x{0:x}, "
+              "status = {1}, error = {2}) => bytes_read = {4}",
+              timeout_usec, Communication::ConnectionStatusAsString(status),
+              error, bytes_read);
 
     if (bytes_read > 0) {
       if (CheckForPacket(buffer, bytes_read, packet))

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 6a6050241695..832760f7f0dc 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -284,7 +284,7 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
     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) {

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 24cf4bf3ee1e..ad28603cc02e 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1661,7 +1661,7 @@ 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";
@@ -1687,13 +1687,10 @@ const char *Thread::StopReasonAsCString(lldb::StopReason reason) {
     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";
@@ -1703,10 +1700,7 @@ const char *Thread::RunModeAsCString(lldb::RunMode mode) {
     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,

diff  --git a/lldb/source/Target/ThreadPlanCallFunction.cpp b/lldb/source/Target/ThreadPlanCallFunction.cpp
index dbe26f42c9bf..f525173f8a51 100644
--- a/lldb/source/Target/ThreadPlanCallFunction.cpp
+++ b/lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -260,9 +260,9 @@ bool ThreadPlanCallFunction::DoPlanExplainsStop(Event *event_ptr) {
     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;

diff  --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
index f3d35a91fcbc..f188d827faae 100644
--- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
@@ -62,8 +62,8 @@ bool ThreadPlanStepOverBreakpoint::DoPlanExplainsStop(Event *event_ptr) {
     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:


        


More information about the lldb-commits mailing list