RE: symbolic debug of loadable modules with kgdb light

[Caz Yokoyama <cazyokoyama at gmail dot com>]

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