RE: symbolic debug of loadable modules with kgdb light

Caz Yokoyama
2009-05-15T21:34:45Z

Hello Pedro,
SysRq expects 1) Ctrl+AltSysRq and "g" from console, or 2) BREAK and "g"
from serial port. I am using 2).
-caz

-----Original Message-----
From: Pedro Alves [mailto:pedro@codesourcery.com]
Sent: Friday, May 15, 2009 2:24 PM
To: gdb-patches@sourceware.org
Cc: Caz Yokoyama; tromey@redhat.com; 'Joel Brobecker'
Subject: Re: symbolic debug of loadable modules with kgdb light

Sounds like you need to fix kgdb instead. Why would it need
a 'g' on connection?

On Friday 15 May 2009 22:15:31, Caz Yokoyama wrote:
> Hello Tom,
> Here is the patch for 2) which enables "break-in Linux kernel 2.6.26 and
> later which kgdb is enabled. When linux-kgdb is set (i.e. not 0), gdb
sends
> BREAK and g when gdb is started and when ^C is typed on gdb.
>
> gdb/main.c - define linux_kgdb
> gdb/remote.c - sends BREAK and g when gdb is started and when ^C is tyed
on
> gdb.
>
> All these modification comes from me.
>
> Index: gdb/main.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/main.c,v
> retrieving revision 1.76
> diff -p -r1.76 main.c
> *** gdb/main.c 27 Apr 2009 10:24:08 -0000 1.76
> --- gdb/main.c 14 May 2009 17:00:05 -0000
> *************** char *gdb_sysroot = 0;
> *** 67,72 ****
> --- 67,75 ----
> /* GDB datadir, used to store data files. */
> char *gdb_datadir = 0;
>
> + /* Whether debugging Linux kernel by using its kgdb */ int linux_kgdb
> + = 0;
> +
> struct ui_file *gdb_stdout;
> struct ui_file *gdb_stderr;
> struct ui_file *gdb_stdlog;
> Index: gdb/remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.354
> diff -p -r1.354 remote.c
> *** gdb/remote.c 16 Apr 2009 19:31:03 -0000 1.354
> --- gdb/remote.c 14 May 2009 17:00:08 -0000
> *************** static struct cmd_list_element *remote_c
> *** 239,244 ****
> --- 239,246 ----
> static struct cmd_list_element *remote_set_cmdlist;
> static struct cmd_list_element *remote_show_cmdlist;
>
> + extern int linux_kgdb;
> +
> /* Description of the remote protocol state for the currently
> connected target. This is per-target state, and independent of the
> selected architecture. */
> *************** remote_start_remote (struct ui_out *uiou
> *** 2606,2611 ****
> --- 2608,2618 ----
> /* Ack any packet which the remote side has already sent. */
> serial_write (remote_desc, "+", 1);
>
> + if (linux_kgdb) {
> + serial_send_break(remote_desc);
> + serial_write(remote_desc, "g", 1);
> + }
> +
> /* 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. */
> *************** remote_stop_as (ptid_t ptid)
> *** 4020,4029 ****
>
> /* 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);
> }
>
> /* This is the generic stop called via the target vector. When a target
> --- 4027,4041 ----
>
> /* Send a break or a ^C, depending on user preference. */
>
> ! if (linux_kgdb) {
> serial_send_break (remote_desc);
> ! serial_write (remote_desc, "g", 1);
> ! } else {
> ! if (remote_break)
> ! serial_send_break (remote_desc);
> ! else
> ! serial_write (remote_desc, "\003", 1);
> ! }
> }
>
> /* This is the generic stop called via the target vector. When a target
> *************** If set, a break, instead of a cntrl-c, i
> *** 9063,9068 ****
> --- 9075,9087 ----
> NULL, NULL, /* FIXME: i18n: Whether to send break
> if interrupted is %s. */
> &setlist, &showlist);
>
> + add_setshow_boolean_cmd ("linux-kgdb", no_class, &linux_kgdb, _("\
> + Set whether Linux kernel is debugged by using kgdb."), _("\ Show
> + whether Linux kernel is debugged by using kgdb."), _("\ If set, send a
> + break and g to the remote target to break-in."),
> + NULL, NULL,
> + &setlist, &showlist);
> +
> /* Install commands for configuring memory read/write packets. */
>
> add_cmd ("remotewritesize", no_class, set_memory_write_packet_size,
_("\
>
> I am working for a patch for 1).
> -caz
>
> -----Original Message-----
> From: Caz Yokoyama [mailto:caz@caztech.com]
> Sent: Saturday, April 25, 2009 5:36 PM
> To: 'tromey@redhat.com'
> Cc: 'Joel Brobecker'; 'gdb-patches@sourceware.org'
> Subject: RE: symbolic debug of loadable modules with kgdb light
>
> Correction
> kgdb light expects BREAK and then ^C.
> ---------
> kgdb light expects BREAK and then g.
> -caz
>
> -----Original Message-----
> From: Caz Yokoyama [mailto:caz@caztech.com]
> Sent: Friday, April 24, 2009 9:48 AM
> To: 'tromey@redhat.com'
> Cc: 'Joel Brobecker'; 'gdb-patches@sourceware.org'
> Subject: RE: symbolic debug of loadable modules with kgdb light
>
> Hello Tom,
> Thank you for pay attention for my patch.
> Have you read following discussion about this? There are 2 points my patch
> add to, 1) symbolic debug of loadable modules with kgdb light, 2) break-in
> by ^C remotely. For 1), Ruby support is integrating into gdb. By using
that,
> it access /proc file system and incorporate a symbol table for a loadable
> module. Someone told me approach by Ruby is more attractive. So, my
> understanding of this is my modification is useless because of Ruby
support.
> BTW, I had kept my modification against cvs source of gdb instead of 6.8.
>
> For 2), gdb emits BREAK or ^C depending when ^C is typed. On the other
hand,
> kgdb light expects BREAK and then ^C. I introduce debugkernel. When
> debugkernel is true, BREAK and then ^C is emitted to target. Someone said
> more elegant scheme is needed instead of debugkernel. So, I lost interest.
> BTW, this is all of my code and less than 10 lines of code modification.
> -caz
>
> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, April 24, 2009 8:32 AM
> To: Caz Yokoyama
> Cc: 'Joel Brobecker'; gdb-patches@sourceware.org
> Subject: Re: symbolic debug of loadable modules with kgdb light
>
> >>>>> "Caz" == Caz Yokoyama <cazyokoyama@gmail.com> writes:
>
> Caz> I have attached the patch against gdb-6.8.
>
> Thanks.
>
> Caz> - This patch is based on
> Caz> http://kgdb.cvs.sourceforge.net/viewvc/kgdb/gdb/. I removed
> Caz> garbage as mush as possible. But it still has the code which I
> Caz> don't know what it is. I don't remove copyright notice which is
> Caz> there.
>
> Do you have copyright assignment papers on file with the FSF? If not,
> let me know and I can get you started on the process. This is a
> requirement for getting any code into gdb.
>
> Did you write this entire patch yourself? I just want to make sure.
> If not, we'll need to get papers from any contributor who wrote more
> than 10 lines of code in the patch.
>
> Caz> - I haven't run testsuite because I could not find how to do that
while
> I
> Caz> run make in testsuite directory.
>
> "make check". You need dejagnu installed. And you actually have to
> run a baseline check without your patch applied, then compare the
> results.
>
> Caz> - I believe that the code follows coding standard. Let me know if
> Caz> not.
>
> I noticed a number of formatting nits. These are no big deal.
>
> More importantly, the patch makes a bunch of apparently
> kernel-debugging-specific changes to generic code. I did not try to
> read it very closely, but basically all of these will need to be cleaned
> up. Some of the hunks we definitely do not want; e.g., moving
> struct value into value.h.
>
> Tom
>
>

--
Pedro Alves