[RFA]: Add IN_EPILOGUE() predicate

[Corinna Vinschen <vinschen at redhat dot com>]

Hi,

some people might recall my patch to gdb.base/recurse.exp, sent to
this list on 2001-09-14 which added the following comment to the
exp file (which actually was a description I got from Michael Snyder):

  # The former version expected the test to return to main().
  # Now it expects the test to return to main or to stop in the
  # function's epilogue.
  # 
  # The problem is that gdb needs to (but doesn't) understand
  # function epilogues in the same way as for prologues.
  # 
  # If there is no hardware watchpoint (such as a x86 debug register),
  # then watchpoints are done "the hard way" by single-stepping the
  # target until the value of the watched variable changes.  If you
  # are single-stepping, you will eventually step into an epilogue.
  # When you do that, the "top" stack frame may become partially
  # deconstructed (as when you pop the frame pointer, for instance),
  # and from that point on, GDB can no longer make sense of the stack.
  # 
  # A test which stops in the epilogue is trying to determine when GDB
  # leaves the stack frame in which the watchpoint was created.  It does
  # this basically by watching for the frame pointer to change.  When
  # the frame pointer changes, the test expects to be back in main, but
  # instead it is still in the epilogue of the callee.

The below patch basically adds the predicate `IN_EPILOGUE(CORE_ADDR addr)'
to gdb.  It's defined to return a non-zero value if the given address
`addr' is in the epilogue of the function.  The epilogue of the function
is defined as the part of a function between the eventually destroying
of the stack frame and the trailing `return to caller' instruction.

Ok, now we have a definition of a predicate which offers (in which
way ever) the information if we're currently in an epilogue or not.
How does that help in the aforementioned case of recurse.exp?

This is part two of the patch, the actual usage of IN_EPILOGUE().

Currently there's only one point in the code at which I have added
a call to IN_EPILOGUE(), breakpoint.c (watchpoint_check)i, line 2308ff:

       reinit_frame_cache ();
       fr = find_frame_addr_in_frame_chain (b->watchpoint_frame);
       within_current_scope = (fr != NULL);
+      /* IN_EPILOGUE() returns a non-zero value if we're still in the
+        function but the stack frame has already been invalidated.
+        Since we can't rely on the values of local variables after
+        the stack has been destroyed, we are treating the watchpoint
+        in that state as `not changed' without further checking. */
+      if (within_current_scope && fr == get_current_frame ()
+          && IN_EPILOGUE (read_pc ()))
+        return WP_VALUE_NOT_CHANGED;
       if (within_current_scope)
        /* If we end up stopping, the current frame will get selected
           in normal_stop.  So this call to select_frame won't affect

The comment says it all.  The problem in watchpoint_check() at that
point is that _if_ we're actually in the epilogue of a function we
can't rely on any value of local variables.  They could have changed
or not, who knows?  However, that's not what we are interested in. 
When the value of a local variable is different by coincidence we
don't mind.

The above added code does IMO what should be done when we're currently
in an epilogue.  It immediately leaves the function watchpoint_check()
without checking the watchpoints.  The epilogue is treated as `twilight
zone'.  This results in that we first leave the current function before
checking for watchpoints again.  Targets suffering from that problem
now leave the function first before the watchpoint will be deleted.
One result:  They pass the recurse.exp test.

If that's not already clear:  IN_EPILOGUE() returns 0 by default,
so if your target doesn't have a problem with the above behaviour or
your target doesn't provide a reliable way to determine the epilogue,
you just don't touch IN_EPILOGUE().  The whole code just behaves as
before then.

The complete patch follows.  I have again only send gdbarch.sh and not
the autogenerated gdbarch.[ch] to save some space.

Hope, that helps,
Corinna


2001-11-01  Corinna Vinschen  <vinschen@redhat.com>

	* arch-utils.c (generic_in_epilogue): New function.
	* arch-utils.h (generic_in_epilogue): Declare extern.
	* breakpoint.c (watchpoint_check): Add test if the pc
	is currently in the epilogue of a function.
	* gdbarch.c: Autogenerated from gdbarch.sh.
	* gdbarch.h: Ditto.
	* gdbarch.sh (function_list): Add `IN_EPILOGUE' definition.

Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.37
diff -u -p -r1.37 arch-utils.c
--- arch-utils.c	2001/10/31 23:21:33	1.37
+++ arch-utils.c	2001/11/01 15:46:06
@@ -111,6 +111,12 @@ generic_in_solib_call_trampoline (CORE_A
   return 0;
 }
 
+int
+generic_in_epilogue (CORE_ADDR pc)
+{
+  return 0;
+}
+
 char *
 legacy_register_name (int i)
 {
Index: arch-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.h,v
retrieving revision 1.22
diff -u -p -r1.22 arch-utils.h
--- arch-utils.h	2001/10/31 23:21:33	1.22
+++ arch-utils.h	2001/11/01 15:46:06
@@ -134,4 +134,6 @@ extern CORE_ADDR generic_skip_trampoline
 
 extern int generic_in_solib_call_trampoline (CORE_ADDR pc, char *name);
 
+extern int generic_in_epilogue (CORE_ADDR pc);
+
 #endif
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.55
diff -u -p -r1.55 breakpoint.c
--- breakpoint.c	2001/10/20 23:54:29	1.55
+++ breakpoint.c	2001/11/01 15:46:10
@@ -2308,6 +2308,14 @@ watchpoint_check (PTR p)
       reinit_frame_cache ();
       fr = find_frame_addr_in_frame_chain (b->watchpoint_frame);
       within_current_scope = (fr != NULL);
+      /* IN_EPILOGUE() returns a non-zero value if we're still in the
+	 function but the stack frame has already been invalidated.
+	 Since we can't rely on the values of local variables after
+	 the stack has been destroyed, we are treating the watchpoint
+	 in that state as `not changed' without further checking. */
+      if (within_current_scope && fr == get_current_frame ()
+          && IN_EPILOGUE (read_pc ()))
+	return WP_VALUE_NOT_CHANGED;
       if (within_current_scope)
 	/* If we end up stopping, the current frame will get selected
 	   in normal_stop.  So this call to select_frame won't affect
Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.84
diff -u -p -r1.84 gdbarch.sh
--- gdbarch.sh	2001/10/31 23:21:33	1.84
+++ gdbarch.sh	2001/11/01 15:46:11
@@ -546,6 +546,15 @@ f:2:SKIP_TRAMPOLINE_CODE:CORE_ADDR:skip_
 # trampoline code in the ".plt" section.  IN_SOLIB_CALL_TRAMPOLINE evaluates
 # to nonzero if we are current stopped in one of these.
 f:2:IN_SOLIB_CALL_TRAMPOLINE:int:in_solib_call_trampoline:CORE_ADDR pc, char *name:pc, name:::generic_in_solib_call_trampoline::0
+# A target might have problems with watchpoints as soon as the stack frame
+# of the current function has been destroyed.  This mostly happens as the
+# first action in a funtion's epilogue.  IN_EPILOGUE() is defined to return
+# a non-zero value if either the given addr is one instruction after the stack
+# destroying instruction up to the trailing return instruction or if we can
+# figure out that the stack frame has already been invalidated regardless
+# of the value of addr.  Targets which don't suffer from that problem could
+# just let this functionality untouched.
+f:2:IN_EPILOGUE:int:in_epilogue:CORE_ADDR addr:addr::0:generic_in_epilogue::0
 EOF
 }