Re: [PATCH] improve python finish breakpoint for exceptions/longjmp case.

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Fri, 21 Sep 2012 16:57:30 +0200, Andrew Burgess wrote:
> @@ -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);

I believe here should be bp_obj->bp->thread as during this function
inferior_thread () may be different, I do not see a temporary inferior switch
here.  But this is a problem already with existing Python Finish Breakpoints:

In the following reproducer breakpoint 3 is correctly thread-specific ("thread
1") but as gdbpy_should_stop does not check thread number it thinks the
breakpoint 3 applies even to thread 2 but frame_id is not valid there so the
breakpoints get deleted.  It sure should not.

FYI I did not review original python/py-finishbreakpoint.c but I do not think
the whole finish_command logic should have been duplicated.  But the original
review was very long which I skipped as a Python one so I may miss something.


Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x00000000004006a3 in g at 5.c:9
	breakpoint already hit 1 time
3       breakpoint     del  y   0x00000000004006e2 in f at 5.c:18 thread 1
	stop only in thread 1
  Id   Target Id         Frame 
  2    Thread 0x7ffff7807700 (LWP 31034) "5" 0x0000000000400706 in start (arg=0x0) at 5.c:23
* 1    Thread 0x7ffff7fe5740 (LWP 31028) "5" g (n=0) at 5.c:9
[Switching to Thread 0x7ffff7807700 (LWP 31034)]

Breakpoint 2, g (n=1) at 5.c:9
9	  var[n] = 1;
MyFinishBreakpoint out of scope
None
(gdb) info breakpoints 
Num     Type           Disp Enb Address            What
2       breakpoint     keep y   0x00000000004006a3 in g at 5.c:9
	breakpoint already hit 2 times
(gdb) 
------------------------------------------------------------------------------
#0  delete_breakpoint (bpt=0x23ee430) at breakpoint.c:13487
#1  in bpfinishpy_out_of_scope (bpfinish_obj=0x21ee130) at ./python/py-finishbreakpoint.c:332
#2  in bpfinishpy_detect_out_scope_cb (b=0x23ee430, args=0x224b550) at ./python/py-finishbreakpoint.c:361
#3  in iterate_over_breakpoints (callback=0x6533d2 <bpfinishpy_detect_out_scope_cb>, data=0x224b550) at breakpoint.c:15639
#4  in bpfinishpy_handle_stop (bs=0x22b6160, print_frame=1) at ./python/py-finishbreakpoint.c:383
#5  in observer_normal_stop_notification_stub (data=0x653518 <bpfinishpy_handle_stop>, args_data=0x7fffffffd3f0) at observer.inc:36
#6  in generic_observer_notify (subject=0x214e130, args=0x7fffffffd3f0) at observer.c:167
#7  in observer_notify_normal_stop (bs=0x22b6160, print_frame=1) at observer.inc:61
#8  in normal_stop () at infrun.c:6135
------------------------------------------------------------------------------
set height 0
set width 0
file ./5
start
set confirm off
source cmd.py
break g
continue
python finishbp = MyFinishBreakpoint (gdb.newest_frame ())
info break
info thread
continue
python print finishbp.return_value
------------------------------------------------------------------------------
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)
                gdb.execute ("where 1")
                gdb.execute ("into thread")
                return True

        def out_of_scope(self):
                print "MyFinishBreakpoint out of scope"
------------------------------------------------------------------------------
#include <pthread.h>
#include <assert.h>
#include <unistd.h>

static volatile int var[2];

void g (int n)
{
  var[n] = 1;
  while (!var[!n]);
}

int v;

void f (int n)
{
  g (n);
  v++;
}

static void *start (void *arg)
{
  while (!var[0]);
  f (1);
  return arg;
}

int main (void)
{
  pthread_t thread1;
  int i;

  i = pthread_create (&thread1, NULL, start, NULL);
  assert (i == 0);
  f (0);
  i = pthread_join (thread1, NULL);
  assert (i == 0);

  return 0;
}
------------------------------------------------------------------------------



Besides that on Fedora 16 x86_64 (using -lmcheck if it matters) this testcases
regresses gdb.python/py-finish-breakpoint.exp for me.  I can debug it more if
it is not reproducible for you, I understand the bug may not be in the new
patch:

(gdb) PASS: gdb.python/py-finish-breakpoint.exp: set FinishBP after the exit()
continue^M
Continuing.^M
[Inferior 1 (process 28725) exited normally]^M
SimpleFinishBreakpoint out of scope^M
thread.c:72: internal-error: inferior_thread: Assertion `tp' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
FAIL: gdb.python/py-finish-breakpoint.exp: catch out of scope after exit (GDB internal error)

Besides that, not sure if it is caused by it or not:

Valgrind output:
==28706== Invalid read of size 1
==28706==    at 0x4C2B0B2: strlen (mc_replace_strmem.c:399)
==28706==    by 0x5E4E440: PyString_FromFormatV (stringobject.c:241)
==28706==    by 0x5E9F62E: PyErr_Format (errors.c:550)
==28706==    by 0x660904: gdbpy_convert_exception (py-utils.c:292)
==28706==    by 0x6535FA: bpfinishpy_detect_out_scope_cb (py-finishbreakpoint.c:377)
==28706==    by 0x6AEB20: iterate_over_breakpoints (breakpoint.c:15639)
==28706==    by 0x6536B1: bpfinishpy_handle_exit (py-finishbreakpoint.c:410)
==28706==    by 0x76B33B: observer_inferior_exit_notification_stub (observer.inc:962)
==28706==    by 0x769E39: generic_observer_notify (observer.c:167)
==28706==    by 0x76B3CD: observer_notify_inferior_exit (observer.inc:987)
==28706==    by 0x84E7AD: exit_inferior_1 (inferior.c:260)
==28706==    by 0x84E840: exit_inferior (inferior.c:279)
==28706==    by 0x758F74: generic_mourn_inferior (target.c:3645)
==28706==    by 0x5DE3BF: inf_ptrace_mourn_inferior (inf-ptrace.c:181)
==28706==    by 0x5EB78C: linux_nat_mourn_inferior (linux-nat.c:4105)
==28706==    by 0x757ADF: target_mourn_inferior (target.c:2804)
==28706==    by 0x713440: handle_inferior_event (infrun.c:3392)
==28706==    by 0x711DA3: wait_for_inferior (infrun.c:2704)
==28706==    by 0x71105D: proceed (infrun.c:2285)
==28706==    by 0x7092C9: continue_1 (infcmd.c:736)
==28706==    by 0x709540: continue_command (infcmd.c:828)
==28706==    by 0x624DA1: do_cfunc (cli-decode.c:114)
==28706==    by 0x627E39: cmd_func (cli-decode.c:1846)
==28706==    by 0x82541C: execute_command (top.c:486)
==28706==    by 0x732E97: command_handler (event-top.c:429)
==28706==    by 0x733482: command_line_handler (event-top.c:630)
==28706==    by 0x87AEB1: rl_callback_read_char (callback.c:220)
==28706==    by 0x7329C8: rl_callback_read_char_wrapper (event-top.c:163)
==28706==    by 0x732DAE: stdin_event_handler (event-top.c:369)
==28706==    by 0x731930: handle_file_event (event-loop.c:827)
==28706==    by 0x730DC5: process_event (event-loop.c:401)
==28706==    by 0x730E71: gdb_do_one_event (event-loop.c:453)
==28706==    by 0x730EE7: start_event_loop (event-loop.c:490)
==28706==    by 0x7329F2: cli_command_loop (event-top.c:176)
==28706==    by 0x7298BE: current_interp_command_loop (interps.c:332)
==28706==    by 0x72A210: captured_command_loop (main.c:226)
==28706==    by 0x7285D9: catch_errors (exceptions.c:546)
==28706==    by 0x72B6BE: captured_main (main.c:999)
==28706==    by 0x7285D9: catch_errors (exceptions.c:546)
==28706==    by 0x72B70A: gdb_main (main.c:1008)
==28706==    by 0x48B945: main (gdb.c:34)
==28706==    by 0x48B945: main (gdb.c:34)
==28706==  Address 0x0 is not stack'd, malloc'd or (recently) free'd



Thanks,
Jan