[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] 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] 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] 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.

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] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

[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.

[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] D37934: Fix compatibility with OpenOCD debug stub.

[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() ==