RE: symbolic debug of loadable modules with kgdb light

Caz Yokoyama
2009-10-02T01:18:30Z

[-- Attachment #1: Type: text/plain, Size: 3240 bytes --]

Hello Joel,
Please start the paperwork for filing an assignment with the FSF. I don't
have a employer. This is a volunteer work while on job hunting.
Please find the updated patch in attachment.
- interrupt_sequence_sysrq_g -> interrupt_sequence_break_g
- Linux kernel -> The Linux kernel
- Tested. No regression.
-caz

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com]
Sent: Thursday, October 01, 2009 4:33 PM
To: Caz Yokoyama
Cc: 'Eli Zaretskii'; gdb-patches@sourceware.org; 'Daniel Jacobowitz'; 'Pedro
Alves'
Subject: Re: symbolic debug of loadable modules with kgdb light

> +2009-10-01 Kazuyoshi Caz Yokoyama <caz@caztech.com>
> +
> + * remote.c (interrupt_sequence_control_c)
> + (interrupt_sequence_break, interrupt_sequence_sysrq_g)
> + (interrupt_sequence_modes): New constants.
> + (interrupt_sequence_mode, interrupt_on_connect): New variable.
> + (show_interrupt_sequence): New function.
> + (set_remotebreak, show_remotebreak): New function.
> + (send_interrupt_sequence): New function.
> + (remote_start_remote): Call send_interrupt_sequence if
> + interrupt_on_connect is true.
> + (remote_stop_as): Call send_interrupt_sequence.
> + (_initialize_remote): Add interrupt-sequence and
interrupt-on-connect,
> + modify remotebreak to call set_remotebreak and show_remotebreak.

Very nice and clean! Just a few nits as shown below, but they are minor
and can be mechanically fixed. So the code part of the patch is
pre-approved, after you address the last comments below (it means that
you have permission to commit the patch, but please make sure to send
a copy of the patch that you end up checking in). I think Eli already
approved the latest version of the documentation part, but he'll
probably want to have a last quick look.

And of course, now that I talk about checking in your patch, I am
now only realizing that you do not seem to have an assignment filed
with the FSF. This is a requirement for us to accept your patch.
Perhaps you have an assignment on file through your employer?
I couldn't find you in their records using either your name or
email domain. Let me know if you'd like me to get you started with
the paperwork. It takes a few weeks, so start ASAP if needed.

> +const char interrupt_sequence_sysrq_g[] = "BREAK-g";

You forgot to rename this constant to interrupt_sequence_break_g,
to match the actual litteral value.

> +/* This boolean variable specifies whether interrupt_sequence is sent
> + to remote target when gdb connect to it.
> + This is mostly needed when you debug Linux kernel.
> + Linux kernel expects BREAK g which is Magic SysRq g for connecting
gdb. */

I've noticed a few English mistakes. I started pointed them out, but in
the end, it's probably simpler if I give you the correct version:

/* This boolean variable specifies whether interrupt_sequence is sent
to the remote target when gdb connects to it.
This is mostly needed when you debug theLinux kernel: The Linux kernel
expects BREAK g which is Magic SysRq g for connecting gdb. */

> + struct cmd_list_element *cmd;
> + static char *cmd_name;

This should not be static. That way, it gets allocated on the stack
and then released after the function returns.

--
Joel

[-- Attachment #2: remotebreak.patch --]
[-- Type: application/octet-stream, Size: 12928 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.10924
diff -u -r1.10924 ChangeLog
--- gdb/ChangeLog 1 Oct 2009 20:09:20 -0000 1.10924
+++ gdb/ChangeLog 2 Oct 2009 00:27:20 -0000
@@ -1,3 +1,18 @@
+2009-10-01 Kazuyoshi Caz Yokoyama <caz@caztech.com>
+
+ * remote.c (interrupt_sequence_control_c)
+ (interrupt_sequence_break, interrupt_sequence_break_g)
+ (interrupt_sequence_modes): New constants.
+ (interrupt_sequence_mode, interrupt_on_connect): New variable.
+ (show_interrupt_sequence): New function.
+ (set_remotebreak, show_remotebreak): New function.
+ (send_interrupt_sequence): New function.
+ (remote_start_remote): Call send_interrupt_sequence if
+ interrupt_on_connect is true.
+ (remote_stop_as): Call send_interrupt_sequence.
+ (_initialize_remote): Add interrupt-sequence and interrupt-on-connect,
+ modify remotebreak to call set_remotebreak and show_remotebreak.
+
2009-10-01 Phil Muldoon <pmuldoon@redhat.com>

* infcall.c (call_function_by_hand): Add a new cleanup branch for
Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.331
diff -u -r1.331 NEWS
--- gdb/NEWS 15 Sep 2009 03:30:04 -0000 1.331
+++ gdb/NEWS 2 Oct 2009 00:27:22 -0000
@@ -234,6 +234,24 @@

* New commands (for set/show, see "New options" below)

+set remote interrupt-sequence [Ctrl-C | BREAK | BREAK-g]
+show remote interrupt-sequence
+ Allow the user to select one of ^C, a BREAK signal or BREAK-g
+ as the sequence to the remote target in order to interrupt the execution.
+ Ctrl-C is a default. Some system prefers BREAK which is high level of
+ serial line for some certain time. Linux kernel prefers BREAK-g, a.k.a
+ Magic SysRq g. It is BREAK signal and character 'g'.
+
+set remote interrupt-on-connect [on | off]
+show remote interrupt-on-connect
+ When interrupt-on-connect is ON, gdb sends interrupt-sequence to
+ remote target when gdb connects to it. This is needed when you debug
+ Linux kernel.
+
+set remotebreak [on | off]
+show remotebreak
+Deprecated. Use "set/show remote interrupt-sequence" instead.
+
catch syscall [NAME(S) | NUMBER(S)]
Catch system calls. Arguments, which should be names of system
calls or their numbers, mean catch only those syscalls. Without
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.372
diff -u -r1.372 remote.c
--- gdb/remote.c 10 Sep 2009 22:47:56 -0000 1.372
+++ gdb/remote.c 2 Oct 2009 00:27:25 -0000
@@ -546,14 +546,76 @@
this can go away. */
static int wait_forever_enabled_p = 1;

+/* Allow the user to specify what sequence to send to the remote
+ when he requests a program interruption: Although ^C is usually
+ what remote systems expect (this is the default, here), it is
+ sometimes preferable to send a break. On other systems such
+ as the Linux kernel, a break followed by g, which is Magic SysRq g
+ is required in order to interrupt the execution. */
+const char interrupt_sequence_control_c[] = "Ctrl-C";
+const char interrupt_sequence_break[] = "BREAK";
+const char interrupt_sequence_break_g[] = "BREAK-g";
+static const char *interrupt_sequence_modes[] =
+ {
+ interrupt_sequence_control_c,
+ interrupt_sequence_break,
+ interrupt_sequence_break_g,
+ NULL
+ };
+static const char *interrupt_sequence_mode = interrupt_sequence_control_c;

-/* This variable chooses whether to send a ^C or a break when the user
- requests program interruption. Although ^C is usually what remote
- systems expect, and that is the default here, sometimes a break is
- preferable instead. */
+static void
+show_interrupt_sequence (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ if (interrupt_sequence_mode == interrupt_sequence_control_c)
+ fprintf_filtered (file,
+ _("Send the ASCII ETX character (Ctrl-c) "
+ "to the remote target to interrupt the "
+ "execution of the program.\n"));
+ else if (interrupt_sequence_mode == interrupt_sequence_break)
+ fprintf_filtered (file,
+ _("send a break signal to the remote target "
+ "to interrupt the execution of the program.\n"));
+ else if (interrupt_sequence_mode == interrupt_sequence_break_g)
+ fprintf_filtered (file,
+ _("Send a break signal and 'g' a.k.a. Magic SysRq g to "
+ "the remote target to interrupt the execution "
+ "of Linux kernel.\n"));
+ else
+ internal_error (__FILE__, __LINE__,
+ _("Invalid value for interrupt_sequence_mode: %s."),
+ interrupt_sequence_mode);
+}

+/* This boolean variable specifies whether interrupt_sequence is sent
+ to the remote target when gdb connects to it.
+ This is mostly needed when you debug the Linux kernel: The Linux kernel
+ expects BREAK g which is Magic SysRq g for connecting gdb. */
+static int interrupt_on_connect = 0;
+
+/* This variable is used to implement the "set/show remotebreak" commands.
+ Since these commands are now deprecated in favor of "set/show remote
+ interrupt-sequence", it no longer has any effect on the code. */
static int remote_break;

+static void
+set_remotebreak (char *args, int from_tty, struct cmd_list_element *c)
+{
+ if (remote_break)
+ interrupt_sequence_mode = interrupt_sequence_break;
+ else
+ interrupt_sequence_mode = interrupt_sequence_control_c;
+}
+
+static void
+show_remotebreak (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+}
+
/* Descriptor for I/O to remote machine. Initialize it to NULL so that
remote_open knows that we don't have a file open when the program
starts. */
@@ -2585,6 +2647,25 @@
int extended_p;
};

+/* Send interrupt_sequence to remote target. */
+static void
+send_interrupt_sequence ()
+{
+ if (interrupt_sequence_mode == interrupt_sequence_control_c)
+ serial_write (remote_desc, "\x03", 1);
+ else if (interrupt_sequence_mode == interrupt_sequence_break)
+ serial_send_break (remote_desc);
+ else if (interrupt_sequence_mode == interrupt_sequence_break_g)
+ {
+ serial_send_break (remote_desc);
+ serial_write (remote_desc, "g", 1);
+ }
+ else
+ internal_error (__FILE__, __LINE__,
+ _("Invalid value for interrupt_sequence_mode: %s."),
+ interrupt_sequence_mode);
+}
+
static void
remote_start_remote (struct ui_out *uiout, void *opaque)
{
@@ -2598,6 +2679,9 @@
/* Ack any packet which the remote side has already sent. */
serial_write (remote_desc, "+", 1);

+ if (interrupt_on_connect)
+ send_interrupt_sequence ();
+
/* The first packet we send to the target is the optional "supported
packets" request. If the target can answer this, it will tell us
which later probes to skip. */
@@ -4021,12 +4105,8 @@
if (rs->cached_wait_status)
return;

- /* Send a break or a ^C, depending on user preference. */
-
- if (remote_break)
- serial_send_break (remote_desc);
- else
- serial_write (remote_desc, "\003", 1);
+ /* Send interrupt_sequence to remote target. */
+ send_interrupt_sequence ();
}

/* This is the generic stop called via the target vector. When a target
@@ -8993,6 +9073,8 @@
_initialize_remote (void)
{
struct remote_state *rs;
+ struct cmd_list_element *cmd;
+ char *cmd_name;

/* architecture specific data */
remote_gdbarch_data_handle =
@@ -9060,8 +9142,31 @@
Set whether to send break if interrupted."), _("\
Show whether to send break if interrupted."), _("\
If set, a break, instead of a cntrl-c, is sent to the remote target."),
- NULL, NULL, /* FIXME: i18n: Whether to send break if interrupted is %s. */
+ set_remotebreak, show_remotebreak,
&setlist, &showlist);
+ cmd_name = "remotebreak";
+ cmd = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+ deprecate_cmd (cmd, "set remote interrupt-sequence");
+ cmd_name = "remotebreak"; /* needed because lookup_cmd updates the pointer */
+ cmd = lookup_cmd (&cmd_name, showlist, "", -1, 1);
+ deprecate_cmd (cmd, "show remote interrupt-sequence");
+
+ add_setshow_enum_cmd ("interrupt-sequence", class_support,
+ interrupt_sequence_modes, &interrupt_sequence_mode, _("\
+Set interrupt sequence to remote target."), _("\
+Show interrupt sequence to remote target."), _("\
+Valid value is \"Ctrl-C\", \"BREAK\" or \"BREAK-g\". The default is \"Ctrl-C\"."),
+ NULL, show_interrupt_sequence,
+ &remote_set_cmdlist,
+ &remote_show_cmdlist);
+
+ add_setshow_boolean_cmd ("interrupt-on-connect", class_support,
+ &interrupt_on_connect, _("\
+Set whether interrupt-sequence is sent to remote target when gdb connects to."), _(" \
+Show whether interrupt-sequence is sent to remote target when gdb connects to."), _(" \
+If set, interrupt sequence is sent to remote target."),
+ NULL, NULL,
+ &remote_set_cmdlist, &remote_show_cmdlist);

/* Install commands for configuring memory read/write packets. */

Index: gdb/doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.958
diff -u -r1.958 ChangeLog
--- gdb/doc/ChangeLog 26 Sep 2009 16:47:13 -0000 1.958
+++ gdb/doc/ChangeLog 2 Oct 2009 00:27:28 -0000
@@ -1,3 +1,8 @@
+2009-10-01 Kazuyoshi Caz Yokoyama <caz@caztech.com>
+
+ * gdb.texinfo (Remote Configuration): Add "set/show remote
+ interrupt-sequence" and "set/show remote interrupt-on-connect" command.
+
2009-09-26 Pierre Muller <muller@ics.u-strasbg.fr>

* gdb.texinfo (Cygwin Native): Mention support for Ctrl-BREAK.
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.629
diff -u -r1.629 gdb.texinfo
--- gdb/doc/gdb.texinfo 26 Sep 2009 16:47:13 -0000 1.629
+++ gdb/doc/gdb.texinfo 2 Oct 2009 00:27:40 -0000
@@ -15011,6 +15011,34 @@
target system. If it is not set, the target will use a default
filename (e.g.@: the last program run).

+@item set remote interrupt-sequence
+@cindex interrupt remote programs
+@cindex select Ctrl-C, BREAK or BREAK-g
+Allow the user to select one of @samp{Ctrl-C}, a @code{BREAK} or
+@samp{BREAK-g} as the
+sequence to the remote target in order to interrupt the execution.
+@samp{Ctrl-C} is a default. Some system prefers @code{BREAK} which
+is high level of serial line for some certain time.
+Linux kernel prefers @samp{BREAK-g}, a.k.a Magic SysRq g.
+It is @code{BREAK} signal followed by character @code{g}.
+
+@item show interrupt-sequence
+Show which of @samp{Ctrl-C}, @code{BREAK} or @code{BREAK-g}
+is sent by @value{GDBN} to interrupt the remote program.
+@code{BREAK-g} is BREAK signal followed by @code{g} and
+also known as Magic SysRq g.
+
+@item set remote interrupt-on-connect
+@cindex send interrupt-sequence on start
+Specify whether interrupt-sequence is sent to remote target when
+@value{GDBN} connects to it. This is mostly needed when you debug
+Linux kernel. Linux kernel expects @code{BREAK} followed by @code{g}
+which is known as Magic SysRq g in order to connect @value{GDBN}.
+
+@item show interrupt-on-connect
+Show whether interrupt-sequence is sent
+to remote target when @value{GDBN} connects to it.
+
@kindex set tcp
@kindex show tcp
@item set tcp auto-retry on
@@ -29817,9 +29845,9 @@
@cindex interrupts (remote protocol)

When a program on the remote target is running, @value{GDBN} may
-attempt to interrupt it by sending a @samp{Ctrl-C} or a @code{BREAK},
-control of which is specified via @value{GDBN}'s @samp{remotebreak}
-setting (@pxref{set remotebreak}).
+attempt to interrupt it by sending a @samp{Ctrl-C}, @code{BREAK} or
+a @code{BREAK} followed by @code{g},
+control of which is specified via @value{GDBN}'s @samp{interrupt-sequence}.

The precise meaning of @code{BREAK} is defined by the transport
mechanism and may, in fact, be undefined. @value{GDBN} does not
@@ -29836,6 +29864,10 @@
(@pxref{X packet}), used for binary downloads, may include an unescaped
@code{0x03} as part of its packet.

+@code{BREAK} followed by @code{g} is also known as Magic SysRq g.
+When Linux kernel receives this sequence from serial port,
+it stops execution and connects to gdb.
+
Stubs are not required to recognize these interrupt mechanisms and the
precise meaning associated with receipt of the interrupt is
implementation defined. If the target supports debugging of multiple