[PATCH] alpha: Use ssize_t to allocate space on stack

[PATCH] alpha: Use ssize_t to allocate space on stack

[Siddhesh Poyarekar]

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

Hi,

This is again an independent portion of the bitpos patch[1].  The
variables used to set up the stack are currently int, which may not be
sufficient for larger types.  This patch expands the variable sizes to
ssize_t.  The patch also adds an additional validation to check for
underflow in the value of the stack pointer after pushing the
arguments.  I don't have any means to run any tests, so I have only
done a compile test for this by configuring with --enable-targets=all.
Does this look safe for commit?

Regards,
Siddhesh

[1] http://sourceware.org/ml/gdb-patches/2012-08/msg00144.html

gdb/ChangeLog:

	* alpha-tdep.c (alpha_push_dummy_call): Expand ACCUMULATE_SIZE,
	REQUIRED_ARG_REGS, OFFSET, LEN, TLEN to ssize_t.  Check for
	underflow in SP.
	(struct alpha_arg): Expand members LEN, OFFSET to ssize_t.

[-- Attachment #2: alpha-ssizet.patch --]
[-- Type: text/x-patch, Size: 2113 bytes --]

Index: gdb/alpha-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.c,v
retrieving revision 1.212
diff -u -r1.212 alpha-tdep.c
--- gdb/alpha-tdep.c	25 Sep 2012 12:48:52 -0000	1.212
+++ gdb/alpha-tdep.c	25 Sep 2012 15:21:46 -0000
@@ -299,18 +299,18 @@
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int i;
-  int accumulate_size = struct_return ? 8 : 0;
+  ssize_t accumulate_size = struct_return ? 8 : 0;
   struct alpha_arg
     {
       const gdb_byte *contents;
-      int len;
-      int offset;
+      ssize_t len;
+      ssize_t offset;
     };
   struct alpha_arg *alpha_args
     = (struct alpha_arg *) alloca (nargs * sizeof (struct alpha_arg));
   struct alpha_arg *m_arg;
   gdb_byte arg_reg_buffer[ALPHA_REGISTER_SIZE * ALPHA_NUM_ARG_REGS];
-  int required_arg_regs;
+  ssize_t required_arg_regs;
   CORE_ADDR func_addr = find_function_addr (function, NULL);
 
   /* The ABI places the address of the called function in T12.  */
@@ -414,6 +414,13 @@
     accumulate_size = 0;
   else
     accumulate_size -= sizeof(arg_reg_buffer);
+
+  /* Check for underflow.  */
+  if (sp - accumulate_size > sp)
+    error (_("Insufficient memory in GDB host for arguments, "
+	     "need %s bytes, but less than %s bytes available."),
+	   plongest (accumulate_size), plongest (CORE_ADDR_MAX - sp));
+
   sp -= accumulate_size;
 
   /* Keep sp aligned to a multiple of 16 as the ABI requires.  */
@@ -423,8 +430,8 @@
   for (i = nargs; m_arg--, --i >= 0;)
     {
       const gdb_byte *contents = m_arg->contents;
-      int offset = m_arg->offset;
-      int len = m_arg->len;
+      ssize_t offset = m_arg->offset;
+      ssize_t len = m_arg->len;
 
       /* Copy the bytes destined for registers into arg_reg_buffer.  */
       if (offset < sizeof(arg_reg_buffer))
@@ -436,7 +443,7 @@
 	    }
 	  else
 	    {
-	      int tlen = sizeof(arg_reg_buffer) - offset;
+	      ssize_t tlen = sizeof(arg_reg_buffer) - offset;
 	      memcpy (arg_reg_buffer + offset, contents, tlen);
 	      offset += tlen;
 	      contents += tlen;

Re: [PATCH] alpha: Use ssize_t to allocate space on stack

[Jan Kratochvil]

On Tue, 25 Sep 2012 17:37:20 +0200, Siddhesh Poyarekar wrote:
> --- gdb/alpha-tdep.c	25 Sep 2012 12:48:52 -0000	1.212
> +++ gdb/alpha-tdep.c	25 Sep 2012 15:21:46 -0000
> @@ -299,18 +299,18 @@
>  {
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    int i;
> -  int accumulate_size = struct_return ? 8 : 0;
> +  ssize_t accumulate_size = struct_return ? 8 : 0;
>    struct alpha_arg
>      {
>        const gdb_byte *contents;
> -      int len;
> -      int offset;
> +      ssize_t len;
> +      ssize_t offset;
>      };
>    struct alpha_arg *alpha_args
>      = (struct alpha_arg *) alloca (nargs * sizeof (struct alpha_arg));
>    struct alpha_arg *m_arg;
>    gdb_byte arg_reg_buffer[ALPHA_REGISTER_SIZE * ALPHA_NUM_ARG_REGS];
> -  int required_arg_regs;
> +  ssize_t required_arg_regs;
>    CORE_ADDR func_addr = find_function_addr (function, NULL);
>  
>    /* The ABI places the address of the called function in T12.  */

At line 409 is also code:
       m_arg->len = TYPE_LENGTH (arg_type);

This is unsafe with extended TYPE_LENGTH width, because LONGEST > ssize_t.


> @@ -414,6 +414,13 @@
>      accumulate_size = 0;
>    else
>      accumulate_size -= sizeof(arg_reg_buffer);
> +
> +  /* Check for underflow.  */
> +  if (sp - accumulate_size > sp)
> +    error (_("Insufficient memory in GDB host for arguments, "
> +	     "need %s bytes, but less than %s bytes available."),
> +	   plongest (accumulate_size), plongest (CORE_ADDR_MAX - sp));
> +
>    sp -= accumulate_size;
>  
>    /* Keep sp aligned to a multiple of 16 as the ABI requires.  */
> @@ -423,8 +430,8 @@
>    for (i = nargs; m_arg--, --i >= 0;)
>      {
>        const gdb_byte *contents = m_arg->contents;
> -      int offset = m_arg->offset;
> -      int len = m_arg->len;
> +      ssize_t offset = m_arg->offset;
> +      ssize_t len = m_arg->len;
>  
>        /* Copy the bytes destined for registers into arg_reg_buffer.  */
>        if (offset < sizeof(arg_reg_buffer))
> @@ -436,7 +443,7 @@
>  	    }
>  	  else
>  	    {
> -	      int tlen = sizeof(arg_reg_buffer) - offset;
> +	      ssize_t tlen = sizeof(arg_reg_buffer) - offset;

FYI this is not needed; but the code may be easier keeping it as you wrote it.


>  	      memcpy (arg_reg_buffer + offset, contents, tlen);
>  	      offset += tlen;
>  	      contents += tlen;


Thanks,
Jan

Re: [PATCH] alpha: Use ssize_t to allocate space on stack

[Siddhesh Poyarekar]

On Fri, 28 Sep 2012 10:29:13 +0200, Jan wrote:
> >    /* The ABI places the address of the called function in T12.  */
> 
> At line 409 is also code:
>        m_arg->len = TYPE_LENGTH (arg_type);
> 
> This is unsafe with extended TYPE_LENGTH width, because LONGEST >
> ssize_t.
> 

OK.  The patch is now part of the bitpos patches, so we can ignore this.

Thanks,
Siddhesh