Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Allow struct 'return' on 32-bit sparc.
@ 2013-02-01 21:13 David Miller
  2013-02-04 23:57 ` Mark Kettenis
  2013-02-08 18:17 ` Ulrich Weigand
  0 siblings, 2 replies; 9+ messages in thread
From: David Miller @ 2013-02-01 21:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: kettenis


I went through a long, winding, and somewhat amusing path to arrive at
this patch.

call-sc.exp tests return value handling for scalars, but some larger
scalars don't get returned in registers and gdb is not able to
override the return value for 'return xxx' commands in most of
those cases.

call-sc.exp tries to accomodate this and make the test still pass when
gdb reports that it has hit this situation.  However the logic used is
not foolproof.

If gdb cannot write the return value, it pops the stackframe only.
What this means is that the return value we end up with is going to
be essentially random.

For example, if the calling convention is like 32-bit sparc, a fixed
stack frame slot is used for these return values.  Whatever happened
to be there before the function call is the value that we will end up
with.

Taking a step back I went to look at why gdb can't override the return
value in this situation in the first place.

Unlike other targets, we can actually properly override the return
value on 32-bit sparc because it is placed in a fixed location offset
from the stack pointer.

On other targets, the function is passed the memory area address as
an argument and returns that address as a return value.  When gdb
pops the stack for the return command, this address return value
won't be set and therefore gdb doesn't have access to the location.

It looks like the various RETURN_VALUE_* cases were split up
specifically with this exact distinction in mind.

So I changed it such that return_command will perform a successful
return value override not just for RETURN_VALUE_REGISTER_CONVENTION,
but for RETURN_VALUE_ABI_PRESERVES_ADDRESS as well.

Then I converted the one target using RETURN_VALUE_ABI_PRESERVES_ADDRESS
to support writing a value in this case.

Any objections?

Meanwhile I'll work on trying to eliminate the deficiencies of the
call-sc.exp test case, even though with this patch it now passes on
32-bit sparc.

gdb/

	* sparc-tdep.c (sparc32_return_value): Handle writing return value when
	using RETURN_VALUE_ABI_PRESERVES_ADDRESS.
	* value.c (convention_for_return): New function.
	(using_struct_return): Implement in terms of convention_for_return.
	* value.h (convention_for_return): Declare.
	* stack.c (return_command): Allow successful overriding of the return
	value when RETURN_VALUE_ABI_PRESERVES_ADDRESS.

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 3ae7395..2b38521 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1370,15 +1370,21 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
   if (sparc_structure_or_union_p (type)
       || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
     {
+      ULONGEST sp;
+      CORE_ADDR addr;
+
       if (readbuf)
 	{
-	  ULONGEST sp;
-	  CORE_ADDR addr;
-
 	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
 	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
 	}
+      if (writebuf)
+	{
+	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
+	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
+	  write_memory (addr, writebuf, TYPE_LENGTH (type));
+	}
 
       return RETURN_VALUE_ABI_PRESERVES_ADDRESS;
     }
diff --git a/gdb/stack.c b/gdb/stack.c
index c8a6a7e..17950a2 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2274,6 +2274,7 @@ down_command (char *count_exp, int from_tty)
 void
 return_command (char *retval_exp, int from_tty)
 {
+  enum return_value_convention rv_conv;
   struct frame_info *thisframe;
   struct gdbarch *gdbarch;
   struct symbol *thisfun;
@@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
       if (thisfun != NULL)
 	function = read_var_value (thisfun, thisframe);
 
+      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
       if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
 	/* If the return-type is "void", don't try to find the
            return-value's location.  However, do still evaluate the
@@ -2334,14 +2336,18 @@ return_command (char *retval_exp, int from_tty)
            is discarded, side effects such as "return i++" still
            occur.  */
 	return_value = NULL;
-      else if (thisfun != NULL
-	       && using_struct_return (gdbarch, function, return_type))
+      else if (thisfun != NULL)
 	{
-	  query_prefix = "The location at which to store the "
-	    "function's return value is unknown.\n"
-	    "If you continue, the return value "
-	    "that you specified will be ignored.\n";
-	  return_value = NULL;
+	  rv_conv = convention_for_return (gdbarch, function, return_type);
+	  if (rv_conv == RETURN_VALUE_STRUCT_CONVENTION
+	      || rv_conv == RETURN_VALUE_ABI_RETURNS_ADDRESS)
+	    {
+	      query_prefix = "The location at which to store the "
+		"function's return value is unknown.\n"
+		"If you continue, the return value "
+		"that you specified will be ignored.\n";
+	      return_value = NULL;
+	    }
 	}
     }
 
@@ -2371,9 +2377,8 @@ return_command (char *retval_exp, int from_tty)
       struct type *return_type = value_type (return_value);
       struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
 
-      gdb_assert (gdbarch_return_value (gdbarch, function, return_type, NULL,
-					NULL, NULL)
-		  == RETURN_VALUE_REGISTER_CONVENTION);
+      gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION
+		  && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS);
       gdbarch_return_value (gdbarch, function, return_type,
 			    get_current_regcache (), NULL /*read*/,
 			    value_contents (return_value) /*write*/);
diff --git a/gdb/value.c b/gdb/value.c
index dbf1c37..07b7dbf 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3323,13 +3323,12 @@ coerce_array (struct value *arg)
 }
 \f
 
-/* Return true if the function returning the specified type is using
-   the convention of returning structures in memory (passing in the
-   address as a hidden first parameter).  */
+/* Return the return value convention that will be used for the
+   specified type.  */
 
-int
-using_struct_return (struct gdbarch *gdbarch,
-		     struct value *function, struct type *value_type)
+enum return_value_convention
+convention_for_return (struct gdbarch *gdbarch,
+		       struct value *function, struct type *value_type)
 {
   enum type_code code = TYPE_CODE (value_type);
 
@@ -3339,11 +3338,22 @@ using_struct_return (struct gdbarch *gdbarch,
   if (code == TYPE_CODE_VOID)
     /* A void return value is never in memory.  See also corresponding
        code in "print_return_value".  */
-    return 0;
+    return RETURN_VALUE_REGISTER_CONVENTION;
 
   /* Probe the architecture for the return-value convention.  */
-  return (gdbarch_return_value (gdbarch, function, value_type,
-				NULL, NULL, NULL)
+  return gdbarch_return_value (gdbarch, function, value_type,
+			       NULL, NULL, NULL);
+}
+
+/* Return true if the function returning the specified type is using
+   the convention of returning structures in memory (passing in the
+   address as a hidden first parameter).  */
+
+int
+using_struct_return (struct gdbarch *gdbarch,
+		     struct value *function, struct type *value_type)
+{
+  return (convention_for_return (gdbarch, function, value_type)
 	  != RETURN_VALUE_REGISTER_CONVENTION);
 }
 
diff --git a/gdb/value.h b/gdb/value.h
index b9013fd..0668fd9 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -696,6 +696,10 @@ extern int value_in (struct value *element, struct value *set);
 extern int value_bit_index (struct type *type, const gdb_byte *addr,
 			    int index);
 
+extern enum return_value_convention
+convention_for_return (struct gdbarch *gdbarch, struct value *function,
+		       struct type *value_type);
+
 extern int using_struct_return (struct gdbarch *gdbarch,
 				struct value *function,
 				struct type *value_type);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-01 21:13 [PATCH] Allow struct 'return' on 32-bit sparc David Miller
@ 2013-02-04 23:57 ` Mark Kettenis
  2013-02-05  1:03   ` David Miller
  2013-02-08 18:17 ` Ulrich Weigand
  1 sibling, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2013-02-04 23:57 UTC (permalink / raw)
  To: davem; +Cc: gdb-patches

> Date: Fri, 01 Feb 2013 16:13:00 -0500 (EST)
> From: David Miller <davem@davemloft.net>
> 
> If gdb cannot write the return value, it pops the stackframe only.
> What this means is that the return value we end up with is going to
> be essentially random.

It does warn you though.

> For example, if the calling convention is like 32-bit sparc, a fixed
> stack frame slot is used for these return values.  Whatever happened
> to be there before the function call is the value that we will end up
> with.
> 
> Taking a step back I went to look at why gdb can't override the return
> value in this situation in the first place.
> 
> Unlike other targets, we can actually properly override the return
> value on 32-bit sparc because it is placed in a fixed location offset
> from the stack pointer.
> 
> On other targets, the function is passed the memory area address as
> an argument and returns that address as a return value.  When gdb
> pops the stack for the return command, this address return value
> won't be set and therefore gdb doesn't have access to the location.
> 
> It looks like the various RETURN_VALUE_* cases were split up
> specifically with this exact distinction in mind.

Yup.  Not sure why I never finished it though.

> So I changed it such that return_command will perform a successful
> return value override not just for RETURN_VALUE_REGISTER_CONVENTION,
> but for RETURN_VALUE_ABI_PRESERVES_ADDRESS as well.
> 
> Then I converted the one target using RETURN_VALUE_ABI_PRESERVES_ADDRESS
> to support writing a value in this case.
> 
> Any objections?

The sparc-tdep.c bits are correct as far as I can see.

I think it would be better if your new function would still have
"struct_return" in its name.  My suggestion would be
"struct_return_convention".

Also,

> diff --git a/gdb/stack.c b/gdb/stack.c
> index c8a6a7e..17950a2 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2274,6 +2274,7 @@ down_command (char *count_exp, int from_tty)
>  void
>  return_command (char *retval_exp, int from_tty)
>  {
> +  enum return_value_convention rv_conv;
>    struct frame_info *thisframe;
>    struct gdbarch *gdbarch;
>    struct symbol *thisfun;
> @@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
>        if (thisfun != NULL)
>  	function = read_var_value (thisfun, thisframe);
>  
> +      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
>        if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
>  	/* If the return-type is "void", don't try to find the
>             return-value's location.  However, do still evaluate the
> @@ -2334,14 +2336,18 @@ return_command (char *retval_exp, int from_tty)
>             is discarded, side effects such as "return i++" still
>             occur.  */
>  	return_value = NULL;
> -      else if (thisfun != NULL
> -	       && using_struct_return (gdbarch, function, return_type))
> +      else if (thisfun != NULL)
>  	{
> -	  query_prefix = "The location at which to store the "
> -	    "function's return value is unknown.\n"
> -	    "If you continue, the return value "
> -	    "that you specified will be ignored.\n";
> -	  return_value = NULL;
> +	  rv_conv = convention_for_return (gdbarch, function, return_type);
> +	  if (rv_conv == RETURN_VALUE_STRUCT_CONVENTION
> +	      || rv_conv == RETURN_VALUE_ABI_RETURNS_ADDRESS)
> +	    {
> +	      query_prefix = "The location at which to store the "
> +		"function's return value is unknown.\n"
> +		"If you continue, the return value "
> +		"that you specified will be ignored.\n";
> +	      return_value = NULL;
> +	    }
>  	}
>      }
>  
> @@ -2371,9 +2377,8 @@ return_command (char *retval_exp, int from_tty)
>        struct type *return_type = value_type (return_value);
>        struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
>  
> -      gdb_assert (gdbarch_return_value (gdbarch, function, return_type, NULL,
> -					NULL, NULL)
> -		  == RETURN_VALUE_REGISTER_CONVENTION);
> +      gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION
> +		  && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS);
>        gdbarch_return_value (gdbarch, function, return_type,
>  			    get_current_regcache (), NULL /*read*/,
>  			    value_contents (return_value) /*write*/);
> diff --git a/gdb/value.c b/gdb/value.c
> index dbf1c37..07b7dbf 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3323,13 +3323,12 @@ coerce_array (struct value *arg)
>  }
>  \f
>  
> -/* Return true if the function returning the specified type is using
> -   the convention of returning structures in memory (passing in the
> -   address as a hidden first parameter).  */
> +/* Return the return value convention that will be used for the
> +   specified type.  */
>  
> -int
> -using_struct_return (struct gdbarch *gdbarch,
> -		     struct value *function, struct type *value_type)
> +enum return_value_convention
> +convention_for_return (struct gdbarch *gdbarch,
> +		       struct value *function, struct type *value_type)
>  {
>    enum type_code code = TYPE_CODE (value_type);
>  
> @@ -3339,11 +3338,22 @@ using_struct_return (struct gdbarch *gdbarch,
>    if (code == TYPE_CODE_VOID)
>      /* A void return value is never in memory.  See also corresponding
>         code in "print_return_value".  */
> -    return 0;
> +    return RETURN_VALUE_REGISTER_CONVENTION;

Return RETURN_VALUE_REGISTER_CONVENTION if there is nothing to return
seems to be a bit odd.  But since convention_for_return() is never
called with code being TYPE_CODE (there is an explicit check for that
in stack.c:return_command()) you could simply leave the check in
using_struct_return().


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-04 23:57 ` Mark Kettenis
@ 2013-02-05  1:03   ` David Miller
  2013-02-05 20:19     ` Mark Kettenis
  2013-02-06 19:41     ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: David Miller @ 2013-02-05  1:03 UTC (permalink / raw)
  To: mark.kettenis; +Cc: gdb-patches

From: Mark Kettenis <mark.kettenis@xs4all.nl>
Date: Tue, 5 Feb 2013 00:57:15 +0100 (CET)

> The sparc-tdep.c bits are correct as far as I can see.
> 
> I think it would be better if your new function would still have
> "struct_return" in its name.  My suggestion would be
> "struct_return_convention".
> 
> Also,
 ...
> Return RETURN_VALUE_REGISTER_CONVENTION if there is nothing to return
> seems to be a bit odd.  But since convention_for_return() is never
> called with code being TYPE_CODE (there is an explicit check for that
> in stack.c:return_command()) you could simply leave the check in
> using_struct_return().

Agreed on all counts, thanks for the feedback.

Here is the updated patch:

gdb/

	* sparc-tdep.c (sparc32_return_value): Handle writing return value when
	using RETURN_VALUE_ABI_PRESERVES_ADDRESS.
	* value.c (struct_return_convention): New function.
	(using_struct_return): Implement in terms of struct_return_convention.
	* value.h (struct_return_convention): Declare.
	* stack.c (return_command): Allow successful overriding of the return
	value when RETURN_VALUE_ABI_PRESERVES_ADDRESS.


diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 3ae7395..2b38521 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1370,15 +1370,21 @@ sparc32_return_value (struct gdbarch *gdbarch, struct value *function,
   if (sparc_structure_or_union_p (type)
       || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16))
     {
+      ULONGEST sp;
+      CORE_ADDR addr;
+
       if (readbuf)
 	{
-	  ULONGEST sp;
-	  CORE_ADDR addr;
-
 	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
 	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
 	}
+      if (writebuf)
+	{
+	  regcache_cooked_read_unsigned (regcache, SPARC_SP_REGNUM, &sp);
+	  addr = read_memory_unsigned_integer (sp + 64, 4, byte_order);
+	  write_memory (addr, writebuf, TYPE_LENGTH (type));
+	}
 
       return RETURN_VALUE_ABI_PRESERVES_ADDRESS;
     }
diff --git a/gdb/stack.c b/gdb/stack.c
index c8a6a7e..218fc01 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2274,6 +2274,7 @@ down_command (char *count_exp, int from_tty)
 void
 return_command (char *retval_exp, int from_tty)
 {
+  enum return_value_convention rv_conv;
   struct frame_info *thisframe;
   struct gdbarch *gdbarch;
   struct symbol *thisfun;
@@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
       if (thisfun != NULL)
 	function = read_var_value (thisfun, thisframe);
 
+      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
       if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
 	/* If the return-type is "void", don't try to find the
            return-value's location.  However, do still evaluate the
@@ -2334,14 +2336,18 @@ return_command (char *retval_exp, int from_tty)
            is discarded, side effects such as "return i++" still
            occur.  */
 	return_value = NULL;
-      else if (thisfun != NULL
-	       && using_struct_return (gdbarch, function, return_type))
+      else if (thisfun != NULL)
 	{
-	  query_prefix = "The location at which to store the "
-	    "function's return value is unknown.\n"
-	    "If you continue, the return value "
-	    "that you specified will be ignored.\n";
-	  return_value = NULL;
+	  rv_conv = struct_return_convention (gdbarch, function, return_type);
+	  if (rv_conv == RETURN_VALUE_STRUCT_CONVENTION
+	      || rv_conv == RETURN_VALUE_ABI_RETURNS_ADDRESS)
+	    {
+	      query_prefix = "The location at which to store the "
+		"function's return value is unknown.\n"
+		"If you continue, the return value "
+		"that you specified will be ignored.\n";
+	      return_value = NULL;
+	    }
 	}
     }
 
@@ -2371,9 +2377,8 @@ return_command (char *retval_exp, int from_tty)
       struct type *return_type = value_type (return_value);
       struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
 
-      gdb_assert (gdbarch_return_value (gdbarch, function, return_type, NULL,
-					NULL, NULL)
-		  == RETURN_VALUE_REGISTER_CONVENTION);
+      gdb_assert (rv_conv != RETURN_VALUE_STRUCT_CONVENTION
+		  && rv_conv != RETURN_VALUE_ABI_RETURNS_ADDRESS);
       gdbarch_return_value (gdbarch, function, return_type,
 			    get_current_regcache (), NULL /*read*/,
 			    value_contents (return_value) /*write*/);
diff --git a/gdb/value.c b/gdb/value.c
index dbf1c37..4b70ece 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3323,6 +3323,23 @@ coerce_array (struct value *arg)
 }
 \f
 
+/* Return the return value convention that will be used for the
+   specified type.  */
+
+enum return_value_convention
+struct_return_convention (struct gdbarch *gdbarch,
+			  struct value *function, struct type *value_type)
+{
+  enum type_code code = TYPE_CODE (value_type);
+
+  if (code == TYPE_CODE_ERROR)
+    error (_("Function return type unknown."));
+
+  /* Probe the architecture for the return-value convention.  */
+  return gdbarch_return_value (gdbarch, function, value_type,
+			       NULL, NULL, NULL);
+}
+
 /* Return true if the function returning the specified type is using
    the convention of returning structures in memory (passing in the
    address as a hidden first parameter).  */
@@ -3331,19 +3348,12 @@ int
 using_struct_return (struct gdbarch *gdbarch,
 		     struct value *function, struct type *value_type)
 {
-  enum type_code code = TYPE_CODE (value_type);
-
-  if (code == TYPE_CODE_ERROR)
-    error (_("Function return type unknown."));
-
-  if (code == TYPE_CODE_VOID)
+  if (TYPE_CODE (value_type) == TYPE_CODE_VOID)
     /* A void return value is never in memory.  See also corresponding
        code in "print_return_value".  */
     return 0;
 
-  /* Probe the architecture for the return-value convention.  */
-  return (gdbarch_return_value (gdbarch, function, value_type,
-				NULL, NULL, NULL)
+  return (struct_return_convention (gdbarch, function, value_type)
 	  != RETURN_VALUE_REGISTER_CONVENTION);
 }
 
diff --git a/gdb/value.h b/gdb/value.h
index b9013fd..c10c3ec 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -696,6 +696,10 @@ extern int value_in (struct value *element, struct value *set);
 extern int value_bit_index (struct type *type, const gdb_byte *addr,
 			    int index);
 
+extern enum return_value_convention
+struct_return_convention (struct gdbarch *gdbarch, struct value *function,
+			  struct type *value_type);
+
 extern int using_struct_return (struct gdbarch *gdbarch,
 				struct value *function,
 				struct type *value_type);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-05  1:03   ` David Miller
@ 2013-02-05 20:19     ` Mark Kettenis
  2013-02-05 20:31       ` David Miller
  2013-02-06 19:41     ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2013-02-05 20:19 UTC (permalink / raw)
  To: davem; +Cc: gdb-patches

> Date: Mon, 04 Feb 2013 20:02:54 -0500 (EST)
> From: David Miller <davem@davemloft.net>
> 
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> Date: Tue, 5 Feb 2013 00:57:15 +0100 (CET)
> 
> > The sparc-tdep.c bits are correct as far as I can see.
> > 
> > I think it would be better if your new function would still have
> > "struct_return" in its name.  My suggestion would be
> > "struct_return_convention".
> > 
> > Also,
>  ...
> > Return RETURN_VALUE_REGISTER_CONVENTION if there is nothing to return
> > seems to be a bit odd.  But since convention_for_return() is never
> > called with code being TYPE_CODE (there is an explicit check for that
> > in stack.c:return_command()) you could simply leave the check in
> > using_struct_return().
> 
> Agreed on all counts, thanks for the feedback.
> 
> Here is the updated patch:
> 
> gdb/
> 
> 	* sparc-tdep.c (sparc32_return_value): Handle writing return value when
> 	using RETURN_VALUE_ABI_PRESERVES_ADDRESS.
> 	* value.c (struct_return_convention): New function.
> 	(using_struct_return): Implement in terms of struct_return_convention.
> 	* value.h (struct_return_convention): Declare.
> 	* stack.c (return_command): Allow successful overriding of the return
> 	value when RETURN_VALUE_ABI_PRESERVES_ADDRESS.

Thanks, that looks good to me.  Perhaps wait a day or two to give
other people a chance to comment on the stack.c/value.c changes.

Mark


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-05 20:19     ` Mark Kettenis
@ 2013-02-05 20:31       ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-02-05 20:31 UTC (permalink / raw)
  To: mark.kettenis; +Cc: gdb-patches

From: Mark Kettenis <mark.kettenis@xs4all.nl>
Date: Tue, 5 Feb 2013 21:18:58 +0100 (CET)

> Perhaps wait a day or two to give other people a chance to comment
> on the stack.c/value.c changes.

Ok, will do, thanks Mark.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-05  1:03   ` David Miller
  2013-02-05 20:19     ` Mark Kettenis
@ 2013-02-06 19:41     ` David Miller
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2013-02-06 19:41 UTC (permalink / raw)
  To: mark.kettenis; +Cc: gdb-patches

From: David Miller <davem@davemloft.net>
Date: Mon, 04 Feb 2013 20:02:54 -0500 (EST)

> Here is the updated patch:
> 
> gdb/
> 
> 	* sparc-tdep.c (sparc32_return_value): Handle writing return value when
> 	using RETURN_VALUE_ABI_PRESERVES_ADDRESS.
> 	* value.c (struct_return_convention): New function.
> 	(using_struct_return): Implement in terms of struct_return_convention.
> 	* value.h (struct_return_convention): Declare.
> 	* stack.c (return_command): Allow successful overriding of the return
> 	value when RETURN_VALUE_ABI_PRESERVES_ADDRESS.

I've committed these changes.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-01 21:13 [PATCH] Allow struct 'return' on 32-bit sparc David Miller
  2013-02-04 23:57 ` Mark Kettenis
@ 2013-02-08 18:17 ` Ulrich Weigand
  2013-02-08 19:10   ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2013-02-08 18:17 UTC (permalink / raw)
  To: David Miller; +Cc: gdb-patches, kettenis

David Miller wrote:

>  void
>  return_command (char *retval_exp, int from_tty)
>  {
> +  enum return_value_convention rv_conv;
>    struct frame_info *thisframe;
>    struct gdbarch *gdbarch;
>    struct symbol *thisfun;

This gives me:

cc1: warnings being treated as errors
/home/kwerner/dailybuild/spu-tc-2013-02-08/gdb-head/src/gdb/stack.c: In function 'return_command':
/home/kwerner/dailybuild/spu-tc-2013-02-08/gdb-head/src/gdb/stack.c:2281: warning: 'rv_conv' may be used uninitialized in this function

when building with GCC 4.1.2 (as of RHEL5).

> @@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
>        if (thisfun != NULL)
>  	function = read_var_value (thisfun, thisframe);
>  
> +      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
>        if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
>  	/* If the return-type is "void", don't try to find the
>             return-value's location.  However, do still evaluate the

Maybe just move this up to the definition?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-08 18:17 ` Ulrich Weigand
@ 2013-02-08 19:10   ` David Miller
  2013-02-08 19:13     ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2013-02-08 19:10 UTC (permalink / raw)
  To: uweigand; +Cc: gdb-patches, kettenis

From: "Ulrich Weigand" <uweigand@de.ibm.com>
Date: Fri, 8 Feb 2013 19:16:39 +0100 (CET)

> David Miller wrote:
> 
>> @@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
>>        if (thisfun != NULL)
>>  	function = read_var_value (thisfun, thisframe);
>>  
>> +      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
>>        if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
>>  	/* If the return-type is "void", don't try to find the
>>             return-value's location.  However, do still evaluate the
> 
> Maybe just move this up to the definition?

Yes, that's a good idea, I'll commit such a fix.

Thanks.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Allow struct 'return' on 32-bit sparc.
  2013-02-08 19:10   ` David Miller
@ 2013-02-08 19:13     ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2013-02-08 19:13 UTC (permalink / raw)
  To: uweigand; +Cc: gdb-patches, kettenis

From: David Miller <davem@davemloft.net>
Date: Fri, 08 Feb 2013 14:10:07 -0500 (EST)

> From: "Ulrich Weigand" <uweigand@de.ibm.com>
> Date: Fri, 8 Feb 2013 19:16:39 +0100 (CET)
> 
>> David Miller wrote:
>> 
>>> @@ -2327,6 +2328,7 @@ return_command (char *retval_exp, int from_tty)
>>>        if (thisfun != NULL)
>>>  	function = read_var_value (thisfun, thisframe);
>>>  
>>> +      rv_conv = RETURN_VALUE_REGISTER_CONVENTION;
>>>        if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
>>>  	/* If the return-type is "void", don't try to find the
>>>             return-value's location.  However, do still evaluate the
>> 
>> Maybe just move this up to the definition?
> 
> Yes, that's a good idea, I'll commit such a fix.

Actually someone beat me to it :-)

2013-02-08  Matthew Gretton-Dann  <matthew.gretton-dann@linaro.org>

	* stack.c (return_command): Work around uninitialized variable
	warning.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-02-08 19:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-01 21:13 [PATCH] Allow struct 'return' on 32-bit sparc David Miller
2013-02-04 23:57 ` Mark Kettenis
2013-02-05  1:03   ` David Miller
2013-02-05 20:19     ` Mark Kettenis
2013-02-05 20:31       ` David Miller
2013-02-06 19:41     ` David Miller
2013-02-08 18:17 ` Ulrich Weigand
2013-02-08 19:10   ` David Miller
2013-02-08 19:13     ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox