Re: [RFC] Add support for PPC Altivec registers in gcore

["Ulrich Weigand" <uweigand@de.ibm.com>]

Carlos Eduardo Seo wrote:

> >> +v:struct core_regset_section *:core_regset_sections:const char *name, int len::::default_regset_sections::(char *) '0'
> > 
> > Not '0' -- just 0.  (That *is* a difference!)
> > 
> 
> Yes, I know. But gcc 4.x will complain about reading through a null 
> pointer if I use 0.

As Andreas pointed out, (char *) '0' is definitely broken -- this
will interpret the numerical value of '0' (i.e. 48 in ASCII) as
pointer value and try to dereference it, which will certainly crash.

I had suggested to use a null pointer as this will be caught at
run-time and printed as "(null)".  However, re-thinking it, this is 
not actually guaranteed by the standard -- which is probably why
you're seeing a warning ...


Unfortunately, I've noticed another, more serious problem:

> Index: src/gdb/arch-utils.c
> ===================================================================
> --- src.orig/gdb/arch-utils.c
> +++ src/gdb/arch-utils.c
> @@ -31,11 +31,19 @@
>  #include "gdbcore.h"
>  #include "osabi.h"
>  #include "target-descriptions.h"
> -
> +#include <sys/procfs.h>
> +#include "gregset.h"
>  #include "version.h"
> +#include "regset.h"
>  
>  #include "floatformat.h"
>  
> +struct core_regset_section default_regset_sections[] =
> +{
> +  { ".reg", sizeof (gdb_gregset_t) },
> +  { ".reg2", sizeof (gdb_fpregset_t) },
> +  { NULL, 0 }
> +};

This isn't really feasible: you cannot simply include the
platform-specific <sys/procfs.h> into a generic file -- it
may not even be available on some platforms; and on other
platforms it may not provide the required gregset_t type.

I'm sorry for all the back-and-forth on this, but it seems
we do have to implement a Linux-specific fallback to that;
I don't see how we can do a generic default.  To do so,
you'll need to let the gdbarch variable default to NULL,
and in linux_nat_do_thread_registers perform the fpregset
fallback if that is the case.  ppc and i386 would then be
the only architectures that install the new gdbarch variable;
over time, the remaining Linux platforms should follow suite
and we can remove the fallbacks.


> Index: src/gdb/ppc-linux-tdep.c
> ===================================================================
> --- src.orig/gdb/ppc-linux-tdep.c
> +++ src/gdb/ppc-linux-tdep.c
> @@ -37,6 +37,16 @@
>  #include "trad-frame.h"
>  #include "frame-unwind.h"
>  #include "tramp-frame.h"
> +#include <sys/procfs.h>

You cannot include a platform-specific header into a -tdep file either,
this file is also used for cross-debugging from arbitrary hosts.

> +#include "gregset.h"
> +
> +static struct core_regset_section ppc_regset_sections[] =
> +{
> +  { ".reg", sizeof (gdb_gregset_t) },
> +  { ".reg2", sizeof (gdb_fpregset_t) },

However, you can simply use all numerical values here -- the sizes
of the register sections in PowerPC core files are constants.

> +  { ".reg-ppc-vmx", 544 },
> +  { NULL, 0 }
> +};


The same applies to the i386 changes:

> +++ src/gdb/i386-linux-tdep.c
> @@ -28,12 +28,24 @@
>  #include "reggroups.h"
>  #include "dwarf2-frame.h"
>  #include "gdb_string.h"
> -
> +#include <sys/procfs.h>
> +#include "gregset.h"
>  #include "i386-tdep.h"
>  #include "i386-linux-tdep.h"
>  #include "glibc-tdep.h"
>  #include "solib-svr4.h"
>  #include "symtab.h"
> +#include "regset.h"
> +
> +static struct core_regset_section i386_regset_sections[] =
> +{
> +  { ".reg", sizeof (gdb_gregset_t) },
> +  { ".reg2", sizeof (gdb_fpregset_t) },
> +#ifdef FILL_FPXREGSET
> +  { ".reg-xfp", sizeof (gdb_fpxregset_t) },
> +#endif
> +  { NULL, 0 }
> +};


One more issue in linux-nat.c:

> +  if (core_regset_p)
> +    while ((++sect_list)->sect_name != NULL)

The ++sect_list still skips the first entry unconditionally.
You should increment sect_list at the *end* of the loop.


Once again, sorry for leading you down a wrong track here.
Thanks for your continued work to fix this problem ...


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com