[Lldb-commits] [lldb] r313442 - Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Vadim Chugunov via lldb-commits 
Author: vadimcn
Date: Fri Sep 15 20:53:13 2017
New Revision: 313442

URL: http://llvm.org/viewvc/llvm-project?rev=313442=rev
Log:
Fix compatibility with OpenOCD debug stub.

OpenOCD sends register classes as two separate  nodes, fixed parser to 
process both of them.

OpenOCD returns "l" in response to "qfThreadInfo", so IsUnsupportedResponse() 
was false and we were ending up without any threads in the process. I think 
it's reasonable to assume that there's always at least one thread.

Modified:

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

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=313442=313441=313442=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Fri Sep 15 20:53:13 2017
@@ -2624,8 +2624,7 @@ size_t GDBRemoteCommunicationClient::Get
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
-IsConnected()) {
+if (thread_ids.size() == 0 && IsConnected()) {
   thread_ids.push_back(1);
 }
   } else {

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=313442=313441=313442=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri Sep 
15 20:53:13 2017
@@ -4168,7 +4168,6 @@ struct GdbServerTargetInfo {
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
-  XMLNode feature_node;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
@@ -4374,8 +4373,8 @@ bool ProcessGDBRemote::GetGDBServerRegis
 
 XMLNode target_node = xml_document.GetRootElement("target");
 if (target_node) {
-  XMLNode feature_node;
-  target_node.ForEachChildElement([_info, _node](
+  std::vector feature_nodes;
+  target_node.ForEachChildElement([_info, _nodes](
   const XMLNode ) -> bool {
 llvm::StringRef name = node.GetName();
 if (name == "architecture") {
@@ -4387,7 +4386,7 @@ bool ProcessGDBRemote::GetGDBServerRegis
   if (!href.empty())
 target_info.includes.push_back(href.str());
 } else if (name == "feature") {
-  feature_node = node;
+  feature_nodes.push_back(node);
 } else if (name == "groups") {
   node.ForEachChildElementWithName(
   "group", [_info](const XMLNode ) -> bool {
@@ -4423,7 +4422,7 @@ bool ProcessGDBRemote::GetGDBServerRegis
   // set the Target's architecture yet, so the ABI is also potentially
   // incorrect.
   ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
-  if (feature_node) {
+  for (auto _node : feature_nodes) {
 ParseRegisters(feature_node, target_info, this->m_register_info,
abi_to_use_sp, cur_reg_num, reg_offset);
   }


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

[Lldb-commits] [lldb] r313437 - Check availability of accept4 in C++ instad of C code.

2017-09-15 Thread Eugene Zemtsov via lldb-commits 
Author: eugene
Date: Fri Sep 15 19:58:49 2017
New Revision: 313437

URL: http://llvm.org/viewvc/llvm-project?rev=313437=rev
Log:
Check availability of accept4 in C++ instad of C code.

Modified:
lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake

Modified: lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake?rev=313437=313436=313437=diff
==
--- lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake Fri Sep 15 19:58:49 2017
@@ -9,7 +9,7 @@ set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SO
 check_symbol_exists(ppoll poll.h HAVE_PPOLL)
 set(CMAKE_REQUIRED_DEFINITIONS)
 check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
-check_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
+check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)


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

[Lldb-commits] [lldb] r313436 - More precise c library feature detection for Android.

2017-09-15 Thread Eugene Zemtsov via lldb-commits 
Author: eugene
Date: Fri Sep 15 19:19:21 2017
New Revision: 313436

URL: http://llvm.org/viewvc/llvm-project?rev=313436=rev
Log:
More precise c library feature detection for Android.

Modified:
lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake
lldb/trunk/source/Host/common/Socket.cpp
lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp

Modified: lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake?rev=313436=313435=313436=diff
==
--- lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake Fri Sep 15 19:19:21 2017
@@ -9,6 +9,7 @@ set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SO
 check_symbol_exists(ppoll poll.h HAVE_PPOLL)
 set(CMAKE_REQUIRED_DEFINITIONS)
 check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
+check_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)

Modified: lldb/trunk/source/Host/common/Socket.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Socket.cpp?rev=313436=313435=313436=diff
==
--- lldb/trunk/source/Host/common/Socket.cpp (original)
+++ lldb/trunk/source/Host/common/Socket.cpp Fri Sep 15 19:19:21 2017
@@ -450,7 +450,7 @@ NativeSocket Socket::AcceptSocket(Native
 close(fd);
   }
   return fd;
-#elif defined(SOCK_CLOEXEC)
+#elif defined(SOCK_CLOEXEC) && defined(HAVE_ACCEPT4)
   int flags = 0;
   if (!child_processes_inherit) {
 flags |= SOCK_CLOEXEC;

Modified: lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp?rev=313436=313435=313436=diff
==
--- lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp (original)
+++ lldb/trunk/source/Host/posix/ProcessLauncherPosixFork.cpp Fri Sep 15 
19:19:21 2017
@@ -29,7 +29,7 @@
 #define PT_TRACE_ME PTRACE_TRACEME
 #endif
 
-#if defined(__ANDROID_API__) && __ANDROID_API__ < 21
+#if defined(__ANDROID_API__) && __ANDROID_API__ < 15
 #include 
 #elif defined(__linux__)
 #include 


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

[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

As long as everyone agrees that no threads from qfThreadInfo means there is one 
thread whose thread ID is 1 then this is ok.


Repository:
  rL LLVM

https://reviews.llvm.org/D37934



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

[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits 
vadimcn added a comment.

This is what I see in the log:

  <  16> send packet: $qfThreadInfo#bb
  <   5> read packet: $l#6c

And here's code that generates it:  
https://github.com/gnu-mcu-eclipse/openocd/blob/b21ab1d683aaee501d45fe8a509a2043123f16fd/src/rtos/rtos.c#L370

I agree, they should have responded with "not supported", but I've never heard 
of a system that allows processes without threads, and openocd is pretty 
popular in embedded, so I think it would make sense to just support this quirk. 
  That's what gdb does anyways...


Repository:
  rL LLVM

https://reviews.llvm.org/D37934



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo updated this revision to Diff 115516.
lemo edited the summary of this revision.
lemo added a comment.

Incorporating CR feedback.


https://reviews.llvm.org/D37923

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Commands/CommandObjectTarget.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -546,7 +546,7 @@
   char buffer[1024];
   int num_printed =
   snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o");
-  assert(num_printed < 1024);
+  lldbassert(num_printed < 1024);
   UNUSED_IF_ASSERT_DISABLED(num_printed);
   success =
   tbreak_regex_cmd_ap->AddRegexCommand(break_regexes[i][0], buffer);
@@ -891,7 +891,7 @@
 const lldb::CommandObjectSP _sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == _sp->GetCommandInterpreter()) &&
+lldbassert((this == _sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   if (name.empty())
@@ -913,7 +913,7 @@
 const lldb::CommandObjectSP _sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == _sp->GetCommandInterpreter()) &&
+lldbassert((this == _sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   if (!name.empty()) {
@@ -1062,7 +1062,7 @@
  lldb::CommandObjectSP _obj_sp,
  llvm::StringRef args_string) {
   if (command_obj_sp.get())
-assert((this == _obj_sp->GetCommandInterpreter()) &&
+lldbassert((this == _obj_sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   std::unique_ptr command_alias_up(
@@ -1839,7 +1839,7 @@
   matches.Clear();
 
   // Only max_return_elements == -1 is supported at present:
-  assert(max_return_elements == -1);
+  lldbassert(max_return_elements == -1);
   bool word_complete;
   num_command_matches = HandleCompletionMatches(
   parsed_line, cursor_index, cursor_char_position, match_start_point,
@@ -2677,6 +2677,57 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  lldbassert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  lldbassert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+  in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream , llvm::StringRef str,
+bool interruptible) {
+  if (str.empty())
+return;
+
+  if (interruptible) {
+// Split the output into lines and poll for interrupt requests
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {
+  size_t chunk_size = 0;
+  for (; chunk_size < size; ++chunk_size) {
+lldbassert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {
+  ++chunk_size;
+  break;
+}
+  }
+  chunk_size = stream.Write(data, chunk_size);
+  lldbassert(size >= chunk_size);
+  data += chunk_size;
+  size -= chunk_size;
+}
+if (size > 0) {
+  stream.Printf("\n... Interrupted.\n");
+}
+  } else {
+stream.PutCString(str);
+  }
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler _handler,
 std::string ) {
   const bool is_interactive = io_handler.GetIsInteractive();
@@ -2700,6 +2751,8 @@
line.c_str());
   }
 
+  StartHandlingCommand();
+
   lldb_private::CommandReturnObject result;
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
@@ -2710,18 +2763,20 @@
 
 if (!result.GetImmediateOutputStream()) {
   llvm::StringRef output = result.GetOutputData();
-  if (!output.empty())
-io_handler.GetOutputStreamFile()->PutCString(output);
+  PrintCommandOutput(*io_handler.GetOutputStreamFile(), output,
+ is_interactive);
 }
 
 // Now emit the command error text from the command we just executed
 if (!result.GetImmediateErrorStream()) {
   llvm::StringRef error =

[Lldb-commits] [lldb] r313407 - Resubmit "[lit] Force site configs to run before source-tree configs"

2017-09-15 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Fri Sep 15 15:10:46 2017
New Revision: 313407

URL: http://llvm.org/viewvc/llvm-project?rev=313407=rev
Log:
Resubmit "[lit] Force site configs to run before source-tree configs"

This is a resubmission of r313270.  It broke standalone builds of
compiler-rt because we were not correctly generating the llvm-lit
script in the standalone build directory.

The fixes incorporated here attempt to find llvm/utils/llvm-lit
from the source tree returned by llvm-config.  If present, it
will generate llvm-lit into the output directory.  Regardless,
the user can specify -DLLVM_EXTERNAL_LIT to point to a specific
lit.py on their file system.  This supports the use case of
someone installing lit via a package manager.  If it cannot find
a source tree, and -DLLVM_EXTERNAL_LIT is either unspecified or
invalid, then we print a warning that tests will not be able
to run.

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

Modified:
lldb/trunk/lit/Unit/lit.cfg
lldb/trunk/lit/lit.cfg

Modified: lldb/trunk/lit/Unit/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Unit/lit.cfg?rev=313407=313406=313407=diff
==
--- lldb/trunk/lit/Unit/lit.cfg (original)
+++ lldb/trunk/lit/Unit/lit.cfg Fri Sep 15 15:10:46 2017
@@ -6,19 +6,6 @@ import os
 
 import lit.formats
 
-# Check that the object root is known.
-if config.test_exec_root is None:
-# Otherwise, we haven't loaded the site specific configuration (the user is
-# probably trying to run on a test file directly, and either the site
-# configuration hasn't been created by the build system, or we are in an
-# out-of-tree build situation).
-
-# Check for 'llvm_unit_site_config' user parameter, and use that if 
available.
-site_cfg = lit_config.params.get('lldb_unit_site_config', None)
-if site_cfg and os.path.exists(site_cfg):
-lit_config.load_config(config, site_cfg)
-raise SystemExit
-
 # name: The name of this test suite.
 config.name = 'lldb-Unit'
 
@@ -31,6 +18,4 @@ config.test_source_root = os.path.join(c
 config.test_exec_root = config.test_source_root
 
 # testFormat: The test format to use to interpret tests.
-if not hasattr(config, 'llvm_build_mode'):
-lit_config.fatal("unable to find llvm_build_mode value on config")
 config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, 'Tests')

Modified: lldb/trunk/lit/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.cfg?rev=313407=313406=313407=diff
==
--- lldb/trunk/lit/lit.cfg (original)
+++ lldb/trunk/lit/lit.cfg Fri Sep 15 15:10:46 2017
@@ -29,94 +29,24 @@ config.suffixes = []
 config.test_source_root = os.path.dirname(__file__)
 
 # test_exec_root: The root path where tests should be run.
-lldb_obj_root = getattr(config, 'lldb_obj_root', None)
-if lldb_obj_root is not None:
-config.test_exec_root = os.path.join(lldb_obj_root, 'lit')
-
-# Set llvm_{src,obj}_root for use by others.
-config.llvm_src_root = getattr(config, 'llvm_src_root', None)
-config.llvm_obj_root = getattr(config, 'llvm_obj_root', None)
+config.test_exec_root = os.path.join(config.lldb_obj_root, 'lit')
 
 # Tweak the PATH to include the tools dir and the scripts dir.
-if lldb_obj_root is not None:
-lldb_tools_dir = getattr(config, 'lldb_tools_dir', None)
-if not lldb_tools_dir:
-lit_config.fatal('No LLDB tools dir set!')
-llvm_tools_dir = getattr(config, 'llvm_tools_dir', None)
-if not llvm_tools_dir:
-lit_config.fatal('No LLVM tools dir set!')
-path = os.path.pathsep.join((lldb_tools_dir, llvm_tools_dir, 
config.environment['PATH']))
-path = os.path.pathsep.join((os.path.join(getattr(config, 'llvm_src_root', 
None),'test','Scripts'),path))
-
-config.environment['PATH'] = path
-
-lldb_libs_dir = getattr(config, 'lldb_libs_dir', None)
-if not lldb_libs_dir:
-lit_config.fatal('No LLDB libs dir set!')
-llvm_libs_dir = getattr(config, 'llvm_libs_dir', None)
-if not llvm_libs_dir:
-lit_config.fatal('No LLVM libs dir set!')
-path = os.path.pathsep.join((lldb_libs_dir, llvm_libs_dir,
- config.environment.get('LD_LIBRARY_PATH','')))
-config.environment['LD_LIBRARY_PATH'] = path
-
-# Propagate LLVM_SRC_ROOT into the environment.
-config.environment['LLVM_SRC_ROOT'] = getattr(config, 'llvm_src_root', '')
-
-# Propagate PYTHON_EXECUTABLE into the environment
-config.environment['PYTHON_EXECUTABLE'] = getattr(config, 
'python_executable',
-  '')
-###
-
-# Check that the object root is known.
-if config.test_exec_root is None:
-# Otherwise, we haven't loaded the site specific configuration (the user is
-# probably trying to run on a test file directly, and either the site
-# configuration hasn't been

[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits 
vadimcn created this revision.
vadimcn added a project: LLDB.

OpenOCD sends register classes as two separate  nodes, fixed parser to 
process both of them.

OpenOCD returns "l" in response to "qfThreadInfo", so IsUnsupportedResponse() 
was false and we were ending up without any threads in the process.   I think 
it's reasonable to assume that there's always at least one thread.


Repository:
  rL LLVM

https://reviews.llvm.org/D37934

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


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4168,7 +4168,6 @@
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
-  XMLNode feature_node;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
@@ -4374,8 +4373,8 @@
 
 XMLNode target_node = xml_document.GetRootElement("target");
 if (target_node) {
-  XMLNode feature_node;
-  target_node.ForEachChildElement([_info, _node](
+  std::vector feature_nodes;
+  target_node.ForEachChildElement([_info, _nodes](
   const XMLNode ) -> bool {
 llvm::StringRef name = node.GetName();
 if (name == "architecture") {
@@ -4387,7 +4386,7 @@
   if (!href.empty())
 target_info.includes.push_back(href.str());
 } else if (name == "feature") {
-  feature_node = node;
+  feature_nodes.push_back(node);
 } else if (name == "groups") {
   node.ForEachChildElementWithName(
   "group", [_info](const XMLNode ) -> bool {
@@ -4423,7 +4422,7 @@
   // set the Target's architecture yet, so the ABI is also potentially
   // incorrect.
   ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
-  if (feature_node) {
+  for (auto feature_node : feature_nodes) {
 ParseRegisters(feature_node, target_info, this->m_register_info,
abi_to_use_sp, cur_reg_num, reg_offset);
   }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2624,8 +2624,7 @@
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
-IsConnected()) {
+if (thread_ids.size() == 0 && IsConnected()) {
   thread_ids.push_back(1);
 }
   } else {


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4168,7 +4168,6 @@
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
-  XMLNode feature_node;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
@@ -4374,8 +4373,8 @@
 
 XMLNode target_node = xml_document.GetRootElement("target");
 if (target_node) {
-  XMLNode feature_node;
-  target_node.ForEachChildElement([_info, _node](
+  std::vector feature_nodes;
+  target_node.ForEachChildElement([_info, _nodes](
   const XMLNode ) -> bool {
 llvm::StringRef name = node.GetName();
 if (name == "architecture") {
@@ -4387,7 +4386,7 @@
   if (!href.empty())
 target_info.includes.push_back(href.str());
 } else if (name == "feature") {
-  feature_node = node;
+  feature_nodes.push_back(node);
 } else if (name == "groups") {
   node.ForEachChildElementWithName(
   "group", [_info](const XMLNode ) -> bool {
@@ -4423,7 +4422,7 @@
   // set the Target's architecture yet, so the ABI is also potentially
   // incorrect.
   ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
-  if (feature_node) {
+  for (auto feature_node : feature_nodes) {
 ParseRegisters(feature_node, target_info, this->m_register_info,
abi_to_use_sp, cur_reg_num, reg_offset);
   }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2624,8 +2624,7 @@
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
-IsConnected()) {
+if (thread_ids.size() ==

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo added inline comments.



Comment at: source/Interpreter/CommandInterpreter.cpp:2713
+  for (; chunk_size < size; ++chunk_size) {
+assert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {

amccarth wrote:
> Should we be that trusting of a caller?  In a non-debug build, if a caller 
> doesn't end the (non-empty) string with '\n', this'll just run past the end 
> of the buffer.  Did I miss something that guarantees the caller won't make a 
> mistake?  Would it be too expensive to play it safe?
There's no assumption that the string ends with \n, see the loop condition: 
chunk_size < size. The assert is just to validate that we don't have embedded 
NULs.



Comment at: source/Interpreter/CommandInterpreter.cpp:2708-2710
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {

clayborg wrote:
> Since we are using "llvm::StringRef" here, why not use its split 
> functionality? Something like:
> ```
> bool done = false;
> while (!done) {
>   auto pair = str.split('\n');
>   auto len = pair.first.size();
>   done = pair.second.empty();
>   // Include newline if we are not done
>   if (!done)
> ++len; 
>   stream.Write(pair.first.data(), 
> }
> ```
> 
> This approach also avoids the issue amccarth mentioned below about not ending 
> with a newline. It is also quite a bit simpler to follow.
I'll give it a try, thanks for the suggestion.



Comment at: source/Interpreter/CommandInterpreter.cpp:2728
+  } else {
+stream.PutCString(str);
+  }

clayborg wrote:
> llvm::StringRef can contain NULLs right? Maybe use
> 
> ```
> stream.Write(data, size);
> ```
The original code (line 2714) was using PutCString(), so this path is just 
preserving the original functionality (which also suggests that the output is 
not expected to have embedded nuls)


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added inline comments.



Comment at: source/Commands/CommandObjectTarget.cpp:2056-2058
+if (m_interpreter.WasInterrupted()) {
+  break;
+}

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2087-2089
+if (m_interpreter.WasInterrupted()) {
+  break;
+}

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2155-2157
+if (m_interpreter.WasInterrupted()) {
+  break;
+}

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2179-2181
+  if (m_interpreter.WasInterrupted()) {
+break;
+  }

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2254-2256
+if (m_interpreter.WasInterrupted()) {
+  break;
+}

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2278-2280
+  if (m_interpreter.WasInterrupted()) {
+break;
+  }

Remove braces



Comment at: source/Commands/CommandObjectTarget.cpp:2348-2350
+if (m_interpreter.WasInterrupted()) {
+  break;
+}

Remove braces



Comment at: source/Interpreter/CommandInterpreter.cpp:2682
+  auto prev_state = 
m_command_state.exchange(CommandHandlingState::eInProgress);
+  assert(prev_state == CommandHandlingState::eIdle);
+}

lldb_assert



Comment at: source/Interpreter/CommandInterpreter.cpp:2687
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  assert(prev_state != CommandHandlingState::eIdle);
+}

lldb_assert



Comment at: source/Interpreter/CommandInterpreter.cpp:2708-2710
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {

Since we are using "llvm::StringRef" here, why not use its split functionality? 
Something like:
```
bool done = false;
while (!done) {
  auto pair = str.split('\n');
  auto len = pair.first.size();
  done = pair.second.empty();
  // Include newline if we are not done
  if (!done)
++len; 
  stream.Write(pair.first.data(), 
}
```

This approach also avoids the issue amccarth mentioned below about not ending 
with a newline. It is also quite a bit simpler to follow.



Comment at: source/Interpreter/CommandInterpreter.cpp:2728
+  } else {
+stream.PutCString(str);
+  }

llvm::StringRef can contain NULLs right? Maybe use

```
stream.Write(data, size);
```


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo added inline comments.



Comment at: source/Commands/CommandObjectTarget.cpp:2058
+  break;
+}
 num_dumped++;

amccarth wrote:
> Many LLVM developers prefer to omit the braces when the body of the 
> control-flow statement is a single statement.
So do the hackers: https://blog.codecentric.de/en/2014/02/curly-braces :) I too 
prefer to omit braces in small test snippets, but in production code it's not 
worth the risk of making a silly mistake.



Comment at: source/Interpreter/CommandInterpreter.cpp:2720
+  chunk_size = stream.Write(data, chunk_size);
+  assert(size >= chunk_size);
+  data += chunk_size;

amccarth wrote:
> This assert should precede the line before it.
Pedantically, it should be both before and after (and for ultimate paranoid 
mode, asserting that Write returns <= than the passed in value)

But the asserts looks for the really nasty case where "size -= chunk_size" 
overflows.



Comment at: tools/driver/Driver.cpp:1189
 
-  exit(signo);
+  _exit(signo);
 }

amccarth wrote:
> Can you add a comment explaining why this uses `_exit` rather than `exit`?  
> It's not obvious to me.
Explained in the SIGINT patch: exit() is not signal-safe 
(http://pubs.opengroup.org/onlinepubs/95399/functions/exit.html)

Calling it from a signal handler can result in all kind of nasty issues, in 
particular exit() does call a lot of stuff, both runtime and user code (ex. 
atexit functions) 




Comment at: tools/lldb-mi/MIDriverMain.cpp:71
 //--
 void sigint_handler(int vSigno) {
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows

amccarth wrote:
> I think this concurrency fix for SIGINT would be better in a separate patch.  
> I understand how it's related to the rest of this patch, but LLVM folks tend 
> to prefer small, incremental patches.
Agreed, I already split this change into separate patches (I wasn't sure if 
people prefer to review two small changes vs a single one with more context)


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

2017-09-15 Thread Francis Ricci via Phabricator via lldb-commits 
fjricci created this revision.

This allows for the stack size to be configured, which isn't
possible with std::thread. Prevents overflowing the stack when
performing complex operations in the task pool on darwin,
where the default pthread stack size is only 512kb.


https://reviews.llvm.org/D37930

Files:
  source/Utility/TaskPool.cpp


Index: source/Utility/TaskPool.cpp
===
--- source/Utility/TaskPool.cpp
+++ source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -54,10 +57,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);


Index: source/Utility/TaskPool.cpp
===
--- source/Utility/TaskPool.cpp
+++ source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -54,10 +57,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37926: Fix the SIGINT handlers

2017-09-15 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Looks fine to me.


https://reviews.llvm.org/D37926



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Adrian McCarthy via Phabricator via lldb-commits 
amccarth added inline comments.



Comment at: source/Commands/CommandObjectTarget.cpp:2058
+  break;
+}
 num_dumped++;

Many LLVM developers prefer to omit the braces when the body of the 
control-flow statement is a single statement.



Comment at: source/Interpreter/CommandInterpreter.cpp:2713
+  for (; chunk_size < size; ++chunk_size) {
+assert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {

Should we be that trusting of a caller?  In a non-debug build, if a caller 
doesn't end the (non-empty) string with '\n', this'll just run past the end of 
the buffer.  Did I miss something that guarantees the caller won't make a 
mistake?  Would it be too expensive to play it safe?



Comment at: source/Interpreter/CommandInterpreter.cpp:2720
+  chunk_size = stream.Write(data, chunk_size);
+  assert(size >= chunk_size);
+  data += chunk_size;

This assert should precede the line before it.



Comment at: tools/driver/Driver.cpp:1182
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();

I'm not sure why these two ifs aren't one, as in:

```
if (g_driver && !g_interrupt_sent.test_and_set())
```



Comment at: tools/driver/Driver.cpp:1189
 
-  exit(signo);
+  _exit(signo);
 }

Can you add a comment explaining why this uses `_exit` rather than `exit`?  
It's not obvious to me.



Comment at: tools/lldb-mi/MIDriverMain.cpp:71
 //--
 void sigint_handler(int vSigno) {
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows

I think this concurrency fix for SIGINT would be better in a separate patch.  I 
understand how it's related to the rest of this patch, but LLVM folks tend to 
prefer small, incremental patches.


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37926: Fix the SIGINT handlers

2017-09-15 Thread Zachary Turner via Phabricator via lldb-commits 
zturner added a comment.

It turns out the function this called, `DispatchInputInterrupt`, already 
acquires a mutex.  Maybe put the synchronization in there?  Then you don't have 
to reproduce the synchronization in both MI and LLDB


https://reviews.llvm.org/D37926



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo updated this revision to Diff 115476.
lemo added a comment.

Split the SIGINT handles fixes into a stanalone patch


https://reviews.llvm.org/D37923

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Commands/CommandObjectTarget.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -2677,6 +2677,58 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  assert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  assert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+  in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream , llvm::StringRef str,
+bool interruptible) {
+  if (str.empty()) {
+return;
+  }
+
+  if (interruptible) {
+// Split the output into lines and poll for interrupt requests
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {
+  size_t chunk_size = 0;
+  for (; chunk_size < size; ++chunk_size) {
+assert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {
+  ++chunk_size;
+  break;
+}
+  }
+  chunk_size = stream.Write(data, chunk_size);
+  assert(size >= chunk_size);
+  data += chunk_size;
+  size -= chunk_size;
+}
+if (size > 0) {
+  stream.Printf("\n... Interrupted.\n");
+}
+  } else {
+stream.PutCString(str);
+  }
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler _handler,
 std::string ) {
   const bool is_interactive = io_handler.GetIsInteractive();
@@ -2700,6 +2752,8 @@
line.c_str());
   }
 
+  StartHandlingCommand();
+
   lldb_private::CommandReturnObject result;
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
@@ -2710,18 +2764,20 @@
 
 if (!result.GetImmediateOutputStream()) {
   llvm::StringRef output = result.GetOutputData();
-  if (!output.empty())
-io_handler.GetOutputStreamFile()->PutCString(output);
+  PrintCommandOutput(*io_handler.GetOutputStreamFile(), output,
+ is_interactive);
 }
 
 // Now emit the command error text from the command we just executed
 if (!result.GetImmediateErrorStream()) {
   llvm::StringRef error = result.GetErrorData();
-  if (!error.empty())
-io_handler.GetErrorStreamFile()->PutCString(error);
+  PrintCommandOutput(*io_handler.GetErrorStreamFile(), error,
+ is_interactive);
 }
   }
 
+  FinishHandlingCommand();
+
   switch (result.GetStatus()) {
   case eReturnStatusInvalid:
   case eReturnStatusSuccessFinishNoResult:
@@ -2777,6 +2833,10 @@
   ExecutionContext exe_ctx(GetExecutionContext());
   Process *process = exe_ctx.GetProcessPtr();
 
+  if (InterruptCommand()) {
+return true;
+  }
+
   if (process) {
 StateType state = process->GetState();
 if (StateIsRunningState(state)) {
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -2053,6 +2053,9 @@
   result.GetOutputStream().EOL();
   result.GetOutputStream().EOL();
 }
+if (m_interpreter.WasInterrupted()) {
+  break;
+}
 num_dumped++;
 DumpModuleSymtab(
 m_interpreter, result.GetOutputStream(),
@@ -2081,6 +2084,9 @@
   result.GetOutputStream().EOL();
   result.GetOutputStream().EOL();
 }
+if (m_interpreter.WasInterrupted()) {
+  break;
+}
 num_dumped++;
 DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
  module, m_options.m_sort_order);
@@ -2146,6 +2152,9 @@
   " modules.\n",
   (uint64_t)num_modules);
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
+if (m_interpreter.WasInterrupted()) {
+

[Lldb-commits] [PATCH] D37926: Fix the SIGINT handlers

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo created this revision.
Herald added a subscriber: ki.stfu.

1. Fix a data race (g_interrupt_sent flag usage was not thread safe, signals 
can be handled on arbitrary threads)
2. exit() is not signal-safe, replaced it with the signal-safe equivalent 
_exit()


https://reviews.llvm.org/D37926

Files:
  driver/Driver.cpp
  lldb-mi/MIDriverMain.cpp


Index: lldb-mi/MIDriverMain.cpp
===
--- lldb-mi/MIDriverMain.cpp
+++ lldb-mi/MIDriverMain.cpp
@@ -72,14 +72,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr  = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   pDebugger->DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
 }
   }
 
Index: driver/Driver.cpp
===
--- driver/Driver.cpp
+++ driver/Driver.cpp
@@ -1177,17 +1177,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
   return;
 }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {


Index: lldb-mi/MIDriverMain.cpp
===
--- lldb-mi/MIDriverMain.cpp
+++ lldb-mi/MIDriverMain.cpp
@@ -72,14 +72,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr  = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   pDebugger->DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
 }
   }
 
Index: driver/Driver.cpp
===
--- driver/Driver.cpp
+++ driver/Driver.cpp
@@ -1177,17 +1177,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
   return;
 }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Adrian McCarthy via Phabricator via lldb-commits 
amccarth added a comment.

I haven't looked at the whole patch yet, but it seems the SIGINT fix is well 
isolated.  That should probably be in a separate patch.


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-15 Thread Leonard Mosescu via Phabricator via lldb-commits 
lemo created this revision.
Herald added a subscriber: ki.stfu.

The core of this change is the new CommandInterpreter::m_command_state, which 
models the state transitions for interactive commands, including an 
"interrupted" state transition.

In general, command interruption requires cooperation from the code executing 
the command, which needs to poll for interruption requests through 
CommandInterpreter::WasInterrupted().

CommandInterpreter::PrintCommandOutput() implements an optionally interruptible 
printing of the command output, which for large outputs was likely the longest 
blocking part

  (ex. target modules dump symtab on a complex binary could take 10+ minutes)

Also included are a few fixes in the handlers for SIGINT (thread safety and 
using the signal-safe _exit() instead of exit())


https://reviews.llvm.org/D37923

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Commands/CommandObjectTarget.cpp
  source/Interpreter/CommandInterpreter.cpp
  tools/driver/Driver.cpp
  tools/lldb-mi/MIDriverMain.cpp

Index: tools/lldb-mi/MIDriverMain.cpp
===
--- tools/lldb-mi/MIDriverMain.cpp
+++ tools/lldb-mi/MIDriverMain.cpp
@@ -72,14 +72,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr  = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   pDebugger->DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
 }
   }
 
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -1177,17 +1177,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
   return;
 }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {
Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -2677,6 +2677,58 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  assert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  assert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+  in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream , llvm::StringRef str,
+bool interruptible) {
+  if (str.empty()) {
+return;
+  }
+
+  if (interruptible) {
+// Split the output into lines and poll for interrupt requests
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {
+  size_t chunk_size = 0;
+  for (; chunk_size < size; ++chunk_size) {
+assert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {
+  ++chunk_size;
+  break;
+}
+  }
+  chunk_size = stream.Write(data, chunk_size);
+  assert(size >= chunk_size);
+  data += chunk_size;
+  size -= chunk_size;
+}
+if (size > 0) {
+  stream.Printf("\n... Interrupted.\n");
+}
+  } else {
+stream.PutCString(str);
+  }
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler _handler,
 std::string ) {
   const bool is_interactive = io_handler.GetIsInteractive();
@@ -2700,6 +2752,8 @@
line.c_str());
   }
 
+  StartHandlingCommand();
+
   lldb_private::CommandReturnObject result;
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
@@ -2710,18 +2764,20 @@
 
 if (!result.GetImmediateOutputStream()) {
   llvm::StringRef output = result.GetOutputData();
-  if (!output.empty())
-

[Lldb-commits] [lldb] r313371 - Remove a couple of warnings pointed out by Ted Woodward.

2017-09-15 Thread Jim Ingham via lldb-commits 
Author: jingham
Date: Fri Sep 15 10:54:37 2017
New Revision: 313371

URL: http://llvm.org/viewvc/llvm-project?rev=313371=rev
Log:
Remove a couple of warnings pointed out by Ted Woodward.

Modified:
lldb/trunk/source/API/SBBreakpointName.cpp

Modified: lldb/trunk/source/API/SBBreakpointName.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBBreakpointName.cpp?rev=313371=313370=313371=diff
==
--- lldb/trunk/source/API/SBBreakpointName.cpp (original)
+++ lldb/trunk/source/API/SBBreakpointName.cpp Fri Sep 15 10:54:37 2017
@@ -77,15 +77,15 @@ public:
   // For now we take a simple approach and only keep the name, and relook
   // up the location when we need it.
   
-  TargetSP GetTarget() {
+  TargetSP GetTarget() const {
 return m_target_wp.lock();
   }
   
-  const char *GetName() {
+  const char *GetName() const {
 return m_name.c_str();
   }
   
-  bool IsValid() {
+  bool IsValid() const {
 return !m_name.empty() && m_target_wp.lock();
   }
   
@@ -102,7 +102,13 @@ public:
   
   const lldb_private::BreakpointName *GetBreakpointName() const
   {
-return GetBreakpointName();
+if (!IsValid())
+  return nullptr;
+TargetSP target_sp = GetTarget();
+if (!target_sp)
+  return nullptr;
+Status error;
+return target_sp->FindBreakpointName(ConstString(m_name), true, error);
   }
   
 private:
@@ -339,7 +345,7 @@ bool SBBreakpointName::GetAutoContinue()
   
   BreakpointName *bp_name = GetBreakpointName();
   if (!bp_name)
-return nullptr;
+return false;
  
   LLDB_LOG(log, "Name: {0}\n", bp_name->GetName());
   std::lock_guard guard(


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