[SO obvious...] make struct_return work for hand_function_call

[SO obvious...] make struct_return work for hand_function_call

[Michael Snyder]

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

God, we've struggled with this for years... the answer is so obvious!

Eli, this generalizes some special-purpose code that you wrote for HP,
which now becomes redundant and may be removed.



[-- Attachment #2: structs2 --]
[-- Type: text/plain, Size: 9499 bytes --]

2003-07-30  Michael Snyder  <msnyder@redhat.com>

	* structs.h (value_being_returned): Add a struct_addr argument.
	* infcall.c (call_function_by_hand): Pass struct_addr to 
	value_being_returned.
	* infcmd.c (print_return_value): Pass zero as struct_addr.
	* values.c (value_being_returned): If struct_addr is passed,
	use it instead of trying to recover it from the inferior.

Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.48
diff -p -r1.48 value.h
*** value.h	5 Jun 2003 20:59:16 -0000	1.48
--- value.h	30 Jul 2003 20:55:22 -0000
*************** extern struct value *value_subscript (st
*** 420,426 ****
  
  extern struct value *value_being_returned (struct type *valtype,
  					   struct regcache *retbuf,
! 					   int struct_return);
  
  extern struct value *value_in (struct value *element, struct value *set);
  
--- 420,427 ----
  
  extern struct value *value_being_returned (struct type *valtype,
  					   struct regcache *retbuf,
! 					   int struct_return,
! 					   CORE_ADDR struct_addr);
  
  extern struct value *value_in (struct value *element, struct value *set);
  
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.50
diff -p -r1.50 values.c
*** values.c	5 Jun 2003 20:59:16 -0000	1.50
--- values.c	30 Jul 2003 20:55:22 -0000
*************** value_from_double (struct type *type, DO
*** 1212,1242 ****
  /* ARGSUSED */
  struct value *
  value_being_returned (struct type *valtype, struct regcache *retbuf,
! 		      int struct_return)
  {
    struct value *val;
    CORE_ADDR addr;
  
!   /* If this is not defined, just use EXTRACT_RETURN_VALUE instead.  */
!   if (EXTRACT_STRUCT_VALUE_ADDRESS_P ())
!     if (struct_return)
!       {
! 	addr = EXTRACT_STRUCT_VALUE_ADDRESS (retbuf);
! 	if (!addr)
! 	  error ("Function return value unknown.");
! 	return value_at (valtype, addr, NULL);
!       }
! 
!   /* If this is not defined, just use EXTRACT_RETURN_VALUE instead.  */
!   if (DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
!     if (struct_return)
!       {
! 	char *buf = deprecated_grub_regcache_for_registers (retbuf);
! 	addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (buf);
! 	if (!addr)
! 	  error ("Function return value unknown.");
! 	return value_at (valtype, addr, NULL);
!       }
  
    val = allocate_value (valtype);
    CHECK_TYPEDEF (valtype);
--- 1212,1247 ----
  /* ARGSUSED */
  struct value *
  value_being_returned (struct type *valtype, struct regcache *retbuf,
! 		      int struct_return, CORE_ADDR struct_addr)
  {
    struct value *val;
    CORE_ADDR addr;
  
!   if (struct_return)
!     {
!       if (struct_addr != 0)
! 	{
! 	  /* Struct return addr supplied by hand_function_call.  */
! 	  return value_at (valtype, struct_addr, NULL);
! 	}
!       /* If one of these is not defined, just use EXTRACT_RETURN_VALUE
! 	 instead.  */
!       else if (EXTRACT_STRUCT_VALUE_ADDRESS_P ())
! 	{
! 	  addr = EXTRACT_STRUCT_VALUE_ADDRESS (retbuf);
! 	  if (!addr)
! 	    error ("Function return value unknown.");
! 	  return value_at (valtype, addr, NULL);
! 	}
!       else if (DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
! 	{
! 	  char *buf = deprecated_grub_regcache_for_registers (retbuf);
! 	  addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (buf);
! 	  if (!addr)
! 	    error ("Function return value unknown.");
! 	  return value_at (valtype, addr, NULL);
! 	}
!     }
  
    val = allocate_value (valtype);
    CHECK_TYPEDEF (valtype);
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.83
diff -p -r1.83 infcmd.c
*** infcmd.c	7 Jul 2003 14:36:58 -0000	1.83
--- infcmd.c	30 Jul 2003 20:55:22 -0000
*************** print_return_value (int structure_return
*** 1077,1086 ****
  
    if (!structure_return)
      {
!       value = value_being_returned (value_type, stop_registers, structure_return);
        stb = ui_out_stream_new (uiout);
        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);
--- 1077,1087 ----
  
    if (!structure_return)
      {
!       value = value_being_returned (value_type, stop_registers, 0, 0);
        stb = ui_out_stream_new (uiout);
        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);
*************** print_return_value (int structure_return
*** 1088,1107 ****
      }
    else
      {
-       /* 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 */
  #ifdef VALUE_RETURNED_FROM_STACK
        value = 0;
        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");
  #else
!       value = value_being_returned (value_type, stop_registers, structure_return);
        stb = ui_out_stream_new (uiout);
        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);
--- 1089,1111 ----
      }
    else
      {
  #ifdef VALUE_RETURNED_FROM_STACK
+       /* 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.  */
        value = 0;
        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");
  #else
!       value = value_being_returned (value_type, stop_registers, 
! 				    structure_return, 0);
        stb = ui_out_stream_new (uiout);
        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);
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.17
diff -p -r1.17 infcall.c
*** infcall.c	16 Jun 2003 16:47:42 -0000	1.17
--- infcall.c	30 Jul 2003 20:55:23 -0000
*************** call_function_by_hand (struct value *fun
*** 421,427 ****
        vector.  Hence this direct call.
  
        A follow-on change is to modify this interface so that it takes
!       thread OR frame OR tpid as a parameter, and returns a dummy
        frame handle.  The handle can then be used further down as a
        parameter to generic_save_dummy_frame_tos().  Hmm, thinking
        about it, since everything is ment to be using generic dummy
--- 421,427 ----
        vector.  Hence this direct call.
  
        A follow-on change is to modify this interface so that it takes
!       thread OR frame OR ptid as a parameter, and returns a dummy
        frame handle.  The handle can then be used further down as a
        parameter to generic_save_dummy_frame_tos().  Hmm, thinking
        about it, since everything is ment to be using generic dummy
*************** call_function_by_hand (struct value *fun
*** 445,451 ****
  	   On a RISC architecture, a void parameterless generic dummy
  	   frame (i.e., no parameters, no result) typically does not
  	   need to push anything the stack and hence can leave SP and
! 	   FP.  Similarly, a framelss (possibly leaf) function does
  	   not push anything on the stack and, hence, that too can
  	   leave FP and SP unchanged.  As a consequence, a sequence of
  	   void parameterless generic dummy frame calls to frameless
--- 445,451 ----
  	   On a RISC architecture, a void parameterless generic dummy
  	   frame (i.e., no parameters, no result) typically does not
  	   need to push anything the stack and hence can leave SP and
! 	   FP.  Similarly, a frameless (possibly leaf) function does
  	   not push anything on the stack and, hence, that too can
  	   leave FP and SP unchanged.  As a consequence, a sequence of
  	   void parameterless generic dummy frame calls to frameless
*************** the function call).", name);
*** 1056,1063 ****
      }
    else
      {
!       struct value *retval = value_being_returned (value_type, retbuf,
! 						   struct_return);
        do_cleanups (retbuf_cleanup);
        return retval;
      }
--- 1056,1065 ----
      }
    else
      {
!       struct value *retval = value_being_returned (value_type,
! 						   retbuf,
! 						   struct_return,
! 						   struct_addr);
        do_cleanups (retbuf_cleanup);
        return retval;
      }

Re: [SO obvious...] make struct_return work for hand_function_call

[Andrew Cagney]

> God, we've struggled with this for years... the answer is so obvious!

Er, struct return already works for hand call functions.  Vis:

   /* 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;
     }

If your architecture provides frame_align(), and does not it will all work.

I think all those changes can be reverted.

> Eli, this generalizes some special-purpose code that you wrote for HP,
> which now becomes redundant and may be removed.

BTW, Elz is Elena.

Andrew

Re: [SO obvious...] make struct_return work for hand_function_call

[Andrew Cagney]

>   /* 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;
>     }

To be clear here, all architectures should implement frame_align.  With 
that method added, the above code is enabled and struct return works. 
Further, problems such as:

     if (gdbarch_frame_align_p (current_gdbarch))
       {
         /* NOTE: cagney/2002-09-18:

            On a RISC architecture, a void parameterless generic dummy
            frame (i.e., no parameters, no result) typically does not
            need to push anything the stack and hence can leave SP and
            FP.  Similarly, a framelss (possibly leaf) function does
            not push anything on the stack and, hence, that too can
            leave FP and SP unchanged.  As a consequence, a sequence of
            void parameterless generic dummy frame calls to frameless
            functions will create a sequence of effectively identical
            frames (SP, FP and TOS and PC the same).  This, not
            suprisingly, results in what appears to be a stack in an
            infinite loop --- when GDB tries to find a generic dummy
            frame on the internal dummy frame stack, it will always
            find the first one.

            To avoid this problem, the code below always grows the
            stack.  That way, two dummy frames can never be identical.
            It does burn a few bytes of stack but that is a small price
            to pay :-).  */

are also fixed.  BTW, frame_align and stack_align are different.  From 
the arch doco:

> @item frame_align (@var{address})
> @anchor{frame_align}
> @findex frame_align
> Define this to adjust @var{address} so that it meets the alignment
> requirements for the start of a new stack frame.  A stack frame's
> alignment requirements are typically stronger than a target processors
> stack alignment requirements (@pxref{STACK_ALIGN}).
> 
> This function is used to ensure that, when creating a dummy frame, both
> the initial stack pointer and (if needed) the address of the return
> value are correctly aligned.
> 
> Unlike @code{STACK_ALIGN}, this function always adjusts the address in
> the direction of stack growth.
> 
> By default, no frame based stack alignment is performed.

Andrew


Andrew