[RFA/sparc64] internal-error printing return value (Ada array)

[RFA/sparc64] internal-error printing return value (Ada array)

[Joel Brobecker]

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

Hello,

When working on GDB for sparc64-sun-solaris, I noticed that GDB
reports an internal error when trying to print the return value
of a function returning an Ada (fixed-size) array. I noticed this
in gdb.ada/array_return.exp:

    FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small
    FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Large

Here is what happens. First the transcript:

    (gdb) b pck.adb:5
    Breakpoint 1 at 0x100009e84: file pck.adb, line 5.
    (gdb) run
    Starting program: /[...]/p 
    
    Breakpoint 1, pck.create_small () at pck.adb:5
    5             return (others => 1);
    (gdb) fin
    Run till exit from #0  pck.create_small () at pck.adb:5
    0x0000000100009fd0 in p () at p.adb:7
    7          Small := Create_Small;
    sparc64-tdep.c:1092: internal-error: sparc64_extract_return_value: Assertion `sp
    arc64_integral_or_pointer_p (type)' failed.

The type in question is a TYPE_CODE_ARRAY. It seems that this case
is not handled yet by the function that extracts the return value:

        sparc64-tdep.c:sparc64_extract_return_value

I am not sure yet whether this case is specific to Ada or if we can
reproduce the same with C, I will double-check. In any case, these
array objects are returned the same way as struct objects: Up to 32 bytes,
they are passed by register, otherwise %o0 contains the address of the
returned object.

I think the best fix for this is to update sparc64_structure_or_union_p
to accept arrays of 32 bytes or less. The name would become slightly
misleading for its actual implementation, and I wouldn't mind changing
it if felt necessary.

I don't know if the same issue applies to 32bit GDB or not. I tried,
but "fin" doesn't stop at the function return, so I never get the
returned value printed by GDB. I'll have a look at this another time.

I also noticed that the same problem happens for arrays larger than
32bytes. In fact, this is not specific to arrays, but to structures
as well. Support for structures larger than 32 bytes seems to be
missing. I can add that too, but let's make this another patch.

2006-11-10  Joel Brobecker  <brobecker@adacore.com>

        * sparc64-tdep.c (sparc64_structure_or_union_p): Accept array
        types if the type length is 32 bytes or less.

Fixes the two FAILs above, no regression.
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: sparc64-tdep.c.diff --]
[-- Type: text/plain, Size: 602 bytes --]

Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.27
diff -u -p -r1.27 sparc64-tdep.c
--- sparc64-tdep.c	22 Aug 2006 20:57:56 -0000	1.27
+++ sparc64-tdep.c	11 Nov 2006 00:24:36 -0000
@@ -114,6 +114,9 @@ sparc64_structure_or_union_p (const stru
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
       return 1;
+    case TYPE_CODE_ARRAY:
+      /* Arrays of up to 32 bytes are returned the same way as record.  */
+      return (TYPE_LENGTH (type) <= 32);
     default:
       break;
     }

Re: [RFA/sparc64] internal-error printing return value (Ada array)

[Mark Kettenis]

>  The type in question is a TYPE_CODE_ARRAY. It seems that this case
>  is not handled yet by the function that extracts the return value:
>
>          sparc64-tdep.c:sparc64_extract_return_value
>
>  I am not sure yet whether this case is specific to Ada or if we can
>  reproduce the same with C, I will double-check.

C doesn't really have arrays.

>  I think the best fix for this is to update sparc64_structure_or_union_p
>  to accept arrays of 32 bytes or less. The name would become slightly
>  misleading for its actual implementation, and I wouldn't mind changing
>  it if felt necessary.
>
>  I don't know if the same issue applies to 32bit GDB or not. I tried,
>  but "fin" doesn't stop at the function return, so I never get the
>  returned value printed by GDB. I'll have a look at this another time.
>
>  I also noticed that the same problem happens for arrays larger than
>  32bytes. In fact, this is not specific to arrays, but to structures
>  as well. Support for structures larger than 32 bytes seems to be
>  missing. I can add that too, but let's make this another patch.
>
>  2006-11-10  Joel Brobecker  <brobecker@adacore.com>
>
>          * sparc64-tdep.c (sparc64_structure_or_union_p): Accept array
>          types if the type length is 32 bytes or less.
>
>  Fixes the two FAILs above, no regression.
>  OK to apply?

I don't think this is the right fix; the length check almost cetainly is.

To decide what is the right fix, we need to investigate this a bit further.
I suspect that Ada arrays arereally treated as structures where all members
have the same type.  So the first question I have is whether these indeed
have "fields".  You should also check how small arrays are passed as
arguments to a function.  Here the magic length will be 16 bytes instead
of 32 bytes.

Mark

Re: [RFA/sparc64] internal-error printing return value (Ada array)

[Joel Brobecker]

> >  2006-11-10  Joel Brobecker  <brobecker@adacore.com>
> >
> >          * sparc64-tdep.c (sparc64_structure_or_union_p): Accept array
> >          types if the type length is 32 bytes or less.
> >
> >  Fixes the two FAILs above, no regression.
> >  OK to apply?
> 
> I don't think this is the right fix; the length check almost cetainly is.
> 
> To decide what is the right fix, we need to investigate this a bit further.
> I suspect that Ada arrays arereally treated as structures where all members
> have the same type.

In Ada, arrays can take many forms, and as a result, you have 
3 types of arrays:

  . statically known arrays (where the array bounds are known at
    compile time), are implemented using a memory buffer. This is
    our case here.

  . Then we have fat pointers: This is a structure that contains
    two pointers, one to a structure containing the array bounds,
    and one pointer to the memory buffer itself. We use that for
    arrays whose bounds are not known at compile time.

  . Lastly, we have thin pointers: This is a pointer to the second
    field of a structure that resembles the fat pointer.

> So the first question I have is whether these indeed
> have "fields".

Is this question still relevant after the description above?
I am not sure I understand it.

> You should also check how small arrays are passed as arguments to a
> function.

This is described by the Ada Reference Manual: Arrays are always
passed by reference. So a function taking a parameter of our static
array type will have the array passed by reference.  As a result,
the the array parameter will be a REF to a TYPE_CODE_ARRAY.

> Here the magic length will be 16 bytes instead of 32 bytes.

I don't understand this part. Why 16 bytes instead of 32?
If the total size of the array is 32 bytes, shouldn't the compiler
return it through %o0 - %o7?

-- 
Joel

Re: [RFA/sparc64] internal-error printing return value (Ada array)

[Mark Kettenis]

> > To decide what is the right fix, we need to investigate this a bit
> > further.
> > I suspect that Ada arrays arereally treated as structures where all
> > members
> > have the same type.
>
>  In Ada, arrays can take many forms, and as a result, you have
>  3 types of arrays:
>
>    . statically known arrays (where the array bounds are known at
>      compile time), are implemented using a memory buffer. This is
>      our case here.
>
>    . Then we have fat pointers: This is a structure that contains
>      two pointers, one to a structure containing the array bounds,
>      and one pointer to the memory buffer itself. We use that for
>      arrays whose bounds are not known at compile time.
>
>    . Lastly, we have thin pointers: This is a pointer to the second
>      field of a structure that resembles the fat pointer.
>
> > So the first question I have is whether these indeed
> > have "fields".
>
>  Is this question still relevant after the description above?
>  I am not sure I understand it.

Hmm, I gueass I should have asked how a gdb `struct type' looks for
these Ada arrays?  In particular, if main_type->nfields is set and
whether main_type->fields is set to something useful.

> > You should also check how small arrays are passed as arguments to a
> > function.
>
>  This is described by the Ada Reference Manual: Arrays are always
>  passed by reference. So a function taking a parameter of our static
>  array type will have the array passed by reference.  As a result,
>  the the array parameter will be a REF to a TYPE_CODE_ARRAY.

So there is no way to pass a TYPE_CODE_ARRAY directly?

> > Here the magic length will be 16 bytes instead of 32 bytes.
>
>  I don't understand this part. Why 16 bytes instead of 32?
>  If the total size of the array is 32 bytes, shouldn't the compiler
>  return it through %o0 - %o7?

The 16-byte limit is for passing structures as an argument to a function.
I presume this is because function arguments occupy 16-byte slots in the
ABI.

Mark

Re: [RFA/sparc64] internal-error printing return value (Ada array)

[Joel Brobecker]

> Hmm, I gueass I should have asked how a gdb `struct type' looks for
> these Ada arrays?  In particular, if main_type->nfields is set and
> whether main_type->fields is set to something useful.

Ah, I understand now (sorry). Yes, main_type->nfields is 1, and
main_type->fields contains the bounds.

> > > You should also check how small arrays are passed as arguments to a
> > > function.
> >
> >  This is described by the Ada Reference Manual: Arrays are always
> >  passed by reference. So a function taking a parameter of our static
> >  array type will have the array passed by reference.  As a result,
> >  the the array parameter will be a REF to a TYPE_CODE_ARRAY.
> 
> So there is no way to pass a TYPE_CODE_ARRAY directly?

Not as a parameter for a function or procedure, no.

> > > Here the magic length will be 16 bytes instead of 32 bytes.
> >
> >  I don't understand this part. Why 16 bytes instead of 32?
> >  If the total size of the array is 32 bytes, shouldn't the compiler
> >  return it through %o0 - %o7?
> 
> The 16-byte limit is for passing structures as an argument to a function.
> I presume this is because function arguments occupy 16-byte slots in the
> ABI.

Ah, OK, I think I see where you are going. I failed to notice that
the function I modified is also used for storing/extracting function
parameters.

In terms of the argument passing, we don't need to worry in our case,
because arrays are passed by reference. In terms of a return value,
the ABI says that structs of up to 32 bytes can be returned.

        3.2.3.3. Structure or Union return values
        
        Structure and union return types up to thirty-two bytes in size
        are returned in registers. The registers are assigned as if the
        value was being passed as the first argument to a function with
        a known prototype.

This is why I used the magic number of 32 in this case. I still think
it is correct, although more comments are certainly required to explain
the above.

What do you think?
-- 
Joel

Re: [RFA/sparc64] internal-error printing return value (Ada array)

[Mark Kettenis]

> > Hmm, I gueass I should have asked how a gdb `struct type' looks for
> > these Ada arrays?  In particular, if main_type->nfields is set and
> > whether main_type->fields is set to something useful.
>
>  Ah, I understand now (sorry). Yes, main_type->nfields is 1, and
>  main_type->fields contains the bounds.

Ok, that's quite different from what we have for TYPE_CODE_STRUCT and
TYPE_CODE_UNION.  This means that adding TYPE_CODE_ARRAY to
sparc64_structure_or_union is the wrong fix.

>  In terms of the argument passing, we don't need to worry in our case,
>  because arrays are passed by reference. In terms of a return value,
>  the ABI says that structs of up to 32 bytes can be returned.
>
>          3.2.3.3. Structure or Union return values
>
>          Structure and union return types up to thirty-two bytes in size
>          are returned in registers. The registers are assigned as if the
>          value was being passed as the first argument to a function with
>          a known prototype.
>
>  This is why I used the magic number of 32 in this case. I still think
>  it is correct, although more comments are certainly required to explain
>  the above.
>
>  What do you think?

Well, we need to worry about argument passing too; Ada might not pass
arrays directly, but other languages might do that.  So I think you should
just add a check for TYPE_CODE_ARRAY to sparc64_extract_return_value() and
sparc64_store_return_value().  You could add it to the same if-block as
the sparc64_structure_of_union() but then you'll need to change the

   if (TYPE_CODE (type) != TYPE_CODE_UNION)

check into

   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)

Mark