[PATCH/RFC] *BSD kernel debugging

[PATCH/RFC] *BSD kernel debugging

[Mark Kettenis]

FreeBSD, NetBSD and OpenBSD all provide the kvm(3) interface for
debugging kernel virtual memory images: kernel crash dumps and live
kernels.  All three include support for this interface in the version
of GDB bundled with the OS, but this code was never contributed back.

I've recently implemented support for kvm(3)-based debugging that
works for all three BSD's.  The interface is fairly simple, just start
GDB on a kernel binary, i.e.

# gdb /bsd
GNU gdb 6.1
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "x86_64-unknown-openbsd3.5"...
(gdb) target kvm
0xffffffff8019c308 in mi_switch ()
(gdb) bt
#0  0xffffffff8019c308 in mi_switch ()
#1  0xffffffff8019baea in ltsleep ()
#2  0xffffffff802a1246 in uvm_scheduler ()
#3  0xffffffff8018ae44 in main ()

The command "target kvm" accepts the name of a kernel dump as an
optional third argument; without it, it uses the memory image of the
currently running kernel.  

All that's needed is a bit of new code (bsd-kvm.[ch]) and a support
function in the appropriate *-nat.c file; because it is built on top
of kvm(3) this is native-only.  I've added a preliminary patch with
some sample code.

If there are no objections I'll check this in in a week or so.

Mark

--- /dev/null	Mon May 17 13:15:56 2004
+++ bsd-kvm.c	Fri Apr 30 15:16:41 2004
@@ -0,0 +1,211 @@
+/* BSD Kernel Data Access Library (libkvm) interface.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include "defs.h"
+#include "frame.h"
+#include "regcache.h"
+#include "target.h"
+
+#include "gdb_assert.h"
+#include <fcntl.h>
+#include <kvm.h>
+#include <nlist.h>
+#include "readline/readline.h"
+#include <sys/param.h>
+#include <sys/user.h>
+
+#include "bsd-kvm.h"
+
+/* Kernel memory interface descriptor.  */
+kvm_t *core_kd;
+
+/* Address of process control block.  */
+struct pcb *bsd_kvm_paddr;
+
+/* Target ops for libkvm interface.  */
+struct target_ops bsd_kvm_ops;
+
+static void
+bsd_kvm_open (char *filename, int from_tty)
+{
+  char errbuf[_POSIX2_LINE_MAX];
+  char *execfile = NULL;
+  kvm_t *temp_kd;
+
+  target_preopen (from_tty);
+
+  if (filename)
+    {
+      char *temp;
+
+      filename = tilde_expand (filename);
+      if (filename[0] != '/')
+	{
+	  temp = concat (current_directory, "/", filename, NULL);
+	  xfree (filename);
+	  filename = temp;
+	}
+    }
+
+  temp_kd = kvm_openfiles (execfile, filename, NULL, O_RDONLY, errbuf);
+  if (temp_kd == NULL)
+    error ("%s", errbuf);
+
+  unpush_target (&bsd_kvm_ops);
+  core_kd = temp_kd;
+  push_target (&bsd_kvm_ops);
+
+  target_fetch_registers (-1);
+
+  flush_cached_frames ();
+  select_frame (get_current_frame ());
+  print_stack_frame (get_selected_frame (), -1, 1);
+}
+
+static void
+bsd_kvm_close (int quitting)
+{
+  if (core_kd)
+    {
+      if (kvm_close (core_kd) == -1)
+	warning ("%s", kvm_geterr(core_kd));
+      core_kd = NULL;
+    }
+}
+
+static int
+bsd_kvm_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len,
+		    int write, struct mem_attrib *attrib,
+		    struct target_ops *ops)
+{
+  if (write)
+    return kvm_write (core_kd, memaddr, myaddr, len);
+  else
+    return kvm_read (core_kd, memaddr, myaddr, len);
+
+  return -1;
+}
+
+static int
+bsd_kvm_fetch_pcb (struct pcb *paddr)
+{
+  struct pcb pcb;
+
+  if (kvm_read (core_kd, (unsigned long) paddr, &pcb, sizeof pcb) == -1)
+    error ("%s", kvm_geterr (core_kd));
+
+  gdb_assert (bsd_kvm_supply_pcb);
+  return bsd_kvm_supply_pcb (current_regcache, &pcb);
+}
+
+static void
+bsd_kvm_fetch_registers (int regnum)
+{
+  struct nlist nl[2];
+
+  if (bsd_kvm_paddr)
+    bsd_kvm_fetch_pcb (bsd_kvm_paddr);
+
+  /* On dumping core, BSD kernels store the faulting context (PCB)
+     in the variable "dumppcb".  */
+  memset (nl, 0, sizeof nl);
+  nl[0].n_name = "_dumppcb";
+
+  if (kvm_nlist (core_kd, nl) == -1)
+    error ("%s", kvm_geterr (core_kd));
+
+  if (nl[0].n_value != 0)
+    {
+      /* Found dumppcb. If it contains a valid context, return
+	 immediately.  */
+      if (bsd_kvm_fetch_pcb ((struct pcb *) nl[0].n_value))
+	return;
+    }
+
+  /* Traditional BSD kernels have a process proc0 that should always
+     be present.  The address of proc0's PCB is stored in the variable
+     "proc0paddr".  */
+
+  memset (nl, 0, sizeof nl);
+  nl[0].n_name = "_proc0paddr";
+
+  if (kvm_nlist (core_kd, nl) == -1)
+    error ("%s", kvm_geterr (core_kd));
+
+  if (nl[0].n_value != 0)
+    {
+      struct pcb *paddr;
+
+      /* Found proc0paddr.  */
+      if (kvm_read (core_kd, nl[0].n_value, &paddr, sizeof paddr) == -1)
+	error ("%s", kvm_geterr (core_kd));
+
+      bsd_kvm_fetch_pcb (paddr);
+      return;
+    }
+
+#ifdef HAVE_STRUCT_THREAD_TD_PCB
+  /* In FreeBSD kernels for 5.0-RELEASE and later, the PCB no longer
+     lives in `struct proc' but in `struct thread'.  The `struct
+     thread' for the initial thread for proc0 can be found in the
+     variable "thread0".  */
+
+  memset (nl, 0, sizeof nl);
+  nl[0].n_name = "_thread0";
+
+  if (kvm_nlist (core_kd, nl) == -1)
+    error ("%s", kvm_geterr (core_kd));
+
+  if (nl[0].n_value != 0)
+    {
+      struct pcb *paddr;
+
+      /* Found thread0.  */
+      nl[1].n_value += offsetof (struct thread, td_pcb);
+      if (kvm_read (core_kd, nl[1].n_value, &paddr, sizeof paddr) == -1)
+	error ("%s", kvm_geterr (core_kd));
+
+      bsd_kvm_fetch_pcb (paddr);
+      return;
+    }
+#endif
+
+  error ("Cannot find a valid PCB");
+}
+
+void
+_initialize_bsd_kvm (void)
+{
+  bsd_kvm_ops.to_shortname = "kvm";
+  bsd_kvm_ops.to_longname = "Kernel memory interface";
+  bsd_kvm_ops.to_doc = "XXX";
+  bsd_kvm_ops.to_open = bsd_kvm_open;
+  bsd_kvm_ops.to_close = bsd_kvm_close;
+  bsd_kvm_ops.to_fetch_registers = bsd_kvm_fetch_registers;
+  bsd_kvm_ops.to_xfer_memory = bsd_kvm_xfer_memory;
+  bsd_kvm_ops.to_stratum = process_stratum;
+  bsd_kvm_ops.to_has_memory = 1;
+  bsd_kvm_ops.to_has_stack = 1;
+  bsd_kvm_ops.to_has_registers = 1;
+  bsd_kvm_ops.to_magic = OPS_MAGIC;
+
+  add_target (&bsd_kvm_ops);
+}
--- /dev/null	Mon May 17 13:15:56 2004
+++ bsd-kvm.h	Fri Apr 30 13:46:54 2004
@@ -0,0 +1,30 @@
+/* BSD Kernel Data Access Library (libkvm) interface.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#ifndef BSD_KVM_H
+#define BSD_KVM_H
+
+struct pcb;
+struct regcache;
+
+extern int bsd_kvm_supply_pcb (struct regcache *regache, struct pcb *pcb);
+
+#endif /* bsd-kvm.h */
Index: amd64fbsd-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64fbsd-nat.c,v
retrieving revision 1.13
diff -u -p -r1.13 amd64fbsd-nat.c
--- amd64fbsd-nat.c	4 Mar 2004 10:35:57 -0000	1.13
+++ amd64fbsd-nat.c	17 May 2004 11:18:55 -0000
@@ -145,6 +145,47 @@ fill_fpregset (fpregset_t *fpregsetp, in
 }
 \f
 
+/* Support for debugging kernel virtual memory images.  */
+
+#include <sys/types.h>
+#include <machine/pcb.h>
+
+#include "bsd-kvm.h"
+
+int
+bsd_kvm_supply_pcb (struct regcache *regcache, struct pcb *pcb)
+{
+  /* The following is true for FreeBSD 5.2:
+
+     The pcb contains %rip, %rbx, %rsp, %rbp, %r12, %r13, %r14, %r15,
+     %ds, %es, %fs and %gs.  This accounts for all callee-saved
+     registers specified by the psABI and then some.  Here %esp
+     contains the stack pointer at the point just after the call to
+     cpu_switch().  From this information we reconstruct the register
+     state as it would like when we just returned from cpu_switch().  */
+
+  /* The stack pointer shouldn't be zero.  */
+  if (pcb->pcb_rsp == 0)
+    return 0;
+
+  pcb->pcb_rsp += 8;
+  regcache_raw_supply (regcache, AMD64_RIP_REGNUM, &pcb->pcb_rip);
+  regcache_raw_supply (regcache, AMD64_RBX_REGNUM, &pcb->pcb_rbx);
+  regcache_raw_supply (regcache, AMD64_RSP_REGNUM, &pcb->pcb_rsp);
+  regcache_raw_supply (regcache, AMD64_RBP_REGNUM, &pcb->pcb_rbp);
+  regcache_raw_supply (regcache, 12, &pcb->pcb_r12);
+  regcache_raw_supply (regcache, 13, &pcb->pcb_r13);
+  regcache_raw_supply (regcache, 14, &pcb->pcb_r14);
+  regcache_raw_supply (regcache, 15, &pcb->pcb_r15);
+  regcache_raw_supply (regcache, AMD64_DS_REGNUM, &pcb->pcb_ds);
+  regcache_raw_supply (regcache, AMD64_ES_REGNUM, &pcb->pcb_es);
+  regcache_raw_supply (regcache, AMD64_FS_REGNUM, &pcb->pcb_fs);
+  regcache_raw_supply (regcache, AMD64_GS_REGNUM, &pcb->pcb_gs);
+
+  return 1;
+}
+\f
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_amd64fbsd_nat (void);
 
Index: amd64obsd-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64obsd-nat.c,v
retrieving revision 1.2
diff -u -p -r1.2 amd64obsd-nat.c
--- amd64obsd-nat.c	25 Feb 2004 20:59:12 -0000	1.2
+++ amd64obsd-nat.c	17 May 2004 11:18:55 -0000
@@ -20,6 +20,8 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
+#include "gdbcore.h"
+#include "regcache.h"
 
 #include "gdb_assert.h"
 
@@ -54,6 +56,60 @@ static int amd64obsd32_r_reg_offset[] =
   22 * 8,			/* %fs */
   23 * 8			/* %gs */
 };
+\f
+
+/* Support for debugging kernel virtual memory images.  */
+
+#include <sys/types.h>
+#include <machine/frame.h>
+#include <machine/pcb.h>
+
+#include "bsd-kvm.h"
+
+int
+bsd_kvm_supply_pcb (struct regcache *regcache, struct pcb *pcb)
+{
+  struct switchframe sf;
+  int regnum;
+
+  /* The following is true for OpenBSD 3.5:
+
+     The pcb contains the stack pointer at the point of the context
+     switch in cpu_switch().  At that point we have a stack frame as
+     described by `struct switchframe', which for OpenBSD 3.5 has the
+     following layout:
+
+     interrupt level
+     %r15
+     %r14
+     %r13
+     %r12
+     %rbp
+     %rbx
+     return address
+
+     Together with %rsp in the pcb, this accounts for all callee-saved
+     registers specified by the psABI.  From this information we
+     reconstruct the register state as it would look when we just
+     returned from cpu_switch().  */
+
+  /* The stack pointer shouldn't be zero.  */
+  if (pcb->pcb_rsp == 0)
+    return 0;
+
+  read_memory (pcb->pcb_rsp, (char *) &sf, sizeof sf);
+  pcb->pcb_rsp += sizeof (struct switchframe);
+  regcache_raw_supply (regcache, 12, &sf.sf_r12);
+  regcache_raw_supply (regcache, 13, &sf.sf_r13);
+  regcache_raw_supply (regcache, 14, &sf.sf_r14);
+  regcache_raw_supply (regcache, 15, &sf.sf_r15);
+  regcache_raw_supply (regcache, AMD64_RSP_REGNUM, &pcb->pcb_rsp);
+  regcache_raw_supply (regcache, AMD64_RBP_REGNUM, &sf.sf_rbp);
+  regcache_raw_supply (regcache, AMD64_RBX_REGNUM, &sf.sf_rbx);
+  regcache_raw_supply (regcache, AMD64_RIP_REGNUM, &sf.sf_rip);
+
+  return 1;
+}
 \f
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
Index: m68kbsd-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/m68kbsd-nat.c,v
retrieving revision 1.1
diff -u -p -r1.1 m68kbsd-nat.c
--- m68kbsd-nat.c	30 Apr 2004 23:28:51 -0000	1.1
+++ m68kbsd-nat.c	17 May 2004 11:18:55 -0000
@@ -20,6 +20,7 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
+#include "gdbcore.h"
 #include "inferior.h"
 #include "regcache.h"
 
@@ -167,4 +168,51 @@ store_inferior_registers (int regnum)
 		  (PTRACE_ARG3_TYPE) &fpregs, 0) == -1)
 	perror_with_name ("Couldn't write floating point status");
     }
+}
+\f
+
+/* Support for debugging kernel virtual memory images.  */
+
+#include <sys/types.h>
+#include <machine/pcb.h>
+
+#include "bsd-kvm.h"
+
+/* OpenBSD doesn't have these.  */
+#ifndef PCB_REGS_FP
+#define PCB_REGS_FP 10
+#endif
+#ifndef PCB_REGS_SP
+#define PCB_REGS_SP 11
+#endif
+
+int
+bsd_kvm_supply_pcb (struct regcache *regcache, struct pcb *pcb)
+{
+  int regnum, tmp;
+  int i = 0;
+
+  /* The following is true for NetBSD 1.6.2:
+
+     The pcb contains %d2...%d7, %a2...%a7 and %ps.  This accounts for
+     all callee-saved registers.  From this information we reconstruct
+     the register state as it would look when we just returned from
+     cpu_switch().  */
+
+  /* The stack pointer shouldn't be zero.  */
+  if (pcb->pcb_regs[PC_REGS_SP] == 0)
+    return 0;
+
+  for (regnum = M68K_D2_REGNUM; regnum <= M68K_D7_REGNUM; regnum++)
+    regcache_raw_supply (regcache, regnum, &pcb->pcb_regs[i++]);
+  for (regnum = M68K_A2_REGNUM; regnum <= M68K_SP_REGNUM; regnum++)
+    regcache_raw_supply (regcache, regnum, &pcb->pcb_regs[i++]);
+
+  tmp = pcb->pcb_ps & 0xffff;
+  regcache_raw_supply (regcache, M68K_PS_REGNUM, &tmp);
+
+  read_memory (pcb->pcb_regs[PCB_REGS_FP] + 4, (char *) &tmp, sizeof tmp);
+  regcache_raw_supply (regcache, M68K_PC_REGNUM, &tmp);
+
+  return 1;
 }
--- /dev/null	Mon May 17 13:15:56 2004
+++ i386nbsd-nat.c	Mon May 17 11:49:34 2004
@@ -0,0 +1,71 @@
+/* Native-dependent code for NetBSD/i386.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include "defs.h"
+#include "gdbcore.h"
+#include "regcache.h"
+
+#include "i386-tdep.h"
+
+/* Support for debugging kernel virtual memory images.  */
+
+#include <sys/types.h>
+#include <machine/frame.h>
+#include <machine/pcb.h>
+
+#include "bsd-kvm.h"
+
+int
+bsd_kvm_supply_pcb (struct regcache *regcache, struct pcb *pcb)
+{
+  struct switchframe sf;
+
+  /* The following is true for NetBSD 1.6.2:
+
+     The pcb contains %esp and %ebp at the point of the context switch
+     in cpu_switch().  At that point we have a stack frame as
+     described by `struct switchframe', which for NetBSD 1.6.2 has the
+     following layout:
+
+     interrupt level
+     %edi
+     %esi
+     %ebx
+     %eip
+
+     we reconstruct the register state as it would look when we just
+     returned from cpu_switch().  */
+
+  /* The stack pointer shouldn't be zero.  */
+  if (pcb->pcb_esp == 0)
+    return 0;
+
+  read_memory (pcb->pcb_esp, (char *) &sf, sizeof sf);
+  pcb->pcb_esp += sizeof (struct switchframe);
+  regcache_raw_supply (regcache, I386_EDI_REGNUM, &sf.sf_edi);
+  regcache_raw_supply (regcache, I386_ESI_REGNUM, &sf.sf_esi);
+  regcache_raw_supply (regcache, I386_EBP_REGNUM, &pcb->pcb_ebp);
+  regcache_raw_supply (regcache, I386_ESP_REGNUM, &pcb->pcb_esp);
+  regcache_raw_supply (regcache, I386_EBX_REGNUM, &sf.sf_ebx);
+  regcache_raw_supply (regcache, I386_EIP_REGNUM, &sf.sf_eip);
+
+  return 1;
+}

Re: [PATCH/RFC] *BSD kernel debugging

[Marcel Moolenaar]

On Mon, May 17, 2004 at 01:32:00PM +0200, Mark Kettenis wrote:
> I've recently implemented support for kvm(3)-based debugging that
> works for all three BSD's.  The interface is fairly simple, just start
> GDB on a kernel binary, i.e.

*snip*

> All that's needed is a bit of new code (bsd-kvm.[ch]) and a support
> function in the appropriate *-nat.c file; because it is built on top
> of kvm(3) this is native-only.  I've added a preliminary patch with
> some sample code.
> 
> If there are no objections I'll check this in in a week or so.

I've been working on a different scheme for FreeBSD that supports kernel
threads and kernel modules and also works on non-i386 and non-amd64
systems without any gdb changes (at least, that's the intend -- I'm not
far enough in the development to have a working prototype of all features).
Anyway, it uses the remote protocol to establish an IPC pipe between gdb
and some front-end that knows about the quirks of the kernel and its core
files and isn't an inherently native solution.

I cannot prevent you from committing this, but if it doesn't address
the items mentioned above, it may not be used on FreeBSD.  Unless I'm
being relieved of gdb duties of course :-)

FYI,

-- 
 Marcel Moolenaar	  USPA: A-39004		 marcel@xcllnt.net

Re: [PATCH/RFC] *BSD kernel debugging

[Mark Kettenis]

   Date: Mon, 17 May 2004 15:48:59 -0400
   From: Andrew Cagney <cagney@gnu.org>

   Can the deck chairs be re-arange a little here?  Instead of having:
	   bsd_kvm_supply_pcb
   picked up by accident, add an _initialize to the *-nat.c files where 
   they can do something more explicit vis:

	   _initialize_foobsd_nat ()
	   {
	     add_bsd_kvm_target (foobsd_kvm_supply_pcb);
	   }

That seems a mighty fine plan.

Consider it done.

Mark

Re: [PATCH/RFC] *BSD kernel debugging

[Marcel Moolenaar]

On Mon, May 17, 2004 at 10:45:39PM +0200, Mark Kettenis wrote:
> 
>    I cannot prevent you from committing this, but if it doesn't address
>    the items mentioned above, it may not be used on FreeBSD.  Unless I'm
>    being relieved of gdb duties of course :-)
> 
> Let's see.  My kvm stuff would still serve a purpose for older
> releases.

[still about FreeBSD] Yes, to certain extend. There's already a port
for gdb6 in the ports collection and I presume it also works on
FreeBSD 4.x. Nonetheless, I like having gdb work without local hacks,
so from that perspective it's definitely valuable.

-- 
 Marcel Moolenaar	  USPA: A-39004		 marcel@xcllnt.net

Re: [PATCH/RFC] *BSD kernel debugging

[Kris Kennaway]

On Mon, May 17, 2004 at 03:17:03PM -0700, Marcel Moolenaar wrote:
> On Mon, May 17, 2004 at 10:45:39PM +0200, Mark Kettenis wrote:
> > 
> >    I cannot prevent you from committing this, but if it doesn't address
> >    the items mentioned above, it may not be used on FreeBSD.  Unless I'm
> >    being relieved of gdb duties of course :-)
> > 
> > Let's see.  My kvm stuff would still serve a purpose for older
> > releases.
> 
> [still about FreeBSD] Yes, to certain extend. There's already a port
> for gdb6 in the ports collection and I presume it also works on
> FreeBSD 4.x. Nonetheless, I like having gdb work without local hacks,
> so from that perspective it's definitely valuable.

Actually no version of gdb in the ports collection works on 4.x.  Some
don't even work on 5.x.

Kris
Attachment:
pgp00000.pgp
Description: PGP signature

-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)

iD8DBQFAqWTrWry0BWjoQKURAtDxAKDlU+7DwuYaDh43dq4GFdERiJRifgCg281q
ZgougBprTK6UTqMSAi13DqE=
=WMbH
-----END PGP SIGNATURE-----
From jimb@redhat.com Tue May 18 01:26:00 2004
From: Jim Blandy <jimb@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: RFA: reorder initializations in rs6000_gdbarch_init
Date: Tue, 18 May 2004 01:26:00 -0000
Message-id: <vt265au8r7p.fsf@zenia.home>
X-SW-Source: 2004-05/msg00520.html
Content-length: 4221

The current code leaves some fields of the tdep structure
uninitialized until the per-mach switch, and then leaves it up to each
case to remember to initialize everything.

This changes the code to initialize everything to neutral values, and
then let each case override the values that are inappropriate.

This fixes the fact that the bfd_mach_ppc_e500 case leaves
ppc_ev0_regnum and ppc_ev31_regnum uninitialized, although I don't
know that that causes any actual misbehavior.  The real motivation is
that it clears the way for the e500 code to override the register
offsets.

2004-05-17  Jim Blandy  <jimb@redhat.com>

	* rs6000-tdep.c (rs6000_gdbarch_init): Initialize tdep fields
	before the mach-specific switch, and then let the individual cases
	override the defaults, rather than leaving them uninitialized
	until the switch and then setting them in each case.

Index: gdb/rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.205
diff -c -p -r1.205 rs6000-tdep.c
*** gdb/rs6000-tdep.c	15 May 2004 05:57:16 -0000	1.205
--- gdb/rs6000-tdep.c	17 May 2004 21:49:54 -0000
*************** rs6000_gdbarch_init (struct gdbarch_info
*** 2821,2826 ****
--- 2821,2830 ----
      tdep->ppc_mq_regnum = -1;
    tdep->ppc_fp0_regnum = 32;
    tdep->ppc_fpscr_regnum = power ? 71 : 70;
+   tdep->ppc_vr0_regnum = -1;
+   tdep->ppc_vrsave_regnum = -1;
+   tdep->ppc_ev0_regnum = -1;
+   tdep->ppc_ev31_regnum = -1;
  
    set_gdbarch_pc_regnum (gdbarch, 64);
    set_gdbarch_sp_regnum (gdbarch, 1);
*************** rs6000_gdbarch_init (struct gdbarch_info
*** 2835,2854 ****
        set_gdbarch_deprecated_store_return_value (gdbarch, rs6000_store_return_value);
      }
  
    if (v->arch == bfd_arch_powerpc)
      switch (v->mach)
        {
        case bfd_mach_ppc: 
  	tdep->ppc_vr0_regnum = 71;
  	tdep->ppc_vrsave_regnum = 104;
- 	tdep->ppc_ev0_regnum = -1;
- 	tdep->ppc_ev31_regnum = -1;
  	break;
        case bfd_mach_ppc_7400:
  	tdep->ppc_vr0_regnum = 119;
  	tdep->ppc_vrsave_regnum = 152;
- 	tdep->ppc_ev0_regnum = -1;
- 	tdep->ppc_ev31_regnum = -1;
  	break;
        case bfd_mach_ppc_e500:
          tdep->ppc_gp0_regnum = 41;
--- 2839,2870 ----
        set_gdbarch_deprecated_store_return_value (gdbarch, rs6000_store_return_value);
      }
  
+   /* Set lr_frame_offset.  */
+   if (wordsize == 8)
+     tdep->lr_frame_offset = 16;
+   else if (sysv_abi)
+     tdep->lr_frame_offset = 4;
+   else
+     tdep->lr_frame_offset = 8;
+ 
+   /* Calculate byte offsets in raw register array.  */
+   tdep->regoff = xmalloc (v->num_tot_regs * sizeof (int));
+   for (i = off = 0; i < v->num_tot_regs; i++)
+     {
+       tdep->regoff[i] = off;
+       off += regsize (v->regs + i, wordsize);
+     }
+ 
    if (v->arch == bfd_arch_powerpc)
      switch (v->mach)
        {
        case bfd_mach_ppc: 
  	tdep->ppc_vr0_regnum = 71;
  	tdep->ppc_vrsave_regnum = 104;
  	break;
        case bfd_mach_ppc_7400:
  	tdep->ppc_vr0_regnum = 119;
  	tdep->ppc_vrsave_regnum = 152;
  	break;
        case bfd_mach_ppc_e500:
          tdep->ppc_gp0_regnum = 41;
*************** rs6000_gdbarch_init (struct gdbarch_info
*** 2868,2899 ****
          set_gdbarch_pseudo_register_read (gdbarch, e500_pseudo_register_read);
          set_gdbarch_pseudo_register_write (gdbarch, e500_pseudo_register_write);
  	break;
-       default:
- 	tdep->ppc_vr0_regnum = -1;
- 	tdep->ppc_vrsave_regnum = -1;
- 	tdep->ppc_ev0_regnum = -1;
- 	tdep->ppc_ev31_regnum = -1;
- 	break;
        }   
  
    /* Sanity check on registers.  */
    gdb_assert (strcmp (tdep->regs[tdep->ppc_gp0_regnum].name, "r0") == 0);
- 
-   /* Set lr_frame_offset.  */
-   if (wordsize == 8)
-     tdep->lr_frame_offset = 16;
-   else if (sysv_abi)
-     tdep->lr_frame_offset = 4;
-   else
-     tdep->lr_frame_offset = 8;
- 
-   /* Calculate byte offsets in raw register array.  */
-   tdep->regoff = xmalloc (v->num_tot_regs * sizeof (int));
-   for (i = off = 0; i < v->num_tot_regs; i++)
-     {
-       tdep->regoff[i] = off;
-       off += regsize (v->regs + i, wordsize);
-     }
  
    /* Select instruction printer.  */
    if (arch == power)
--- 2884,2893 ----