[lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()

[mgorny]

Summary

Replace the uses of StringConvert combined with hand-rolled array
splitting with llvm::StringRef.split() and llvm::to_integer().

[teemperor]

All the llvm::StringRef & iteration variables can just be llvm::StringRef without the reference. Also some base values got lost here (see inline comments).

Beside that this LGTM, thanks!

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

1667

Base 16 arg is lost here.

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

390

This needs to forward the base arg (there are some callers that seem to parse hex).

1464

base 16 lost here.

[mgorny]

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

1667

Good catch, thanks.

[mgorny]

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

1667

Actually, scratch that. The values here include 0x prefix. and apparently llvm::to_integer() handles that correctly only if base isn't specified.

Though it might be reasonable to fix llvm::to_integer() to allow matching prefix, I guess.

[mgorny]

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

1667

I've pinged @labath about this and he suggested just stripping the 0x prefix (and fixing the sender not to send it). I'll do the first part in this patch, and then look into updating the code and docs to rid of 0x prefix.

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

 #include <cmath>
 #include <sys/stat.h>
 
 #include <numeric>
 #include <sstream>
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/HostInfo.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/LLDBAssert.h"
if (SendPacketAndWaitForResponse(packet, response) ==
           }
         } else if (name.equals("error")) {
           StringExtractorGDBRemote error_extractor(value);
           std::string error_string;
           // Now convert the HEX bytes into a string value
           error_extractor.GetHexByteString(error_string);
           error.SetErrorString(error_string.c_str());
         } else if (name.equals("dirty-pages")) {
+          llvm::SmallVector<llvm::StringRef, 16> split_value;
           std::vector<addr_t> dirty_page_list;
-          std::string comma_sep_str = value.str();
-          size_t comma_pos;
+          value.split(split_value, ',');
+          for (llvm::StringRef x : split_value) {
             addr_t page;
-          while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) {
-            comma_sep_str[comma_pos] = '\0';
-            page = StringConvert::ToUInt64(comma_sep_str.c_str(),
-                                           LLDB_INVALID_ADDRESS, 16);
-            if (page != LLDB_INVALID_ADDRESS)
+            x.consume_front("0x");
+            if (llvm::to_integer(x, page, 16))
               dirty_page_list.push_back(page);
-            comma_sep_str.erase(0, comma_pos + 1);
           }
-          page = StringConvert::ToUInt64(comma_sep_str.c_str(),
-                                         LLDB_INVALID_ADDRESS, 16);
-          if (page != LLDB_INVALID_ADDRESS)
-            dirty_page_list.push_back(page);
           region_info.SetDirtyPageList(dirty_page_list);
         }
       }
 
       if (m_target_vm_page_size != 0)
         region_info.SetPageSize(m_target_vm_page_size);
 
       if (region_info.GetRange().IsValid()) {

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

protected:
   DataExtractor GetAuxvData() override;
 
   StructuredData::ObjectSP GetExtendedInfoForThread(lldb::tid_t tid);
 
   void GetMaxMemorySize();
 
   bool CalculateThreadStopInfo(ThreadGDBRemote *thread);
 
-  size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadPCsFromStopReplyThreadsValue(llvm::StringRef value);
 
   size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
   bool StartAsyncThread();
 
   void StopAsyncThread();

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

 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Host/PseudoTerminal.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/CommandObjectMultiword.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupBoolean.h"
if (target_definition_sp) {
         return true;
       }
     }
   }
   return false;
 }
 
 static size_t SplitCommaSeparatedRegisterNumberString(
-    const llvm::StringRef &comma_separated_regiter_numbers,
+    const llvm::StringRef &comma_separated_register_numbers,
     std::vector<uint32_t> &regnums, int base) {
   regnums.clear();
-  std::pair<llvm::StringRef, llvm::StringRef> value_pair;
-  value_pair.second = comma_separated_regiter_numbers;
-  do {
-    value_pair = value_pair.second.split(',');
-    if (!value_pair.first.empty()) {
-      uint32_t reg = StringConvert::ToUInt32(value_pair.first.str().c_str(),
-                                             LLDB_INVALID_REGNUM, base);
-      if (reg != LLDB_INVALID_REGNUM)
-        regnums.push_back(reg);
-    }
-  } while (!value_pair.second.empty());
+  llvm::SmallVector<llvm::StringRef, 4> split_string;
+  comma_separated_register_numbers.split(split_string, ',');
+  for (llvm::StringRef x : split_string) {
+    uint32_t reg;
+    if (llvm::to_integer(x, reg, base))
   return regnums.size();
 }
 
 void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
   if (!force && m_register_info_sp)
     return;
 
   m_register_info_sp = std::make_shared<GDBRemoteDynamicRegisterInfo>();
if (pid_tid && pid_tid->first == pid) {
           tid != StringExtractorGDBRemote::AllProcesses)
         m_thread_ids.push_back(tid);
     }
   } while (thread_ids.GetChar() == ',');
 
   return m_thread_ids.size();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
+    llvm::StringRef value) {
   m_thread_pcs.clear();
-  size_t comma_pos;
+  llvm::SmallVector<llvm::StringRef, 16> split_value;
+  value.split(split_value, ',');
+  for (llvm::StringRef x : split_value) {
     lldb::addr_t pc;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-    value[comma_pos] = '\0';
-    pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-    if (pc != LLDB_INVALID_ADDRESS)
+    if (llvm::to_integer(x, pc, 16))
       m_thread_pcs.push_back(pc);
-    value.erase(0, comma_pos + 1);
   }
-  pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_ADDRESS)
-    m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
 
 bool ProcessGDBRemote::UpdateThreadIDList() {
   std::lock_guard<std::recursive_mutex> guard(m_thread_list_real.GetMutex());
 
   if (m_jthreadsinfo_sp) {
     // If we have the JSON threads info, we can get the thread list from that