Re: [PATCH] [sparc] inferior SEGV while calling Ada subprogram

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Thu, 30 Sep 2010 19:40:51 -0700
> 
> This is one of these issues that makes you wonder how we managed
> to miss it after all these years...  But it's probably just because
> our users simply never call subprograms that take an array as one
> of the parameters - it's just the AdaCore testsuite that does :).
> 
> The problem can be reproduced on sparc-solaris using gdb.ada/arrayparam.exp:
> 
>    (gdb) print call_me ("bonjour")
>    
>    Program received signal SIGSEGV, Segmentation fault.
>    0x000190d0 in pck.call_me (str=...) at /[...]/gdb.ada/arrayparam/pck.adb:18
>    18         procedure Call_Me (Str : String) is
>    The program being debugged was signaled while in a function called from GDB.
>    [...]
> 
> The SEGV is due to an unaligned access.  This is because Ada arrays
> can take several forms, and the one that is used in this case is
> a reference to a structure containing 2 pointers: One pointer to
> the array of characters, and one pointer to another structure containing
> the bounds. We call them `fat pointers'.
> 
> During the function call setup, there is some special code for Ada
> inside value_arg_coerce that converts a C-type array into Ada arrays:
> 
>   /* Perform any Ada-specific coercion first.  */
>   if (current_language->la_language == language_ada)
>     arg = ada_convert_actual (arg, type, gdbarch, sp);
> 
> What this function does is allocating some memory on the stack
> in order to store the array contents, the bounds, and the descriptor.
> We start with the bounds, followed by the array, followed by the
> descriptor. And in between each piece, we re-align the stack.
> 
> That's when I realized that sparc-tdep and sparc64-tdep do not
> provide a routine that re-aligns the stack pointer.  So, with
> the array above (7 bytes), once the array was allocated on stack,
> the new stack-pointer value was badly misaligned. As a result,
> the array descriptors gets allocated at a mis-aligned address,
> and trying to load the addresses inside that descriptor triggered
> a SEGV.
> 
> gdb/ChangeLog:
> 
>         * sparc-tdep.c (sparc32_frame_align): New function.
>         (sparc32_gdbarch_init): Set the frame_align gdbarch method.
>         * sparc64-tdep.c (sparc64_frame_align): New function.
>         (sparc64_gdbarch_init): Set the frame_align gdbarch method.
> 
> This fixes gdb.ada/arrayparam.exp. It also runs clean on AdaCore's
> testsuite (Python-based), for sparc-solaris and sparc64-solaris.
> However, as said previously, I am barred from running the testsuite
> on any Solaris machine until further notice (it looks like one of
> the testcases causes the machine to crash pretty badly - and I haven't
> be able to get a window where it's OK to investigate that).
> 
> I don't see how this could hurt, though.
> 
> OK to commit?

Looks ok to me.  However...

> diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
> index 29a12cf..a35db79 100644
> --- a/gdb/sparc-tdep.c
> +++ b/gdb/sparc-tdep.c
> @@ -398,6 +398,15 @@ sparc32_pseudo_register_write (struct gdbarch *gdbarch,
>  }
>  
>  
> +/* Implement the frame_align gdbarch method.  */
> +

That comment is pretty meaningless.  Can you leave it out?

> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index 3cd6109..978501a 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -937,6 +937,15 @@ sparc64_store_arguments (struct regcache *regcache, int nargs,
>    return sp;
>  }
>  
> +/* Implement the frame_align gdbarch method.  */
> +

Same here.