[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL330247: Report more precise error message when attach fails 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45573?vs=142194&id=142907#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45573

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
@@ -126,6 +126,8 @@
 return self.qfThreadInfo()
 if packet == "qC":
 return self.qC()
+if packet == "QEnableErrorStrings":
+return self.QEnableErrorStrings()
 if packet == "?":
 return self.haltReason()
 if packet[0] == "H":
@@ -137,6 +139,9 @@
 if data is not None:
 return self._qXferResponse(data, has_more)
 return ""
+if packet.startswith("vAttach;"):
+pid = packet.partition(';')[2]
+return self.vAttach(int(pid, 16))
 if packet[0] == "Z":
 return self.setBreakpoint(packet)
 return self.other(packet)
@@ -177,6 +182,9 @@
 def qC(self):
 return "QC0"
 
+def QEnableErrorStrings(self):
+return "OK"
+
 def haltReason(self):
 # SIGINT is 2, return type is 2 digit hex string
 return "S02"
@@ -187,6 +195,9 @@
 def _qXferResponse(self, data, has_more):
 return "%s%s" % ("m" if has_more else "l", escape_binary(data))
 
+def vAttach(self, pid):
+raise self.UnexpectedPacketException()
+
 def selectThread(self, op, thread_id):
 return "OK"
 
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -11,3 +11,28 @@
 target = self.createTarget("a.yaml")
 process = self.connect(target)
 self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])
+
+def test_attach_fail(self):
+error_msg = "mock-error-msg"
+
+class MyResponder(MockGDBServerResponder):
+# Pretend we don't have any process during the initial queries.
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vAttach(self, pid):
+return "E42;" + error_msg.encode("hex")
+
+self.server.responder = MyResponder()
+
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, 
[lldb.eStateConnected])
+
+error = lldb.SBError()
+target.AttachToProcessWithID(lldb.SBListener(), 47, error)
+self.assertEquals(error_msg, error.GetCString())
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3804,11 +3804,9 @@
 response.GetError() == 0x87) {
   process->SetExitStatus(-1, "cannot attach to process due to "
  "System Integrity Protection");
-}
-// E01 code from vAttach means that the attach failed
-if (::strstr(continue_cstr, "vAttach") != NULL &&
-response.GetError() == 0x1) {
-  process->SetExitStatus(-1, "unable to attach");
+} else if (::strstr(continue_cstr, "vAttach") != NULL &&
+   response.GetStatus().Fail()) {
+  process->SetExitStatus(-1, response.GetStatus().AsCString());
 } else {
   process->SetExitStatus(-1, "lost connection");
 }


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

That looks fine.


https://reviews.llvm.org/D45573



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

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

lgtm


https://reviews.llvm.org/D45573



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

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM module comment




Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3809
 }
 // E01 code from vAttach means that the attach failed
 if (::strstr(continue_cstr, "vAttach") != NULL &&

Outdated comment


https://reviews.llvm.org/D45573



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

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jingham, JDevlieghere.

If the remote stub sends a specific error message instead of just a E??
code, we can use this to display a more informative error message
instead of just the generic "unable to attach" message.

I write a test for this using the SB API.
On the console this will show up like:
(lldb) process attach ...
error: attach failed: 

if the stub supports error messages, or:
error: attach failed: "Error ??"

if it doesn't.


https://reviews.llvm.org/D45573

Files:
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  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
@@ -3808,8 +3808,8 @@
 }
 // E01 code from vAttach means that the attach failed
 if (::strstr(continue_cstr, "vAttach") != NULL &&
-response.GetError() == 0x1) {
-  process->SetExitStatus(-1, "unable to attach");
+response.GetStatus().Fail()) {
+  process->SetExitStatus(-1, response.GetStatus().AsCString());
 } else {
   process->SetExitStatus(-1, "lost connection");
 }
Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===
--- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
@@ -126,6 +126,8 @@
 return self.qfThreadInfo()
 if packet == "qC":
 return self.qC()
+if packet == "QEnableErrorStrings":
+return self.QEnableErrorStrings()
 if packet == "?":
 return self.haltReason()
 if packet[0] == "H":
@@ -137,6 +139,9 @@
 if data is not None:
 return self._qXferResponse(data, has_more)
 return ""
+if packet.startswith("vAttach;"):
+pid = packet.partition(';')[2]
+return self.vAttach(int(pid, 16))
 if packet[0] == "Z":
 return self.setBreakpoint(packet)
 return self.other(packet)
@@ -177,6 +182,9 @@
 def qC(self):
 return "QC0"
 
+def QEnableErrorStrings(self):
+return "OK"
+
 def haltReason(self):
 # SIGINT is 2, return type is 2 digit hex string
 return "S02"
@@ -187,6 +195,9 @@
 def _qXferResponse(self, data, has_more):
 return "%s%s" % ("m" if has_more else "l", escape_binary(data))
 
+def vAttach(self, pid):
+raise self.UnexpectedPacketException()
+
 def selectThread(self, op, thread_id):
 return "OK"
 
Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -11,3 +11,28 @@
 target = self.createTarget("a.yaml")
 process = self.connect(target)
 self.assertPacketLogContains(["qProcessInfo", "qfThreadInfo"])
+
+def test_attach_fail(self):
+error_msg = "mock-error-msg"
+
+class MyResponder(MockGDBServerResponder):
+# Pretend we don't have any process during the initial queries.
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vAttach(self, pid):
+return "E42;" + error_msg.encode("hex")
+
+self.server.responder = MyResponder()
+
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, 
[lldb.eStateConnected])
+
+error = lldb.SBError()
+target.AttachToProcessWithID(lldb.SBListener(), 47, error)
+self.assertEquals(error_msg, error.GetCString())


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3808,8 +3808,8 @@
 }
 // E01 code from vAttach means that the attach failed
 if (::strstr(continue_cstr, "vAttach") != NULL &&
-response.GetError() ==