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: [RFC] Python Finish Breakpoints


> Kevin> breakpoint.h:
> Kevin> enum py_bp_type
> Kevin> ? {
> Kevin> ? ? py_bp_none, ? ? ? ? /* No Python object. ?*/
>
> I don't think this one is needed.
>
> Kevin> ? ? py_bp_standard, ? ? /* Ordinary breakpoint object. ?*/
> Kevin> ? ? py_bp_finish ? ? ? ?/* FinishBreakpoint object. ?*/
>
> These should be uppercase, but it seems to me that if there are just 2
> states you might as well use an ordinary boolean(-ish) flag.

OK, I wanted to let a room free for further Python-specific breakpoint
handling, but if you feel like it's not necessary ...
I changed it to "int is_py_finish_bp"

> Kevin> as per your two comments, I now only store the `struct type' ?of the
> Kevin> function and the return value,
>
> You need to store a gdb.Type wrapper.
> A 'struct type' can also be invalidated when an objfile is destroyed.

I wouldn't mind, but I can't see how gdb.Type ensures validity, as far
as I've seen, there is no "is_valid" method and I can't and no further
verification during the Python -> C translation:

struct type *
type_object_to_type (PyObject *obj)
{
  if (! PyObject_TypeCheck (obj, &type_object_type))
    return NULL;
  return ((type_object *) obj)->type;
}


> Kevin> diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint-cc.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint-cc.exp
>
> Tom> Funny file name.
>
> Kevin> funny but correct, or too funny? ;)
>
> It is more usual in the gdb test suite to give the .cc and .exp files
> the same base name.

so py-finish-breakpoint2.{exp,cc,py} should look more serious!

>> Other cases to consider are -
>> * inferior exec
>> * inferior exit
>> * explicit inferior function call
>> * "return" command
> Kevin> I'll work on the tests for the next version of the patch ("return"
> Kevin> should already be covered)
>
> I will wait for this to do more review.

these situations should now be covered in `'py-finish-breakpoint.exp"

> Kevin> @defivar FinishBreakpoint out_of_scope_notif
> Kevin> This attribute will be @code{True} until the @code{out_of_scope} method has
> Kevin> been called and @code{False} afterwards. This attribute is writeable, so out
> Kevin> of scope notifications can be re-enabled.
> Kevin> @end defivar
>
> I still don't really understand under what circumstances it is useful
> for a program to set this attribute.
>
> Kevin> - avoid calling `out_of_scope' every normal_stop when the breakpoint
> Kevin> is not anymore in the callstack
>
> I think it would be ok to just leave this up to the subclass to handle.

I thought about just getting rid of this part of the patch, but it
really seem important to me.
Here is a (pseudo-code) example to try to demonstrate that:

function inner (param) {
  if (param)
    return 1
  else
    throw exception
}

function outter (param) {
  return innter(param)
}

function main () {
  try:
    outter (TRUE)
  except:
    pass

  try:
    outter (FALSE)
  except:
    pass

  try:
    outter (TRUE)
  except:
    pass
}

in a Python script, you want to know the return value of OUTTER (in a
script --> you don't know the actual shape of the Main function -- for
instance think about the 'write(2)` function which returns the size of
the data actually written)

you'll need one gdb.Breakpoint and several gdb.FinishBreakpoint:

class OutterBreakpoint(gdb.Breakpoint):
  def __init__(self):
    gdb.Breakpoint.__init__(self, "outter", internal=True)
    self.silent = True

  def stop():
    OutterFinishBreakpoint(gdb.newest_frame())
    return False

class OutterFinishBreakpoint(gdb.FinishBreakpoint):
  def stop():
    print "outter finished with :", self.return_value
    self.enable = False
    gdb.post_event(self.delete)

  def out_of_scope()
    self.enable = False
    gdb.post_event(self.delete)

This `out_of_scope' function is the easiest way to cleanup the
FinishBreakpoint when the finished function didn't return correctly
(and is only useful in this situation, I admit)

> Kevin> - allow the script to re-activate notification when it wants to
> Kevin> 're-use' the FinishBreakpoint (instead of deleting it / creating a new
> Kevin> one)
>
> I am not sure when this makes sense.

that's the opposite situation, you know know the function will only be
triggered from one (limitted set of) place, so you don't want to
create/delete BPs each time:


class OutterBreakpoint(gdb.Breakpoint):
  ...
  def stop():
    if self.finish is None:
        self.finish = OutterFinishBreakpoint(gdb.newest_frame())
    return False

class OutterFinishBreakpoint(gdb.FinishBreakpoint):
  ...
  def stop():
     #don't delete
     #treat normal termination

  def out_of_scope():
     #treat exception termination
     self.out_of_scope_notif = True # allows to catch further
exception termination


I also added a verification which checks that `out_of_scope'  is only
trigged for the inferior for the inferior which owns the breakpoint:

+bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
...
+      else if (finish_bp->out_of_scope_notif
+          && b->pspace == current_inferior()->pspace
+          && (!target_has_registers || frame_find_by_id(b->frame_id) == NULL))
+        {
+          bpfinishpy_out_of_scope (finish_bp);
+        }


thanks,

Kevin

Attachment: finish_bp2.txt
Description: Text document


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