public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
@ 2017-08-22 14:05 Sergio Durigan Junior
  2017-08-22 18:09 ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2017-08-22 14:05 UTC (permalink / raw)
  To: GDB Patches; +Cc: Jan Kratochvil, Pedro Alves, Sergio Durigan Junior

The fix for PR gdb/20609:

  commit bb805577d2b212411fb7b0a2d01644567fac4e8d
  Author: Jan Kratochvil <jan.kratochvil@redhat.com>
  Date:   Thu Sep 29 17:38:16 2016 +0200

Introduced the concept of deferring the call to breakpoint_re_set on
certain useful occasions.  However, there is one specific scenario
where delaying needs to be done and still isn't: the case when we're
starting a GDB to debug a remote inferior without specifying a local
binary, as in for example:

  ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \
    -ex "set remote exec-file /bin/ls" -ex r

In this case, when calling exec_file_locate_attach to locate the
inferior, GDB is incorrectly resetting the breakpoints without a
thread/inferior even running, which causes an assertion to be
triggered:

  binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n)

The right thing to do is to defer resetting the breakpoints when
locating the binary, which is what this patch does.

A testcase is also provided.  Regtested on buildbot.

gdb/ChangeLog:
2017-08-22  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR remote/21852
	* remote.c (remote_add_inferior): Tell "exec_file_locate_attach"
	to defer calling breakpoint_re_set.

gdb/testsuite/ChangeLog:
2017-08-22  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR remote/21852
	* gdb.server/normal.c: New file, copied from gdb.base.
	* gdb.server/run-without-local-binary.exp: New file.
---
 gdb/remote.c                                       |  2 +-
 gdb/testsuite/gdb.server/normal.c                  | 24 ++++++++++
 .../gdb.server/run-without-local-binary.exp        | 56 ++++++++++++++++++++++
 3 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.server/normal.c
 create mode 100644 gdb/testsuite/gdb.server/run-without-local-binary.exp

diff --git a/gdb/remote.c b/gdb/remote.c
index ff59a0f81f..c9bcb163e7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1825,7 +1825,7 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
   /* If no main executable is currently open then attempt to
      open the file that was executed to create this inferior.  */
   if (try_open_exec && get_exec_file (0) == NULL)
-    exec_file_locate_attach (pid, 0, 1);
+    exec_file_locate_attach (pid, 1, 1);
 
   return inf;
 }
diff --git a/gdb/testsuite/gdb.server/normal.c b/gdb/testsuite/gdb.server/normal.c
new file mode 100644
index 0000000000..f0eb1541fd
--- /dev/null
+++ b/gdb/testsuite/gdb.server/normal.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013-2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This test is just a normal return 0.  */
+
+int
+main (int argc, char *argv[])
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/run-without-local-binary.exp b/gdb/testsuite/gdb.server/run-without-local-binary.exp
new file mode 100644
index 0000000000..5af2d1bb25
--- /dev/null
+++ b/gdb/testsuite/gdb.server/run-without-local-binary.exp
@@ -0,0 +1,56 @@
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+load_lib gdbserver-support.exp
+
+if {[skip_gdbserver_tests]} {
+    return
+}
+
+standard_testfile normal.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Test running GDB without providing a local binary for it.  In order
+# to do that, we unset GDBFLAGS before running GDB.  We also start
+# gdbserver "by hand".  For more details about this test, see PR
+# remote/21852.
+save_vars { GDBFLAGS } {
+    set GDBFLAGS ""
+    gdb_exit
+    gdb_start
+
+    gdb_test_no_output "set remote exec-file $binfile" \
+	"set remote exec-file"
+
+    # Let's start gdbserver in extended-remote mode now.
+    set res [gdbserver_start "--multi" ""]
+    set gdbserver_protocol [lindex $res 0]
+    if { [string first "extended-" $gdbserver_protocol] != 0} {
+	set gdbserver_protocol "extended-$gdbserver_protocol"
+    }
+    set gdbserver_gdbport [lindex $res 1]
+    set use_gdb_stub 0
+
+    gdb_test "target ${gdbserver_protocol} ${gdbserver_gdbport}" \
+	"Remote debugging using $gdbserver_gdbport" \
+	"connect to gdbserver"
+
+    gdb_test "run" \
+	"\\\[Inferior $decimal \\\(process $decimal\\\) exited normally\\\]" \
+	"run test program until the end"
+}
-- 
2.13.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-22 14:05 [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB Sergio Durigan Junior
@ 2017-08-22 18:09 ` Pedro Alves
  2017-08-22 22:56   ` Sergio Durigan Junior
  2017-08-23 17:36   ` Sergio Durigan Junior
  0 siblings, 2 replies; 7+ messages in thread
From: Pedro Alves @ 2017-08-22 18:09 UTC (permalink / raw)
  To: Sergio Durigan Junior, GDB Patches; +Cc: Jan Kratochvil

On 08/22/2017 03:04 PM, Sergio Durigan Junior wrote:
> The fix for PR gdb/20609:
> 
>   commit bb805577d2b212411fb7b0a2d01644567fac4e8d
>   Author: Jan Kratochvil <jan.kratochvil@redhat.com>
>   Date:   Thu Sep 29 17:38:16 2016 +0200
> 
> Introduced the concept of deferring the call to breakpoint_re_set on
> certain useful occasions.  However, there is one specific scenario
> where delaying needs to be done and still isn't: the case when we're
> starting a GDB to debug a remote inferior without specifying a local
> binary, as in for example:
> 
>   ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \
>     -ex "set remote exec-file /bin/ls" -ex r
> 
> In this case, when calling exec_file_locate_attach to locate the
> inferior, GDB is incorrectly resetting the breakpoints without a
> thread/inferior even running, which causes an assertion to be
> triggered:
> 
>   binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed.
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>   Quit this debugging session? (y or n)
> 
> The right thing to do is to defer resetting the breakpoints when
> locating the binary, which is what this patch does.

Hmm, I think we're missing more rationale.  There may well be
other reasons for doing that, but this case just looks like a
case of remote.c breaking invariants to me -- making inferior_ptid
point to a non-existing thread and then calling common code is
recipe for disaster.  Seems to me that the fix is just to
not do that?  See patch below.  It fixes your test for me
as well, though I haven't run the full testsuite.

From: Pedro Alves <palves@redhat.com>
Date: 2017-08-22 18:42:27 +0100

fix remote.c

---

 gdb/remote.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index ff59a0f..ea21163 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3831,19 +3831,16 @@ add_current_inferior_and_thread (char *wait_status)
 {
   struct remote_state *rs = get_remote_state ();
   int fake_pid_p = 0;
-  ptid_t ptid;
 
   inferior_ptid = null_ptid;
 
   /* Now, if we have thread information, update inferior_ptid.  */
-  ptid = get_current_thread (wait_status);
+  ptid_t curr_ptid = get_current_thread (wait_status);
 
-  if (!ptid_equal (ptid, null_ptid))
+  if (curr_ptid != null_ptid)
     {
       if (!remote_multi_process_p (rs))
 	fake_pid_p = 1;
-
-      inferior_ptid = ptid;
     }
   else
     {
@@ -3851,14 +3848,17 @@ add_current_inferior_and_thread (char *wait_status)
 	 (such as kill) won't work.  This variable serves (at least)
 	 double duty as both the pid of the target process (if it has
 	 such), and as a flag indicating that a target is active.  */
-      inferior_ptid = magic_null_ptid;
+      curr_ptid = magic_null_ptid;
       fake_pid_p = 1;
     }
 
-  remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1, 1);
+  remote_add_inferior (fake_pid_p, ptid_get_pid (curr_ptid), -1, 1);
 
-  /* Add the main thread.  */
-  add_thread_silent (inferior_ptid);
+  /* Add the main thread and switch to it.  Don't try reading
+     registers yes, since we haven't fetched the target description
+     yet.  */
+  thread_info *tp = add_thread_silent (curr_ptid);
+  switch_to_thread_no_regs (tp);
 }
 
 /* Print info about a thread that was found already stopped on

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-22 18:09 ` Pedro Alves
@ 2017-08-22 22:56   ` Sergio Durigan Junior
  2017-08-22 23:19     ` Pedro Alves
  2017-08-23 17:36   ` Sergio Durigan Junior
  1 sibling, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2017-08-22 22:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, Jan Kratochvil

On Tuesday, August 22 2017, Pedro Alves wrote:

> On 08/22/2017 03:04 PM, Sergio Durigan Junior wrote:
>> The fix for PR gdb/20609:
>> 
>>   commit bb805577d2b212411fb7b0a2d01644567fac4e8d
>>   Author: Jan Kratochvil <jan.kratochvil@redhat.com>
>>   Date:   Thu Sep 29 17:38:16 2016 +0200
>> 
>> Introduced the concept of deferring the call to breakpoint_re_set on
>> certain useful occasions.  However, there is one specific scenario
>> where delaying needs to be done and still isn't: the case when we're
>> starting a GDB to debug a remote inferior without specifying a local
>> binary, as in for example:
>> 
>>   ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \
>>     -ex "set remote exec-file /bin/ls" -ex r
>> 
>> In this case, when calling exec_file_locate_attach to locate the
>> inferior, GDB is incorrectly resetting the breakpoints without a
>> thread/inferior even running, which causes an assertion to be
>> triggered:
>> 
>>   binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed.
>>   A problem internal to GDB has been detected,
>>   further debugging may prove unreliable.
>>   Quit this debugging session? (y or n)
>> 
>> The right thing to do is to defer resetting the breakpoints when
>> locating the binary, which is what this patch does.
>
> Hmm, I think we're missing more rationale.  There may well be
> other reasons for doing that, but this case just looks like a
> case of remote.c breaking invariants to me -- making inferior_ptid
> point to a non-existing thread and then calling common code is
> recipe for disaster.  Seems to me that the fix is just to
> not do that?  See patch below.  It fixes your test for me
> as well, though I haven't run the full testsuite.

Thanks for the review.

Well, what can I say.  My fix looked right from my perspective, and I
confess that at the beginning I had the same thought: remote.c is
causing the problem by making inferior_ptid point to a non existing
thread.  However, I quickly found that the culprit was on the call chain
leading to exec_file_locate_attach and concentrated my focus on that.

Your patch looks more complete and to the point indeed.  Although it
seems to me, from what I observed, that calling breakpoint_re_set on
exec_file_locate_attach when dealing with a remote inferior doesn't make
sense either.

Anyway, I'll resubmit my patch using your approach and leave my first
patch aside for a bit, until I hear what you think about not calling
breakpoint_re_set on this specific case.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-22 22:56   ` Sergio Durigan Junior
@ 2017-08-22 23:19     ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2017-08-22 23:19 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: GDB Patches, Jan Kratochvil

On 08/22/2017 11:56 PM, Sergio Durigan Junior wrote:
> On Tuesday, August 22 2017, Pedro Alves wrote:
> 
>> On 08/22/2017 03:04 PM, Sergio Durigan Junior wrote:
>>> The fix for PR gdb/20609:
>>>
>>>   commit bb805577d2b212411fb7b0a2d01644567fac4e8d
>>>   Author: Jan Kratochvil <jan.kratochvil@redhat.com>
>>>   Date:   Thu Sep 29 17:38:16 2016 +0200
>>>
>>> Introduced the concept of deferring the call to breakpoint_re_set on
>>> certain useful occasions.  However, there is one specific scenario
>>> where delaying needs to be done and still isn't: the case when we're
>>> starting a GDB to debug a remote inferior without specifying a local
>>> binary, as in for example:
>>>
>>>   ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \
>>>     -ex "set remote exec-file /bin/ls" -ex r
>>>
>>> In this case, when calling exec_file_locate_attach to locate the
>>> inferior, GDB is incorrectly resetting the breakpoints without a
>>> thread/inferior even running, which causes an assertion to be
>>> triggered:
>>>
>>>   binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed.
>>>   A problem internal to GDB has been detected,
>>>   further debugging may prove unreliable.
>>>   Quit this debugging session? (y or n)
>>>
>>> The right thing to do is to defer resetting the breakpoints when
>>> locating the binary, which is what this patch does.
>>
>> Hmm, I think we're missing more rationale.  There may well be
>> other reasons for doing that, but this case just looks like a
>> case of remote.c breaking invariants to me -- making inferior_ptid
>> point to a non-existing thread and then calling common code is
>> recipe for disaster.  Seems to me that the fix is just to
>> not do that?  See patch below.  It fixes your test for me
>> as well, though I haven't run the full testsuite.
> 
> Thanks for the review.
> 
> Well, what can I say.  My fix looked right from my perspective, and I
> confess that at the beginning I had the same thought: remote.c is
> causing the problem by making inferior_ptid point to a non existing
> thread.  However, I quickly found that the culprit was on the call chain
> leading to exec_file_locate_attach and concentrated my focus on that.
> 
> Your patch looks more complete and to the point indeed.  Although it
> seems to me, from what I observed, that calling breakpoint_re_set on
> exec_file_locate_attach when dealing with a remote inferior doesn't make
> sense either.

... doesn't make sense because?  The thing is we're missing an explicit
rationale from first principles.

> Anyway, I'll resubmit my patch using your approach and leave my first
> patch aside for a bit, until I hear what you think about not calling
> breakpoint_re_set on this specific case.

Well, I was hoping you'd go first.  :-)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-22 18:09 ` Pedro Alves
  2017-08-22 22:56   ` Sergio Durigan Junior
@ 2017-08-23 17:36   ` Sergio Durigan Junior
  2017-08-23 19:32     ` Pedro Alves
  1 sibling, 1 reply; 7+ messages in thread
From: Sergio Durigan Junior @ 2017-08-23 17:36 UTC (permalink / raw)
  To: GDB Patches; +Cc: Pedro Alves, Sergio Durigan Junior

There is an assertion that is triggering when we start GDB and
instruct it to debug a remote inferior, but don't provide a local
binary, like:

  ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \
    -ex "set remote exec-file /bin/ls" -ex r

In this case, when calling exec_file_locate_attach to locate the
inferior, GDB is incorrectly resetting the breakpoints without a
thread/inferior even running, which causes an assertion to be
triggered:

  binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n)

This happens because add_current_inferior_and_thread (on remote.c) is
breaking an invariant: making inferior_ptid point to a non-existing
thread and then calling common code, which in this case is
breakpoint_re_set.  The fix is to make sure that inferior_ptid points
to null_ptid if there is no thread present.

A testcase is provided.  Regtested on buildbot.

gdb/ChangeLog:
2017-08-23  Pedro Alvex  <palves@redhat.com>

	PR remote/21852
	* remote.c (add_current_inferior_and_thread): Set inferior_ptid
	to null_ptid and switch to thread without reading the registers
	after adding the inferior.

gdb/testsuite/ChangeLog:
2017-08-23  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR remote/21852
	* gdb.server/normal.c: New file, copied from gdb.base.
	* gdb.server/run-without-local-binary.exp: New file.
---
 gdb/remote.c                                       | 18 +++----
 gdb/testsuite/gdb.server/normal.c                  | 24 +++++++++
 .../gdb.server/run-without-local-binary.exp        | 60 ++++++++++++++++++++++
 3 files changed, 93 insertions(+), 9 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/normal.c
 create mode 100644 gdb/testsuite/gdb.server/run-without-local-binary.exp

diff --git a/gdb/remote.c b/gdb/remote.c
index ff59a0f81f..2249533258 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3831,19 +3831,16 @@ add_current_inferior_and_thread (char *wait_status)
 {
   struct remote_state *rs = get_remote_state ();
   int fake_pid_p = 0;
-  ptid_t ptid;
 
   inferior_ptid = null_ptid;
 
   /* Now, if we have thread information, update inferior_ptid.  */
-  ptid = get_current_thread (wait_status);
+  ptid_t curr_ptid = get_current_thread (wait_status);
 
-  if (!ptid_equal (ptid, null_ptid))
+  if (curr_ptid != null_ptid)
     {
       if (!remote_multi_process_p (rs))
 	fake_pid_p = 1;
-
-      inferior_ptid = ptid;
     }
   else
     {
@@ -3851,14 +3848,17 @@ add_current_inferior_and_thread (char *wait_status)
 	 (such as kill) won't work.  This variable serves (at least)
 	 double duty as both the pid of the target process (if it has
 	 such), and as a flag indicating that a target is active.  */
-      inferior_ptid = magic_null_ptid;
+      curr_ptid = magic_null_ptid;
       fake_pid_p = 1;
     }
 
-  remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1, 1);
+  remote_add_inferior (fake_pid_p, ptid_get_pid (curr_ptid), -1, 1);
 
-  /* Add the main thread.  */
-  add_thread_silent (inferior_ptid);
+  /* Add the main thread and switch to it.  Don't try reading
+     registers yet, since we haven't fetched the target description
+     yet.  */
+  thread_info *tp = add_thread_silent (curr_ptid);
+  switch_to_thread_no_regs (tp);
 }
 
 /* Print info about a thread that was found already stopped on
diff --git a/gdb/testsuite/gdb.server/normal.c b/gdb/testsuite/gdb.server/normal.c
new file mode 100644
index 0000000000..f0eb1541fd
--- /dev/null
+++ b/gdb/testsuite/gdb.server/normal.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013-2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This test is just a normal return 0.  */
+
+int
+main (int argc, char *argv[])
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/run-without-local-binary.exp b/gdb/testsuite/gdb.server/run-without-local-binary.exp
new file mode 100644
index 0000000000..ea4b043a7a
--- /dev/null
+++ b/gdb/testsuite/gdb.server/run-without-local-binary.exp
@@ -0,0 +1,60 @@
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+load_lib gdbserver-support.exp
+
+if {[skip_gdbserver_tests]} {
+    return
+}
+
+standard_testfile normal.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Test running GDB without providing a local binary for it.  In order
+# to do that, we unset GDBFLAGS before running GDB.  We also start
+# gdbserver "by hand".  For more details about this test, see PR
+# remote/21852.
+save_vars { GDBFLAGS } {
+    set GDBFLAGS ""
+    gdb_exit
+    gdb_start
+
+    gdb_test_no_output "set remote exec-file $binfile" \
+	"set remote exec-file"
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*" "disconnect"
+
+    # Let's start gdbserver in extended-remote mode now.
+    set res [gdbserver_start "--multi" ""]
+    set gdbserver_protocol [lindex $res 0]
+    if { [string first "extended-" $gdbserver_protocol] != 0} {
+	set gdbserver_protocol "extended-$gdbserver_protocol"
+    }
+    set gdbserver_gdbport [lindex $res 1]
+    set use_gdb_stub 0
+
+    gdb_test "target ${gdbserver_protocol} ${gdbserver_gdbport}" \
+	"Remote debugging using $gdbserver_gdbport" \
+	"connect to gdbserver"
+
+    gdb_test "run" \
+	"\\\[Inferior $decimal \\\(process $decimal\\\) exited normally\\\]" \
+	"run test program until the end"
+}
-- 
2.13.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-23 17:36   ` Sergio Durigan Junior
@ 2017-08-23 19:32     ` Pedro Alves
  2017-08-23 21:29       ` Sergio Durigan Junior
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2017-08-23 19:32 UTC (permalink / raw)
  To: Sergio Durigan Junior, GDB Patches

On 08/23/2017 06:36 PM, Sergio Durigan Junior wrote:

> A testcase is provided.  Regtested on buildbot.

Looks good to me.  A question below.

> gdb/ChangeLog:
> 2017-08-23  Pedro Alvex  <palves@redhat.com>

Love my new name, thanks.  :-)

> +    # Let's start gdbserver in extended-remote mode now.
> +    set res [gdbserver_start "--multi" ""]
> +    set gdbserver_protocol [lindex $res 0]
> +    if { [string first "extended-" $gdbserver_protocol] != 0} {
> +	set gdbserver_protocol "extended-$gdbserver_protocol"
> +    }
> +    set gdbserver_gdbport [lindex $res 1]
> +    set use_gdb_stub 0

I guess this can't use gdbserver_start_extended because that
somehow assumes you're starting with a binary?  Can you add
a comment?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
  2017-08-23 19:32     ` Pedro Alves
@ 2017-08-23 21:29       ` Sergio Durigan Junior
  0 siblings, 0 replies; 7+ messages in thread
From: Sergio Durigan Junior @ 2017-08-23 21:29 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches

On Wednesday, August 23 2017, Pedro Alves wrote:

> On 08/23/2017 06:36 PM, Sergio Durigan Junior wrote:
>
>> A testcase is provided.  Regtested on buildbot.
>
> Looks good to me.  A question below.
>
>> gdb/ChangeLog:
>> 2017-08-23  Pedro Alvex  <palves@redhat.com>
>
> Love my new name, thanks.  :-)

Sigh.  Sorry, fixed.

>> +    # Let's start gdbserver in extended-remote mode now.
>> +    set res [gdbserver_start "--multi" ""]
>> +    set gdbserver_protocol [lindex $res 0]
>> +    if { [string first "extended-" $gdbserver_protocol] != 0} {
>> +	set gdbserver_protocol "extended-$gdbserver_protocol"
>> +    }
>> +    set gdbserver_gdbport [lindex $res 1]
>> +    set use_gdb_stub 0
>
> I guess this can't use gdbserver_start_extended because that
> somehow assumes you're starting with a binary?  Can you add
> a comment?

Exactly.  And sure, I added a comment.

Pushed:
  87215ad1651ca3094d813eae06233fd7259b37e5

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-08-23 21:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-22 14:05 [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB Sergio Durigan Junior
2017-08-22 18:09 ` Pedro Alves
2017-08-22 22:56   ` Sergio Durigan Junior
2017-08-22 23:19     ` Pedro Alves
2017-08-23 17:36   ` Sergio Durigan Junior
2017-08-23 19:32     ` Pedro Alves
2017-08-23 21:29       ` Sergio Durigan Junior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).