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

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good




Comment at: source/Utility/TaskPool.cpp:61
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();

(nit): Can you create a named constant for the min stack size as just based on 
this line I would have no idea what does that number do?


https://reviews.llvm.org/D37930



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

[Lldb-commits] [lldb] r313525 - Fix Linux remote debugging after r313442

[Tamas Berghammer via lldb-commits]

Author: tberghammer
Date: Mon Sep 18 03:24:48 2017
New Revision: 313525

URL: http://llvm.org/viewvc/llvm-project?rev=313525&view=rev
Log:
Fix Linux remote debugging after r313442

On Linux lldb-server sends an OK response to qfThreadInfo if no process
is started yet. I don't know why would LLDB issue a qfThreadInfo packet
before starting a process but creating a fake thread ID in case of an
OK or Error respoinse sounds bad anyway so lets not do it.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.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=313525&r1=313524&r2=313525&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Mon Sep 18 03:24:48 2017
@@ -2624,7 +2624,8 @@ size_t GDBRemoteCommunicationClient::Get
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (thread_ids.size() == 0 && IsConnected()) {
+if ((response.IsUnsupportedResponse() || response.IsNormalResponse()) &&
+thread_ids.size() == 0 && IsConnected()) {
   thread_ids.push_back(1);
 }
   } else {


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

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

[Tamas Berghammer via lldb-commits]

Hi Vadim,

This change broke remote debugging on Linux and Android as for some reason
LLDB sends a qfThreadInfo on those platforms before starting a process (not
sure why, will investigate when I have a bit more time) and lldb-server
sends an OK response to it. After your change it will generate a valid
thread ID what will cause LLDB to get confused and fail to lunch a process.
I submitted a fix as r313525 what should work both for OpenOCD and for
Linux/Android but can you verify the OpenOCD part?

Thanks,
Tamas

On Sat, Sep 16, 2017 at 4:54 AM Vadim Chugunov via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: vadimcn
> Date: Fri Sep 15 20:53:13 2017
> New Revision: 313442
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313442&view=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&r1=313441&r2=313442&view=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&r1=313441&r2=313442&view=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
> &target_info,
> @@ -4374,8 +4373,8 @@ bool ProcessGDBRemote::GetGDBServerRegis
>
>  XMLNode target_node = xml_document.GetRootElement("target");
>  if (target_node) {
> -  XMLNode feature_node;
> -  target_node.ForEachChildElement([&target_info, &feature_node](
> +  std::vector feature_nodes;
> +  target_node.ForEachChildElement([&target_info, &feature_nodes](
>const XMLNode &node) -> 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", [&target_info](const XMLNode &node) -> 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 &feature_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 mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r313537 - Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via lldb-commits]

Author: fjricci
Date: Mon Sep 18 08:18:48 2017
New Revision: 313537

URL: http://llvm.org/viewvc/llvm-project?rev=313537&view=rev
Log:
Use ThreadLauncher to launch TaskPool threads

Summary:
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.

Reviewers: labath, tberghammer, clayborg

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Utility/TaskPool.cpp

Modified: lldb/trunk/source/Utility/TaskPool.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/TaskPool.cpp?rev=313537&r1=313536&r2=313537&view=diff
==
--- lldb/trunk/source/Utility/TaskPool.cpp (original)
+++ lldb/trunk/source/Utility/TaskPool.cpp Mon Sep 18 08:18:48 2017
@@ -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 @@ public:
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -45,6 +48,7 @@ TaskPoolImpl::TaskPoolImpl() : m_thread_
 
 void TaskPoolImpl::AddTask(std::function &&task_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,17 @@ void TaskPoolImpl::AddTask(std::function
 // 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, min_stack_size)
+.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] D33213: Use socketpair on all Unix platforms

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

This should be good to go. Watch the buildbots for failures.


https://reviews.llvm.org/D33213



___
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

[Francis Ricci via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313537: Use ThreadLauncher to launch TaskPool threads 
(authored by fjricci).

Changed prior to commit:
  https://reviews.llvm.org/D37930?vs=115484&id=115658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37930

Files:
  lldb/trunk/source/Utility/TaskPool.cpp


Index: lldb/trunk/source/Utility/TaskPool.cpp
===
--- lldb/trunk/source/Utility/TaskPool.cpp
+++ lldb/trunk/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;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &&task_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,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, min_stack_size)
+.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: lldb/trunk/source/Utility/TaskPool.cpp
===
--- lldb/trunk/source/Utility/TaskPool.cpp
+++ lldb/trunk/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;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &&task_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,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, min_stack_size)
+.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] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The default return value of WillResume should be no error. Sorry for not 
catching this. The core file Process subclasses will need to override this 
manually.


Repository:
  rL LLVM

https://reviews.llvm.org/D37651



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

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

[Greg Clayton via lldb-commits]

That is the reason I asked for the patch to verify "l" was the only thing 
received before it goes and makes up a fake thread ID...

> On Sep 18, 2017, at 4:44 AM, Tamas Berghammer via lldb-commits 
>  wrote:
> 
> Hi Vadim,
> 
> This change broke remote debugging on Linux and Android as for some reason 
> LLDB sends a qfThreadInfo on those platforms before starting a process (not 
> sure why, will investigate when I have a bit more time) and lldb-server sends 
> an OK response to it. After your change it will generate a valid thread ID 
> what will cause LLDB to get confused and fail to lunch a process. I submitted 
> a fix as r313525 what should work both for OpenOCD and for Linux/Android but 
> can you verify the OpenOCD part?
> 
> Thanks,
> Tamas
> 
> On Sat, Sep 16, 2017 at 4:54 AM Vadim Chugunov via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> Author: vadimcn
> Date: Fri Sep 15 20:53:13 2017
> New Revision: 313442
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=313442&view=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&r1=313441&r2=313442&view=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&r1=313441&r2=313442&view=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 &target_info,
> @@ -4374,8 +4373,8 @@ bool ProcessGDBRemote::GetGDBServerRegis
> 
>  XMLNode target_node = xml_document.GetRootElement("target");
>  if (target_node) {
> -  XMLNode feature_node;
> -  target_node.ForEachChildElement([&target_info, &feature_node](
> +  std::vector feature_nodes;
> +  target_node.ForEachChildElement([&target_info, &feature_nodes](
>const XMLNode &node) -> 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", [&target_info](const XMLNode &node) -> 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 &feature_node : feature_nodes) {
>  ParseRegisters(feature_node, target_info, this->m_register_info,
>

[Lldb-commits] [lldb] r313539 - Revert "Use ThreadLauncher to launch TaskPool threads"

[Francis Ricci via lldb-commits]

Author: fjricci
Date: Mon Sep 18 08:43:59 2017
New Revision: 313539

URL: http://llvm.org/viewvc/llvm-project?rev=313539&view=rev
Log:
Revert "Use ThreadLauncher to launch TaskPool threads"

This reverts commit r313537 because it fails to link on linux buildbots

Modified:
lldb/trunk/source/Utility/TaskPool.cpp

Modified: lldb/trunk/source/Utility/TaskPool.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/TaskPool.cpp?rev=313539&r1=313538&r2=313539&view=diff
==
--- lldb/trunk/source/Utility/TaskPool.cpp (original)
+++ lldb/trunk/source/Utility/TaskPool.cpp Mon Sep 18 08:43:59 2017
@@ -8,7 +8,6 @@
 
//===--===//
 
 #include "lldb/Utility/TaskPool.h"
-#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -24,8 +23,6 @@ public:
 private:
   TaskPoolImpl();
 
-  static lldb::thread_result_t WorkerPtr(void *pool);
-
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -48,7 +45,6 @@ TaskPoolImpl::TaskPoolImpl() : m_thread_
 
 void TaskPoolImpl::AddTask(std::function &&task_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
-  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -58,17 +54,10 @@ void TaskPoolImpl::AddTask(std::function
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
-   this, nullptr, min_stack_size)
-.Release();
+std::thread(Worker, this).detach();
   }
 }
 
-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] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

fjricci reopened this revision.
fjricci added a comment.
This revision is now accepted and ready to land.

Is ThreadLauncher unavailable in this code for some reason? The link failed on 
linux buildbots (building lldb on Darwin was fine locally): 
http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13311/steps/run%20unit%20tests/logs/stdio


Repository:
  rL LLVM

https://reviews.llvm.org/D37930



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

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

[Tamas Berghammer via lldb-commits]

Are you building with cmake or with XCode? If you are using cmake then you
should be able to reproduce it with running "ninja check-lldb-unit". It
failed to link one of the unittest targets because UtilityTests don't have
an explicit dependency on lldbHost (where ThreadLauncher is defined). You
can fix it by the following patch: https://reviews.llvm.org/P8037

Tamas

On Mon, Sep 18, 2017 at 4:47 PM Francis Ricci via Phabricator <
revi...@reviews.llvm.org> wrote:

> fjricci reopened this revision.
> fjricci added a comment.
> This revision is now accepted and ready to land.
>
> Is ThreadLauncher unavailable in this code for some reason? The link
> failed on linux buildbots (building lldb on Darwin was fine locally):
> http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13311/steps/run%20unit%20tests/logs/stdio
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D37930
>
>
>
>
___
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.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

If we get a response of "l" for the "qfThreadInfo", it might be worth trying to 
send a "https://reviews.llvm.org/H1"; packet (select thread ID 1 for subsequent 
continues and steps) to see if the server knows about thread 1 before 
proceeding? This patch also broke other platforms that respond with "OK" to the 
"qfThreadInfo". So we really need to do some extra verification that "l" was 
the only things received in response to the "qfThreadInfo".


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] [lldb] r313540 - Revert "Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)"

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Sep 18 08:59:44 2017
New Revision: 313540

URL: http://llvm.org/viewvc/llvm-project?rev=313540&view=rev
Log:
Revert "Fix for bug 34532 - A few rough corners related to post-mortem 
debugging (core/minidump)"

Broke Windows and FreeBSD (at least).

This reverts commit 628ca7052b4a5dbace0f6205409113e12c8a78fa.

Modified:
lldb/trunk/include/lldb/Target/Process.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=313540&r1=313539&r2=313540&view=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Sep 18 08:59:44 2017
@@ -1248,13 +1248,7 @@ public:
   /// @return
   /// Returns an error object.
   //--
-  virtual Status WillResume() {
-Status error;
-error.SetErrorStringWithFormat(
-"error: %s does not support resuming processes",
-GetPluginName().GetCString());
-return error;
-  }
+  virtual Status WillResume() { return Status(); }
 
   //--
   /// Resumes all of a process's threads as configured using the

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py?rev=313540&r1=313539&r2=313540&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 Mon Sep 18 08:59:44 2017
@@ -244,34 +244,6 @@ class LinuxCoreTestCase(TestBase):
 end_region.GetRegionBase())
 self.assertEqual(end_region.GetRegionEnd(), lldb.LLDB_INVALID_ADDRESS)
 
-def check_state(self, process):
-with open(os.devnull) as devnul:
-# sanitize test output
-self.dbg.SetOutputFileHandle(devnul, False)
-self.dbg.SetErrorFileHandle(devnul, False)
-
-self.assertTrue(process.is_stopped)
-
-# Process.Continue
-error = process.Continue()
-self.assertFalse(error.Success())
-self.assertTrue(process.is_stopped)
-
-# Thread.StepOut
-thread = process.GetSelectedThread()
-thread.StepOut()
-self.assertTrue(process.is_stopped)
-
-# command line
-self.dbg.HandleCommand('s')
-self.assertTrue(process.is_stopped)
-self.dbg.HandleCommand('c')
-self.assertTrue(process.is_stopped)
-
-# restore file handles
-self.dbg.SetOutputFileHandle(None, False)
-self.dbg.SetErrorFileHandle(None, False)
-
 def do_test(self, filename, pid, region_count):
 target = self.dbg.CreateTarget(filename + ".out")
 process = target.LoadCore(filename + ".core")
@@ -279,8 +251,6 @@ class LinuxCoreTestCase(TestBase):
 self.assertEqual(process.GetNumThreads(), 1)
 self.assertEqual(process.GetProcessID(), pid)
 
-self.check_state(process)
-
 thread = process.GetSelectedThread()
 self.assertTrue(thread)
 self.assertEqual(thread.GetThreadID(), pid)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=313540&r1=313539&r2=313540&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Mon Sep 18 08:59:44 2017
@@ -31,34 +31,6 @@ class MiniDumpNewTestCase(TestBase):
 lldb.DBG.SetSelectedPlatform(self._initial_platform)
 super(MiniDumpNewTestCase, self).tearDown()
 
-def check_state(self):
-with open(os.devnull) as devnul:
-# sanitize test output
-self.dbg.SetOutputFileHandle(devnul, False)
-self.dbg.SetErrorFileHandle(devnul, False)
-
-self.assertTrue(s

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

[Stephane Sezer via lldb-commits]

What difference does this make?

On Fri, Sep 15, 2017 at 8:00 PM Eugene Zemtsov via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: eugene
> Date: Fri Sep 15 19:58:49 2017
> New Revision: 313437
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313437&view=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&r1=313436&r2=313437&view=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
>
-- 
-- 
Stephane Sezer
___
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.

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

This obvious solution works well for me and seems safe.

  auto isEmptyList = response.IsNormalResponse() && response.GetStringRef() == 
"l";
  
  if ((response.IsUnsupportedResponse() || isEmptyList) ...


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.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I like the solution detailed above by tatyana-krasnukha


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] D33213: Use socketpair on all Unix platforms

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Pavel & Eugene haven't marked it accepted yet, but from the comments is looks 
like they are both okay with the change.  So it looks to me like everything is 
done but checking the code in.

If you have checkin privileges, then just check it in and as Greg says keep an 
eye on the bots.  If you don't then speak up and one of us will check it in for 
you.


https://reviews.llvm.org/D33213



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

[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms

[Eugene Zemtsov via Phabricator via lldb-commits]

eugene accepted this revision.
eugene added a comment.

I did mark it Accepted.


https://reviews.llvm.org/D33213



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

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

[Eugene Zemtsov via lldb-commits]

In GNU C Library, accept4 is guarded by __USE_GNU macro. Whether this macro
is defined or not can depend on the compiler and its arguments for C and
C++.
LLDB uses accept4 in C++ code thus it makes sense to test availability of
this function in C++ mode.

On Mon, Sep 18, 2017 at 9:58 AM, Stephane Sezer  wrote:

> What difference does this make?
>
> On Fri, Sep 15, 2017 at 8:00 PM Eugene Zemtsov via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> Author: eugene
>> Date: Fri Sep 15 19:58:49 2017
>> New Revision: 313437
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313437&view=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&r1=313436&r2=313437&view=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
>>
> --
> --
> Stephane Sezer
>



-- 
Thanks,
Eugene Zemtsov.
___
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.

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

Sorry, for not catching this regression!   I've checked that attaching to a 
standard gdbserver still worked, but was not aware of the other scenarios.

Does lldb-server return 'OK' string in response to 'qfThreadInfo'?   According 
to this 
,
 'OK' is not a valid response to qfThreadInfo. (Is there a better reference for 
gdb-remote protocol?)

Also, Tamas Berghammer has already submitted a fix 
, does is 
still not resolve this issue?


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] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

fjricci updated this revision to Diff 115691.
fjricci added a comment.
Herald added subscribers: JDevlieghere, mgorny.

Move TaskPool from Utility to Host


https://reviews.llvm.org/D37930

Files:
  include/lldb/Host/TaskPool.h
  include/lldb/Utility/TaskPool.h
  lldb.xcodeproj/project.pbxproj
  source/Host/CMakeLists.txt
  source/Host/common/TaskPool.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Utility/CMakeLists.txt
  source/Utility/TaskPool.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/TaskPoolTest.cpp
  unittests/Utility/CMakeLists.txt
  unittests/Utility/TaskPoolTest.cpp

Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -8,7 +8,6 @@
   StatusTest.cpp
   StringExtractorTest.cpp
   StructuredDataTest.cpp
-  TaskPoolTest.cpp
   TildeExpressionResolverTest.cpp
   TimeoutTest.cpp
   TimerTest.cpp
Index: unittests/Host/TaskPoolTest.cpp
===
--- unittests/Host/TaskPoolTest.cpp
+++ unittests/Host/TaskPoolTest.cpp
@@ -1,6 +1,6 @@
 #include "gtest/gtest.h"
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
 
 TEST(TaskPoolTest, AddTask) {
   auto fn = [](int x) { return x * x + 1; };
Index: unittests/Host/CMakeLists.txt
===
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -6,6 +6,7 @@
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
+  TaskPoolTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: source/Utility/CMakeLists.txt
===
--- source/Utility/CMakeLists.txt
+++ source/Utility/CMakeLists.txt
@@ -29,7 +29,6 @@
   StringLexer.cpp
   StringList.cpp
   StructuredData.cpp
-  TaskPool.cpp
   TildeExpressionResolver.cpp
   Timer.cpp
   UserID.cpp
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -53,7 +53,7 @@
 
 #include "lldb/Target/Language.h"
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
 
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
Index: source/Host/common/TaskPool.cpp
===
--- source/Host/common/TaskPool.cpp
+++ source/Host/common/TaskPool.cpp
@@ -7,7 +7,8 @@
 //
 //===--===//
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/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;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &&task_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,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, min_stack_size)
+.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/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -31,6 +31,7 @@
   common/SoftwareBreakpoint.cpp
   common/StringConvert.cpp
   common/Symbols.cpp
+  common/TaskPool.cpp
   common/TCPSocket.cpp
   common/Terminal.cpp
   common/ThreadLauncher.cpp
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -2664,8 +2664,8 @@
 		6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ArmUnwindInfo.cpp; path = source/Symbol/ArmUnwindInfo.cpp; sourceTree = ""; };
 		6D9AB3DC1BB2B74E003F2289 /* TypeMap.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = TypeMap.cpp; path = source/Symbol/TypeMap.cpp; sourceTree = ""; };
 		6D9AB3DE1BB2B76B003F2289 /* TypeMap.h 

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

[Vadim Chugunov via lldb-commits]

Yes, this works for OpenOCD as well.  Thanks!

On Mon, Sep 18, 2017 at 4:44 AM, Tamas Berghammer 
wrote:

> Hi Vadim,
>
> This change broke remote debugging on Linux and Android as for some reason
> LLDB sends a qfThreadInfo on those platforms before starting a process (not
> sure why, will investigate when I have a bit more time) and lldb-server
> sends an OK response to it. After your change it will generate a valid
> thread ID what will cause LLDB to get confused and fail to lunch a process.
> I submitted a fix as r313525 what should work both for OpenOCD and for
> Linux/Android but can you verify the OpenOCD part?
>
> Thanks,
> Tamas
>
> On Sat, Sep 16, 2017 at 4:54 AM Vadim Chugunov via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> Author: vadimcn
>> Date: Fri Sep 15 20:53:13 2017
>> New Revision: 313442
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313442&view=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&r1=313441&r2=313442&view=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&r1=313441&r2=313442&view=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
>> &target_info,
>> @@ -4374,8 +4373,8 @@ bool ProcessGDBRemote::GetGDBServerRegis
>>
>>  XMLNode target_node = xml_document.GetRootElement("target");
>>  if (target_node) {
>> -  XMLNode feature_node;
>> -  target_node.ForEachChildElement([&target_info, &feature_node](
>> +  std::vector feature_nodes;
>> +  target_node.ForEachChildElement([&target_info, &feature_nodes](
>>const XMLNode &node) -> 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", [&target_info](const XMLNode &node) -> 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 &feature_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 mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

Maybe LLDB should use `qAttached` to determine if there's an active process?  
OpenOCD seems to implement 

 that one correctly.


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

[Leonard Mosescu via Phabricator via lldb-commits]

lemo added inline comments.



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

lemo wrote:
> 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.
I forgot to reply to this: I tried using str.split() as suggested, with a small 
tweak to avoid reading past the end of the first slice (++len to include the 
'\n'):

while (!str.empty() && !WasInterrupted()) {
  auto pair = str.split('\n');
  stream.Write(pair.first.data(), pair.first.size());
  if (str.size() != pair.first.size())
stream.PutChar('\n');
  str = pair.second;
}
if (!str.empty()) {
  stream.Printf("\n... Interrupted.\n");
}

While this version is shorter there I see a number of small problems with it:
1. It requires a close look at the StringRef::split() interface (ex. done = 
pair.second.empty() is actually not the correct check since it will eat the 
final \n)
2. While conceptually straightforward, the StringRef::split() interface is not 
ideal in this case - see #1
3. This version assumes that stream.Write() actually writes everything (ie. not 
handling the return size)
4. In order to avoid reading past the end of the first slice we need to do a 
separate PutChar() for each \n

So in the end I prefer the original version, which although a bit more verbose, 
tracks the invariants clearly, is potentially faster and handles strings 
regardless if they are terminated with \n or not.


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

[Leonard Mosescu via Phabricator via lldb-commits]

lemo added inline comments.



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

lemo wrote:
> lemo wrote:
> > 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.
> I forgot to reply to this: I tried using str.split() as suggested, with a 
> small tweak to avoid reading past the end of the first slice (++len to 
> include the '\n'):
> 
> while (!str.empty() && !WasInterrupted()) {
>   auto pair = str.split('\n');
>   stream.Write(pair.first.data(), pair.first.size());
>   if (str.size() != pair.first.size())
> stream.PutChar('\n');
>   str = pair.second;
> }
> if (!str.empty()) {
>   stream.Printf("\n... Interrupted.\n");
> }
> 
> While this version is shorter there I see a number of small problems with it:
> 1. It requires a close look at the StringRef::split() interface (ex. done = 
> pair.second.empty() is actually not the correct check since it will eat the 
> final \n)
> 2. While conceptually straightforward, the StringRef::split() interface is 
> not ideal in this case - see #1
> 3. This version assumes that stream.Write() actually writes everything (ie. 
> not handling the return size)
> 4. In order to avoid reading past the end of the first slice we need to do a 
> separate PutChar() for each \n
> 
> So in the end I prefer the original version, which although a bit more 
> verbose, tracks the invariants clearly, is potentially faster and handles 
> strings regardless if they are terminated with \n or not.
Thoughts?


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

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



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

lemo wrote:
> lemo wrote:
> > lemo wrote:
> > > 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.
> > I forgot to reply to this: I tried using str.split() as suggested, with a 
> > small tweak to avoid reading past the end of the first slice (++len to 
> > include the '\n'):
> > 
> > while (!str.empty() && !WasInterrupted()) {
> >   auto pair = str.split('\n');
> >   stream.Write(pair.first.data(), pair.first.size());
> >   if (str.size() != pair.first.size())
> > stream.PutChar('\n');
> >   str = pair.second;
> > }
> > if (!str.empty()) {
> >   stream.Printf("\n... Interrupted.\n");
> > }
> > 
> > While this version is shorter there I see a number of small problems with 
> > it:
> > 1. It requires a close look at the StringRef::split() interface (ex. done = 
> > pair.second.empty() is actually not the correct check since it will eat the 
> > final \n)
> > 2. While conceptually straightforward, the StringRef::split() interface is 
> > not ideal in this case - see #1
> > 3. This version assumes that stream.Write() actually writes everything (ie. 
> > not handling the return size)
> > 4. In order to avoid reading past the end of the first slice we need to do 
> > a separate PutChar() for each \n
> > 
> > So in the end I prefer the original version, which although a bit more 
> > verbose, tracks the invariants clearly, is potentially faster and handles 
> > strings regardless if they are terminated with \n or not.
> Thoughts?
I haven't followed the rest of this too closely, so forgive me if this is a 
dumb question, but it looks like by the time we reach this function, the 
command is already complete.  Why would you want to interrupt it if it's 
already done?  Couldn't you just print the entire output in one call to 
`stream.Write()` at this point?


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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I think Zach might be correct. If we already gathered the data, why not just 
output it all? Lets answer this first before returning to which implementation 
to use when parsing it up into lines and dumping it. Seems a shame to throw any 
data away.


https://reviews.llvm.org/D37923



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

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

[Leonard Mosescu via lldb-commits]

It's a good question - here's a two part answer:

1. The actual printing of the output is the longest blocking in many cases
(as mentioned in the description: the actual data gathering for "target
module dump symtab" can take 1-2sec, but printing it can take 20min. For
quick experiment, try dis -p -c 1).
2. This change provides the scaffolding for cooperative interruption that
can be used were appropriate, not just the printing part. I did this for
"target" commands (see the changes in CommandObjectTarget.cpp), and it's
very easy to do the same in other places as needed.




On Mon, Sep 18, 2017 at 3:05 PM, Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> I think Zach might be correct. If we already gathered the data, why not
> just output it all? Lets answer this first before returning to which
> implementation to use when parsing it up into lines and dumping it. Seems a
> shame to throw any data away.
>
>
> 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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In the original discussion of this feature (it was on the mailing list), it 
sounded like the specific cast that  motivated Leonard in adding this feature 
was when there's a command that's in the process of generating tons of output, 
and you want to interrupt the tons of output.  Not the work, just the output.  
So the fact that is discards output was part of the design.


https://reviews.llvm.org/D37923



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

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

[Zachary Turner via lldb-commits]

On Mon, Sep 18, 2017 at 3:13 PM Leonard Mosescu  wrote:

> It's a good question - here's a two part answer:
>
> 1. The actual printing of the output is the longest blocking in many cases
> (as mentioned in the description: the actual data gathering for "target
> module dump symtab" can take 1-2sec, but printing it can take 20min. For
> quick experiment, try dis -p -c 1).
> 2. This change provides the scaffolding for cooperative interruption that
> can be used were appropriate, not just the printing part. I did this for
> "target" commands (see the changes in CommandObjectTarget.cpp), and it's
> very easy to do the same in other places as needed.
>
>
Makes sense.  Can you try `llvm::line_iterator` then instead of the
hand-splitting?  See `llvm/Support/LineIterator.h`
___
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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

And in general, interruption should be as responsive as you can make it.  When 
I tell lldb to "Stop right now" it really should do as close as possible to 
"stop right now".  Not, :-"excuse me I have 10 more pages of output to dump".  
It's not uncommon to dump some info, see what you want and not need to see the 
rest.  Interrupt in that case is expressing that "don't show me any more" so we 
should honor that.


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] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Leonard Mosescu via Phabricator via lldb-commits]

lemo updated this revision to Diff 115732.
lemo added a comment.

Fixed the FreeBSD/Windows break : the intention was to keep 
Process::WillResume() and Process::DoResume() "in-sync", but this had the 
unfortunate consequence of breaking Process sub-classes which don't override 
WillResume().

The safer approach is to keep Process::WillResume() untouched and only override 
it in the minidump and core implementations.


https://reviews.llvm.org/D37651

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
  source/Commands/CommandObjectThread.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.h
  source/Plugins/Process/minidump/ProcessMinidump.h
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,7 +1621,12 @@
   log->Printf("Process::Resume: -- TrySetRunning failed, not resuming.");
 return error;
   }
-  return PrivateResume();
+  Status error = PrivateResume();
+  if (!error.Success()) {
+// Undo running state change
+m_public_run_lock.SetStopped();
+  }
+  return error;
 }
 
 Status Process::ResumeSynchronous(Stream *stream) {
@@ -1650,6 +1655,9 @@
   error.SetErrorStringWithFormat(
   "process not in stopped state after synchronous resume: %s",
   StateAsCString(state));
+  } else {
+// Undo running state change
+m_public_run_lock.SetStopped();
   }
 
   // Undo the hijacking of process events...
Index: source/Plugins/Process/minidump/ProcessMinidump.h
===
--- source/Plugins/Process/minidump/ProcessMinidump.h
+++ source/Plugins/Process/minidump/ProcessMinidump.h
@@ -82,6 +82,14 @@
 
   bool GetProcessInfo(ProcessInstanceInfo &info) override;
 
+  Status WillResume() override {
+Status error;
+error.SetErrorStringWithFormat(
+"error: %s does not support resuming processes",
+GetPluginName().GetCString());
+return error;
+  }
+
   MinidumpParser m_minidump_parser;
 
 protected:
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -84,11 +84,21 @@
 
   void RefreshStateAfterStop() override;
 
+  lldb_private::Status WillResume() override {
+lldb_private::Status error;
+error.SetErrorStringWithFormat(
+"error: %s does not support resuming processes",
+GetPluginName().GetCString());
+return error;
+  }
+
   //--
   // Process Queries
   //--
   bool IsAlive() override;
 
+  bool WarnBeforeDetach() const override { return false; }
+
   //--
   // Process Memory
   //--
Index: source/Commands/CommandObjectThread.cpp
===
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -94,7 +94,7 @@
 bool all_threads = false;
 if (command.GetArgumentCount() == 0) {
   Thread *thread = m_exe_ctx.GetThreadPtr();
-  if (!HandleOneThread(thread->GetID(), result))
+  if (!thread || !HandleOneThread(thread->GetID(), result))
 return false;
   return result.Succeeded();
 } else if (command.GetArgumentCount() == 1) {
@@ -775,6 +775,12 @@
   else
 error = process->Resume();
 
+  if (!error.Success()) {
+result.AppendMessage(error.AsCString());
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+
   // There is a race condition where this thread will return up the call
   // stack to the main command handler
   // and show an (lldb) prompt before HandlePrivateEvent (from
Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -31,6 +31,34 @@
 lldb.DBG.SetSelectedPlatform(self._initial_platform)
 super(MiniDumpNewTestCase, self).tearDown()
 
+def check_state(self):
+with open(os.devnull) as devnul:
+# sanitize test output
+self.dbg.SetOutputFileHandle(devnul, False)
+self.dbg.SetErrorFileHandle(devnul, False)
+
+self.assertTrue(self.process.is_stopped)
+
+# Process.Continue
+e

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

ok. Then back to the "can we use StringRef"? We might be able to check if the 
second.data() is NULL? It might be NULL if the string doesn't end with a 
newline. It it does, it might be empty but contain a pointer to the '\0' byte?


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] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks fine.


https://reviews.llvm.org/D37651



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

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

[Leonard Mosescu via lldb-commits]

Looking at line_iterator, it seems to be designed to work with
MemoryBuffer, which in turn seems specialized for dealing with file content
(so while it may be possible to force init a MemoryBuffer from a StringRef
it seems a bit awkward to me). Also, line_iterator has extra stuff which we
don't need here yet requires extra care (skipping empty lines and comments,
tracking line numbers...)


On Mon, Sep 18, 2017 at 3:18 PM, Zachary Turner  wrote:

>
>
> On Mon, Sep 18, 2017 at 3:13 PM Leonard Mosescu 
> wrote:
>
>> It's a good question - here's a two part answer:
>>
>> 1. The actual printing of the output is the longest blocking in many
>> cases (as mentioned in the description: the actual data gathering for
>> "target module dump symtab" can take 1-2sec, but printing it can take
>> 20min. For quick experiment, try dis -p -c 1).
>> 2. This change provides the scaffolding for cooperative interruption that
>> can be used were appropriate, not just the printing part. I did this for
>> "target" commands (see the changes in CommandObjectTarget.cpp), and it's
>> very easy to do the same in other places as needed.
>>
>>
> Makes sense.  Can you try `llvm::line_iterator` then instead of the
> hand-splitting?  See `llvm/Support/LineIterator.h`
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Leonard Mosescu via lldb-commits]

Hi Greg, are you referring to the manual line splitting vs. using
StringRef::split()? This is how it's documented:

/// Split into two substrings around the first occurrence of a separator
/// character.
///
/// If \p Separator is in the string, then the result is a pair (LHS, RHS)
/// such that (*this == LHS + Separator + RHS) is true and RHS is
/// maximal. If \p Separator is not in the string, then the result is a
/// pair (LHS, RHS) where (*this == LHS) and (RHS == "").

Yes, RHS == "" happens to be be implemented as a StringRef with nullptr
data, but this seems to make the implementation even more depended on the
subtle details. My point was that second == "" in two cases: separator is
not in the string AND separator is _last_ character in the string.

Did I get the question right?

On Mon, Sep 18, 2017 at 3:51 PM, Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> ok. Then back to the "can we use StringRef"? We might be able to check if
> the second.data() is NULL? It might be NULL if the string doesn't end with
> a newline. It it does, it might be empty but contain a pointer to the '\0'
> byte?
>
>
> https://reviews.llvm.org/D37923
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits