Re: [rfa/ppc] Use frame_align(); delete hack

Re: [rfa/ppc] Use frame_align(); delete hack

[Kevin Buettner]

On Sep 24,  7:07pm, Andrew Cagney wrote:

> In the process it delets that global variable 
> struct return hack.  Struct returns involving dummy functions are 
> instead handled by the dummy frame code.

Where/how does this happen?

Kevin

Re: [rfa/ppc] Use frame_align(); delete hack

[Andrew Cagney]

> On Sep 24,  7:07pm, Andrew Cagney wrote:
> 
> 
>> In the process it delets that global variable 
>> struct return hack.  Struct returns involving dummy functions are 
>> instead handled by the dummy frame code.
> 
> 
> Where/how does this happen?

In hand_function_call(), near the end:

>     /* NOTE: cagney/2002-09-10: Only when the stack has been correctly
>        aligned (using frame_align()) do we can trust STRUCT_ADDR and
>        fetch the return value direct from the stack.  This lack of
>        trust comes about because legacy targets have a nasty habit of
>        silently, and local to PUSH_ARGUMENTS(), moving STRUCT_ADDR.
>        For such targets, just hope that value_being_returned() can
>        find the adjusted value.  */
>     if (struct_return && gdbarch_frame_align_p (current_gdbarch))
>       {
>         struct value *retval = value_at (value_type, struct_addr, NULL);
>         do_cleanups (retbuf_cleanup);
>         return retval;
>       }
>     else
>       {
> 	struct value *retval = value_being_returned (value_type, retbuf,
> 						     struct_return);
> 	do_cleanups (retbuf_cleanup);
> 	return retval;
>       }
>   }

Andrew

Re: [rfa/ppc] Use frame_align(); delete hack

[Kevin Buettner]

On Sep 24,  7:07pm, Andrew Cagney wrote:

> The extract struct return function is also modified so that it, for want 
> of a better guess, returns register 3.  The other option is to have that 
> function return 0 (since it really doesn't know).
[...]
> +  /* FIXME: This will only work at random.  The caller passes the
> +     struct_return address in register 3, but it is not preserved.  It
> +     may still be there, or this may be a random value.  */
> +  ULONGEST val;
> +  regcache_cooked_read_unsigned (regcache, 3, &val);
> +  return val;

I think returning 0 here is a better idea.  If the function that
you're returning from has made any (non-nullary) calls, r3 will have
been overwritten.  Even if it doesn't make any calls, r3 is still fair
game for the compiler to use as a temporary.

I'd like to see a comment pointing out the fact that gdb knows about
and is able to print struct return values in inferior function calls
due to the fact that gdb is the caller (and therefore knows the address
of the struct return buffer).

Otherwise, okay.

Kevin

Re: [rfa/ppc] Use frame_align(); delete hack

[Andrew Cagney]

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

> 
> I think returning 0 here is a better idea.  If the function that
> you're returning from has made any (non-nullary) calls, r3 will have
> been overwritten.  Even if it doesn't make any calls, r3 is still fair
> game for the compiler to use as a temporary.
> 
> I'd like to see a comment pointing out the fact that gdb knows about
> and is able to print struct return values in inferior function calls
> due to the fact that gdb is the caller (and therefore knows the address
> of the struct return buffer).


Done.  Two comments and a bug report.
I checked in the attached.

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 4261 bytes --]

2002-09-26  Andrew Cagney  <ac131313@redhat.com>

	* rs6000-tdep.c (rs6000_extract_struct_value_address): Return 0.
	(rs6000_struct_return_address): Delete variable.
	(rs6000_store_struct_return): Update.
	(rs6000_gdbarch_init): Set extract_struct_value_address instead of
	deprecated_extract_struct_value_address.
	(rs6000_frame_align): New function.
	(rs6000_gdbarch_init): Set frame_align.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.83
diff -u -r1.83 rs6000-tdep.c
--- rs6000-tdep.c	25 Sep 2002 13:34:53 -0000	1.83
+++ rs6000-tdep.c	26 Sep 2002 22:27:24 -0000
@@ -1032,6 +1032,13 @@
     }
 }
 
+/* All the ABI's require 16 byte alignment.  */
+static CORE_ADDR
+rs6000_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  return (addr & -16);
+}
+
 /* Pass the arguments in either registers, or in the stack. In RS/6000,
    the first eight words of the argument list (that might be less than
    eight parameters if some parameters occupy more than one word) are
@@ -1363,13 +1370,6 @@
     }
 }
 
-/* Keep structure return address in this variable.
-   FIXME:  This is a horrid kludge which should not be allowed to continue
-   living.  This only allows a single nested call to a structure-returning
-   function.  Come on, guys!  -- gnu@cygnus.com, Aug 92  */
-
-static CORE_ADDR rs6000_struct_return_address;
-
 /* Return whether handle_inferior_event() should proceed through code
    starting at PC in function NAME when stepping.
 
@@ -1989,18 +1989,12 @@
 }
 
 /* Store the address of the place in which to copy the structure the
-   subroutine will return.  This is called from call_function.
-
-   In RS/6000, struct return addresses are passed as an extra parameter in r3.
-   In function return, callee is not responsible of returning this address
-   back.  Since gdb needs to find it, we will store in a designated variable
-   `rs6000_struct_return_address'.  */
+   subroutine will return.  */
 
 static void
 rs6000_store_struct_return (CORE_ADDR addr, CORE_ADDR sp)
 {
   write_register (3, addr);
-  rs6000_struct_return_address = addr;
 }
 
 /* Write into appropriate registers a function return value
@@ -2058,9 +2052,20 @@
    as a CORE_ADDR (or an expression that can be used as one).  */
 
 static CORE_ADDR
-rs6000_extract_struct_value_address (char *regbuf)
+rs6000_extract_struct_value_address (struct regcache *regcache)
 {
-  return rs6000_struct_return_address;
+  /* FIXME: cagney/2002-09-26: PR gdb/724: When making an inferior
+     function call GDB knows the address of the struct return value
+     and hence, should not need to call this function.  Unfortunately,
+     the current hand_function_call() code only saves the most recent
+     struct address leading to occasional calls.  The code should
+     instead maintain a stack of such addresses (in the dummy frame
+     object).  */
+  /* NOTE: cagney/2002-09-26: Return 0 which indicates that we've
+     really got no idea where the return value is being stored.  While
+     r3, on function entry, contained the address it will have since
+     been reused (scratch) and hence wouldn't be valid */
+  return 0;
 }
 
 /* Return whether PC is in a dummy function call.
@@ -2887,6 +2892,7 @@
   set_gdbarch_call_dummy_stack_adjust_p (gdbarch, 0);
   set_gdbarch_get_saved_register (gdbarch, generic_unwind_get_saved_register);
   set_gdbarch_fix_call_dummy (gdbarch, rs6000_fix_call_dummy);
+  set_gdbarch_frame_align (gdbarch, rs6000_frame_align);
   set_gdbarch_push_dummy_frame (gdbarch, generic_push_dummy_frame);
   set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);
   set_gdbarch_push_return_address (gdbarch, ppc_push_return_address);
@@ -2910,7 +2916,7 @@
     set_gdbarch_push_arguments (gdbarch, rs6000_push_arguments);
 
   set_gdbarch_store_struct_return (gdbarch, rs6000_store_struct_return);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, rs6000_extract_struct_value_address);
+  set_gdbarch_extract_struct_value_address (gdbarch, rs6000_extract_struct_value_address);
   set_gdbarch_pop_frame (gdbarch, rs6000_pop_frame);
 
   set_gdbarch_skip_prologue (gdbarch, rs6000_skip_prologue);