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

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, JDevlieghere.
mgorny requested review of this revision.

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


https://reviews.llvm.org/D110472

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -359,7 +359,7 @@
 
   bool CalculateThreadStopInfo(ThreadGDBRemote *thread);
 
-  size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadPCsFromStopReplyThreadsValue(llvm::StringRef value);
 
   size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -43,7 +43,6 @@
 #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"
@@ -381,20 +380,16 @@
 }
 
 static size_t SplitCommaSeparatedRegisterNumberString(
-const llvm::StringRef &comma_separated_regiter_numbers,
+const llvm::StringRef &comma_separated_register_numbers,
 std::vector ®nums, int base) {
   regnums.clear();
-  std::pair 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 split_string;
+  comma_separated_register_numbers.split(split_string, ',');
+  for (llvm::StringRef &x : split_string) {
+uint32_t reg;
+if (llvm::to_integer(x, reg))
+  regnums.push_back(reg);
+  }
   return regnums.size();
 }
 
@@ -1459,21 +1454,16 @@
   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;
-  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)
+  llvm::SmallVector split_value;
+  value.split(split_value, ',');
+  for (llvm::StringRef &x : split_value) {
+lldb::addr_t pc;
+if (llvm::to_integer(x, pc))
   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();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -16,7 +16,6 @@
 
 #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"
@@ -1660,22 +1659,14 @@
   error_extractor.GetHexByteString(error_string);
   error.SetErrorString(error_string.c_str());
 } else if (name.equals("dirty-pages")) {
+  llvm::SmallVector split_value;
   std::vector dirty_page_list;
-  std::string comma_sep_str = value.str();
-  size_t comma_pos;
-  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)
+  value.split(split_value, ',');
+  for (llvm::StringRef &x : split_value) {
+addr_t page;
+if (llvm::to_integer(x, page))
   dirty_page_list.push_back(page);
-comma_sep_str.erase(0, comm

[Lldb-commits] [PATCH] D110447: [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 375024.
mgorny added a comment.

Reverting debugserver part, this *beep* doesn't use LLVM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110447/new/

https://reviews.llvm.org/D110447

Files:
  lldb/source/Interpreter/OptionValueArray.cpp
  lldb/source/Interpreter/OptionValueFileSpecList.cpp
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/source/Interpreter/OptionValueSInt64.cpp
  lldb/source/Interpreter/OptionValueUInt64.cpp
  lldb/source/Interpreter/Property.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/unittests/debugserver/RNBSocketTest.cpp

Index: lldb/unittests/debugserver/RNBSocketTest.cpp
===
--- lldb/unittests/debugserver/RNBSocketTest.cpp
+++ lldb/unittests/debugserver/RNBSocketTest.cpp
@@ -15,7 +15,6 @@
 #include "RNBDefs.h"
 #include "RNBSocket.h"
 #include "lldb/Host/Socket.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "llvm/Testing/Support/Error.h"
 
Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -26,7 +26,6 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/Socket.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
@@ -238,7 +237,7 @@
 if (colon_pos != std::string::npos) {
   connection_host = final_host_and_port.substr(0, colon_pos);
   connection_port = final_host_and_port.substr(colon_pos + 1);
-  connection_portno = StringConvert::ToUInt32(connection_port.c_str(), 0);
+  llvm::to_integer(connection_port, connection_portno);
 }
 
 
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -12,7 +12,6 @@
 #include "Plugins/Process/Utility/MipsLinuxSignals.h"
 #include "Plugins/Process/Utility/NetBSDSignals.h"
 #include "lldb/Host/HostInfo.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
@@ -156,9 +155,8 @@
   return pos->first;
   }
 
-  const int32_t signo =
-  StringConvert::ToSInt32(name, LLDB_INVALID_SIGNAL_NUMBER, 0);
-  if (signo != LLDB_INVALID_SIGNAL_NUMBER)
+  int32_t signo;
+  if (llvm::to_integer(name, signo))
 return signo;
   return LLDB_INVALID_SIGNAL_NUMBER;
 }
Index: lldb/source/Symbol/SymbolContext.cpp
===
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -977,13 +976,11 @@
 m_type |= eFileSpecified;
 break;
   case eLineStartSpecified:
-m_start_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
+if (llvm::to_integer(spec_string, m_start_line))
   m_type |= eLineStartSpecified;
 break;
   case eLineEndSpecified:
-m_end_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
+if (llvm::to_integer(spec_string, m_end_line))
   m_type |= eLineEndSpecified;
 break;
   case eFunctionSpecified:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -9,7 +9,6 @@
 #include "DWARFUnit.h"
 
 #include "lldb/Core/Module.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/StreamString.h"
@@ -687,12 +686,9 @@
 llvm::SmallVector matches;
 if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
   &matches)) {
-  m_producer_version_major =
-  StringConvert::ToUInt32(matches[1].str().c_str(), UINT32_MAX, 10);
-  m_producer_version_minor =
-  StringConvert::ToUInt32(matches[2].str().c_str(

[Lldb-commits] [PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

2021-09-25 Thread Markus Böck via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0b61f43b6096: [CMake] Consistently use the LibXml2::LibXml2 
target instead of… (authored by zero9178).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109975/new/

https://reviews.llvm.org/D109975

Files:
  clang/tools/c-index-test/CMakeLists.txt
  lldb/source/Host/CMakeLists.txt


Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -137,7 +137,7 @@
   list(APPEND EXTRA_LIBS kvm)
 endif()
 if (LLDB_ENABLE_LIBXML2)
-  list(APPEND EXTRA_LIBS ${LIBXML2_LIBRARIES})
+  list(APPEND EXTRA_LIBS LibXml2::LibXml2)
 endif()
 if (HAVE_LIBDL)
   list(APPEND EXTRA_LIBS ${CMAKE_DL_LIBS})
Index: clang/tools/c-index-test/CMakeLists.txt
===
--- clang/tools/c-index-test/CMakeLists.txt
+++ clang/tools/c-index-test/CMakeLists.txt
@@ -40,12 +40,7 @@
 
 # If libxml2 is available, make it available for c-index-test.
 if (CLANG_HAVE_LIBXML)
-  if ((CMAKE_OSX_SYSROOT) AND (EXISTS 
${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR}))
-include_directories(SYSTEM ${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR})
-  else()
-include_directories(SYSTEM ${LIBXML2_INCLUDE_DIR})
-  endif()
-  target_link_libraries(c-index-test PRIVATE ${LIBXML2_LIBRARIES})
+  target_link_libraries(c-index-test PRIVATE LibXml2::LibXml2)
 endif()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)


Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -137,7 +137,7 @@
   list(APPEND EXTRA_LIBS kvm)
 endif()
 if (LLDB_ENABLE_LIBXML2)
-  list(APPEND EXTRA_LIBS ${LIBXML2_LIBRARIES})
+  list(APPEND EXTRA_LIBS LibXml2::LibXml2)
 endif()
 if (HAVE_LIBDL)
   list(APPEND EXTRA_LIBS ${CMAKE_DL_LIBS})
Index: clang/tools/c-index-test/CMakeLists.txt
===
--- clang/tools/c-index-test/CMakeLists.txt
+++ clang/tools/c-index-test/CMakeLists.txt
@@ -40,12 +40,7 @@
 
 # If libxml2 is available, make it available for c-index-test.
 if (CLANG_HAVE_LIBXML)
-  if ((CMAKE_OSX_SYSROOT) AND (EXISTS ${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR}))
-include_directories(SYSTEM ${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR})
-  else()
-include_directories(SYSTEM ${LIBXML2_INCLUDE_DIR})
-  endif()
-  target_link_libraries(c-index-test PRIVATE ${LIBXML2_LIBRARIES})
+  target_link_libraries(c-index-test PRIVATE LibXml2::LibXml2)
 endif()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 0b61f43 - [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

2021-09-25 Thread Markus Böck via lldb-commits

Author: Markus Böck
Date: 2021-09-25T13:13:11+02:00
New Revision: 0b61f43b6096a9e98652991cba34e8ad44d35101

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

LOG: [CMake] Consistently use the LibXml2::LibXml2 target instead of 
LIBXML2_LIBRARIES

Linking against the LibXml2::LibXml2 target has the advantage of not only 
importing the library, but also adding the include path as well as any 
definitions the library requires. In case of a static build of libxml2, eg. a 
define is set on Windows to remove any DLL imports and export.

LLVM already makes use of the target, but c-index-test and lldb were still 
linking against the library only.

The workaround for Mac OS-X that I removed seems to have also been made 
redundant since https://reviews.llvm.org/D84563 I believe

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

Added: 


Modified: 
clang/tools/c-index-test/CMakeLists.txt
lldb/source/Host/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/c-index-test/CMakeLists.txt 
b/clang/tools/c-index-test/CMakeLists.txt
index ceef4b08637cc..99c6081db2d63 100644
--- a/clang/tools/c-index-test/CMakeLists.txt
+++ b/clang/tools/c-index-test/CMakeLists.txt
@@ -40,12 +40,7 @@ set_target_properties(c-index-test
 
 # If libxml2 is available, make it available for c-index-test.
 if (CLANG_HAVE_LIBXML)
-  if ((CMAKE_OSX_SYSROOT) AND (EXISTS 
${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR}))
-include_directories(SYSTEM ${CMAKE_OSX_SYSROOT}/${LIBXML2_INCLUDE_DIR})
-  else()
-include_directories(SYSTEM ${LIBXML2_INCLUDE_DIR})
-  endif()
-  target_link_libraries(c-index-test PRIVATE ${LIBXML2_LIBRARIES})
+  target_link_libraries(c-index-test PRIVATE LibXml2::LibXml2)
 endif()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)

diff  --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index a018fd6c183dc..c18e8ce004b08 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -137,7 +137,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
   list(APPEND EXTRA_LIBS kvm)
 endif()
 if (LLDB_ENABLE_LIBXML2)
-  list(APPEND EXTRA_LIBS ${LIBXML2_LIBRARIES})
+  list(APPEND EXTRA_LIBS LibXml2::LibXml2)
 endif()
 if (HAVE_LIBDL)
   list(APPEND EXTRA_LIBS ${CMAKE_DL_LIBS})



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110447: [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

This patch makes me very happy. I have one comment regarding one small change 
in behaviour, but otherwise this LGTM.

If you have the time, it would be nice to drop FIXME's above the few places 
where the return value of `to_integer` is not checked. Those always make great 
beginner bugs.




Comment at: lldb/source/Symbol/SymbolContext.cpp:986
-m_end_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
   m_type |= eLineEndSpecified;

This change and the one above make this return true on a parse failure. LLDB is 
of course one step ahead of us here and never even checks the return value 
anywhere (...), but I think it still makes sense to preserve the behaviour in 
this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110447/new/

https://reviews.llvm.org/D110447

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110447: [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D110447#3022340 , @teemperor wrote:

> If you have the time, it would be nice to drop FIXME's above the few places 
> where the return value of `to_integer` is not checked. Those always make 
> great beginner bugs.

Beginner? I'm pretty sure these things are really hard ;-).

That said, in most of the cases 'not checking the return value' is combined 
with setting a fallback value earlier, i.e. if `llvm::to_integer()` fails, the 
variable remains with its default/fallback value. But yeah, I suppose it would 
be nice to rethink these things and maybe add explicit error handling in place 
of silent fallbacks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110447/new/

https://reviews.llvm.org/D110447

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D110447: [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Symbol/SymbolContext.cpp:986
-m_end_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
   m_type |= eLineEndSpecified;

teemperor wrote:
> This change and the one above make this return true on a parse failure. LLDB 
> is of course one step ahead of us here and never even checks the return value 
> anywhere (...), but I think it still makes sense to preserve the behaviour in 
> this patch.
Ahh, good catch. Will fix in a minute.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110447/new/

https://reviews.llvm.org/D110447

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 3a6ba36 - [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-09-25T14:19:19+02:00
New Revision: 3a6ba3675177cb5e47dee325f300aced4cd864ed

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

LOG: [lldb] Convert misc. StringConvert uses

Replace misc. StringConvert uses with llvm::to_integer()
and llvm::to_float(), except for cases where further refactoring is
planned.  The purpose of this change is to eliminate the StringConvert
API that is duplicate to LLVM, and less correct in behavior at the same
time.

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

Added: 


Modified: 
lldb/source/Interpreter/OptionValueArray.cpp
lldb/source/Interpreter/OptionValueFileSpecList.cpp
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/source/Interpreter/OptionValueSInt64.cpp
lldb/source/Interpreter/OptionValueUInt64.cpp
lldb/source/Interpreter/Property.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Target/UnixSignals.cpp
lldb/tools/lldb-server/lldb-gdbserver.cpp
lldb/unittests/debugserver/RNBSocketTest.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/OptionValueArray.cpp 
b/lldb/source/Interpreter/OptionValueArray.cpp
index b1545bdebf10e..4468fe57702e6 100644
--- a/lldb/source/Interpreter/OptionValueArray.cpp
+++ b/lldb/source/Interpreter/OptionValueArray.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Interpreter/OptionValueArray.h"
 
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Stream.h"
 
@@ -167,13 +166,12 @@ Status OptionValueArray::SetArgs(const Args &args, 
VarSetOperationType op) {
   case eVarSetOperationInsertBefore:
   case eVarSetOperationInsertAfter:
 if (argc > 1) {
-  uint32_t idx =
-  StringConvert::ToUInt32(args.GetArgumentAtIndex(0), UINT32_MAX);
+  uint32_t idx;
   const uint32_t count = GetSize();
-  if (idx > count) {
+  if (!llvm::to_integer(args.GetArgumentAtIndex(0), idx) || idx > count) {
 error.SetErrorStringWithFormat(
-"invalid insert array index %u, index must be 0 through %u", idx,
-count);
+"invalid insert array index %s, index must be 0 through %u",
+args.GetArgumentAtIndex(0), count);
   } else {
 if (op == eVarSetOperationInsertAfter)
   ++idx;
@@ -207,9 +205,8 @@ Status OptionValueArray::SetArgs(const Args &args, 
VarSetOperationType op) {
   bool all_indexes_valid = true;
   size_t i;
   for (i = 0; i < argc; ++i) {
-const size_t idx =
-StringConvert::ToSInt32(args.GetArgumentAtIndex(i), INT32_MAX);
-if (idx >= size) {
+size_t idx;
+if (!llvm::to_integer(args.GetArgumentAtIndex(i), idx) || idx >= size) 
{
   all_indexes_valid = false;
   break;
 } else
@@ -249,13 +246,12 @@ Status OptionValueArray::SetArgs(const Args &args, 
VarSetOperationType op) {
 
   case eVarSetOperationReplace:
 if (argc > 1) {
-  uint32_t idx =
-  StringConvert::ToUInt32(args.GetArgumentAtIndex(0), UINT32_MAX);
+  uint32_t idx;
   const uint32_t count = GetSize();
-  if (idx > count) {
+  if (!llvm::to_integer(args.GetArgumentAtIndex(0), idx) || idx > count) {
 error.SetErrorStringWithFormat(
-"invalid replace array index %u, index must be 0 through %u", idx,
-count);
+"invalid replace array index %s, index must be 0 through %u",
+args.GetArgumentAtIndex(0), count);
   } else {
 for (size_t i = 1; i < argc; ++i, ++idx) {
   lldb::OptionValueSP value_sp(CreateValueFromCStringForTypeMask(

diff  --git a/lldb/source/Interpreter/OptionValueFileSpecList.cpp 
b/lldb/source/Interpreter/OptionValueFileSpecList.cpp
index 2160fd61d4282..6566eee09d738 100644
--- a/lldb/source/Interpreter/OptionValueFileSpecList.cpp
+++ b/lldb/source/Interpreter/OptionValueFileSpecList.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Interpreter/OptionValueFileSpecList.h"
 
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Stream.h"
 
@@ -57,13 +56,12 @@ Status 
OptionValueFileSpecList::SetValueFromString(llvm::StringRef value,
 
   case eVarSetOperationReplace:
 if (argc > 1) {
-  uint32_t idx =
-  StringConvert::ToUInt32(args.GetArgumentAtIndex(0), UINT32_MAX);
+  uint32_t idx;
   const uin

[Lldb-commits] [PATCH] D110447: [lldb] Convert misc. StringConvert uses

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a6ba3675177: [lldb] Convert misc. StringConvert uses 
(authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110447?vs=375024&id=375030#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110447/new/

https://reviews.llvm.org/D110447

Files:
  lldb/source/Interpreter/OptionValueArray.cpp
  lldb/source/Interpreter/OptionValueFileSpecList.cpp
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/source/Interpreter/OptionValueSInt64.cpp
  lldb/source/Interpreter/OptionValueUInt64.cpp
  lldb/source/Interpreter/Property.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/unittests/debugserver/RNBSocketTest.cpp

Index: lldb/unittests/debugserver/RNBSocketTest.cpp
===
--- lldb/unittests/debugserver/RNBSocketTest.cpp
+++ lldb/unittests/debugserver/RNBSocketTest.cpp
@@ -15,7 +15,6 @@
 #include "RNBDefs.h"
 #include "RNBSocket.h"
 #include "lldb/Host/Socket.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "llvm/Testing/Support/Error.h"
 
Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -26,7 +26,6 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/Socket.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
@@ -238,7 +237,8 @@
 if (colon_pos != std::string::npos) {
   connection_host = final_host_and_port.substr(0, colon_pos);
   connection_port = final_host_and_port.substr(colon_pos + 1);
-  connection_portno = StringConvert::ToUInt32(connection_port.c_str(), 0);
+  // FIXME: improve error handling
+  llvm::to_integer(connection_port, connection_portno);
 }
 
 
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -12,7 +12,6 @@
 #include "Plugins/Process/Utility/MipsLinuxSignals.h"
 #include "Plugins/Process/Utility/NetBSDSignals.h"
 #include "lldb/Host/HostInfo.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
@@ -156,9 +155,8 @@
   return pos->first;
   }
 
-  const int32_t signo =
-  StringConvert::ToSInt32(name, LLDB_INVALID_SIGNAL_NUMBER, 0);
-  if (signo != LLDB_INVALID_SIGNAL_NUMBER)
+  int32_t signo;
+  if (llvm::to_integer(name, signo))
 return signo;
   return LLDB_INVALID_SIGNAL_NUMBER;
 }
Index: lldb/source/Symbol/SymbolContext.cpp
===
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -977,13 +976,11 @@
 m_type |= eFileSpecified;
 break;
   case eLineStartSpecified:
-m_start_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
+if ((return_value = llvm::to_integer(spec_string, m_start_line)))
   m_type |= eLineStartSpecified;
 break;
   case eLineEndSpecified:
-m_end_line = StringConvert::ToSInt32(spec_string, 0, 0, &return_value);
-if (return_value)
+if ((return_value = llvm::to_integer(spec_string, m_end_line)))
   m_type |= eLineEndSpecified;
 break;
   case eFunctionSpecified:
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -9,7 +9,6 @@
 #include "DWARFUnit.h"
 
 #include "lldb/Core/Module.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/StreamString.h"
@@ -687,12 +686,10 @@
 llvm::SmallVector matches;
 if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
  

[Lldb-commits] [PATCH] D110478: [lldb] Move StringConvert inside debugserver

2021-09-25 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste, JDevlieghere, 
jasonmolenda.
mgorny requested review of this revision.

The StringConvert API is no longer used anywhere but in debugserver.
Since debugserver does not use LLVM API, we cannot replace it with
llvm::to_integer() and llvm::to_float() there.  Let's just move
the sources into debugserver.

(note: this commit message assumes merging the other parent diffs)


https://reviews.llvm.org/D110478

Files:
  lldb/include/lldb/Host/StringConvert.h
  lldb/include/lldb/module.modulemap
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/StringConvert.cpp
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/debugserver/source/JSON.cpp
  lldb/tools/debugserver/source/StringConvert.cpp
  lldb/tools/debugserver/source/StringConvert.h

Index: lldb/tools/debugserver/source/StringConvert.h
===
--- lldb/tools/debugserver/source/StringConvert.h
+++ lldb/tools/debugserver/source/StringConvert.h
@@ -6,24 +6,13 @@
 //
 //===--===//
 
-#ifndef LLDB_HOST_STRINGCONVERT_H
-#define LLDB_HOST_STRINGCONVERT_H
+#ifndef LLDB_TOOLS_DEBUGSERVER_SOURCE_STRINGCONVERT_H
+#define LLDB_TOOLS_DEBUGSERVER_SOURCE_STRINGCONVERT_H
 
 #include 
 
-namespace lldb_private {
-
 namespace StringConvert {
 
-/// \namespace StringConvert StringConvert.h "lldb/Host/StringConvert.h"
-/// Utility classes for converting strings into Integers
-
-int32_t ToSInt32(const char *s, int32_t fail_value = 0, int base = 0,
- bool *success_ptr = nullptr);
-
-uint32_t ToUInt32(const char *s, uint32_t fail_value = 0, int base = 0,
-  bool *success_ptr = nullptr);
-
 int64_t ToSInt64(const char *s, int64_t fail_value = 0, int base = 0,
  bool *success_ptr = nullptr);
 
@@ -32,7 +21,7 @@
 
 double ToDouble(const char *s, double fail_value = 0.0,
 bool *success_ptr = nullptr);
+
 } // namespace StringConvert
-} // namespace lldb_private
 
 #endif
Index: lldb/tools/debugserver/source/StringConvert.cpp
===
--- lldb/tools/debugserver/source/StringConvert.cpp
+++ lldb/tools/debugserver/source/StringConvert.cpp
@@ -8,43 +8,10 @@
 
 #include 
 
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 
-namespace lldb_private {
 namespace StringConvert {
 
-int32_t ToSInt32(const char *s, int32_t fail_value, int base,
- bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const long sval = ::strtol(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = ((sval <= INT32_MAX) && (sval >= INT32_MIN));
-  return (int32_t)sval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
-uint32_t ToUInt32(const char *s, uint32_t fail_value, int base,
-  bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const unsigned long uval = ::strtoul(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = (uval <= UINT32_MAX);
-  return (uint32_t)uval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
 int64_t ToSInt64(const char *s, int64_t fail_value, int base,
  bool *success_ptr) {
   if (s && s[0]) {
@@ -91,5 +58,5 @@
 *success_ptr = false;
   return fail_value;
 }
-}
-}
+
+} // namespace StringConvert
Index: lldb/tools/debugserver/source/JSON.cpp
===
--- lldb/tools/debugserver/source/JSON.cpp
+++ lldb/tools/debugserver/source/JSON.cpp
@@ -13,12 +13,10 @@
 #include 
 
 // C++ includes
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 #include 
 #include 
 
-using namespace lldb_private;
-
 std::string JSONString::json_string_quote_metachars(const std::string &s) {
   if (s.find('"') == std::string::npos)
 return s;
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -206,8 +206,8 @@
   DNBThreadResumeActions.cpp
   JSON.cpp
   StdStringExtractor.cpp
+  StringConvert.cpp
   # JSON reader depends on the following LLDB-common files
-  ${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
   ${LLDB_SOURCE_DIR}/source/Host/common/SocketAddress.cpp
   # end JSON reader dependencies
   libdebugserver.cpp
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -37,7 +37,6 @@
   common/PseudoTerminal.cpp
   common/SocketAddress.c

[Lldb-commits] [PATCH] D110478: [lldb] Move StringConvert inside debugserver

2021-09-25 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110478/new/

https://reviews.llvm.org/D110478

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


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

2021-09-25 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

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!




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1667
+addr_t page;
+if (llvm::to_integer(x, page))
   dirty_page_list.push_back(page);

Base 16 arg is lost here.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:390
+uint32_t reg;
+if (llvm::to_integer(x, reg))
+  regnums.push_back(reg);

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



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1464
+lldb::addr_t pc;
+if (llvm::to_integer(x, pc))
   m_thread_pcs.push_back(pc);

base 16 lost here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110472/new/

https://reviews.llvm.org/D110472

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] eee687a - [lldb] Add minidump save-core functionality to ELF object files

2021-09-25 Thread Raphael Isemann via lldb-commits
I guess you might have already figured that out in a Google-internal
chat, but msan actually requires that the used libc++ is also compiled
with msan enabled. Totally forgot that this is a requirement for msan
(sorry).

I don't think there is a public LLDB msan bot (?), but you can
probably try the msan bot script as described here and then just see
if you can patch it to enable LLDB:
https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild


Am Di., 7. Sept. 2021 um 10:43 Uhr schrieb Andy Yankovsky :
>
> Replying on behalf of Andrej, since there's some issue with his email policy
>
> Andrej Korman:
>
> Hi,
>
> unfortunately I was not able to reproduce this as build with MemorySanitizer 
> fails earlier with:
>
> Uninitialized bytes in __interceptor_memchr at offset 6 inside 
> [0x702006c0, 7) ==3708245==WARNING: MemorySanitizer: 
> use-of-uninitialized-value #0 0x4f9908 in llvm::StringRef::find(char, 
> unsigned long) const 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/include/llvm/ADT/StringRef.h:319:29
>  #1 0xcb7639 in 
> llvm::StringRef::split(llvm::SmallVectorImpl&, char, int, 
> bool) const 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/lib/Support/StringRef.cpp:341:20
>  #2 0xcd0dbd in llvm::Triple::Triple(llvm::Twine const&) 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/lib/Support/Triple.cpp:770:19
>  #3 0xd85899 in llvm::sys::getProcessTriple[abi:cxx11]() 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/lib/Support/Host.cpp:1750:10
>  #4 0xc18393 in (anonymous 
> namespace)::CommandLineParser::ParseCommandLineOptions(int, char const* 
> const*, llvm::StringRef, llvm::raw_ostream*, bool) 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/lib/Support/CommandLine.cpp:1353:17
>  #5 0xc17fc4 in llvm::cl::ParseCommandLineOptions(int, char const* const*, 
> llvm::StringRef, llvm::raw_ostream*, char const*, bool) 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/lib/Support/CommandLine.cpp:1320:24
>  #6 0xb9a3cc in main 
> /usr/local/google/home/andrejkorman/work/llvm-project/llvm/utils/TableGen/TableGen.cpp:283:3
>  #7 0x7f5fc9d30d09 in __libc_start_main csu/../csu/libc-start.c:308:16 #8 
> 0x42ad99 in _start 
> (/usr/local/google/home/andrejkorman/work/llvm-project/build_x64/bin/llvm-tblgen+0x42ad99)
>
> and the same with the lldb-tblgen and clang-tblgen. The command used for the 
> build was:
>
> cmake -G Ninja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ 
> -DLLVM_PARALLEL_LINK_JOBS=5 -DLLVM_BUILD_TESTS=ON -DLLVM_USE_SANITIZER=Memory 
> -DLLVM_ENABLE_PROJECTS='lldb;clang' ../llvm
>
> After providing -DLLVM_TABLEGEN, -DLLDB_TABLEGEN and -DCLANG_TABLEGEN builded 
> separately, it fails on clang-ast-dump. This exact issue also happened on 2 
> more Linux machines running on x86_64. I wonder if this is expected and if 
> possible how to get past this errors.
>
> Thank you for any information on this issue and sorry for the inconvenience. 
> I wasn't, unfortunately, able to find the root cause only by looking at the 
> code that is producing this memory issue.
>
> On Mon, 6 Sept 2021 at 12:21, Raphael Isemann  wrote:
>>
>> Not sure if there is a public builder with sanitizer enabled, but
>> compiling LLDB with -DLLVM_USE_SANITIZER=Memory and then doing ninja
>> check-lldb should be enough to reproduce this IIUC.
>>
>> Am Mo., 6. Sept. 2021 um 12:17 Uhr schrieb Andy Yankovsky via
>> lldb-commits :
>> >
>> > Thanks for flagging this! Adding the author of the change.
>> >
>> > Does it fail somewhere on the llvm builders? Is there an easy way to 
>> > reproduce this locally?
>> >
>> >
>> > On Thu, 2 Sept 2021 at 01:53, Richard Smith  wrote:
>> >>
>> >> The new test fails under MSan:
>> >>
>> >> Uninitialized bytes in __interceptor_write at offset 2 inside 
>> >> [0x7fb1f42ed000, 18438530)
>> >> ==3871==WARNING: MemorySanitizer: use-of-uninitialized-value
>> >> #0 0x55f5706515d9 in RetryAfterSignal> >> unsigned long), int, const void *, unsigned long> 
>> >> llvm-project/llvm/include/llvm/Support/Errno.h:38:11
>> >> #1 0x55f5706515d9 in lldb_private::NativeFile::Write(void const*, 
>> >> unsigned long&) llvm-project/lldb/source/Host/common/File.cpp:585:9
>> >> #2 0x55f570badbf5 in 
>> >> MinidumpFileBuilder::Dump(std::__msan::unique_ptr> >> std::__msan::default_delete >&) const 
>> >> llvm-project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:739:22
>> >> #3 0x55f570bb075c in 
>> >> ObjectFileMinidump::SaveCore(std::__msan::shared_ptr
>> >>  const&, lldb_private::FileSpec const&, lldb::SaveCoreStyle&, 
>> >> lldb_private::Status&)  
>> >> llvm-project/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:114:19
>> >> #4 0x55f57048960b in 
>> >> lldb_private::PluginManager::SaveCore(std::__msan::shared_ptr
>> >>  const&, lldb_private::FileSpec const&, lldb::SaveCoreStyle&, 
>> >> lldb_private::ConstString)  
>> >> llvm-project/lldb/source/Core/PluginManager.