This is the mail archive of the gdb-patches@sourceware.org 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]

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5


On Mon, 26 Mar 2012 23:45:27 +0200, Mark Kettenis wrote:
> Well, I'd prefer not to have to re-read the entire discussion to
> review your diff.  But are you saying that you now do think ON_STACK
> is the right way to go and I can simply forget about the history?

We need some place without any installed FDE or a place with our own
(unwind-terminating) FDE.  _start was proven is no longer the right place (on
ppc*), PLT stubs are also no longer the right place (with recent ld).

So remains stack or maybe some malloc-allocated memory but from those ON_STACK
seems to be easier.  I had some concerns about security frameworks limiting
stack executability but I have no real world countercase, it seems to be all
handled correctly.

Still despite [patch 1/2] now correctly removes stale breakpoints from stack
in some cases the breakpoints may remain there such as if inferior longjmps
out of the inferior called function.  With ON_STACK inferior functions it is
a real problem, stale inferior call breakpoints were not a problem before.
breakpoint.h:
    /* The breakpoint at the end of a call dummy.  */
    /* FIXME: What if the function we are calling longjmp()s out of
       the call, or the user gets out with the "return" command?  We
       currently have no way of cleaning up the breakpoint in these
       (obscure) situations.  (Probably can solve this by noticing
       longjmp, "return", etc., it's similar to noticing when a
       watchpoint on a local variable goes out of scope (with hardware
       support for watchpoints)).  */
    bp_call_dummy,

The "return" command is implemented in [patch 1/2] (and the comment is not
updated) but retrospectively I see we may need to also handle the out-of-scope
breakpoints.

OTOH I did not implement it because in some cases backtrace fails temporarily
due to some missing CFIs and it would immediately remove such inferior return
breakpoints.  It happens with
	Watchpoint %d deleted because the program has left the block\n\
	in which its expression is valid.\n"),
but a deleted watchpoint is not so serious like deleted inferior call return
breakpoint.

There may be rather some logic to consider a frame X stale if we successfully
unwind next frame, X is not found and X is not inner to the next frame.
But
(a) There were some plans to remove frame_id_inner completely as it breaks
    with -fsplit-stack for example.
(b) bp_watchpoint_scope should be also reworked the same way that time.

Another way is to really handle such stale breakpoints in the longjmp
breakpoint caught by GDB.  I did not investigate more this way.

But TBH longjmp is a hack, one should use proper C++ exceptions instead and
those get caught properly for inferior calls.  These were some of the reasons
I did not go so far to try deal more with longjmps out of inferior calls,
which are bizarre enough on their own.


Thanks,
Jan


gdb/
2012-03-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* breakpoint.h (bp_call_dummy): Update the comment.
	* dummy-frame.c: Include gdbthread.h.
	(pop_dummy_frame_bpt): New function.
	(pop_dummy_frame): Initialie DUMMY earlier.  Call pop_dummy_frame_bpt.

gdb/testsuite/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
	(maintenance info breakpoints): New tests.

--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -95,10 +95,10 @@ enum bptype
 
     /* The breakpoint at the end of a call dummy.  */
     /* FIXME: What if the function we are calling longjmp()s out of
-       the call, or the user gets out with the "return" command?  We
-       currently have no way of cleaning up the breakpoint in these
-       (obscure) situations.  (Probably can solve this by noticing
-       longjmp, "return", etc., it's similar to noticing when a
+       the call?  "return" command is handled by pop_dummy_frame_bpt.
+       We currently have no way of cleaning up the breakpoint in such
+       (obscure) situation.  (Probably can solve this by noticing
+       longjmp, etc., it's similar to noticing when a
        watchpoint on a local variable goes out of scope (with hardware
        support for watchpoints)).  */
     bp_call_dummy,
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -29,6 +29,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "observer.h"
+#include "gdbthread.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
   xfree (dummy);
 }
 
+/* Delete any breakpoint B which is a momentary breakpoint for return from
+   inferior call matching DUMMY_VOIDP.  */
+
+static int
+pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
+{
+  struct dummy_frame *dummy = dummy_voidp;
+
+  if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)
+      && b->thread == pid_to_thread_id (inferior_ptid))
+    delete_breakpoint (b);
+
+  /* Continue the traversal.  */
+  return 0;
+}
+
 /* Pop *DUMMY_PTR, restoring program state to that before the
    frame was created.  */
 
 static void
 pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
-  struct dummy_frame *dummy;
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  restore_infcall_suspend_state (dummy->caller_state);
 
-  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
+  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
 
   /* restore_infcall_control_state frees inf_state,
      all that remains is to pop *dummy_ptr.  */
-  dummy = *dummy_ptr;
   *dummy_ptr = dummy->next;
   xfree (dummy);
 
--- a/gdb/testsuite/gdb.base/call-signal-resume.exp
+++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
@@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
 gdb_test_no_output "set confirm off"
 gdb_test "return" ""
 
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re "call dummy.*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
 # Resume execution, the program should continue without any signal.
 
 gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"


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