core regs vs. proc-service regs mess

In trying to get core files working properly again on
Linux/Sparc and
Daniel I now see the problems you referred to the other day.

What we have currently on Linux/Sparc is:

1) ptrace() interfaces have one layout for regsets
2) ELF core files store regsets in a different
   format

Now proc-service.c wants fill_gregset() et al. to
provide things in the ptrace() style layout, that is
what libthread_db expects under Linux.

But linux_nat_make_corefile_notes() expects the
fill_gregset()
et al. to provide ELF core file format registers.

This is completely contradictory, and I don't have any idea
how to
cleanly resolve this outside of duplicating the entirety of
linux_nat_make_corefile_notes() on Linux/Sparc which is
silly.

Daniel any ideas?  Maybe we
can have a "linux_tdep" struct where we
can place a "->to_fill_core_gregset()" type
method or similar?

>
>  In trying to get core files working properly again on
Linux/Sparc and
>  Daniel I now see the problems you referred to the
other day.
>
>  What we have currently on Linux/Sparc is:
>
>  1) ptrace() interfaces have one layout for regsets
>  2) ELF core files store regsets in a different
>     format
>
>  Now proc-service.c wants fill_gregset() et al. to
>  provide things in the ptrace() style layout, that is
>  what libthread_db expects under Linux.
>
>  But linux_nat_make_corefile_notes() expects the
fill_gregset()
>  et al. to provide ELF core file format registers.

Hi Dave,

Yes this is annoying, and ultimately caused because the
ptrace(2) and
core(5) interfaces on Linux were never properly designed
(heck they're not
even fully documented) and inconsistent between the
different hardware
platforms.  For most platforms the ptrace(2) and core(5)
layout are the
same, but not on sparc (presumably because ptrace(2) on
sparc tries to
be compatible with SunOS/Solaris).

>  This is completely contradictory, and I don't have
any idea how to
>  cleanly resolve this outside of duplicating the
entirety of
>  linux_nat_make_corefile_notes() on Linux/Sparc which
is silly.
>
>  Daniel any ideas?  Maybe we
can have a "linux_tdep" struct where we
>  can place a "->to_fill_core_gregset()"
type method or similar?

What really should happen is to convert
linux_nat_make_corefile_notes() to
use the regset_from_core_section() interface.  Unfortunately
that is quite
a bit of work since most Linux targets don't provide that
interface yet.

Mark

On Wed, Apr 05, 2006 at 10:25:21AM +0200, Mark Kettenis
wrote:
> >  This is completely contradictory, and I don't
have any idea how to
> >  cleanly resolve this outside of duplicating the
entirety of
> >  linux_nat_make_corefile_notes() on Linux/Sparc
which is silly.
> >
> >  Daniel any ideas?  Maybe we
can have a "linux_tdep" struct where we
> >  can place a
"->to_fill_core_gregset()" type method or
similar?
> 
> What really should happen is to convert
linux_nat_make_corefile_notes() to
> use the regset_from_core_section() interface. 
Unfortunately that is quite
> a bit of work since most Linux targets don't provide
that interface yet.

Shouldn't this be pretty simple:
gdbarch_regset_from_core_section_p?

-- 
Daniel Jacobowitz
CodeSourcery

From: Daniel Jacobowitz <drowfalse.org>
Date: Wed, 5 Apr 2006 09:23:00 -0400

> On Wed, Apr 05, 2006 at 10:25:21AM +0200, Mark Kettenis
wrote:
> > >  This is completely contradictory, and I
don't have any idea how to
> > >  cleanly resolve this outside of duplicating
the entirety of
> > >  linux_nat_make_corefile_notes() on
Linux/Sparc which is silly.
> > >
> > >  Daniel any ideas?  Maybe we
can have a "linux_tdep" struct where we
> > >  can place a
"->to_fill_core_gregset()" type method or
similar?
> > 
> > What really should happen is to convert
linux_nat_make_corefile_notes() to
> > use the regset_from_core_section() interface. 
Unfortunately that is quite
> > a bit of work since most Linux targets don't
provide that interface yet.
> 
> Shouldn't this be pretty simple:
gdbarch_regset_from_core_section_p?

I'll work on converting the Linux/Sparc targets over to the
regset_from_core_section() interface.

Then I'll give the linux_nat_make_corefile_notes() change a
try with the test that Daniel is suggesting here to decide
how to get at the core regs.

From: Daniel Jacobowitz <drowfalse.org>
Date: Wed, 5 Apr 2006 09:23:00 -0400

> On Wed, Apr 05, 2006 at 10:25:21AM +0200, Mark Kettenis
wrote:
> > >  This is completely contradictory, and I
don't have any idea how to
> > >  cleanly resolve this outside of duplicating
the entirety of
> > >  linux_nat_make_corefile_notes() on
Linux/Sparc which is silly.
> > >
> > >  Daniel any ideas?  Maybe we
can have a "linux_tdep" struct where we
> > >  can place a
"->to_fill_core_gregset()" type method or
similar?
> > 
> > What really should happen is to convert
linux_nat_make_corefile_notes() to
> > use the regset_from_core_section() interface. 
Unfortunately that is quite
> > a bit of work since most Linux targets don't
provide that interface yet.
> 
> Shouldn't this be pretty simple:
gdbarch_regset_from_core_section_p?

So something like the following patch?  I didn't know what
to do with
the fpxregset stuff, I guess we'll just have to pick some
random
string for that?  I couldn't find a ELF section name string
assigned
to that stuff.

I tested this patch along with Linux/Sparc
regset_from_core_section()
support, and it all seems to work fine.

--- linux-nat.c.~1~	2006-04-05 18:08:11.000000000 -0700
+++ linux-nat.c	2006-04-05 18:41:57.000000000 -0700
 -36,6
+36,7 
 #include "gdbthread.h"
 #include "gdbcmd.h"
 #include "regcache.h"
+#include "regset.h"
 #include "inf-ptrace.h"
 #include "auxv.h"
 #include <sys/param.h>		/* for MAXPATHLEN */
 -2529,15
+2530,33 
   gdb_fpxregset_t fpxregs;
 #endif
   unsigned long lwp = ptid_get_lwp (ptid);
+  struct gdbarch *gdbarch = current_gdbarch;
+  const struct regset *regset;
 
-  fill_gregset (&gregs, -1);
+  if (gdbarch && gdbarch_regset_from_core_section_p
(gdbarch)
+      && (regset = gdbarch_regset_from_core_section
(gdbarch, ".reg",
+						     sizeof gregs)) != NULL)
+    {
+      regset->collect_regset (regset, current_regcache,
-1,
+			      &gregs, sizeof gregs);
+    }
+  else
+    fill_gregset (&gregs, -1);
   note_data = (char *) elfcore_write_prstatus (obfd,
 					       note_data,
 					       note_size,
 					       lwp,
 					       stop_signal, &gregs);
 
-  fill_fpregset (&fpregs, -1);
+  if (gdbarch && gdbarch_regset_from_core_section_p
(gdbarch)
+      && (regset = gdbarch_regset_from_core_section
(gdbarch, ".reg2",
+						     sizeof fpregs)) != NULL)
+    {
+      regset->collect_regset (regset, current_regcache,
-1,
+			      &fpregs, sizeof fpregs);
+    }
+  else
+    fill_fpregset (&fpregs, -1);
   note_data = (char *) elfcore_write_prfpreg (obfd,
 					      note_data,
 					      note_size,

On Wed, Apr 05, 2006 at 10:06:44PM -0700, David S. Miller
wrote:
> So something like the following patch?  I didn't know
what to do with
> the fpxregset stuff, I guess we'll just have to pick
some random
> string for that?  I couldn't find a ELF section name
string assigned
> to that stuff.

It's .reg-xfp; created by bfd/elf.c:elfcore_grok_prxfpreg.

> +  if (gdbarch &&
gdbarch_regset_from_core_section_p (gdbarch)
> +      && (regset =
gdbarch_regset_from_core_section (gdbarch,
".reg",
> +						     sizeof gregs)) != NULL)

No point testing gdbarch for NULL; it won't be.  Also, I
think
gdbarch_regset_from_core_section_p is enough.  If that
returns NULL,
then skip the regset.  We don't need to cater to incomplete
implementations of the core functions.

Oh, and GDB prefers not to use assignments in if statements.

But otherwise, yeah, this is what I had in mind.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

From: Daniel Jacobowitz <drowfalse.org>
Date: Thu, 6 Apr 2006 09:32:50 -0400

> It's .reg-xfp; created by
bfd/elf.c:elfcore_grok_prxfpreg.
 ...
> No point testing gdbarch for NULL; it won't be.  Also,
I think
> gdbarch_regset_from_core_section_p is enough.  If that
returns NULL,
> then skip the regset.  We don't need to cater to
incomplete
> implementations of the core functions.
> 
> Oh, and GDB prefers not to use assignments in if
statements.
> 
> But otherwise, yeah, this is what I had in mind. 
Thanks.

Thanks for the feedback.

Can I get approval for the Linux/Sparc dwarf2 bit I posted
yesterday?

  http://sources.redhat.com/ml/gdb-patches/2006-04/m
sg00065.html
 
I want to get that in first, then I'll finish up a final
version of
these core regs changes.

Also, I haven't gotten approval for that one-liner
break.exp fix
I posted the other day which we discussed in depth a few
days
ago:

  http://sources.redhat.com/ml/gdb-patches/2006-04/m
sg00051.html

If I could get an ACK or NACK on that I'd really appreciate
it.

Thanks a lot!