[RFA/sparc] "finish" does not work if function returns array.

[RFA/sparc] "finish" does not work if function returns array.

[Joel Brobecker]

This is something that is not explicitly covered by the ABI, which
would explain why it's not covered by the code yet.  In Ada, functions
can return arrays, for instance:

   type Data_Small is array (1 .. 2) of Integer;
   function Create_Small return Data_Small;

Trying to do a "finish" from Create_Small just runs-away:

    (gdb) b create_small
    Breakpoint 1 at 0x18b30: 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

    Program exited normally.

What happens is that functions returning an array are like functions
returning a structure.  As such, the ABI requires the caller to setup
an "unimp" instruction right after the delay slot of the call instruction.
For instance, in our case above, the code in the caller looks like this:

       0x00018e78 <+12>:    call  0x18de0 <pck__create_small>
       0x00018e7c <+16>:    nop
       0x00018e80 <+20>:    illtrap  0x8

Before this patch, GDB would place the finish breakpoint where the
unimp/illtrap instruction is located, instead of placing it on the
next instruction.  As a result, the finish breakpoint never gets hit,
and the program runs away...

Interestingly, another symptom of the same problem is trying to step
over that function:

    (gdb) start
    Breakpoint 1 at 0x18e70: file p.adb, line 8.
    Starting program: /[...]/p
    p () at p.adb:8
    8          Small := Create_Small;
    (gdb) next

    Program exited normally.

Ooops!

2010-04-30  Joel Brobecker  <brobecker@adacore.com>

	* sparc-tdep.c (sparc_structure_or_union_p): Return non-zero
	for array types.
	* sparc64-tdep.c (sparc64_structure_or_union_p): Likewise.

Fixed thusly.

I don't think that the same situation applies to sparc64, since I don't
believe that they kept that "unimp" convention for functions returning
a struct or a union.  However, the fact that arrays are like struct should
still apply on sparc64 - hence the decision to fix sparc64-tdep as well.

I have tested this patch on sparc-solaris and sparc64-solaris (2.8).
The issue is that I still am forbidden from running the GDB testsuite
on solaris machines (it causes our machine to crash pretty badly).
So it's been tested with AdaCore's testsuite only.  I would appreciate
if anyone running OpenSolaris could test it; But I have otherwise done
my best to test it.

OK to apply?

thanks,
-- 
Joel

---
 gdb/sparc-tdep.c   |    7 ++++++-
 gdb/sparc64-tdep.c |    7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index a2bae9f..f882505 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -221,7 +221,11 @@ sparc_floating_p (const struct type *type)
   return 0;
 }
 
-/* Check whether TYPE is "Structure or Union".  */
+/* Check whether TYPE is "Structure or Union".
+
+   In terms of subprogram calls, arrays are treated the same as struct
+   and union types.  So this function also returns non-zero for array
+   types.  */
 
 static int
 sparc_structure_or_union_p (const struct type *type)
@@ -230,6 +234,7 @@ sparc_structure_or_union_p (const struct type *type)
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
+    case TYPE_CODE_ARRAY:
       return 1;
     default:
       break;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 9a34834..08d6f1a 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -103,7 +103,11 @@ sparc64_floating_p (const struct type *type)
   return 0;
 }
 
-/* Check whether TYPE is "Structure or Union".  */
+/* Check whether TYPE is "Structure or Union".
+
+   In terms of subprogram calls, arrays are treated the same as struct
+   and union types.  So this function also returns non-zero for array
+   types.  */
 
 static int
 sparc64_structure_or_union_p (const struct type *type)
@@ -112,6 +116,7 @@ sparc64_structure_or_union_p (const struct type *type)
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
+    case TYPE_CODE_ARRAY:
       return 1;
     default:
       break;
-- 
1.6.3.3

Re: [RFA/sparc] "finish" does not work if function returns array.

[Mark Kettenis]

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 30 Apr 2010 15:30:20 -0700
> 
> This is something that is not explicitly covered by the ABI, which
> would explain why it's not covered by the code yet.  In Ada, functions
> can return arrays, for instance:
> 
>    type Data_Small is array (1 .. 2) of Integer;
>    function Create_Small return Data_Small;

Yeah, it's a serious problem that ABIs only tend to cover the C
language.

>  gdb/sparc-tdep.c   |    7 ++++++-
>  gdb/sparc64-tdep.c |    7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
> index a2bae9f..f882505 100644
> --- a/gdb/sparc-tdep.c
> +++ b/gdb/sparc-tdep.c
> @@ -221,7 +221,11 @@ sparc_floating_p (const struct type *type)
>    return 0;
>  }
>  
> -/* Check whether TYPE is "Structure or Union".  */
> +/* Check whether TYPE is "Structure or Union".
> +
> +   In terms of subprogram calls, arrays are treated the same as struct
> +   and union types.  So this function also returns non-zero for array
> +   types.  */

Could you turn that into "In terms of Ada subprogram calls..."?

Re: [RFA/sparc] "finish" does not work if function returns array.

[Joel Brobecker]

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

> > -/* Check whether TYPE is "Structure or Union".  */
> > +/* Check whether TYPE is "Structure or Union".
> > +
> > +   In terms of subprogram calls, arrays are treated the same as struct
> > +   and union types.  So this function also returns non-zero for array
> > +   types.  */
> 
> Could you turn that into "In terms of Ada subprogram calls..."?

Sure! Thanks for the quick review.

Here is a new version of the patch.

-- 
Joel

[-- Attachment #2: arrays-returns-on-sparc.diff --]
[-- Type: text/x-diff, Size: 1895 bytes --]

commit c39300fabadf2ee77ab98accdff7aa6c7df463f0
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Fri Apr 30 15:19:16 2010 -0700

"finish" does not work on sparc if function returns array.

2010-04-30  Joel Brobecker  <brobecker@adacore.com>

	* sparc-tdep.c (sparc_structure_or_union_p): Return non-zero
	for array types.
	* sparc64-tdep.c (sparc64_structure_or_union_p): Likewise.

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index a2bae9f..29a12cf 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -221,7 +221,11 @@ sparc_floating_p (const struct type *type)
   return 0;
 }
 
-/* Check whether TYPE is "Structure or Union".  */
+/* Check whether TYPE is "Structure or Union".
+
+   In terms of Ada subprogram calls, arrays are treated the same as
+   struct and union types.  So this function also returns non-zero
+   for array types.  */
 
 static int
 sparc_structure_or_union_p (const struct type *type)
@@ -230,6 +234,7 @@ sparc_structure_or_union_p (const struct type *type)
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
+    case TYPE_CODE_ARRAY:
       return 1;
     default:
       break;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 9a34834..3cd6109 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -103,7 +103,11 @@ sparc64_floating_p (const struct type *type)
   return 0;
 }
 
-/* Check whether TYPE is "Structure or Union".  */
+/* Check whether TYPE is "Structure or Union".
+
+   In terms of Ada subprogram calls, arrays are treated the same as
+   struct and union types.  So this function also returns non-zero
+   for array types.  */
 
 static int
 sparc64_structure_or_union_p (const struct type *type)
@@ -112,6 +116,7 @@ sparc64_structure_or_union_p (const struct type *type)
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
+    case TYPE_CODE_ARRAY:
       return 1;
     default:
       break;

Re: [RFA/sparc] "finish" does not work if function returns array.

[Joel Brobecker]

> "finish" does not work on sparc if function returns array.
> 
> 2010-04-30  Joel Brobecker  <brobecker@adacore.com>
> 
> 	* sparc-tdep.c (sparc_structure_or_union_p): Return non-zero
> 	for array types.
> 	* sparc64-tdep.c (sparc64_structure_or_union_p): Likewise.

FYI: I just checked in the revised version.

-- 
Joel