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: [PATCH] improve python finish breakpoint for exceptions/longjmp case.


On Tue, 06 Nov 2012 15:23:49 +0100, Andrew Burgess wrote:
> On 30/10/2012 5:40 PM, Jan Kratochvil wrote:
> > On Thu, 25 Oct 2012 21:23:19 +0200, Andrew Burgess wrote:
> > 
> > Anyway we agree it is not transparent to "finish" anyway but this is more
> > a problem there are no observer-like breakpoints:
> > 
> > ==> finish.c <==
> > void g (void) {}
> > void f (void) {
> >   g ();
> > }
> > int main (void) {
> >   f ();
> >   return 0;
> > }
> > 
> > ==> finish.cmd <==
> > file ./finish
> > start
> > step
> > break g
> > commands
> >  echo hook-g\n
> >  continue
> > end
> > finish
> > 
> > -------------------
> > 
> > hook-g
> > Actual:
> > [Inferior 1 (process 13204) exited normally]
> > (gdb) _
> > Expected:
> > main () at finish.c:7
> > 7	  return 0;
> > (gdb) _
> > 
> 
> So currently the finish command stops whenever gdb stops, even though in
> this case once we stop (at g) we then automatically continue again.

I agree.


> In the example is it that you'd like to consider the "g" breakpoint
> observer-like (just because it contains a continue),

Right, but I see it was only my wish, GDB does not behave that way.


> or are you suggesting that the "finish" command should not auto-terminate
> once gdb stops?

I was suggesting "finish" should continue its "finish" work after the "g"
breakpoint commands were run.  But I see it does not work that way.

Let's forget about this part.


> I can't really see how the above example relates to the issue with my patch, however,

I agree.


> I have a new patch in which I believe the level of risk of code corruption should be less.

The problem is it is not as safe as set_longjmp_breakpoint_for_call_dummy
which I was suggest to hook into.  You are hooked by
	observer_attach_normal_stop (bpfinishpy_handle_stop);
but that does not catch for example a "return" command.


==> 65.c <==
static int h (void) { return 1; }
static int f (void) { return h (); }
static int g (void) { return h (); }
int main (void) {
  f ();
  g ();
  return 0;
}

==> 65.cmd <==
set confirm off
set height 0
set width 0
shell rm -f 65
shell gcc -o 65 65.c -Wall -g
file ./65
start
source 65.py
break h
continue
info break
python finishbp = MyFinishBreakpoint (gdb.newest_frame ())
info break
return
return
bt
continue
info break
python print finishbp.return_value

==> 65.py <==
class MyFinishBreakpoint (gdb.FinishBreakpoint):
        def __init__(self, frame):
               	gdb.FinishBreakpoint.__init__ (self, frame)
               	print "MyFinishBreakpoint init"

       	def stop(self):
                print "MyFinishBreakpoint stop"
                print "return_value is: %d" % int (self.return_value)
                return True

        def out_of_scope(self):
               	print "MyFinishBreakpoint out of scope"

=============

and its output:

Temporary breakpoint 1 at 0x400511: file 65.c, line 5.

Temporary breakpoint 1, main () at 65.c:5
5	  f ();
Breakpoint 2 at 0x4004f0: file 65.c, line 1.

Breakpoint 2, h () at 65.c:1
1	static int h (void) { return 1; }
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x00000000004004f0 in h at 65.c:1
	breakpoint already hit 1 time
Temporary breakpoint 3 at 0x400500: file 65.c, line 2.
MyFinishBreakpoint init
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x00000000004004f0 in h at 65.c:1
	breakpoint already hit 1 time
3       breakpoint     del  y   0x0000000000400500 in f at 65.c:2 thread 1
	stop only in thread 1
#0  main () at 65.c:6

Breakpoint 2, h () at 65.c:1
1	static int h (void) { return 1; }
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x00000000004004f0 in h at 65.c:1
	breakpoint already hit 2 times
3       breakpoint     del  y   0x0000000000400500 in f at 65.c:2 thread 1
	stop only in thread 1
None
(gdb) _


You can see in the end we already exited f() and entered it again but the
finish breakpoint still exists, out-of-scope detection did not catch it.

Unfortunately set_longjmp_breakpoint_for_call_dummy is a bit dummy frame
specific so it would need some extensions to be suitable for this purpose.

I believe it would be good to hook also into frame_pop().


> As far as I can see the only remaining issue is that using other commands
> that set/clear the longjmp breakpoints once a FinishBreakpoint is active
> (finish/step/etc) will remove the longjmp breakpoint, we'll then fallback to
> only calling the out-of-scope callback at the next stop.  To solve this I'd
> like to change the longjmp breakpoint mechanism to support having multiple
> longjmp breakpoint sets active at once;

You would need a new type besides bp_longjmp and bp_exception.  This is why
I created that bp_longjmp_call_dummy which I was suggesting to extend also for
your purposes.  But I am OK with new bp_longjmp_py_finish_bpt or something.

I do not like much that you share tp->initiating_frame but I haven't found
a countercase reproducer.


> but I'd rather do this in a follow-up patch, and as the new behaviour is no
> worse than the original behaviour this feels like a reasonable compromise to
> me (but you might disagree).

I am OK with that, in fact I leave further Python API extensions on the other
maintainers.


[...]
> @@ -161,7 +168,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    PyObject *frame_obj = NULL;
>    int thread;
>    struct frame_info *frame, *prev_frame = NULL;
> -  struct frame_id frame_id;
> +  struct frame_id prev_frame_id, init_frame_id;
>    PyObject *internal = NULL;
>    int internal_bp = 0;
>    CORE_ADDR finish_pc, pc;
> @@ -184,6 +191,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>  
>    if (frame == NULL)
>      goto invalid_frame;
> +
> +  init_frame_id = get_frame_id (frame);
>    
>    TRY_CATCH (except, RETURN_MASK_ALL)
>      {
> @@ -201,8 +210,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>          }
>        else
>          {
> -          frame_id = get_frame_id (prev_frame);
> -          if (frame_id_eq (frame_id, null_frame_id))
> +          prev_frame_id = get_frame_id (prev_frame);
> +          if (frame_id_eq (prev_frame_id, null_frame_id))
>              PyErr_SetString (PyExc_ValueError,
>                               _("Invalid ID for the `frame' object."));
>          }
> @@ -295,11 +304,18 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>                           AUTO_BOOLEAN_TRUE,
>                           &bkpt_breakpoint_ops,
>                           0, 1, internal_bp, 0);
> +      set_longjmp_breakpoint (inferior_thread (), init_frame_id);

Here should be prev_frame_id instead.  I find it maybe a bit confusing but it
is so, check 'case 2' in infrun.c 'case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME'.
finish_command puts there the previous frame of where you execute the "finish"
command.

Countercase:

==> 64.c <==
#include <setjmp.h>
static jmp_buf env;
static int h (void) {
  if (setjmp (env))
    return 1;
  longjmp (env, 1);
  return 2;
}
int main (void) { return h (); }

==> 64.cmd <==
set confirm off
set height 0
set width 0
shell rm -f 64
shell gcc -o 64 64.c -Wall -g
file ./64
start
source 64.py
break h
continue
info break
python finishbp = MyFinishBreakpoint (gdb.newest_frame ())
info break
continue
python print finishbp.return_value

==> 64.py <==
class MyFinishBreakpoint (gdb.FinishBreakpoint):
        def __init__(self, frame):
               	gdb.FinishBreakpoint.__init__ (self, frame)
               	print "MyFinishBreakpoint init"

       	def stop(self):
                print "MyFinishBreakpoint stop"
                print "return_value is: %d" % int (self.return_value)
                return True

        def out_of_scope(self):
               	print "MyFinishBreakpoint out of scope"

=============

and its output:

Temporary breakpoint 1 at 0x4005aa: file 64.c, line 9.

Temporary breakpoint 1, main () at 64.c:9
9	int main (void) { return h (); }
Breakpoint 2 at 0x400580: file 64.c, line 4.

Breakpoint 2, h () at 64.c:4
4	  if (setjmp (env))
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x0000000000400580 in h at 64.c:4
	breakpoint already hit 1 time
Temporary breakpoint 3 at 0x4005af: file 64.c, line 9.
MyFinishBreakpoint init
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x0000000000400580 in h at 64.c:4
	breakpoint already hit 1 time
3       breakpoint     del  y   0x00000000004005af in main at 64.c:9 thread 1
	stop only in thread 1
0x000000000040058a in h () at 64.c:4
4	  if (setjmp (env))
None
(gdb) _

Existence of the Python FinishBreakpoint causes a stop during longjmp inside h
itself but that should not be caught, the function still has not returned.


> +
> +      /* Set frame to NULL for sanity, creating the breakpoint could cause
> +	 us to switch threads, thus blowing away the frame cache, rendering
> +	 the frame pointer invalid.  */
> +      frame = NULL;
>      }
>    GDB_PY_SET_HANDLE_EXCEPTION (except);
>    
> -  self_bpfinish->py_bp.bp->frame_id = frame_id;
> +  self_bpfinish->py_bp.bp->frame_id = prev_frame_id;
>    self_bpfinish->py_bp.is_finish_bp = 1;
> +  self_bpfinish->initiating_frame = init_frame_id;
>    
>    /* Bind the breakpoint with the current program space.  */
>    self_bpfinish->py_bp.bp->pspace = current_program_space;


Thanks,
Jan


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