On Mon, Oct 24, 2011 at 6:06 PM, Phil Muldoon wrote: > Kevin Pouget writes: > > I have some comments regarding the Python bits. Thanks for that, I replied inline >> >> +  ** A new class "gdb.FinishBreakpoint" is provided to catch the return >> +     of a function.  This class is based on the "finish" command >> +     available in the CLI. >> + >>    ** Type objects for struct and union types now allow access to >>       the fields using standard Python dictionary (mapping) methods. >>       For example, "some_type['myfield']" now works, as does >> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c >> index ba1b08f..ffd2ef6 100644 >> --- a/gdb/breakpoint.c >> +++ b/gdb/breakpoint.c >> @@ -5700,6 +5700,7 @@ init_raw_breakpoint_without_location (struct breakpoint *b, >>    b->frame_id = null_frame_id; >>    b->condition_not_parsed = 0; >>    b->py_bp_object = NULL; >> +  b->is_py_finish_bp = 0; > > Is there any reason why this need to be in the breakpoint struct? I > think this should be encapsulated in breakpoint PyObject, and a accessor > method provided for it.  As a finish breakpoint can only ever be > instantiated by Python, there will always be a py_bp_object attached to > make the call. just to put it back in context (in was back in May ...), here is the rational behind the flag: On Thu, May 19, 2011 at 6:20 PM, Tom Tromey wrote: > gdbpy_is_stopped_at_finish_bp is not safe to call here -- it assumes the > GIL has been acquired, which it has not. I would rather it not be changed > to acquire the GIL, however. I think one of two other approaches would > be preferable. > > One way you could handle this is to add a new constant to enum bptype. > This is likely to be pretty invasive. > > Another way would be to add a flag to the struct breakpoint itself. > > Yet another way would be a new breakpoint_ops method. --> PyObject_TypeCheck (obj, &finish_breakpoint_object_type) is not safe to call everywhere I've refactored the code according to your comment anyway, it make sense, so now there are two version: bpfinishpy_is_finish_bp (PyObject *obj) --> strong one, used in "gdbpy_breakpoint_created" bpfinishpy_bp_is_finish_bp (struct breakpoint_object *bp_obj) --> 'weak' one, used in 'gdbpy_is_stopped_at_finish_bp', does't require the GIL >> +/* struct implementing the gdb.FinishBreakpoint object by extending >> +   the gdb.Breakpoint class.  */ >> +struct finish_breakpoint_object >> +{ >> +  /* gdb.Breakpoint base class.  */ >> +  struct breakpoint_object py_bp; >> +  /* gdb.Type object of the function finished by this breakpoint.  */ >> +  PyObject *function_type; >> +  /* gdb.Type object of the value return by the breakpointed function.  */ >> +  PyObject *return_type; >> +  /* When stopped at this FinishBreakpoint, value returned by the function; >> +     Py_None if the value is not computable; >> +     NULL if GDB is not stopped at a FinishBreakpoint.  */ >> +  PyObject *return_value; >> +}; > > I think these comments should wrap? They wrap for me in emacs. I'm not sure about the exact meaning of 'wrap' here, but I assume it's about the new line between computable and NULL; I've reformatted it. Thanks for your comments, Kevin 2011-10-25 Kevin Pouget Introduce gdb.FinishBreakpoints in Python * Makefile.in (SUBDIR_PYTHON_OBS): Add py-finishbreakpoint.o. (SUBDIR_PYTHON_SRCS): Add python/py-finishbreakpoint.c. Add build rule for this file. * infcmd.c (get_return_value): New function. (print_return_value): Split to create get_return_value. * inferior.h (get_return_value): New prototype. * infrun.c: Include python/python.h. (stop_registers): Mention FinishBreakpoint in description. (normal_stop): Set stop_registers if stopped at FinishBreakpoint. * python/py-breakpoint.c (breakpoint_object_type): Make non-static. (bppy_pending_object): Likewise (gdbpy_should_stop): Disable temporary breakpoints. (gdbpy_breakpoint_created): Set is_py_finish_bp is necessary. (struct breakpoint_object): Move to python-internal.h (BPPY_REQUIRE_VALID): Likewise. (BPPY_SET_REQUIRE_VALID): Likewise. (gdbpy_breakpoint_created): Initialize is_finish_bp. * python/python-internal.h (breakpoint_object_type): Add as extern. (bppy_pending_object): Likewise. (typedef struct breakpoint_object) Removed. (struct breakpoint_object): Moved from py-breakpoint.c. Add field is_finish_bp. (BPPY_REQUIRE_VALID): Likewise. (BPPY_SET_REQUIRE_VALID): Likewise. (frame_object_to_frame_info): New prototype. (gdbpy_initialize_finishbreakpoints): New prototype. (bpfinishpy_bp_is_finish_bp): Likewise. (bpfinishpy_is_finish_bp): Likewise. * python/py-finishbreakpoint.c: New file. * python/py-frame.c(frame_object_to_frame_info): Make non-static and accept PyObject instead of frame_object. (frapy_is_valid): Don't cast to frame_object. (frapy_name): Likewise. (frapy_type): Likewise. (frapy_unwind_stop_reason): Likewise. (frapy_pc): Likewise. (frapy_block): Likewise. (frapy_function): Likewise. (frapy_older): Likewise. (frapy_newer): Likewise. (frapy_find_sal): Likewise. (frapy_read_var): Likewise. (frapy_select): Likewise. * python/python.c (gdbpy_is_stopped_at_finish_bp): New noop function. (_initialize_python): Add gdbpy_initialize_finishbreakpoints. * python/python.h: Include breakpoint.h (gdbpy_is_stopped_at_finish_bp): New prototype. doc/ * gdb.texinfo (Breakpoints In Python): New subsection: Finish Breakpoints. (Python API): Add menu entry for Finish Breakpoints testsuite/ * gdb.python/py-breakpoint.exp (mult_line): Define and use variable instead of line number. * gdb.python/py-finish-breakpoint.c: New file. * gdb.python/py-finish-breakpoint.exp: New file. * gdb.python/py-finish-breakpoint.py: New file. * gdb.python/py-finish-breakpoint2.cc: New file. * gdb.python/py-finish-breakpoint2.exp: New file. * gdb.python/py-finish-breakpoint2.py: New file.