* [PATCH] fix error sending signal to dead process
@ 2012-04-05 12:48 Patrick Monnerat
2012-04-19 0:22 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Monnerat @ 2012-04-05 12:48 UTC (permalink / raw)
To: insight
[-- Attachment #1: Type: text/plain, Size: 677 bytes --]
Using insight-7.4.50 (cvs snapshot 20120403), continuing or stepping
after a fault (i.e. segfault) occurred results in sending a signal to
the dead process.
This is due to hook gdbtk_annotate_signal() trying to get thread_info
structure with a null pid.
Continuing in the same insight session (by re-running the program)
finally ends in insight segfaulting.
The attached patch fixes this problem. It simply do not pass signal to
tcl if the pid is null.
Cheers,
Patrick
P.S.: the bug report system linked from your web site
(http://sources.redhat.com/cgi-bin/gnatsweb.pl?database=insight&user=gue
st&password=guest&cmd=login) is down (HTTP error 500).
[-- Attachment #2: insight-7.4.50-sig2dead.patch --]
[-- Type: application/octet-stream, Size: 912 bytes --]
diff -Naur insight-7.4.50.orig/gdb/gdbtk/generic/gdbtk-hooks.c insight-7.4.50.new/gdb/gdbtk/generic/gdbtk-hooks.c
--- insight-7.4.50.orig/gdb/gdbtk/generic/gdbtk-hooks.c 2012-03-28 15:09:12.000000000 +0200
+++ insight-7.4.50.new/gdb/gdbtk/generic/gdbtk-hooks.c 2012-04-05 12:44:20.284306992 +0200
@@ -804,7 +804,7 @@
gdbtk_annotate_signal (void)
{
char *buf;
- struct thread_info *tp = inferior_thread ();
+ struct thread_info *tp;
/* Inform gui that the target has stopped. This is
a necessary stop button evil. We don't want signal notification
@@ -812,6 +812,11 @@
timeout. */
Tcl_Eval (gdbtk_interp, "gdbtk_stop_idle_callback");
+ if (ptid_equal(inferior_ptid, null_ptid))
+ return;
+
+ tp = inferior_thread ();
+
buf = xstrprintf ("gdbtk_signal %s {%s}",
target_signal_to_name (tp->suspend.stop_signal),
target_signal_to_string (tp->suspend.stop_signal));
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix error sending signal to dead process
2012-04-05 12:48 [PATCH] fix error sending signal to dead process Patrick Monnerat
@ 2012-04-19 0:22 ` Keith Seitz
2012-04-19 15:19 ` Patrick Monnerat
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2012-04-19 0:22 UTC (permalink / raw)
To: Patrick Monnerat; +Cc: insight
On 04/05/2012 05:45 AM, Patrick Monnerat wrote:
> The attached patch fixes this problem. It simply do not pass signal to
> tcl if the pid is null.
A nit or two, comments below.
> P.S.: the bug report system linked from your web site
> (http://sources.redhat.com/cgi-bin/gnatsweb.pl?database=insight&user=gue
> st&password=guest&cmd=login) is down (HTTP error 500).
Gah! It looks like sourceware is now 100% bugzilla, but insight was
never "converted" (aka I thought the "old" bug database was still
around). I'll update the page to ask viewers to submit bugs here for the
interim. Thank you for bringing this to my attention.
> diff -Naur insight-7.4.50.orig/gdb/gdbtk/generic/gdbtk-hooks.c insight-7.4.50.new/gdb/gdbtk/generic/gdbtk-hooks.c
> --- insight-7.4.50.orig/gdb/gdbtk/generic/gdbtk-hooks.c 2012-03-28 15:09:12.000000000 +0200
> +++ insight-7.4.50.new/gdb/gdbtk/generic/gdbtk-hooks.c 2012-04-05 12:44:20.284306992 +0200
> @@ -804,7 +804,7 @@
> gdbtk_annotate_signal (void)
> {
> char *buf;
> - struct thread_info *tp = inferior_thread ();
> + struct thread_info *tp;
>
> /* Inform gui that the target has stopped. This is
> a necessary stop button evil. We don't want signal notification
> @@ -812,6 +812,11 @@
> timeout. */
> Tcl_Eval (gdbtk_interp, "gdbtk_stop_idle_callback");
>
> + if (ptid_equal(inferior_ptid, null_ptid))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
There should be spaces between function names and opening parenthesis.
[Yeah, it's rather lame, but it is GNU coding standard. Insight follows
GDB's conventions.]
ChangeLog entry?
> + return;
> +
> + tp = inferior_thread ();
> +
> buf = xstrprintf ("gdbtk_signal %s {%s}",
> target_signal_to_name (tp->suspend.stop_signal),
> target_signal_to_string (tp->suspend.stop_signal));
[Aside: Eew. I can't believe Tcl_Eval is still being used!]
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] fix error sending signal to dead process
2012-04-19 0:22 ` Keith Seitz
@ 2012-04-19 15:19 ` Patrick Monnerat
2012-04-19 16:59 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Monnerat @ 2012-04-19 15:19 UTC (permalink / raw)
To: Keith Seitz; +Cc: insight
[-- Attachment #1: Type: text/plain, Size: 677 bytes --]
Keith Seitz wrote:
> There should be spaces between function names and opening parenthesis.
> [Yeah, it's rather lame, but it is GNU coding standard. Insight
follows GDB's conventions.]
> ChangeLog entry?
The new attached patch fixes both of your remarks. It has been made
against today's cvs snapshot.
> [Aside: Eew. I can't believe Tcl_Eval is still being used!]
Well... I'm not enough involved in insight's guts and tcl to share your
estonishment. You're probably right. But I can say at least this call is
clear enough to understand what's happening :-)
In fact I'm only the insight's packager for Fedora.
Thanks for your help.
Cheers,
Patrick
[-- Attachment #2: insight-7.4.50-sig2dead.patch --]
[-- Type: application/octet-stream, Size: 1362 bytes --]
diff -Naur src.orig/gdb/gdbtk/ChangeLog src.new/gdb/gdbtk/ChangeLog
--- src.orig/gdb/gdbtk/ChangeLog 2012-04-19 11:41:24.000000000 +0200
+++ src.new/gdb/gdbtk/ChangeLog 2012-04-19 17:04:04.601462401 +0200
@@ -1,3 +1,8 @@
+2012-04-19 Patrick Monnerat <pm@datasphere.ch>
+
+ * generic/gdbtk-hooks.c (gdbtk_annotate_signal): Avoid
+ dereferencing a null (i.e.: dead) process/thread.
+
2012-04-19 Roland Schwingel <roland.schwingel@onevision.com>
* generic/gdbtk-bp.c: Updated copyright.
diff -Naur src.orig/gdb/gdbtk/generic/gdbtk-hooks.c src.new/gdb/gdbtk/generic/gdbtk-hooks.c
--- src.orig/gdb/gdbtk/generic/gdbtk-hooks.c 2012-03-28 15:09:12.000000000 +0200
+++ src.new/gdb/gdbtk/generic/gdbtk-hooks.c 2012-04-19 16:57:27.929884730 +0200
@@ -804,7 +804,7 @@
gdbtk_annotate_signal (void)
{
char *buf;
- struct thread_info *tp = inferior_thread ();
+ struct thread_info *tp;
/* Inform gui that the target has stopped. This is
a necessary stop button evil. We don't want signal notification
@@ -812,6 +812,11 @@
timeout. */
Tcl_Eval (gdbtk_interp, "gdbtk_stop_idle_callback");
+ if (ptid_equal (inferior_ptid, null_ptid))
+ return;
+
+ tp = inferior_thread ();
+
buf = xstrprintf ("gdbtk_signal %s {%s}",
target_signal_to_name (tp->suspend.stop_signal),
target_signal_to_string (tp->suspend.stop_signal));
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix error sending signal to dead process
2012-04-19 15:19 ` Patrick Monnerat
@ 2012-04-19 16:59 ` Keith Seitz
2012-04-19 17:51 ` Patrick Monnerat
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2012-04-19 16:59 UTC (permalink / raw)
To: Patrick Monnerat; +Cc: insight
On 04/19/2012 08:19 AM, Patrick Monnerat wrote:
> The new attached patch fixes both of your remarks. It has been made
> against today's cvs snapshot.
Perfect! Please commit.
> In fact I'm only the insight's packager for Fedora.
I only just recently learned that this had been done, despite the fact
that all my computers run Fedora! So, thank you for taking the
initiative here!
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] fix error sending signal to dead process
2012-04-19 16:59 ` Keith Seitz
@ 2012-04-19 17:51 ` Patrick Monnerat
2012-04-19 19:50 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Monnerat @ 2012-04-19 17:51 UTC (permalink / raw)
To: Keith Seitz; +Cc: insight
Keith Seitz wrote:
> Perfect! Please commit.
I think you or someone else have to commit: AFAIK, I only have anonymous
read access to the CVS...
Thanks for doing it :-)
Cheers,
Patrick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix error sending signal to dead process
2012-04-19 17:51 ` Patrick Monnerat
@ 2012-04-19 19:50 ` Keith Seitz
0 siblings, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2012-04-19 19:50 UTC (permalink / raw)
To: Patrick Monnerat; +Cc: insight
On 04/19/2012 10:50 AM, Patrick Monnerat wrote:
> I think you or someone else have to commit: AFAIK, I only have anonymous
> read access to the CVS...
> Thanks for doing it :-)
I've committed your patch. Thanks again for your help!
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-19 19:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-05 12:48 [PATCH] fix error sending signal to dead process Patrick Monnerat
2012-04-19 0:22 ` Keith Seitz
2012-04-19 15:19 ` Patrick Monnerat
2012-04-19 16:59 ` Keith Seitz
2012-04-19 17:51 ` Patrick Monnerat
2012-04-19 19:50 ` Keith Seitz
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).