[RFA] wrong pointer type length

[RFA] wrong pointer type length

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

Hello,

We've seen a problem that involves stabs on AIX. What we were trying
to do was to print the value of a parameter which is just a pointer
to a structure. GDB prints the value of this pointer as 0x0. The
code comes from our testsuite, and is unfortunately confidential.

We traced down the problem to the fact that the native assembler
seems to be changing up the order in which the stabs entries are
generated. Not sure why. Perhaps Ulrich might know a bit more.
In any case, the assembly file contains the following stabx entries
in that order:

        .stabx "root__union_record_t:Tt42=s12x_part:40,0,32;[...]
        .stabx  "root__union_record_a:t43=*42",0,140,0
        .stabx  "u:p74=k43",808,130,0

However, after being compiled, the order has been changed to:

        1. root__union_record_t:Tt42=s12x_part:40,0,32;[...]
        2. u:p74=k43
        3. root__union_record_a:t43=*42

As a result, when GDB reads in the stabs, it does the following:

  1. Read type 42, and create the associated struct type
  2. Read in parameter 74, finds that it's related to type 43,
     which is not known at this point, and thus uses an undefined
     type for now. Then it makes a const variant of type 43.
  3. Read type 43, finds that it is a pointer to type 42, so
     replaces our undefined type 43 with a pointer to type 42.

The problem is in the creation of the pointer type: We smash the
current type chain, and thus end up not updating the length of
all the variants:

       /* We have storage, but need to reset it.  */
       {
         ntype = *typeptr;
         objfile = TYPE_OBJFILE (ntype);
 -->     smash_type (ntype);
         TYPE_OBJFILE (ntype) = objfile;

The attached patch fixes this.

2007-05-16  Joel Brobecker  <brobecker@adacore.com>

        * gdbtypes.c (make_pointer_type): Preserve the pointer type chain
        and set the length of all the variants of the pointer type.

Tested on x86-linux and ppc-aix, no regressions.  OK to commit?

As a followup to this patch, I think that the same needs to be done
for at least make_reference_type, but I don't have hard evidence that
this will ever be needed right now. If it is agreed that this function
also needs an update, it can be made independently in a followup patch.

I don't think that make_function_type would have a non-empty chain.
So the change shouldn't be needed there. I checked the rest of the
file, and I more or less convinced myself that the code was fine too.

Cheers,
-- 
Joel

[-- Attachment #2: ptrtype.diff --]
[-- Type: text/plain, Size: 1167 bytes --]

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.114
diff -u -p -r1.114 gdbtypes.c
--- gdbtypes.c	28 Feb 2007 19:42:08 -0000	1.114
+++ gdbtypes.c	16 May 2007 20:34:08 -0000
@@ -276,6 +276,7 @@ make_pointer_type (struct type *type, st
 {
   struct type *ntype;	/* New type */
   struct objfile *objfile;
+  struct type *chain;
 
   ntype = TYPE_POINTER_TYPE (type);
 
@@ -301,7 +302,9 @@ make_pointer_type (struct type *type, st
     {
       ntype = *typeptr;
       objfile = TYPE_OBJFILE (ntype);
+      chain = TYPE_CHAIN (ntype);
       smash_type (ntype);
+      TYPE_CHAIN (ntype) = chain;
       TYPE_OBJFILE (ntype) = objfile;
     }
 
@@ -321,6 +324,14 @@ make_pointer_type (struct type *type, st
   if (!TYPE_POINTER_TYPE (type))	/* Remember it, if don't have one.  */
     TYPE_POINTER_TYPE (type) = ntype;
 
+  /* Update the length of all the other variants of this type.  */
+  chain = TYPE_CHAIN (ntype);
+  while (chain != ntype)
+    {
+      TYPE_LENGTH (chain) = TYPE_LENGTH (ntype);
+      chain = TYPE_CHAIN (chain);
+    }
+
   return ntype;
 }
 

Re: [RFA] wrong pointer type length

[Daniel Jacobowitz]

On Thu, May 17, 2007 at 09:19:19AM -0700, Joel Brobecker wrote:
> 2007-05-16  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdbtypes.c (make_pointer_type): Preserve the pointer type chain
>         and set the length of all the variants of the pointer type.
> 
> Tested on x86-linux and ppc-aix, no regressions.  OK to commit?

OK, I suppose.

> As a followup to this patch, I think that the same needs to be done
> for at least make_reference_type, but I don't have hard evidence that
> this will ever be needed right now. If it is agreed that this function
> also needs an update, it can be made independently in a followup patch.

Yes please.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] wrong pointer type length

[Joel Brobecker]

> > 2007-05-16  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gdbtypes.c (make_pointer_type): Preserve the pointer type chain
> >         and set the length of all the variants of the pointer type.
> > 
> > Tested on x86-linux and ppc-aix, no regressions.  OK to commit?
> 
> OK, I suppose.

That was fast! Thank you. This part has been checked in.

> > As a followup to this patch, I think that the same needs to be done
> > for at least make_reference_type, but I don't have hard evidence that
> > this will ever be needed right now. If it is agreed that this function
> > also needs an update, it can be made independently in a followup patch.
> 
> Yes please.

I'm working on the second part right now...

-- 
Joel

Re: [RFA] wrong pointer type length

[Jim Blandy]

Joel Brobecker <brobecker@adacore.com> writes:
> We traced down the problem to the fact that the native assembler
> seems to be changing up the order in which the stabs entries are
> generated. Not sure why. Perhaps Ulrich might know a bit more.
> In any case, the assembly file contains the following stabx entries
> in that order:
>
>         .stabx "root__union_record_t:Tt42=s12x_part:40,0,32;[...]
>         .stabx  "root__union_record_a:t43=*42",0,140,0
>         .stabx  "u:p74=k43",808,130,0
>
> However, after being compiled, the order has been changed to:
>
>         1. root__union_record_t:Tt42=s12x_part:40,0,32;[...]
>         2. u:p74=k43
>         3. root__union_record_a:t43=*42

If the assembler is doing this in other cases, you're going to be
pretty hosed.  STABS requires records to appear in the order
specified.

Re: [RFA] wrong pointer type length

[Joel Brobecker]

> If the assembler is doing this in other cases, you're going to be
> pretty hosed.  STABS requires records to appear in the order
> specified.

I know :-(. Unfortunately, we can't change the assembler, so I've added
no too long ago a second list of undefined types  that should allow us
to handle this type of wild reordering. See:

    http://www.sourceware.org/ml/gdb-patches/2007-02/msg00387.html

-- 
Joel

[RFA] wrong pointer type length part 2 (of 2)

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

> > > As a followup to this patch, I think that the same needs to be done
> > > for at least make_reference_type, but I don't have hard evidence that
> > > this will ever be needed right now. If it is agreed that this function
> > > also needs an update, it can be made independently in a followup patch.
> > 
> > Yes please.
> 
> I'm working on the second part right now...

OK, here is the second part that deals with reference types.

2007-05-17  Joel Brobecker  <brobecker@adacore.com>

        * gdbtypes.c (make_reference_type): Preserve the type chain
        and set the length of all the variants of the pointer type.

Tested on x86-linux (elf/dwarf), and ppc-aix (xcoff/stabs).
OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: reftype.diff --]
[-- Type: text/plain, Size: 1173 bytes --]

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.115
diff -u -p -r1.115 gdbtypes.c
--- gdbtypes.c	17 May 2007 16:38:25 -0000	1.115
+++ gdbtypes.c	17 May 2007 16:40:53 -0000
@@ -354,6 +354,7 @@ make_reference_type (struct type *type, 
 {
   struct type *ntype;	/* New type */
   struct objfile *objfile;
+  struct type *chain;
 
   ntype = TYPE_REFERENCE_TYPE (type);
 
@@ -379,7 +380,9 @@ make_reference_type (struct type *type, 
     {
       ntype = *typeptr;
       objfile = TYPE_OBJFILE (ntype);
+      chain = TYPE_CHAIN (ntype);
       smash_type (ntype);
+      TYPE_CHAIN (ntype) = chain;
       TYPE_OBJFILE (ntype) = objfile;
     }
 
@@ -395,6 +398,14 @@ make_reference_type (struct type *type, 
   if (!TYPE_REFERENCE_TYPE (type))	/* Remember it, if don't have one.  */
     TYPE_REFERENCE_TYPE (type) = ntype;
 
+  /* Update the length of all the other variants of this type.  */
+  chain = TYPE_CHAIN (ntype);
+  while (chain != ntype)
+    {
+      TYPE_LENGTH (chain) = TYPE_LENGTH (ntype);
+      chain = TYPE_CHAIN (chain);
+    }
+
   return ntype;
 }
 

Re: [RFA] wrong pointer type length part 2 (of 2)

[Daniel Jacobowitz]

On Thu, May 17, 2007 at 11:51:44AM -0700, Joel Brobecker wrote:
> > > > As a followup to this patch, I think that the same needs to be done
> > > > for at least make_reference_type, but I don't have hard evidence that
> > > > this will ever be needed right now. If it is agreed that this function
> > > > also needs an update, it can be made independently in a followup patch.
> > > 
> > > Yes please.
> > 
> > I'm working on the second part right now...
> 
> OK, here is the second part that deals with reference types.
> 
> 2007-05-17  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdbtypes.c (make_reference_type): Preserve the type chain
>         and set the length of all the variants of the pointer type.
> 
> Tested on x86-linux (elf/dwarf), and ppc-aix (xcoff/stabs).
> OK to commit?

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] wrong pointer type length part 2 (of 2)

[Joel Brobecker]

> > 2007-05-17  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gdbtypes.c (make_reference_type): Preserve the type chain
> >         and set the length of all the variants of the pointer type.
> > 
> > Tested on x86-linux (elf/dwarf), and ppc-aix (xcoff/stabs).
> > OK to commit?
> 
> OK.

Thanks. Checked in too.

-- 
Joel