2003-10-23 Andrew Cagney <cagney@redhat.com>

	* Makefile.in (stack.o): Add $(regcache_h).
	* stack.c: Include "regcache.h"
	(return_command): Rewrite.  Use get_frame_id and
	get_selected_frame.  Eliminate "deprecated_selected_frame".  Warn
	about unhandled return-values.
	* value.h (set_return_value): Delete declaration.
	* values.c (set_return_value): Delete function.

5 files changed, 110 insertions, 111 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1ea1b4fb8d0..9d4e324f1c6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2003-10-23  Andrew Cagney  <cagney@redhat.com>
+
+	* Makefile.in (stack.o): Add $(regcache_h).
+	* stack.c: Include "regcache.h"
+	(return_command): Rewrite.  Use get_frame_id and
+	get_selected_frame.  Eliminate "deprecated_selected_frame".  Warn
+	about unhandled return-values.
+	* value.h (set_return_value): Delete declaration.
+	* values.c (set_return_value): Delete function.
+
 2003-10-23  Jeff Johnston  <jjohnstn@redhat.com>
 
 	* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 73dbe758ff5..d5f2d0a18fa 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2341,7 +2341,7 @@ stack.o: stack.c $(defs_h) $(gdb_string_h) $(value_h) $(symtab_h) \
 	$(gdbtypes_h) $(expression_h) $(language_h) $(frame_h) $(gdbcmd_h) \
 	$(gdbcore_h) $(target_h) $(source_h) $(breakpoint_h) $(demangle_h) \
 	$(inferior_h) $(annotate_h) $(ui_out_h) $(block_h) $(stack_h) \
-	$(gdb_assert_h) $(dictionary_h) $(reggroups_h)
+	$(gdb_assert_h) $(dictionary_h) $(reggroups_h) $(regcache_h)
 standalone.o: standalone.c $(gdb_stat_h) $(defs_h) $(symtab_h) $(frame_h) \
 	$(inferior_h) $(gdb_wait_h)
 std-regs.o: std-regs.c $(defs_h) $(user_regs_h) $(frame_h) $(gdbtypes_h) \
diff --git a/gdb/stack.c b/gdb/stack.c
index c52017c9635..144f7785827 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -44,6 +44,7 @@
 #include "gdb_assert.h"
 #include "dictionary.h"
 #include "reggroups.h"
+#include "regcache.h"
 
 /* Prototypes for exported functions. */
 
@@ -55,8 +56,6 @@ void (*selected_frame_level_changed_hook) (int);
 
 void _initialize_stack (void);
 
-void return_command (char *, int);
-
 /* Prototypes for local functions. */
 
 static void down_command (char *, int);
@@ -1819,94 +1818,138 @@ void
 return_command (char *retval_exp, int from_tty)
 {
   struct symbol *thisfun;
-  CORE_ADDR selected_frame_addr;
-  CORE_ADDR selected_frame_pc;
-  struct frame_info *frame;
   struct value *return_value = NULL;
+  const char *query_prefix = "";
 
-  if (deprecated_selected_frame == NULL)
+  /* FIXME: cagney/2003-10-20: Perform a minimal existance test on the
+     target.  If that fails, error out.  For the moment don't rely on
+     get_selected_frame as it's error message is the the singularly
+     obscure "No registers".  */
+  if (!target_has_registers)
     error ("No selected frame.");
-  thisfun = get_frame_function (deprecated_selected_frame);
-  selected_frame_addr = get_frame_base (deprecated_selected_frame);
-  selected_frame_pc = get_frame_pc (deprecated_selected_frame);
-
-  /* Compute the return value (if any -- possibly getting errors here).  */
+  thisfun = get_frame_function (get_selected_frame ());
 
+  /* Compute the return value.  If the computation triggers an error,
+     let it bail.  If the return type can't be handled, set
+     RETURN_VALUE to NULL, and QUERY_PREFIX to an informational
+     message.  */
   if (retval_exp)
     {
       struct type *return_type = NULL;
 
+      /* Compute the return value.  Should the computation fail, this
+         call throws an error.  */
       return_value = parse_and_eval (retval_exp);
 
-      /* Cast return value to the return type of the function.  */
+      /* Cast return value to the return type of the function.  Should
+         the cast fail, this call throws an error.  */
       if (thisfun != NULL)
 	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
       if (return_type == NULL)
 	return_type = builtin_type_int;
       return_value = value_cast (return_type, return_value);
 
-      /* Make sure we have fully evaluated it, since
-         it might live in the stack frame we're about to pop.  */
+      /* Make sure the value is fully evaluated.  It may live in the
+         stack frame we're about to pop.  */
       if (VALUE_LAZY (return_value))
 	value_fetch_lazy (return_value);
-    }
-
-  /* If interactive, require confirmation.  */
 
-  if (from_tty)
-    {
-      if (thisfun != 0)
+      /* Check that this architecture can handle the function's return
+         type.  In the case of "struct convention", still do the
+         "return", just also warn the user.  */
+      if (gdbarch_return_value_p (current_gdbarch))
 	{
-	  if (!query ("Make %s return now? ", SYMBOL_PRINT_NAME (thisfun)))
-	    {
-	      error ("Not confirmed.");
-	      /* NOTREACHED */
-	    }
+	  if (gdbarch_return_value (current_gdbarch, return_type,
+				    NULL, NULL, NULL)
+	      == RETURN_VALUE_REGISTER_CONVENTION)
+	    return_value = NULL;
 	}
-      else if (!query ("Make selected stack frame return now? "))
-	error ("Not confirmed.");
+      else
+	{
+	  /* NOTE: cagney/2003-10-20: The double check is to ensure
+	     that the STORE_RETURN_VALUE call, further down, is not
+	     applied to a struct or union return-value.  It wasn't
+	     allowed previously, so don't start allowing it now.  An
+	     ABI that uses "register convention" to return small
+	     structures and should implement the "return_value"
+	     architecture method.  */
+	  if (using_struct_return (return_type, 0)
+	      || TYPE_CODE (return_type) == TYPE_CODE_STRUCT
+	      || TYPE_CODE (return_type) == TYPE_CODE_UNION)
+	    return_value = NULL;
+	}
+      if (return_value == NULL)
+	query_prefix = "\
+The location at which to store the function's return value is unknown.\n";
     }
 
-  /* FIXME: cagney/2003-01-18: Rather than pop each frame in turn,
-     this code should just go straight to the relevant frame and pop
-     that.  */
-
-  /* Do the real work.  Pop until the specified frame is current.  We
-     use this method because the deprecated_selected_frame is not
-     valid after a frame_pop().  The pc comparison makes this work
-     even if the selected frame shares its fp with another frame.  */
-
-  /* FIXME: cagney/32003-03-12: This code should use frame_id_eq().
-     Unfortunatly, that function doesn't yet include the PC in any
-     frame ID comparison.  */
+  /* Does an interactive user really want to do this?  Include
+     information, such as how well GDB can handle the return value, in
+     the query message.  */
+  if (from_tty)
+    {
+      int confirmed;
+      if (thisfun == NULL)
+	confirmed = query ("%sMake selected stack frame return now? ",
+			   query_prefix);
+      else
+	confirmed = query ("%sMake %s return now? ", query_prefix,
+			   SYMBOL_PRINT_NAME (thisfun));
+      if (!confirmed)
+	error ("Not confirmed");
+    }
 
-  while (selected_frame_addr != get_frame_base (frame = get_current_frame ())
-	 || selected_frame_pc != get_frame_pc (frame))
-    frame_pop (get_current_frame ());
+  /* NOTE: cagney/2003-01-18: Is this silly?  Rather than pop each
+     frame in turn, should this code just go straight to the relevant
+     frame and pop that?  */
 
-  /* Then pop that frame.  */
+  /* First discard all frames inner-to the selected frame (making the
+     selected frame current).  */
+  {
+    struct frame_id selected_id = get_frame_id (get_selected_frame ());
+    while (!frame_id_eq (selected_id, get_frame_id (get_current_frame ())))
+      {
+	if (frame_id_inner (selected_id, get_frame_id (get_current_frame ())))
+	  /* Caught in the safety net, oops!  We've gone way past the
+             selected frame.  */
+	  error ("Problem while popping stack frames (corrupt stack?)");
+	frame_pop (get_current_frame ());
+      }
+  }
 
+  /* Second discard the selected frame (which is now also the current
+     frame).  */
   frame_pop (get_current_frame ());
 
-  /* Compute the return value (if any) and store in the place
-     for return values.  */
-
-  if (retval_exp)
-    set_return_value (return_value);
-
-  /* If we are at the end of a call dummy now, pop the dummy frame too.  */
+  /* Store RETURN_VAUE in the just-returned register set.  */
+  if (return_value != NULL)
+    {
+      struct type *return_type = VALUE_TYPE (return_value);
+      if (!gdbarch_return_value_p (current_gdbarch))
+	{
+	  STORE_RETURN_VALUE (return_type, current_regcache,
+			      VALUE_CONTENTS (return_value));
+	}
+      else
+	{
+	  gdb_assert (gdbarch_return_value (current_gdbarch, return_type,
+					    NULL, NULL, NULL)
+		      == RETURN_VALUE_REGISTER_CONVENTION);
+	  gdbarch_return_value (current_gdbarch, return_type, current_regcache,
+				VALUE_CONTENTS (return_value), NULL);
+	}
+    }
 
-  /* FIXME: cagney/2003-01-18: This is silly.  Instead of popping all
-     the frames except the dummy, and then, as an afterthought,
-     popping the dummy frame, this code should just pop through to the
-     dummy frame.  */
-  
+  /* If we are at the end of a call dummy now, pop the dummy frame
+     too.  */
+  /* NOTE: cagney/2003-01-18: Is this silly?  Instead of popping all
+     the frames in sequence, should this code just pop the dummy frame
+     directly?  */
   if (CALL_DUMMY_HAS_COMPLETED (read_pc(), read_sp (),
 				get_frame_base (get_current_frame ())))
     frame_pop (get_current_frame ());
 
   /* If interactive, print the frame that is now current.  */
-
   if (from_tty)
     frame_command ("0", 1);
   else
diff --git a/gdb/value.h b/gdb/value.h
index 1ee176a7d4b..690edb96e89 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -425,8 +425,6 @@ extern int value_bit_index (struct type *type, char *addr, int index);
 
 extern int using_struct_return (struct type *value_type, int gcc_p);
 
-extern void set_return_value (struct value *val);
-
 extern struct value *evaluate_expression (struct expression *exp);
 
 extern struct value *evaluate_type (struct expression *exp);
diff --git a/gdb/values.c b/gdb/values.c
index da5c18022d6..aba500cfbec 100644
--- a/gdb/values.c
+++ b/gdb/values.c
@@ -1306,58 +1306,6 @@ using_struct_return (struct type *value_type, int gcc_p)
 	  == RETURN_VALUE_STRUCT_CONVENTION);
 }
 
-/* Store VAL so it will be returned if a function returns now.
-   Does not verify that VAL's type matches what the current
-   function wants to return.  */
-
-void
-set_return_value (struct value *val)
-{
-  struct type *type = check_typedef (VALUE_TYPE (val));
-  enum type_code code = TYPE_CODE (type);
-
-  if (code == TYPE_CODE_ERROR)
-    error ("Function return type unknown.");
-
-  if (gdbarch_return_value_p (current_gdbarch))
-    {
-      switch (gdbarch_return_value (current_gdbarch, type, NULL, NULL, NULL))
-	{
-	case RETURN_VALUE_REGISTER_CONVENTION:
-	  /* Success.  The architecture can deal with it, write it to
-             the regcache.  */
-	  gdbarch_return_value (current_gdbarch, type, current_regcache,
-				VALUE_CONTENTS (val), NULL);
-	  return;
-	case RETURN_VALUE_STRUCT_CONVENTION:
-	  /* Failure.  For the moment, assume that it is not possible
-             to find the location, on the stack, at which the "struct
-             return" value should be stored.  Only a warning because
-             an error aborts the "return" command leaving GDB in a
-             weird state.  */
-	  warning ("Location of return value unknown");
-	  return;
-	}
-    }
-
-
-  if (code == TYPE_CODE_STRUCT
-      || code == TYPE_CODE_UNION)	/* FIXME, implement struct return.  */
-    /* FIXME: cagney/2003-10-20: This should be an internal-warning.
-       The problem is that while GDB's core supports "struct return"
-       using "register convention", many architectures haven't been
-       updated to implement the mechanisms needed to make it work.
-       It's a warning, and not an error, as otherwize it will jump out
-       of the "return" command leaving both GDB and the user in a very
-       confused state.  */
-    {
-      warning ("This architecture does not support specifying a struct or union return-value.");
-      return;
-    }
-
-  STORE_RETURN_VALUE (type, current_regcache, VALUE_CONTENTS (val));
-}
-
 void
 _initialize_values (void)
 {