[PATCH/RFC/RFA] Print in-memory struct return values

[PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

The current GDB doesn't print the return value when using `finish' for
functions return structures that are not returned in registers.  Note
that this is a regression from GDB 6.0 for many systems.  Anyway, the
attached patch provides a way to fix this, and adds the necessary
support to the i386 target.

If there are no comments, I'll check this in in a few days.  Eli, is
the doc bit OK?

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>
 
	* gdbarch.sh (gdbarch_return_value_address): New method.
	* gdbarch.h, gdbarch.c: Re-generate.
	* infcmd.c (print_return_value): Use new
	gdbarch_return_value_address method if provided.
	* i386-tdep.c (i386_return_value_address): New function.
	(i386_gdbarch_init): Set return_value_address.

 
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.108
diff -u -p -r1.108 infcmd.c
--- infcmd.c 28 Apr 2004 16:36:25 -0000 1.108
+++ infcmd.c 7 May 2004 17:05:04 -0000
@@ -1061,9 +1061,6 @@ print_return_value (int struct_return, s
       /* The return value can be found in the inferior's registers.  */
       value = register_value_being_returned (value_type, stop_registers);
     }
-  /* FIXME: cagney/2004-01-17: When both return_value and
-     extract_returned_value_address are available, should use that to
-     find the address of and then extract the returned value.  */
   /* FIXME: 2003-09-27: When returning from a nested inferior function
      call, it's possible (with no help from the architecture vector)
      to locate and return/print a "struct return" value.  This is just
@@ -1079,11 +1076,23 @@ print_return_value (int struct_return, s
       gdb_assert (gdbarch_return_value (current_gdbarch, value_type,
 					NULL, NULL, NULL)
 		  == RETURN_VALUE_STRUCT_CONVENTION);
-      ui_out_text (uiout, "Value returned has type: ");
-      ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type));
-      ui_out_text (uiout, ".");
-      ui_out_text (uiout, " Cannot determine contents\n");
-      return;
+
+      if (gdbarch_return_value_address_p (current_gdbarch))
+	{
+	  CORE_ADDR addr;
+
+	  addr = gdbarch_return_value_address (current_gdbarch,
+					       stop_registers);
+	  value = value_at (value_type, addr, NULL);
+	}
+      else
+	{
+	  ui_out_text (uiout, "Value returned has type: ");
+	  ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type));
+	  ui_out_text (uiout, ".");
+	  ui_out_text (uiout, " Cannot determine contents\n");
+	  return;
+	}
     }
   else
     {
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.311
diff -u -p -r1.311 gdbarch.sh
--- gdbarch.sh 5 May 2004 15:42:52 -0000 1.311
+++ gdbarch.sh 7 May 2004 17:05:05 -0000
@@ -594,6 +594,8 @@ F:2:DEPRECATED_STORE_STRUCT_RETURN:void:
 
 M:::enum return_value_convention:return_value:struct type *valtype, struct regcache *regcache, void *readbuf, const void *writebuf:valtype, regcache, readbuf, writebuf
 
+M:::CORE_ADDR:return_value_address:struct regcache *regcache:regcache
+
 # The deprecated methods RETURN_VALUE_ON_STACK, EXTRACT_RETURN_VALUE,
 # STORE_RETURN_VALUE and USE_STRUCT_CONVENTION have all been folded
 # into RETURN_VALUE.
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.190
diff -u -p -r1.190 i386-tdep.c
--- i386-tdep.c 30 Apr 2004 21:13:58 -0000 1.190
+++ i386-tdep.c 7 May 2004 17:05:05 -0000
@@ -1373,6 +1373,20 @@ i386_return_value (struct gdbarch *gdbar
 
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
+
+/* Return the address where the return value can be found for
+   functions that use the REGISTER_VALUE_STRUCT_RETURN convention.
+   REGCACHE contains a copy of the registers from the just returned
+   function.  */
+
+static CORE_ADDR
+i386_return_value_address (struct gdbarch *gdbarch, struct regcache *regcache)
+{
+  ULONGEST addr;
+
+  regcache_cooked_read_unsigned (regcache, I386_EAX_REGNUM, &addr);
+  return addr;
+}
 \f
 
 /* Return the GDB type object for the "standard" data type of data in
@@ -2041,6 +2055,7 @@ i386_gdbarch_init (struct gdbarch_info i
   set_gdbarch_value_to_register (gdbarch, i386_value_to_register);
 
   set_gdbarch_return_value (gdbarch, i386_return_value);
+  set_gdbarch_return_value_address (gdbarch, i386_return_value_address);
 
   set_gdbarch_skip_prologue (gdbarch, i386_skip_prologue);
 
Index: doc/ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* gdbint.texinfo (Target Architecture Definition): Document
	gdbarch_return_value_address.

Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.198
diff -u -p -r1.198 gdbint.texinfo
--- doc/gdbint.texinfo 1 May 2004 16:52:30 -0000 1.198
+++ doc/gdbint.texinfo 7 May 2004 17:05:07 -0000
@@ -3786,6 +3786,16 @@ to the inner most frame.  While replacin
 @code{struct frame_info} @var{frame} parameter would remove that
 limitation there has yet to be a demonstrated need for such a change.}
 
+@item CORE_ADDR gdbarch_return_value_address (struct gdbarch *@var{gdbarch}, struct regcache *@var{regcache})
+@findex gdbarch_return_value_address
+@anchor{gdbarch_return_value_address} Return the address where the
+return value can be found for functions that use the
+@code{RETURN_VALUE_STRUCT_CONVENTION} return-value convention
+(@pxref{gdbarch_return_value}).  A copy of the registers from the just
+returned function can be found in @var{regcache}.  @emph{This method
+should only be provided if the address of the return value can be found
+reliably from the register contents upon function return.}
+
 @item SKIP_PERMANENT_BREAKPOINT
 @findex SKIP_PERMANENT_BREAKPOINT
 Advance the inferior's PC past a permanent breakpoint.  @value{GDBN} normally

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Andrew Cagney]

> The current GDB doesn't print the return value when using `finish' for
> functions return structures that are not returned in registers.  Note
> that this is a regression from GDB 6.0 for many systems.  Anyway, the
> attached patch provides a way to fix this, and adds the necessary
> support to the i386 target.
> 
> If there are no comments, I'll check this in in a few days.  Eli, is
> the doc bit OK?

Why not add another member to `enum return_value_convention' so that 
return_value() can directly differentate between these two cases?

Andrew

> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
>  
> 	* gdbarch.sh (gdbarch_return_value_address): New method.
> 	* gdbarch.h, gdbarch.c: Re-generate.
> 	* infcmd.c (print_return_value): Use new
> 	gdbarch_return_value_address method if provided.
> 	* i386-tdep.c (i386_return_value_address): New function.
> 	(i386_gdbarch_init): Set return_value_address.
> 

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   Date: Fri, 07 May 2004 18:35:28 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > The current GDB doesn't print the return value when using `finish' for
   > functions return structures that are not returned in registers.  Note
   > that this is a regression from GDB 6.0 for many systems.  Anyway, the
   > attached patch provides a way to fix this, and adds the necessary
   > support to the i386 target.
   > 
   > If there are no comments, I'll check this in in a few days.  Eli, is
   > the doc bit OK?

   Why not add another member to `enum return_value_convention' so that 
   return_value() can directly differentate between these two cases?

I thought about that.  Returning an address from gdbarch_return_value
is a bit weird.  Hmm, it just occurs to me that we make a copy of the
value and have gdbarch_return_value return that.  Hmm, back to the
drawing board.  Expect a new patch soon.

Mark

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   Date: Fri, 07 May 2004 18:35:28 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > The current GDB doesn't print the return value when using `finish' for
   > functions return structures that are not returned in registers.  Note
   > that this is a regression from GDB 6.0 for many systems.  Anyway, the
   > attached patch provides a way to fix this, and adds the necessary
   > support to the i386 target.
   > 
   > If there are no comments, I'll check this in in a few days.  Eli, is
   > the doc bit OK?

   Why not add another member to `enum return_value_convention' so that 
   return_value() can directly differentate between these two cases?

I called the new member RETURN_VALUE_ADDRESS_CONVENTION.  I'm not
really satisfied with that name, but I couldn't think of something
better.  The implementation is fairly simple, but I took the
opportunity to re-arrange the code such that the legacy stuff is
separated out, that's why the patch looks a bit more invasive.

Andrew, do you have any objections?

While implementing this stuff, it occured to me that the return value
of gdbarch_return_value() really should be a set of bit flags instead
of an enum.  We should have flags that indicate:

* Whether GDB should allocate some memory to store the return value.

* Whether the location of the return value is known when we've just
  returned from a function.

* Whether the location of the return value is known when we're
  currently executing a function.

I think the ABI's I've seen thus far cover at least six of the eight
posible combinations.  Thoughts?  I'd like to check in the attached
patch regardless of what we decide.

Mark

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Andrew Cagney]

Ouch.  That's really ugly.  I thought RETURN_VALUE_STRUCT_CONVENTION
was already long enough.  I'd really like to avoid thos really long
names.  Hmm what if I use:
RETURN_VALUE_ABI_RETURNS_ADDRESS
RETURN_VALUE_ABI_PRESERVES_ADDRESS
yes.

Andrew

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   Date: Sat, 08 May 2004 17:12:21 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > Ouch.  That's really ugly.  I thought RETURN_VALUE_STRUCT_CONVENTION
   > was already long enough.  I'd really like to avoid thos really long
   > names.  Hmm what if I use:
   > 
   > RETURN_VALUE_ABI_RETURNS_ADDRESS
   > RETURN_VALUE_ABI_PRESERVES_ADDRESS

   yes.

So I checked in the attached.  The patches also corners
RETURN_VALUE_ABI_PRESERVES_ADDRESS, but doesn't implement it yet.
I'll do that when I touch SPARC again.

The patch induces some PASS->KFAIL changes in the testsuite.  This is
actually a problem with the testsuite.  The tests in question assume
that if "finish" prints a return value, "return" should work.
RETURN_VALUE_ABI_RETURNS_ADDRESS makes this assumption invalid.  We
should probably just remove those tests, but perhaps we can do
something smarter.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>
 
	* defs.h (enum return_value_convention): Add
	RETURN_VALUE_ABI_RETURNS_ADDRESS and
	RETURN_VALUE_ABI_PRESERVES_ADDRESS.
	* infcmd.c (legacy_return_value): New function.
	(print_return_value): Rwerite to implement
	RETURN_VALUE_ABI_RETURNS_ADDRESS.
	* values.c (using_struct_return): Check for inequality to
	RETURN_VALUE_REGISTER_CONVENTION instead of equality to
	RETURN_VALUE_STRUCT_CONVENTION.
	* i386-tdep.c (i386_return_value): Implement
	RETURN_VALUE_ABI_RETURNS_ADDRESS.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.145
diff -u -p -r1.145 defs.h
--- defs.h 30 Apr 2004 20:44:58 -0000 1.145
+++ defs.h 8 May 2004 22:55:59 -0000
@@ -250,7 +250,19 @@ enum return_value_convention
      should be stored.  While typically, and historically, used for
      large structs, this is convention is applied to values of many
      different types.  */
-  RETURN_VALUE_STRUCT_CONVENTION
+  RETURN_VALUE_STRUCT_CONVENTION,
+  /* Like the "struct return convention" above, but where the ABI
+     guarantees that the called function stores the address at which
+     the value being returned is stored in a well-defined location,
+     such as a register or memory slot in the stack frame.  Don't use
+     this if the ABI doesn't explicitly guarantees this.  */
+  RETURN_VALUE_ABI_RETURNS_ADDRESS,
+  /* Like the "struct return convention" above, but where the ABI
+     guarantees that the address at which the value being returned is
+     stored will be available in a well-defined location, such as a
+     register or memory slot in the stack frame.  Don't use this if
+     the ABI doesn't explicitly guarantees this.  */
+  RETURN_VALUE_ABI_PRESERVES_ADDRESS,
 };
 
 /* the cleanup list records things that have to be undone
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.108
diff -u -p -r1.108 infcmd.c
--- infcmd.c 28 Apr 2004 16:36:25 -0000 1.108
+++ infcmd.c 8 May 2004 22:55:59 -0000
@@ -1047,79 +1047,97 @@ advance_command (char *arg, int from_tty
 }
 \f
 
+static struct value *
+legacy_return_value (int struct_return, struct type *value_type)
+{
+  struct value *value;
+
+  if (!struct_return)
+    {
+      /* The return value can be found in the inferior's registers.  */
+      return register_value_being_returned (value_type, stop_registers);
+    }
+
+  if (DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
+    {
+      CORE_ADDR addr;
+
+      addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (stop_registers);
+      if (!addr)
+	error ("Function return value unknown.");
+      return value_at (value_type, addr, NULL);
+    }
+
+  /* It is "struct return" yet the value is being extracted,
+     presumably from registers, using EXTRACT_RETURN_VALUE.  This
+     doesn't make sense.  Unfortunately, the legacy interfaces allowed
+     this behavior.  Sigh!  */
+  value = allocate_value (value_type);
+  CHECK_TYPEDEF (value_type);
+  /* If the function returns void, don't bother fetching the return
+     value.  */
+  EXTRACT_RETURN_VALUE (value_type, stop_registers,
+			VALUE_CONTENTS_RAW (value));
+  return value;
+}
+
 /* Print the result of a function at the end of a 'finish' command.  */
 
 static void
 print_return_value (int struct_return, struct type *value_type)
 {
+  struct gdbarch *gdbarch = current_gdbarch;
   struct cleanup *old_chain;
   struct ui_stream *stb;
-  struct value *value;
+  struct value *value = NULL;
 
-  if (!struct_return)
-    {
-      /* The return value can be found in the inferior's registers.  */
-      value = register_value_being_returned (value_type, stop_registers);
-    }
-  /* FIXME: cagney/2004-01-17: When both return_value and
-     extract_returned_value_address are available, should use that to
-     find the address of and then extract the returned value.  */
   /* FIXME: 2003-09-27: When returning from a nested inferior function
      call, it's possible (with no help from the architecture vector)
      to locate and return/print a "struct return" value.  This is just
      a more complicated case of what is already being done in in the
      inferior function call code.  In fact, when inferior function
      calls are made async, this will likely be made the norm.  */
-  else if (gdbarch_return_value_p (current_gdbarch))
-    /* We cannot determine the contents of the structure because it is
-       on the stack, and we don't know where, since we did not
-       initiate the call, as opposed to the call_function_by_hand
-       case.  */
-    {
-      gdb_assert (gdbarch_return_value (current_gdbarch, value_type,
-					NULL, NULL, NULL)
-		  == RETURN_VALUE_STRUCT_CONVENTION);
-      ui_out_text (uiout, "Value returned has type: ");
-      ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type));
-      ui_out_text (uiout, ".");
-      ui_out_text (uiout, " Cannot determine contents\n");
-      return;
-    }
-  else
+
+  if (gdbarch_return_value_p (gdbarch))
     {
-      if (DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
-	{
-	  CORE_ADDR addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (stop_registers);
-	  if (!addr)
-	    error ("Function return value unknown.");
-	  value = value_at (value_type, addr, NULL);
-	}
-      else
+      switch (gdbarch_return_value (gdbarch, value_type, NULL, NULL, NULL))
 	{
-	  /* It is "struct return" yet the value is being extracted,
-             presumably from registers, using EXTRACT_RETURN_VALUE.
-             This doesn't make sense.  Unfortunately, the legacy
-             interfaces allowed this behavior.  Sigh!  */
+	case RETURN_VALUE_REGISTER_CONVENTION:
+	case RETURN_VALUE_ABI_RETURNS_ADDRESS:
 	  value = allocate_value (value_type);
 	  CHECK_TYPEDEF (value_type);
-	  /* If the function returns void, don't bother fetching the
-	     return value.  */
-	  EXTRACT_RETURN_VALUE (value_type, stop_registers,
-				VALUE_CONTENTS_RAW (value));
+	  gdbarch_return_value (current_gdbarch, value_type, stop_registers,
+				VALUE_CONTENTS_RAW (value), NULL);
+	  break;
+
+	case RETURN_VALUE_STRUCT_CONVENTION:
+	  break;
 	}
     }
+  else
+    value = legacy_return_value (struct_return, value_type);
 
-  /* Print it.  */
-  stb = ui_out_stream_new (uiout);
-  old_chain = make_cleanup_ui_out_stream_delete (stb);
-  ui_out_text (uiout, "Value returned is ");
-  ui_out_field_fmt (uiout, "gdb-result-var", "$%d",
-		    record_latest_value (value));
-  ui_out_text (uiout, " = ");
-  value_print (value, stb->stream, 0, Val_no_prettyprint);
-  ui_out_field_stream (uiout, "return-value", stb);
-  ui_out_text (uiout, "\n");
-  do_cleanups (old_chain);
+  if (value)
+    {
+      /* Print it.  */
+      stb = ui_out_stream_new (uiout);
+      old_chain = make_cleanup_ui_out_stream_delete (stb);
+      ui_out_text (uiout, "Value returned is ");
+      ui_out_field_fmt (uiout, "gdb-result-var", "$%d",
+			record_latest_value (value));
+      ui_out_text (uiout, " = ");
+      value_print (value, stb->stream, 0, Val_no_prettyprint);
+      ui_out_field_stream (uiout, "return-value", stb);
+      ui_out_text (uiout, "\n");
+      do_cleanups (old_chain);
+    }
+  else
+    {
+      ui_out_text (uiout, "Value returned has type: ");
+      ui_out_field_string (uiout, "return-type", TYPE_NAME (value_type));
+      ui_out_text (uiout, ".");
+      ui_out_text (uiout, " Cannot determine contents\n");
+    }
 }
 
 /* Stuff that needs to be done by the finish command after the target
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.64
diff -u -p -r1.64 values.c
--- values.c 26 Jan 2004 20:52:12 -0000 1.64
+++ values.c 8 May 2004 22:56:00 -0000
@@ -1308,7 +1308,7 @@ using_struct_return (struct type *value_
   /* Probe the architecture for the return-value convention.  */
   return (gdbarch_return_value (current_gdbarch, value_type,
 				NULL, NULL, NULL)
-	  == RETURN_VALUE_STRUCT_CONVENTION);
+	  != RETURN_VALUE_REGISTER_CONVENTION);
 }
 
 void
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.190
diff -u -p -r1.190 i386-tdep.c
--- i386-tdep.c 30 Apr 2004 21:13:58 -0000 1.190
+++ i386-tdep.c 8 May 2004 22:56:00 -0000
@@ -1352,7 +1352,28 @@ i386_return_value (struct gdbarch *gdbar
 
   if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
       && !i386_reg_struct_return_p (gdbarch, type))
-    return RETURN_VALUE_STRUCT_CONVENTION;
+    {
+      /* The System V ABI says that:
+
+	 "A function that returns a structure or union also sets %eax
+	 to the value of the original address of the caller's area
+	 before it returns.  Thus when the caller receives control
+	 again, the address of the returned object resides in register
+	 %eax and can be used to access the object."
+
+	 So the ABI guarantees that we can always find the return
+	 value just after the function has returned.  */
+
+      if (readbuf)
+	{
+	  ULONGEST addr;
+
+	  regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &addr);
+	  read_memory (addr, readbuf, TYPE_LENGTH (type));
+	}
+
+      return RETURN_VALUE_ABI_RETURNS_ADDRESS;
+    }
 
   /* This special case is for structures consisting of a single
      `float' or `double' member.  These structures are returned in

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Andrew Cagney]

   Date: Sat, 08 May 2004 17:12:21 -0400
   From: Andrew Cagney <cagney@gnu.org>
   > Ouch.  That's really ugly.  I thought RETURN_VALUE_STRUCT_CONVENTION
   > was already long enough.  I'd really like to avoid thos really long
   > names.  Hmm what if I use:
   > 
   > RETURN_VALUE_ABI_RETURNS_ADDRESS
   > RETURN_VALUE_ABI_PRESERVES_ADDRESS

   yes.

So I checked in the attached.  The patches also corners
RETURN_VALUE_ABI_PRESERVES_ADDRESS, but doesn't implement it yet.
I'll do that when I touch SPARC again.
The patch induces some PASS->KFAIL changes in the testsuite.  This is
actually a problem with the testsuite.  The tests in question assume
that if "finish" prints a return value, "return" should work.
RETURN_VALUE_ABI_RETURNS_ADDRESS makes this assumption invalid.  We
should probably just remove those tests, but perhaps we can do
something smarter.
How come it doesn't work?

Popping the caller's frame should (assuming the unwind info is correct) 
restore the struct-return address register to the value that the callee 
expects.  return_value can then use that register value to find the 
location at which to store the struct?

That cross check is there because, in the past, people decided getting 
all these edge cases right was too hard and ignored them.  If the 
previous paragraph doesn't hold, something smarter is needed.

Andrew

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   Date: Sat, 08 May 2004 20:07:55 -0400
   From: Andrew Cagney <cagney@gnu.org>

   >    Date: Sat, 08 May 2004 17:12:21 -0400
   >    From: Andrew Cagney <cagney@gnu.org>
   > 
   >    > Ouch.  That's really ugly.  I thought RETURN_VALUE_STRUCT_CONVENTION
   >    > was already long enough.  I'd really like to avoid thos really long
   >    > names.  Hmm what if I use:
   >    > 
   >    > RETURN_VALUE_ABI_RETURNS_ADDRESS
   >    > RETURN_VALUE_ABI_PRESERVES_ADDRESS
   > 
   >    yes.
   > 
   > So I checked in the attached.  The patches also corners
   > RETURN_VALUE_ABI_PRESERVES_ADDRESS, but doesn't implement it yet.
   > I'll do that when I touch SPARC again.
   > 
   > The patch induces some PASS->KFAIL changes in the testsuite.  This is
   > actually a problem with the testsuite.  The tests in question assume
   > that if "finish" prints a return value, "return" should work.
   > RETURN_VALUE_ABI_RETURNS_ADDRESS makes this assumption invalid.  We
   > should probably just remove those tests, but perhaps we can do
   > something smarter.

   How come it doesn't work?

   Popping the caller's frame should (assuming the unwind info is correct) 
   restore the struct-return address register to the value that the callee 
   expects.  return_value can then use that register value to find the 
   location at which to store the struct?

The problem is in the "assuming the unwind info is correct".  It's not
there.  It will probably never be there.  This isn't a normal register
save/restore.  The incoming address will be on the stack.  The callee
moves it into %eax somewhere, but we don't know where and when.  You
simply can't express this in DWARF2 CFI.

Now, we could do some nifty prologue/epilogue analysis and make a good
guess at where the address is stored.  Compilers seem to either do the
move in the prologue (as the SVR4 ABI suggests, see
i386_analyze_struct_return()) or leave the address on the stack until
the epilogue (as GCC does).  However, we can never be sure.  This
would lead to the unreliabilities you were trying to get rid of when
gdbarch_return_value() was instroduced.

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Andrew Cagney]

   How come it doesn't work?

   Popping the caller's frame should (assuming the unwind info is correct) 
   restore the struct-return address register to the value that the callee 
   expects.  return_value can then use that register value to find the 
   location at which to store the struct?

The problem is in the "assuming the unwind info is correct".  It's not
there.  It will probably never be there.  This isn't a normal register
save/restore.  The incoming address will be on the stack.  The callee
moves it into %eax somewhere, but we don't know where and when.  You
simply can't express this in DWARF2 CFI.
You can.  %eax would start out as on the stack, and then later be marked 
as ``in register''.  It's just the reverse flow of what happens to other 
registers.

Now, we could do some nifty prologue/epilogue analysis and make a good
guess at where the address is stored.  Compilers seem to either do the
move in the prologue (as the SVR4 ABI suggests, see
i386_analyze_struct_return()) or leave the address on the stack until
the epilogue (as GCC does).  However, we can never be sure.  This
would lead to the unreliabilities you were trying to get rid of when
gdbarch_return_value() was instroduced.
Ah.  This is what needs to be added to the return_value documentation.

Andrew

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Michael Snyder]

Mark Kettenis wrote:
   Date: Fri, 07 May 2004 18:35:28 -0400
   From: Andrew Cagney <cagney@gnu.org>
   > The current GDB doesn't print the return value when using `finish' for
   > functions return structures that are not returned in registers.  Note
   > that this is a regression from GDB 6.0 for many systems.  Anyway, the
   > attached patch provides a way to fix this, and adds the necessary
   > support to the i386 target.
   > 
   > If there are no comments, I'll check this in in a few days.  Eli, is
   > the doc bit OK?

   Why not add another member to `enum return_value_convention' so that 
   return_value() can directly differentate between these two cases?

I called the new member RETURN_VALUE_ADDRESS_CONVENTION.  I'm not
really satisfied with that name, but I couldn't think of something
better. 
RETURN_VALUE_BY_REFERENCE?  This is, in effect, an invisible
reference variable.

The implementation is fairly simple, but I took the
opportunity to re-arrange the code such that the legacy stuff is
separated out, that's why the patch looks a bit more invasive.
Andrew, do you have any objections?

While implementing this stuff, it occured to me that the return value
of gdbarch_return_value() really should be a set of bit flags instead
of an enum.  We should have flags that indicate:
* Whether GDB should allocate some memory to store the return value.

* Whether the location of the return value is known when we've just
  returned from a function.
* Whether the location of the return value is known when we're
  currently executing a function.
I think the ABI's I've seen thus far cover at least six of the eight
posible combinations.  Thoughts?  I'd like to check in the attached
patch regardless of what we decide.
Mark

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   Date: Wed, 12 May 2004 17:46:58 +0000
   From: Michael Snyder <msnyder@redhat.com>

   Mark Kettenis wrote:
   >    Date: Fri, 07 May 2004 18:35:28 -0400
   >    From: Andrew Cagney <cagney@gnu.org>
   > 
   >    > The current GDB doesn't print the return value when using `finish' for
   >    > functions return structures that are not returned in registers.  Note
   >    > that this is a regression from GDB 6.0 for many systems.  Anyway, the
   >    > attached patch provides a way to fix this, and adds the necessary
   >    > support to the i386 target.
   >    > 
   >    > If there are no comments, I'll check this in in a few days.  Eli, is
   >    > the doc bit OK?
   > 
   >    Why not add another member to `enum return_value_convention' so that 
   >    return_value() can directly differentate between these two cases?
   > 
   > I called the new member RETURN_VALUE_ADDRESS_CONVENTION.  I'm not
   > really satisfied with that name, but I couldn't think of something
   > better. 

   RETURN_VALUE_BY_REFERENCE?  This is, in effect, an invisible
   reference variable.

Well, yes.  However, the patch is already in, with names that pleased
Andrew a bit more.

Anyway, to what extent are the m68k ABI's documented to return the
structure return address in %d0 and/or %a0?  For the
PCC_STATIC_STRUCT_RETURN targets this is clear, but how is this for
the systems where the caller is supposed to pass the memory area.  Do
you have access to the m68k SVR4 ABI?  What does it say about this?

Mark

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Andreas Schwab]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

Mark Kettenis <kettenis@chello.nl> writes:

> Anyway, to what extent are the m68k ABI's documented to return the
> structure return address in %d0 and/or %a0?  For the
> PCC_STATIC_STRUCT_RETURN targets this is clear, but how is this for
> the systems where the caller is supposed to pass the memory area.  Do
> you have access to the m68k SVR4 ABI?  What does it say about this?

From the 68000 supplement:

    Functions Returning Structures or Unions

    As mentioned above, when a function returns a structure or union, it
    expects the caller to provide space for the return value and to place
    its address in register %a0.  Having the caller supply the return
    object's space allows re-entrancy.

    A function returning a structure or union also sets %a0 to the value
    it finds in %a0.  Thus when the caller receives control again, the
    address of the returned object resides in register %a0.  ...

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH/RFC/RFA] Print in-memory struct return values

[Mark Kettenis]

   From: Andreas Schwab <schwab@suse.de>
   Date: Sun, 16 May 2004 00:12:39 +0200

   Mark Kettenis <kettenis@chello.nl> writes:

   >From the 68000 supplement:

       Functions Returning Structures or Unions

       As mentioned above, when a function returns a structure or union, it
       expects the caller to provide space for the return value and to place
       its address in register %a0.  Having the caller supply the return
       object's space allows re-entrancy.

       A function returning a structure or union also sets %a0 to the value
       it finds in %a0.  Thus when the caller receives control again, the
       address of the returned object resides in register %a0.  ...

Thanks Andreas.  That was what I was looking for.  Michael, are you
aware of any ABI documentation for the not-quite-SVR4 embedded m68k
targets?  Are those GCC only?

Mark