[rfc; rfa:i386] Eliminate save_dummy_frame_tos

[rfc; rfa:i386] Eliminate save_dummy_frame_tos

[Andrew Cagney]

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

Hello,

This patch removes the need for setting the save_dummy_frame_tos method 
(d10v and i386).  Instead the frame ID stack addr returned by 
push_dummy_call is saved (by generic_save_dummy_frame_tos) and used as 
the dummy breakpoint ID.

The i386 and d10v don't show any regressions.

Mark, look right?
I'd like to add a comment to the ``return sp + 8'' only I'm not actually 
sure what's going on here :-/

Andrew

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

2003-06-13  Andrew Cagney  <cagney@redhat.com>

	* infcall.c (call_function_by_hand): When UNWIND_DUMMY_ID is
	available, do not use the FP and always save the TOS.
	* dummy-frame.c (dummy_frame_this_id): Do not assert
	SAVE_DUMMY_FRAME_TOS.
	* i386-tdep.c (i386_save_dummy_frame_tos): Delete function.
	(i386_gdbarch_init): Do not set save_dummy_frame_tos.
	(i386_push_dummy_call): Add 8 to the returned SP.
	* frame.c (legacy_frame_p): Do not require SAVE_DUMMY_FRAME_TOS.
	* d10v-tdep.c (d10v_unwind_dummy_id): Use d10v_unwind_sp.
	(d10v_gdbarch_init): Do not set save_dummy_frame_tos.

Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.124
diff -u -r1.124 d10v-tdep.c
--- d10v-tdep.c	9 Jun 2003 17:35:56 -0000	1.124
+++ d10v-tdep.c	13 Jun 2003 04:54:14 -0000
@@ -1477,9 +1477,8 @@
 static struct frame_id
 d10v_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
-  ULONGEST base;
-  frame_unwind_unsigned_register (next_frame, D10V_SP_REGNUM, &base);
-  return frame_id_build (d10v_make_daddr (base), frame_pc_unwind (next_frame));
+  return frame_id_build (d10v_unwind_sp (gdbarch, next_frame),
+			 frame_pc_unwind (next_frame));
 }
 
 static gdbarch_init_ftype d10v_gdbarch_init;
@@ -1593,9 +1592,10 @@
   frame_unwind_append_predicate (gdbarch, d10v_frame_p);
   frame_base_set_default (gdbarch, &d10v_frame_base);
 
-  /* Methods for saving / extracting a dummy frame's ID.  */
+  /* Methods for saving / extracting a dummy frame's ID.  The ID's
+     stack address must match the SP value returned by
+     PUSH_DUMMY_CALL, and saved by generic_save_dummy_frame_tos.  */
   set_gdbarch_unwind_dummy_id (gdbarch, d10v_unwind_dummy_id);
-  set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);
 
   /* Return the unwound PC value.  */
   set_gdbarch_unwind_pc (gdbarch, d10v_unwind_pc);
Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.23
diff -u -r1.23 dummy-frame.c
--- dummy-frame.c	22 May 2003 18:37:04 -0000	1.23
+++ dummy-frame.c	13 Jun 2003 04:54:18 -0000
@@ -360,10 +360,6 @@
      just asking for trouble.  */
   if (gdbarch_unwind_dummy_id_p (current_gdbarch))
     {
-      /* Assume call_function_by_hand(), via SAVE_DUMMY_FRAME_TOS,
-	 previously saved the dummy frame's ID.  Things only work if
-	 the two return the same value.  */
-      gdb_assert (SAVE_DUMMY_FRAME_TOS_P ());
       /* Use an architecture specific method to extract the prev's
 	 dummy ID from the next frame.  Note that this method uses
 	 frame_register_unwind to obtain the register values needed to
Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.153
diff -u -r1.153 i386-tdep.c
--- i386-tdep.c	11 Jun 2003 19:38:26 -0000	1.153
+++ i386-tdep.c	13 Jun 2003 04:54:27 -0000
@@ -960,12 +960,6 @@
   i386_frame_base_address
 };
 
-static void
-i386_save_dummy_frame_tos (CORE_ADDR sp)
-{
-  generic_save_dummy_frame_tos (sp + 8);
-}
-
 static struct frame_id
 i386_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
@@ -1058,7 +1052,7 @@
   /* ...and fake a frame pointer.  */
   regcache_cooked_write (regcache, I386_EBP_REGNUM, buf);
 
-  return sp;
+  return sp + 8;
 }
 
 /* These registers are used for returning integers (and on some
@@ -1716,7 +1710,6 @@
   set_gdbarch_print_insn (gdbarch, i386_print_insn);
 
   set_gdbarch_unwind_dummy_id (gdbarch, i386_unwind_dummy_id);
-  set_gdbarch_save_dummy_frame_tos (gdbarch, i386_save_dummy_frame_tos);
 
   set_gdbarch_unwind_pc (gdbarch, i386_unwind_pc);
 
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.15
diff -u -r1.15 infcall.c
--- infcall.c	8 Jun 2003 18:27:13 -0000	1.15
+++ infcall.c	13 Jun 2003 04:54:28 -0000
@@ -825,6 +825,14 @@
 
   if (SAVE_DUMMY_FRAME_TOS_P ())
     SAVE_DUMMY_FRAME_TOS (sp);
+  else if (gdbarch_unwind_dummy_id_p (current_gdbarch))
+    {
+      /* Sanity.  The exact same SP value is returned by
+	 PUSH_DUMMY_CALL, saved as the dummy-frame TOS, and used by
+	 unwind_dummy_id to form the frame ID's stack address.  */
+      gdb_assert (DEPRECATED_USE_GENERIC_DUMMY_FRAMES);
+      generic_save_dummy_frame_tos (sp);
+    }
 
   /* Now proceed, having reached the desired place.  */
   clear_proceed_status ();
@@ -843,17 +851,29 @@
        set_momentary_breakpoint.  We need to give the breakpoint a
        frame ID so that the breakpoint code can correctly re-identify
        the dummy breakpoint.  */
-    /* The assumption here is that push_dummy_call() returned the
-       stack part of the frame ID.  Unfortunatly, many older
-       architectures were, via a convoluted mess, relying on the
-       poorly defined and greatly overloaded DEPRECATED_TARGET_READ_FP
-       or DEPRECATED_FP_REGNUM to supply the value.  */
-    if (DEPRECATED_TARGET_READ_FP_P ())
-      frame = frame_id_build (DEPRECATED_TARGET_READ_FP (), sal.pc);
-    else if (DEPRECATED_FP_REGNUM >= 0)
-      frame = frame_id_build (read_register (DEPRECATED_FP_REGNUM), sal.pc);
+    if (gdbarch_unwind_dummy_id_p (current_gdbarch))
+      {
+	/* Sanity.  The exact same SP value is returned by
+	 PUSH_DUMMY_CALL, saved as the dummy-frame TOS, and used by
+	 unwind_dummy_id to form the frame ID's stack address.  */
+	gdb_assert (DEPRECATED_USE_GENERIC_DUMMY_FRAMES);
+	frame = frame_id_build (sp, sal.pc);
+      }
     else
-      frame = frame_id_build (sp, sal.pc);
+      {
+	/* The assumption here is that push_dummy_call() returned the
+	   stack part of the frame ID.  Unfortunatly, many older
+	   architectures were, via a convoluted mess, relying on the
+	   poorly defined and greatly overloaded
+	   DEPRECATED_TARGET_READ_FP or DEPRECATED_FP_REGNUM to supply
+	   the value.  */
+	if (DEPRECATED_TARGET_READ_FP_P ())
+	  frame = frame_id_build (DEPRECATED_TARGET_READ_FP (), sal.pc);
+	else if (DEPRECATED_FP_REGNUM >= 0)
+	  frame = frame_id_build (read_register (DEPRECATED_FP_REGNUM), sal.pc);
+	else
+	  frame = frame_id_build (sp, sal.pc);
+      }
     bpt = set_momentary_breakpoint (sal, frame, bp_call_dummy);
     bpt->disposition = disp_del;
   }

Re: [rfc; rfa:i386] Eliminate save_dummy_frame_tos

[Mark Kettenis]

Andrew Cagney <ac131313@redhat.com> writes:

> This is a multi-part message in MIME format.
> --------------050407020306070609080700
> Content-Type: text/plain; charset=us-ascii; format=flowed
> Content-Transfer-Encoding: 7bit
> 
> Hello,
> 
> This patch removes the need for setting the save_dummy_frame_tos method 
> (d10v and i386).  Instead the frame ID stack addr returned by 
> push_dummy_call is saved (by generic_save_dummy_frame_tos) and used as 
> the dummy breakpoint ID.
> 
> The i386 and d10v don't show any regressions.
> 
> Mark, look right?

Looks good to me.  We can make similar changes to amd64, but if you
don't want to make those, I'll do a followup patch to yours.

> I'd like to add a comment to the ``return sp + 8'' only I'm not actually 
> sure what's going on here :-/

It certainly deserves a comment.  I'm just not sure.  This "+ 8" is
all over the place (i386_frame_this_id, i386_sigtramp_frame_this_id,
i386_unwind_dummy_id).  It's there, since all frame unwinders for a
given target have to agree (within a certain margin) on the defenition
of the stack address of a frame.  Otherwise frame_id_inner() won't
work correctly.  Since DWARF2/GCC uses the stack address *before* the
function call as a frame's CFA.  On the i386, when %ebp is used as a
frame pointer, the offset between the contents %ebp and the CFA as
defined by GCC.

Mark

Re: [rfc; rfa:i386] Eliminate save_dummy_frame_tos

[Andrew Cagney]

> Andrew Cagney <ac131313@redhat.com> writes:
> 
> 
>> This is a multi-part message in MIME format.
>> --------------050407020306070609080700
>> Content-Type: text/plain; charset=us-ascii; format=flowed
>> Content-Transfer-Encoding: 7bit
>> 
>> Hello,
>> 
>> This patch removes the need for setting the save_dummy_frame_tos method 
>> (d10v and i386).  Instead the frame ID stack addr returned by 
>> push_dummy_call is saved (by generic_save_dummy_frame_tos) and used as 
>> the dummy breakpoint ID.
>> 
>> The i386 and d10v don't show any regressions.
>> 
>> Mark, look right?
> 
> 
> Looks good to me.  We can make similar changes to amd64, but if you
> don't want to make those, I'll do a followup patch to yours.

I feel lucky ....

>> I'd like to add a comment to the ``return sp + 8'' only I'm not actually 
>> sure what's going on here :-/
> 
> 
> It certainly deserves a comment.  I'm just not sure.  This "+ 8" is
> all over the place (i386_frame_this_id, i386_sigtramp_frame_this_id,
> i386_unwind_dummy_id).  It's there, since all frame unwinders for a
> given target have to agree (within a certain margin) on the defenition
> of the stack address of a frame.  Otherwise frame_id_inner() won't
> work correctly.  Since DWARF2/GCC uses the stack address *before* the
> function call as a frame's CFA.  On the i386, when %ebp is used as a
> frame pointer, the offset between the contents %ebp and the CFA as
> defined by GCC.

Stolen.  I've checked in the attached.  It's slightly different to the 
original in that:

- it also tweaks x86-64 and alpha
- when unwind_dummy_id_p it always uses generic_save_dummy_frame_tos 
which forces the 4 ducks:
-- push_dummy_call
-- unwind_dummy_id
-- the dummy breakpoint's frame ID
-- the dummy frame's stack addr
to be in a row.

I'll follow-up with extra gdbarch comments.

Andrew