Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] improve python finish breakpoint for exceptions/longjmp case.
@ 2012-09-21 14:58 Andrew Burgess
  2012-09-24 10:22 ` Kevin Pouget
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Andrew Burgess @ 2012-09-21 14:58 UTC (permalink / raw)
  To: gdb-patches

The documentation for the python FinishBreakpoints suggests that when a longjmp or c++ exception is used to terminate a function the out_of_scope method will be called.  A quick look inside gdb/python/py-finishbreakpoint.c shows that we take no action to ensure that we will break on exceptions or longjmps (for example using breakpoint.c:set_longjmp_breakpoint), instead if we leave a function using for longjmp/exception we rely on hitting some other stop point to trigger the call to the out_of_scope method.

A further issue is that the testing for FinishBreakpoints, in gdb/testsuite/gdb.python/py-finish-breakpoint2.exp, the test action titled "check FinishBreakpoint in catch()" expects the "stop" method to fire rather than the "out_of_scope" method, this is due to the generated code (on x86 and maybe other targets), the first breakpoint we hit after throwing the exception happens to be the finish breakpoint, however this is not guaranteed, and means that (a) the test does not match the documentation, and (b) the test is platform specific.

I have a patch below that improves and extends the testing to cover more cases, and a patch for py-finishbreakpoint.c that goes some way to fixing some of the problems.

There are a couple of comments to go with the patch,

 1. I've changed the c++ test program from using c++ streams to using cstdio.  I have to confess self interest here, on my local target the c++ streams are too large to build in, so any test that uses them will not work for me.  I'm not the only target for which this is the case, see the comments in gdb/testsuite/lib/gdb.exp.  Though we _obviously_ have to have good test coverage for gdb with c++ streams, I think it would be a shame if some important behaviours are only tested on targets that support c++ streams.  However, if this is a problem then I can change the test back to using c++ streams.

 2. The patch does not solve all the problems with FinishBreakpoints.  I use set_longjmp_breakpoint to create the breakpoints for the lonjmp/exceptions, however, between creating an instance of FinishBreakpoint, when the breakpoints are created, and hitting these breakpoints, we are free to issue any commands we like, including things like step/finish/etc all of which use set_longjmp_breakpoint themselves.  As these commands (step/finish/etc) complete, they remove the longjmp breakpoints, preventing the FinishBreakpoint from stopping correctly.  This behaviour is tested in my updated test and currently kfails; before committing this patch I'll raise a defect and fill in the new bug id.

I have some ideas about how to fix #2 but I'm open to suggestions.  As the gdb is no worse than it was in case #2, and is significantly better in other cases, I'd like to push this patch now (given that I'll raise a bug for broken case).

Let me know what you think, OK to commit?

Thanks,
Andrew

gdb/ChangeLog

2012-09-21  Andrew Burgess  <aburgess@broadcom.com>

	* python/py-finishbreakpoint.c (struct finish_breakpoint_object)
	<initiating_frame>: New field.
	(bpfinishpy_post_stop_hook): Disable the longjmp breakpoints when
	we stop at a finish breakpoint.  Have the finish breakpoint
	deleted at the next stop, wherever that might be.
	(bpfinishpy_init): Set longjmp breakpoints.  Remember frame we're
	in when creating the finish breakpoint.
	(bpfinishpy_detect_out_scope_cb): Look for frame we are hoping to
	finish when deciding if we're out of scope, not frame of parent.

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 56ab775..a2cd980 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -53,6 +53,9 @@ struct finish_breakpoint_object
      the function; Py_None if the value is not computable; NULL if GDB is
      not stopped at a FinishBreakpoint.  */
   PyObject *return_value;
+  /* The initiating frame for this operation, used to decide when we have
+     left this frame.  */
+  struct frame_id initiating_frame;
 };
 
 /* Python function to get the 'return_value' attribute of
@@ -141,6 +144,10 @@ bpfinishpy_post_stop_hook (struct breakpoint_object *bp_obj)
       /* Can't delete it here, but it will be removed at the next stop.  */
       disable_breakpoint (bp_obj->bp);
       gdb_assert (bp_obj->bp->disposition == disp_del);
+      bp_obj->bp->disposition = disp_del_at_next_stop;
+
+      /* Disable all the longjmp breakpoints too.  */
+      delete_longjmp_breakpoint_at_next_stop (inferior_thread ()->num);
     }
   if (except.reason < 0)
     {
@@ -295,11 +302,13 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
                          AUTO_BOOLEAN_TRUE,
                          &bkpt_breakpoint_ops,
                          0, 1, internal_bp, 0);
+      set_longjmp_breakpoint (inferior_thread (), null_frame_id);
     }
   GDB_PY_SET_HANDLE_EXCEPTION (except);
   
   self_bpfinish->py_bp.bp->frame_id = frame_id;
   self_bpfinish->py_bp.is_finish_bp = 1;
+  self_bpfinish->initiating_frame = get_frame_id (frame);
   
   /* Bind the breakpoint with the current program space.  */
   self_bpfinish->py_bp.bp->pspace = current_program_space;
@@ -329,6 +338,7 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
           gdbpy_print_stack ();
     }
 
+  delete_longjmp_breakpoint_at_next_stop (inferior_thread ()->num);
   delete_breakpoint (bpfinish_obj->py_bp.bp);
 }
 
@@ -355,9 +365,11 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
         {
           TRY_CATCH (except, RETURN_MASK_ALL)
             {
+	      struct frame_id fid = finish_bp->initiating_frame;
+
               if (b->pspace == current_inferior ()->pspace
                   && (!target_has_registers
-                      || frame_find_by_id (b->frame_id) == NULL))
+                      || frame_find_by_id (fid) == NULL))
                 bpfinishpy_out_of_scope (finish_bp);
             }
           if (except.reason < 0)

2012-09-21  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.python/py-finish-breakpoint2.cc: Add extra levels of nesting
	to allow more testing opportunities.
	* gdb.python/py-finish-breakpoint2.exp: Additional test cases.

diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
index 8cc756f..930e6bc 100644
--- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
@@ -22,7 +22,8 @@
 void
 throw_exception_1 (int e)
 {
-  throw new int (e);
+  if (e > 5)
+    throw new int (e);
 }
 
 void
@@ -32,28 +33,79 @@ throw_exception (int e)
 }
 
 int
-main (void)
+do_test (int e)
 {
-  int i;
+  int i = 0;
   try
     {
-      throw_exception_1 (10);
+      throw_exception_1 (e);
+
+      i += 1;
     }
   catch (const int *e)
     {
         std::cerr << "Exception #" << *e << std::endl;
     }
-  i += 1; /* Break after exception 1.  */
+  i += 1;
 
   try
     {
-      throw_exception (10);
+      throw_exception (e);
+
+      i += 1;
     }
   catch (const int *e)
     {
         std::cerr << "Exception #" << *e << std::endl;
     }
-  i += 1; /* Break after exception 2.  */
+  i += 1;
+
+  return i;
+}
+
+int
+call_do_test (int e)
+{
+  int i;
+
+  i = do_test (e);
+
+  throw_exception_1 (e);
+
+  return i;
+}
+
+int
+do_nested_test (int e)
+{
+  int i = 0;
+
+  try
+    {
+      call_do_test (e);
+
+      i += 1;
+    }
+  catch (const int *e)
+    {
+      std::cerr << "Exception #" << *e << std::endl;
+    }
+  i += 1;
 
   return i;
 }
+
+
+int
+main (void)
+{
+  int i = 0;
+
+  i += do_test (10);
+
+  i += do_test (4);
+
+  i += do_nested_test (10);
+
+  return i; /* Additional breakpoint */
+}
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
index 3b08ef8..f1403d9 100644
--- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
@@ -37,7 +37,7 @@ if ![runto_main] then {
 # Check FinishBreakpoints against C++ exceptions
 #
 
-gdb_breakpoint [gdb_get_line_number "Break after exception 2"]
+gdb_breakpoint [gdb_get_line_number "Additional breakpoint"]
 
 gdb_test "source $pyfile" ".*Python script imported.*" \
          "import python scripts"
@@ -47,9 +47,43 @@ gdb_test "continue" "Breakpoint .*throw_exception_1.*" "run to exception 1"
 
 gdb_test "python print len(gdb.breakpoints())" "3" "check BP count"
 gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
-gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "check FinishBreakpoint in catch()"
+gdb_test "continue" ".*do_test \\(e=10\\).*catch \\(const int \\*e\\).*exception did not finish.*" "FinishBreakpoint with exception thrown caught in parent"
 gdb_test "python print len(gdb.breakpoints())" "3" "check finish BP removal"
 
 gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second exception"
 gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
-gdb_test "continue" ".*exception did not finish.*" "FinishBreakpoint with exception thrown not caught"
+gdb_test "continue" ".*do_test \\(e=10\\).*catch \\(const int \\*e\\).*exception did not finish.*" "FinishBreakpoint with exception thrown caught in grandparent"
+gdb_test "python print len(gdb.breakpoints())" "3" "check finish BP removal"
+
+gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to first no throw test"
+gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
+gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "FinishBreakpoint with exception not thrown"
+gdb_test "python print len(gdb.breakpoints())" "3" "check finish BP removal"
+
+gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second no throw test"
+gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
+gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "FinishBreakpoint with exception not thrown"
+gdb_test "python print len(gdb.breakpoints())" "3" "check finish BP removal"
+
+# Now exercies the nested test example, we're creating an
+# ExceptionFinishBreakpoint inside a frame, then going to continue into
+# further child frames before using the "finish" command, finally, we'll
+# continue, and look for the original ExceptionFinishBreakpoint frame to
+# finish.
+
+gdb_breakpoint "call_do_test"
+gdb_test "continue" ".*Breakpoint.* call_do_test.*" "continue to nested test."
+gdb_test "python print len(gdb.breakpoints())" "4" "check BP count before nested test."
+gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
+gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second no throw test"
+gdb_test "finish" ".*do_test \\(e=10\\).*catch \\(const int \\*e\\).*" "finish with exception being thrown, caught in parent"
+
+gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second no throw test"
+gdb_test "finish" ".*do_test \\(e=10\\).*catch \\(const int \\*e\\).*" "finish with exception being thrown, caught in grand-parent"
+
+gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second no throw test"
+
+setup_kfail "BUG-ID" "*-*-*"
+gdb_test "continue" ".*catch \\(const int \\*e\\).*exception did not finish.*" "FinishBreakpoint nested with exception thrown caught in parent"
+
+gdb_test "python print len(gdb.breakpoints())" "4" "check finish BP removal"


^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-11-22 18:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-21 14:58 [PATCH] improve python finish breakpoint for exceptions/longjmp case Andrew Burgess
2012-09-24 10:22 ` Kevin Pouget
2012-10-01 16:33   ` Andrew Burgess
2012-10-01 16:30 ` Andrew Burgess
2012-10-10 21:08   ` Andrew Burgess
2012-10-11  6:46     ` Phil Muldoon
2012-10-11 16:32 ` Jan Kratochvil
2012-10-15 20:40   ` Andrew Burgess
2012-10-17 16:28     ` Jan Kratochvil
2012-10-24 20:48       ` Andrew Burgess
2012-10-25  6:13         ` Jan Kratochvil
2012-10-25 19:23       ` Andrew Burgess
2012-10-30 17:41         ` Jan Kratochvil
2012-11-06 14:24           ` Andrew Burgess
2012-11-22 18:15             ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox