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

* valops.c (hand_function_call): Align the initial stack pointer
	and STRUCT_ADDR using frame_align.  When STRUCT_RETURN and
	FRAME_ALIGN_P, use STRUCT_ADDR to obtain the called function's
	return value.
	* mips-tdep.c (mips_frame_align): New function.
	(mips_gdbarch_init): Set frame_align.
	* gdbarch.sh (FRAME_ALIGN): New method.
	* gdbarch.h, gdbarch.c: Re-generate.

gdb/ChangeLog

@@ -1,3 +1,14 @@
+2002-09-18  Andrew Cagney  <ac131313@redhat.com>
+
+	* valops.c (hand_function_call): Align the initial stack pointer
+	and STRUCT_ADDR using frame_align.  When STRUCT_RETURN and
+	FRAME_ALIGN_P, use STRUCT_ADDR to obtain the called function's
+	return value.
+	* mips-tdep.c (mips_frame_align): New function.
+	(mips_gdbarch_init): Set frame_align.
+	* gdbarch.sh (FRAME_ALIGN): New method.
+	* gdbarch.h, gdbarch.c: Re-generate.
+
 2002-09-18  Michal Ludvig  <mludvig@suse.cz>
 
 	* x86-64-linux-nat.c (x86_64_regmap): Added CS and SS

gdb/gdbarch.c

@@ -244,6 +244,7 @@ struct gdbarch
   gdbarch_saved_pc_after_call_ftype *saved_pc_after_call;
   gdbarch_frame_num_args_ftype *frame_num_args;
   gdbarch_stack_align_ftype *stack_align;
+  gdbarch_frame_align_ftype *frame_align;
   int extra_stack_alignment_needed;
   gdbarch_reg_struct_has_addr_ftype *reg_struct_has_addr;
   gdbarch_save_dummy_frame_tos_ftype *save_dummy_frame_tos;
@@ -419,6 +420,7 @@ struct gdbarch startup_gdbarch =
   0,
   0,
   0,
+  0,
   generic_in_function_epilogue_p,
   construct_inferior_arguments,
   0,
@@ -775,6 +777,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
       && (gdbarch->frame_num_args == 0))
     fprintf_unfiltered (log, "\n\tframe_num_args");
   /* Skip verify of stack_align, has predicate */
+  /* Skip verify of frame_align, has predicate */
   /* Skip verify of extra_stack_alignment_needed, invalid_p == 0 */
   /* Skip verify of reg_struct_has_addr, has predicate */
   /* Skip verify of save_dummy_frame_tos, has predicate */
@@ -825,6 +828,10 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: GDB_MULTI_ARCH = %d\n",
                       GDB_MULTI_ARCH);
+  if (GDB_MULTI_ARCH)
+    fprintf_unfiltered (file,
+                        "gdbarch_dump: frame_align = 0x%08lx\n",
+                        (long) current_gdbarch->frame_align);
   if (GDB_MULTI_ARCH)
     fprintf_unfiltered (file,
                         "gdbarch_dump: in_function_epilogue_p = 0x%08lx\n",
@@ -4509,6 +4516,32 @@ set_gdbarch_stack_align (struct gdbarch *gdbarch,
   gdbarch->stack_align = stack_align;
 }
 
+int
+gdbarch_frame_align_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->frame_align != 0;
+}
+
+CORE_ADDR
+gdbarch_frame_align (struct gdbarch *gdbarch, CORE_ADDR address)
+{
+  gdb_assert (gdbarch != NULL);
+  if (gdbarch->frame_align == 0)
+    internal_error (__FILE__, __LINE__,
+                    "gdbarch: gdbarch_frame_align invalid");
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_frame_align called\n");
+  return gdbarch->frame_align (gdbarch, address);
+}
+
+void
+set_gdbarch_frame_align (struct gdbarch *gdbarch,
+                         gdbarch_frame_align_ftype frame_align)
+{
+  gdbarch->frame_align = frame_align;
+}
+
 int
 gdbarch_extra_stack_alignment_needed (struct gdbarch *gdbarch)
 {

gdb/gdbarch.h

@@ -2014,6 +2014,12 @@ extern void set_gdbarch_stack_align (struct gdbarch *gdbarch, gdbarch_stack_alig
 #endif
 #endif
 
+extern int gdbarch_frame_align_p (struct gdbarch *gdbarch);
+
+typedef CORE_ADDR (gdbarch_frame_align_ftype) (struct gdbarch *gdbarch, CORE_ADDR address);
+extern CORE_ADDR gdbarch_frame_align (struct gdbarch *gdbarch, CORE_ADDR address);
+extern void set_gdbarch_frame_align (struct gdbarch *gdbarch, gdbarch_frame_align_ftype *frame_align);
+
 /* Default (value) for non- multi-arch platforms. */
 #if (!GDB_MULTI_ARCH) && !defined (EXTRA_STACK_ALIGNMENT_NEEDED)
 #define EXTRA_STACK_ALIGNMENT_NEEDED (1)

gdb/gdbarch.sh

@@ -572,6 +572,7 @@ f:2:SAVED_PC_AFTER_CALL:CORE_ADDR:saved_pc_after_call:struct frame_info *frame:f
 f:2:FRAME_NUM_ARGS:int:frame_num_args:struct frame_info *frame:frame::0:0
 #
 F:2:STACK_ALIGN:CORE_ADDR:stack_align:CORE_ADDR sp:sp::0:0
+M:::CORE_ADDR:frame_align:CORE_ADDR address:address
 v:2:EXTRA_STACK_ALIGNMENT_NEEDED:int:extra_stack_alignment_needed::::0:1::0:::
 F:2:REG_STRUCT_HAS_ADDR:int:reg_struct_has_addr:int gcc_p, struct type *type:gcc_p, type::0:0
 F:2:SAVE_DUMMY_FRAME_TOS:void:save_dummy_frame_tos:CORE_ADDR sp:sp::0:0

gdb/mips-tdep.c

@@ -2584,6 +2584,14 @@ mips_type_needs_double_align (struct type *type)
 #define ROUND_DOWN(n,a) ((n) & ~((a)-1))
 #define ROUND_UP(n,a) (((n)+(a)-1) & ~((a)-1))
 
+/* Adjust the address downward (direction of stack growth) so that it
+   is correctly aligned for a new stack frame.  */
+static CORE_ADDR
+mips_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  return ROUND_DOWN (addr, 16);
+}
+
 static CORE_ADDR
 mips_eabi_push_arguments (int nargs,
 			  struct value **args,
@@ -5963,6 +5971,7 @@ mips_gdbarch_init (struct gdbarch_info info,
   set_gdbarch_call_dummy_words (gdbarch, mips_call_dummy_words);
   set_gdbarch_sizeof_call_dummy_words (gdbarch, sizeof (mips_call_dummy_words));
   set_gdbarch_push_return_address (gdbarch, mips_push_return_address);
+  set_gdbarch_frame_align (gdbarch, mips_frame_align);
   set_gdbarch_register_convertible (gdbarch, mips_register_convertible);
   set_gdbarch_register_convert_to_virtual (gdbarch, 
 					   mips_register_convert_to_virtual);

gdb/valops.c

@@ -1351,7 +1351,55 @@ hand_function_call (struct value *function, int nargs, struct value **args)
      they are saved on the stack in the inferior.  */
   PUSH_DUMMY_FRAME;
 
-  old_sp = sp = read_sp ();
+  old_sp = read_sp ();
+
+  /* Ensure that the initial SP is correctly aligned.  */
+  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
+	 :-).  */
+      sp = gdbarch_frame_align (current_gdbarch, old_sp);
+      if (sp == old_sp)
+	{
+	  if (INNER_THAN (1, 2))
+	    /* Stack grows down.  */
+	    sp = gdbarch_frame_align (current_gdbarch, old_sp - 1);
+	  else
+	    /* Stack grows up.  */
+	    sp = gdbarch_frame_align (current_gdbarch, old_sp + 1);
+	}
+      gdb_assert ((INNER_THAN (1, 2) && sp <= old_sp)
+		  || (INNER_THAN (2, 1) && sp >= old_sp));
+    }
+  else
+    /* FIXME: cagney/2002-09-18: Hey, you loose!  Who knows how badly
+       aligned the SP is!  Further, per comment above, if the generic
+       dummy frame ends up empty (because nothing is pushed) GDB won't
+       be able to correctly perform back traces.  If a target is
+       having trouble with backtraces, first thing to do is add
+       FRAME_ALIGN() to its architecture vector.  After that, try
+       adding SAVE_DUMMY_FRAME_TOS() and modifying FRAME_CHAIN so that
+       when the next outer frame is a generic dummy, it returns the
+       current frame's base.  */
+    sp = old_sp;
 
   if (INNER_THAN (1, 2))
     {
@@ -1366,6 +1414,11 @@ hand_function_call (struct value *function, int nargs, struct value **args)
       sp += sizeof_dummy1;
     }
 
+  /* NOTE: cagney/2002-09-10: Don't bother re-adjusting the stack
+     after allocating space for the call dummy.  A target can specify
+     a SIZEOF_DUMMY1 (via SIZEOF_CALL_DUMMY_WORDS) such that all local
+     alignment requirements are met.  */
+
   funaddr = find_function_addr (function, &value_type);
   CHECK_TYPEDEF (value_type);
 
@@ -1562,7 +1615,8 @@ You must use a pointer to function type variable. Command ignored.", arg_name);
 
 
   /* Reserve space for the return structure to be written on the
-     stack, if necessary */
+     stack, if necessary.  Make certain that the value is correctly
+     aligned. */
 
   if (struct_return)
     {
@@ -1574,15 +1628,23 @@ You must use a pointer to function type variable. Command ignored.", arg_name);
 	len = STACK_ALIGN (len);
       if (INNER_THAN (1, 2))
 	{
-	  /* stack grows downward */
+	  /* Stack grows downward.  Align STRUCT_ADDR and SP after
+             making space for the return value.  */
 	  sp -= len;
+	  if (gdbarch_frame_align_p (current_gdbarch))
+	    sp = gdbarch_frame_align (current_gdbarch, sp);
 	  struct_addr = sp;
 	}
       else
 	{
-	  /* stack grows upward */
+	  /* Stack grows upward.  Align the frame, allocate space, and
+             then again, re-align the frame??? */
+	  if (gdbarch_frame_align_p (current_gdbarch))
+	    sp = gdbarch_frame_align (current_gdbarch, sp);
 	  struct_addr = sp;
 	  sp += len;
+	  if (gdbarch_frame_align_p (current_gdbarch))
+	    sp = gdbarch_frame_align (current_gdbarch, sp);
 	}
     }
 
@@ -1778,14 +1840,13 @@ the function call).", name);
     do_cleanups (inf_status_cleanup);
 
     /* Figure out the value returned by the function.  */
-/* elz: I defined this new macro for the hppa architecture only.
+    /* elz: I defined this new macro for the hppa architecture only.
-   this gives us a way to get the value returned by the function from the stack,
+       this gives us a way to get the value returned by the function
-   at the same address we told the function to put it.
+       from the stack, at the same address we told the function to put
-   We cannot assume on the pa that r28 still contains the address of the returned
+       it.  We cannot assume on the pa that r28 still contains the
-   structure. Usually this will be overwritten by the callee.
+       address of the returned structure. Usually this will be
-   I don't know about other architectures, so I defined this macro
+       overwritten by the callee.  I don't know about other
- */
+       architectures, so I defined this macro */
-
 #ifdef VALUE_RETURNED_FROM_STACK
     if (struct_return)
       {
@@ -1793,12 +1854,26 @@ the function call).", name);
 	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
-      struct value *retval = value_being_returned (value_type, retbuf, struct_return);
+       fetch the return value direct from the stack.  This lack of
-      do_cleanups (retbuf_cleanup);
+       trust comes about because legacy targets have a nasty habit of
-      return retval;
+       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;
+      }
   }
 }