* [PATCH] remote: Fix a crash on longjmp breakpoint removal
@ 2011-12-13 21:13 Maciej W. Rozycki
2011-12-20 16:00 ` Tom Tromey
0 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2011-12-13 21:13 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Hi,
I have observed a crash, where GDB attempts to send a packet to gdbserver
after the remote target has already exited (and the gdbserver process
terminated) -- here's the tail of an example remote session transcript:
Sending packet: $vCont;c#a8...Packet received: W00
[Inferior 1 (Remote target) exited normally]
Sending packet: $Hg0#df...
This happens on the MIPS/Linux target while GDB is single-stepping with
the "step" command.
I have tracked it down to remote_close calling discard_all_inferiors,
which in turn eventually calls delete_thread_of_inferior, which calls
clear_thread_inferior_resources, which calls delete_longjmp_breakpoint,
which calls remote_remove_breakpoint, which needs to send some packets to
the remote target to get the breakpoint removed. But at this point the
remote file descriptor has already been closed; after a W00 stop reply
gdbserver does not expect any further input anyway.
I have looked through ChangeLogs and I believe this is a regression
caused by:
2010-12-09 Tom Tromey <tromey@redhat.com>
PR c++/9593:
* thread.c (clear_thread_inferior_resources): Call
delete_longjmp_breakpoint.
[...]
I have been able to get rid of the crash with the change below, no test
suite regressions on mips-linux-gnu (remote target). OK to apply?
2011-12-13 Maciej W. Rozycki <macro@codesourcery.com>
gdb/
* thread.c (clear_thread_inferior_resources): Don't call
delete_longjmp_breakpoint if there's no inferior.
Maciej
gdb-inferior-resources-crash.diff
Index: gdb-fsf-trunk-quilt/gdb/thread.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/thread.c 2011-11-02 21:28:38.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/thread.c 2011-12-13 19:42:25.805620748 +0000
@@ -119,7 +119,8 @@ clear_thread_inferior_resources (struct
do_all_intermediate_continuations_thread (tp, 1);
do_all_continuations_thread (tp, 1);
- delete_longjmp_breakpoint (tp->num);
+ if (!ptid_equal (inferior_ptid, null_ptid))
+ delete_longjmp_breakpoint (tp->num);
}
static void
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2011-12-13 21:13 [PATCH] remote: Fix a crash on longjmp breakpoint removal Maciej W. Rozycki
@ 2011-12-20 16:00 ` Tom Tromey
2012-03-07 20:08 ` Maciej W. Rozycki
0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2011-12-20 16:00 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: gdb-patches, Keith Seitz
>>>>> "Maciej" == Maciej W Rozycki <macro@codesourcery.com> writes:
Maciej> I have observed a crash, where GDB attempts to send a packet to
Maciej> gdbserver after the remote target has already exited (and the
Maciej> gdbserver process terminated) -- here's the tail of an example
Maciej> remote session transcript:
Maciej> I have tracked it down to remote_close calling
Maciej> discard_all_inferiors, which in turn eventually calls
Maciej> delete_thread_of_inferior, which calls
Maciej> clear_thread_inferior_resources, which calls
Maciej> delete_longjmp_breakpoint, which calls remote_remove_breakpoint,
Maciej> which needs to send some packets to the remote target to get the
Maciej> breakpoint removed. But at this point the remote file
Maciej> descriptor has already been closed; after a W00 stop reply
Maciej> gdbserver does not expect any further input anyway.
You and Keith both came to the same diagnosis, but you have different
patches:
http://sourceware.org/ml/gdb-patches/2011-11/msg00212.html
... our patch review delays seem to have bitten us this time.
Or anyway bitten the two of you, sorry about that.
I am not sure which approach is better. I don't think I know enough
about remote.c to say.
Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2011-12-20 16:00 ` Tom Tromey
@ 2012-03-07 20:08 ` Maciej W. Rozycki
2012-03-07 20:47 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2012-03-07 20:08 UTC (permalink / raw)
To: Tom Tromey, Pedro Alves; +Cc: gdb-patches, Keith Seitz
On Tue, 20 Dec 2011, Tom Tromey wrote:
> Maciej> I have observed a crash, where GDB attempts to send a packet to
> Maciej> gdbserver after the remote target has already exited (and the
> Maciej> gdbserver process terminated) -- here's the tail of an example
> Maciej> remote session transcript:
>
> Maciej> I have tracked it down to remote_close calling
> Maciej> discard_all_inferiors, which in turn eventually calls
> Maciej> delete_thread_of_inferior, which calls
> Maciej> clear_thread_inferior_resources, which calls
> Maciej> delete_longjmp_breakpoint, which calls remote_remove_breakpoint,
> Maciej> which needs to send some packets to the remote target to get the
> Maciej> breakpoint removed. But at this point the remote file
> Maciej> descriptor has already been closed; after a W00 stop reply
> Maciej> gdbserver does not expect any further input anyway.
>
> You and Keith both came to the same diagnosis, but you have different
> patches:
>
> http://sourceware.org/ml/gdb-patches/2011-11/msg00212.html
>
> ... our patch review delays seem to have bitten us this time.
> Or anyway bitten the two of you, sorry about that.
>
> I am not sure which approach is better. I don't think I know enough
> about remote.c to say.
So I have finally got back to it and noticed that this:
http://sourceware.org/ml/gdb-patches/2012-01/msg00701.html
has already fixed the problem -- shall we close gdb/13333 then? I think
we should still add Keith's test case to cover any possible future
breakage in this area -- the fix didn't include any. I suggest that we
retrofit the ChangeLog entry with a reference too.
Maciej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-07 20:08 ` Maciej W. Rozycki
@ 2012-03-07 20:47 ` Pedro Alves
2012-03-07 20:48 ` Keith Seitz
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2012-03-07 20:47 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Tom Tromey, gdb-patches, Keith Seitz
On 03/07/2012 08:08 PM, Maciej W. Rozycki wrote:
> So I have finally got back to it and noticed that this:
>
> http://sourceware.org/ml/gdb-patches/2012-01/msg00701.html
>
> has already fixed the problem -- shall we close gdb/13333 then?
If it's fixed, yes.
> I think we should still add Keith's test case to cover any possible future
> breakage in this area -- the fix didn't include any.
py-finish-breakpoint.exp was failing against gdbserver without the fix.
I don't mind adding a new test. Keith's was using gdb_expect, which we
very much like to avoid. Can it be made to use gdb_test or
gdb_test_multiple?
> I suggest that we retrofit the ChangeLog entry with a reference too.
Please go ahead.
--
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-07 20:47 ` Pedro Alves
@ 2012-03-07 20:48 ` Keith Seitz
2012-03-10 2:55 ` Maciej W. Rozycki
0 siblings, 1 reply; 9+ messages in thread
From: Keith Seitz @ 2012-03-07 20:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: Maciej W. Rozycki, gdb-patches
On 03/07/2012 12:46 PM, Pedro Alves wrote:
> py-finish-breakpoint.exp was failing against gdbserver without the fix.
> I don't mind adding a new test. Keith's was using gdb_expect, which we
> very much like to avoid. Can it be made to use gdb_test or
> gdb_test_multiple?
I should think so. The test is pretty simple. I just keep forgetting
about gdb_test_multiple!
Keith
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-07 20:48 ` Keith Seitz
@ 2012-03-10 2:55 ` Maciej W. Rozycki
2012-03-10 4:48 ` Keith Seitz
0 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2012-03-10 2:55 UTC (permalink / raw)
To: Keith Seitz; +Cc: Pedro Alves, gdb-patches
On Wed, 7 Mar 2012, Keith Seitz wrote:
> > py-finish-breakpoint.exp was failing against gdbserver without the fix.
Hmm, is this test case general enough to be run in all the relevant
configurations -- i.e. is there no special provision for Python support
required?
> > I don't mind adding a new test. Keith's was using gdb_expect, which we
> > very much like to avoid. Can it be made to use gdb_test or
> > gdb_test_multiple?
>
> I should think so. The test is pretty simple. I just keep forgetting about
> gdb_test_multiple!
Keith, would you care updating your test case then? I'm going to be away
for two weeks and won't be able to look into it any sooner.
Maciej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-10 2:55 ` Maciej W. Rozycki
@ 2012-03-10 4:48 ` Keith Seitz
2012-03-10 5:11 ` Keith Seitz
0 siblings, 1 reply; 9+ messages in thread
From: Keith Seitz @ 2012-03-10 4:48 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Pedro Alves, gdb-patches
On 03/09/2012 06:55 PM, Maciej W. Rozycki wrote:
> Keith, would you care updating your test case then? I'm going to be away
> for two weeks and won't be able to look into it any sooner.
Sure thing.
Keith
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-10 4:48 ` Keith Seitz
@ 2012-03-10 5:11 ` Keith Seitz
2012-03-12 18:13 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Keith Seitz @ 2012-03-10 5:11 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Pedro Alves, gdb-patches
On 03/09/2012 08:48 PM, Keith Seitz wrote:
> On 03/09/2012 06:55 PM, Maciej W. Rozycki wrote:
>> Keith, would you care updating your test case then? I'm going to be away
>> for two weeks and won't be able to look into it any sooner.
>
> Sure thing.
I've changed the test a little bit to safeguard against looping forever.
Pedro, does this look acceptable?
Keith
testsuite/ChangeLog
2012-03-09 Keith Seitz <keiths@redhat.com>
PR gdb/13333
* gdb.server/server-run.exp: Check if gdb crashes when
the inferior and gdbserver exit while stepping.
Index: testsuite/gdb.server/server-run.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.server/server-run.exp,v
retrieving revision 1.12
diff -u -p -r1.12 server-run.exp
--- testsuite/gdb.server/server-run.exp 15 Feb 2012 12:51:17 -0000 1.12
+++ testsuite/gdb.server/server-run.exp 10 Mar 2012 05:08:49 -0000
@@ -51,3 +51,24 @@ if { [istarget *-*-linux*] } {
gdb_breakpoint main
gdb_test "continue" "Breakpoint.* main .*" "continue to main"
+
+# gdb/13333 - Check if gdb crashes while stepping over an
+# inferior exit.
+set max 10
+gdb_test_multiple "next" "step until inferior exits" {
+ -re ".*Inferior 1 \\\(process \[0-9\]+\\\) exited
normally.*$gdb_prompt $" {
+ # The inferior exited, and gdb did not crash.
+ pass "step until inferior exits"
+ }
+
+ -re "$gdb_prompt $" {
+ # We got a prompt -- keep stepping
+ incr max -1
+ if {$max == 0} {
+ unresolved "step until inferior exits"
+ } else {
+ send_gdb "next\n"
+ exp_continue
+ }
+ }
+}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] remote: Fix a crash on longjmp breakpoint removal
2012-03-10 5:11 ` Keith Seitz
@ 2012-03-12 18:13 ` Pedro Alves
0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2012-03-12 18:13 UTC (permalink / raw)
To: Keith Seitz; +Cc: Maciej W. Rozycki, Pedro Alves, gdb-patches
On 03/10/2012 05:11 AM, Keith Seitz wrote:
> On 03/09/2012 08:48 PM, Keith Seitz wrote:
>> On 03/09/2012 06:55 PM, Maciej W. Rozycki wrote:
>>> Keith, would you care updating your test case then? I'm going to be away
>>> for two weeks and won't be able to look into it any sooner.
>>
>> Sure thing.
>
> I've changed the test a little bit to safeguard against looping forever. Pedro, does this look acceptable?
>
I was going to say there's really nothing target specific in the test, and
that thus it's best to put it under gdb.base/ (well I just did :-) ), but then I
remembered Tom's quite new gdb.base/nextoverexit.exp test that tests the exact
same (albeit in an even simpler form). I've checked out a tree from just before
my fix, and run it against nextoverexit.exp w/ gdbserver (didn't exist at the
time, so I copied it over), and indeed GDB crashes.
So we don't really need this new test; it's redundant. Sorry for not realizing
this before you went for the trouble of updating the test... :-/
--
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-03-12 18:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-13 21:13 [PATCH] remote: Fix a crash on longjmp breakpoint removal Maciej W. Rozycki
2011-12-20 16:00 ` Tom Tromey
2012-03-07 20:08 ` Maciej W. Rozycki
2012-03-07 20:47 ` Pedro Alves
2012-03-07 20:48 ` Keith Seitz
2012-03-10 2:55 ` Maciej W. Rozycki
2012-03-10 4:48 ` Keith Seitz
2012-03-10 5:11 ` Keith Seitz
2012-03-12 18:13 ` Pedro Alves
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).