This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch/rfc] Simplify by assuming that the inferior frame was aligned


Hello,

This simplifies inferior function call code by assuming that "struct_return" is always correct and always using it to extract the "struct return value".

The MIPS inferior function call code was invalidating "struct_addr" by doing some local alignment. Now that the MIPS has a frame_align method that problem no longer occures (I've found no other code pulling similar tricks). Consequently, I believe that the attached simplification is now safe.

It's got lots of follow through. In particular it allows the restructuring of value_being_returned and print_return_value, and the elimination of VALUE_RETURNED_FROM_STACK.

Note that the change is only one line! The rest is to alert the coder as to the status of various parts of GDB.

Baring comment, I'll look to commit this in 3-4 days,
Andrew
2003-09-27  Andrew Cagney  <cagney@redhat.com>

	* infcall.c (call_function_by_hand): When STRUCT_RETURN, always
	use STRUCT_ADDR.  When not using "struct return convention", pass
	"0" to "value_being_returned".  Add FIXMEs.
	* infcmd.c (print_return_value): Pass an explicit 0/1 to
	value_being_returned.  Add comments.
	* values.c (value_being_returned): Add FIXME.
	* hppa-tdep.c (hppa_extract_struct_value_address): Add FIXME.
	(hppa_value_returned_from_stack): Add FIXME.

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.99
diff -u -r1.99 hppa-tdep.c
--- hppa-tdep.c	25 Sep 2003 20:44:01 -0000	1.99
+++ hppa-tdep.c	28 Sep 2003 01:43:08 -0000
@@ -2067,6 +2067,9 @@
    This function does the same stuff as value_being_returned in values.c, but
    gets the value from the stack rather than from the buffer where all the
    registers were saved when the function called completed. */
+/* FIXME: cagney/2003-09-27: This function is no longer needed.  The
+   inferior function call code now directly handles the case described
+   above.  */
 struct value *
 hppa_value_returned_from_stack (struct type *valtype, CORE_ADDR addr)
 {
@@ -5040,6 +5043,10 @@
      the address size is equal to the size of an int* _on the host_...
      One possible implementation that crossed my mind is to use
      extract_address.  */
+  /* FIXME: cagney/2003-09-27: This function can probably go.  ELZ
+     writes: We cannot assume on the pa that r28 still contains the
+     address of the returned structure. Usually this will be
+     overwritten by the callee.  */
   return (*(int *)(regbuf + DEPRECATED_REGISTER_BYTE (28)));
 }
 
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.29
diff -u -r1.29 infcall.c
--- infcall.c	14 Sep 2003 16:32:13 -0000	1.29
+++ infcall.c	28 Sep 2003 01:43:09 -0000
@@ -1076,6 +1076,8 @@
      address of the returned structure. Usually this will be
      overwritten by the callee.  I don't know about other
      architectures, so I defined this macro */
+  /* FIXME: cagney/2003-09-27: This is no longer needed.  The problem
+     is now handled directly be by the code below.  */
 #ifdef VALUE_RETURNED_FROM_STACK
   if (struct_return)
     {
@@ -1083,23 +1085,28 @@
       return VALUE_RETURNED_FROM_STACK (value_type, struct_addr);
     }
 #endif
-  /* 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))
+  if (struct_return)
     {
+      /* NOTE: cagney/2003-09-27: This assumes that PUSH_DUMMY_CALL
+	 has correctly stored STRUCT_ADDR in the target.  In the past
+	 that hasn't been the case, the old MIPS PUSH_ARGUMENTS
+	 (PUSH_DUMMY_CALL precursor) would silently move the location
+	 of the struct return value making STRUCT_ADDR bogus.  If
+	 you're seeing problems with values being returned using the
+	 "struct return convention", check that PUSH_DUMMY_CALL isn't
+	 playing tricks.  */
       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);
+      /* This call to value_being_returned is never made when the
+         function uses "struct return convention".  Hence, pass "0"
+         instead of STRUCT_RETURN.  Besides, VALUE_TYPE, in
+         combination with RETURN_VALUE() (nee USE_STRUCT_CONVENTION)
+         can be used to re-construct the value of STRUCT_RETURN. */
+      struct value *retval = value_being_returned (value_type, retbuf, 0);
       do_cleanups (retbuf_cleanup);
       return retval;
     }
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.91
diff -u -r1.91 infcmd.c
--- infcmd.c	21 Sep 2003 01:26:44 -0000	1.91
+++ infcmd.c	28 Sep 2003 01:43:10 -0000
@@ -1070,7 +1070,7 @@
 
   if (!structure_return)
     {
-      value = value_being_returned (value_type, stop_registers, structure_return);
+      value = value_being_returned (value_type, stop_registers, 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));
@@ -1081,6 +1081,18 @@
     }
   else
     {
+      /* FIXME: 2003-09-27: This code block should be handling the
+         "use struct convention" case, and not the function
+         value_being_returned.  This would allow the dramatic
+         simplification of value_being_returned (perhaphs renamed to
+         register_value_being_returned).  */
+      /* 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.  */
       /* 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 */
@@ -1091,7 +1103,7 @@
       ui_out_text (uiout, ".");
       ui_out_text (uiout, " Cannot determine contents\n");
 #else
-      value = value_being_returned (value_type, stop_registers, structure_return);
+      value = value_being_returned (value_type, stop_registers, 1);
       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));
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.57
diff -u -r1.57 values.c
--- values.c	21 Sep 2003 01:26:45 -0000	1.57
+++ values.c	28 Sep 2003 01:43:11 -0000
@@ -1216,6 +1216,12 @@
    0 when it is using the value returning conventions (this often
    means returning pointer to where structure is vs. returning value). */
 
+/* FIXME: cagney/2003-09-27: Should move the "struct return
+   convention" code to the only call site in print_return_value that
+   needs it.  This function can then be renamed to
+   "register_value_being_returned" and with the "struct_return"
+   parameter dropped.  */
+
 struct value *
 value_being_returned (struct type *valtype, struct regcache *retbuf,
 		      int struct_return)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]