Re: [PATCH] Add bp_location to Python interface

Kevin Pouget
2011-12-13T15:29:31Z

[-- Attachment #1: Type: text/plain, Size: 15657 bytes --]

On Fri, Dec 9, 2011 at 3:10 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Kevin Pouget <kevin.pouget@gmail.com> writes:
>
>> On Thu, Dec 8, 2011 at 3:28 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>>
>
>>>> +A breakpoint location is represented with a @code{gdb.BpLocation} object,
>>>> +which offers the following attributes (all read only) and methods.
>>>> +Please note that breakpoint locations are very transient entities in
>>>> +@value{GDBN}, so one should avoid keeping references to them.
>>
>>>
>>> While it is good to note that, I'm not sure  what we are explaining here
>>> other than when the breakpoint is deleted, all location objects that are
>>> associated with that object are invalid too.  Or, are you noting we
>>> should allow the user to interact after the fact? If that is the case,
>>> what is "is_valid" for?  Why note the transient nature of the object?
>>
>>> I'm not sure what we are explaining here other than when the breakpoint is deleted
>>
>> that's what I wanted to emphasize here, bp_locations are cleaned/reset
>> more often than 'high-level' breakpoints are removed. My knowledge
>> about that is quite limited, but for instance this (cleaned up) stack:
>>
>> #0  gdbpy_bplocation_free py-bploc.c:60
>> #1  free_bp_location (loc=) breakpoint.c:5685
>> #2  decref_bp_location (blp=) breakpoint.c:5707
>> #3  update_global_location_list (should_insert=1)  breakpoint.c:10575
>> #4  update_breakpoint_locations (b=, sals=...,     sals_end=...)
>> breakpoint.c:11787
>> #5  breakpoint_re_set_default (b=) breakpoint.c:11937
>> #6  breakpoint_re_set_one (bint=) breakpoint.c:11968
>> #8  breakpoint_re_set () breakpoint.c:11992
>> #9  solib_add (pattern=0x0, from_tty=0, target=,  readsyms=1) solib.c:926
>> #10 bpstat_what (bs_head=) breakpoint.c:4487
>> #11 handle_inferior_event (ecs=) infrun.c:4394
>> #12 wait_for_inferior ()
>>
>> shows that bplocations might be removed when a shared library is loaded
>
> Good point, I did not think of this scenario.  Looking at several
> sections of GDB code, update_global_location_list can just be called
> whenever.
>
>>> If the breakpoint is deleted and the user still has reference to a
>>> location object, I think we should just run a validation routine and
>>> refuse to do anything but raise an exception at that point (like
>>> is_valid, but triggered for all function/attribute calls).
>>
>> hum, I think that's what I already do in the code :)
>>
>> I've updated the gdb.BpLocation.is_valid documentation, maybe it's clearer?
>
> Yeah, I saw it later.  Forgot to delete that comment!
>
>>> @defun BpLocation.is_valid ()
>>> Returns @code{True} if the @code{gdb.BpLocation} object is valid,
>>> @code{False} if not.  A @code{gdb.BpLocation} object may be invalidated by
>>> GDB at any moment for internal reasons. All other @code{gdb.BpLocation} methods
>>> and attributes will throw an exception if the object is invalid.
>>> @end defun
>>
>>>> +
>>>> +@defvar BpLocation.owner
>>>> +This attribute holds a reference to the @code{gdb.Breakpoint} object which
>>>> +owns this location.
>>>> +@end defvar
>>
>>> Note which attributes are read only/writable.  This and others.
>> I specify a few lines above that all the attributes are read only, but
>> I've noted it on each attribute as well.
>
> Right, but there is no guarantee that the user will read the entirety
> of that section's documentation.  So we always put attribute access rights
> in the attribute section.

ok, I thought that as soon as it was clearly mentioned somewhere, that
was enough. But it doesn't cost anything to me to write it for each
attribute, so no problem!

>>> Also many different breakpoints can be at one location.  I'm not sure if it
>>> is worth pointing out here that "this breakpoint" only owns the
>>> location insofar as the scope of that breakpoint (there could be other
>>> breakpoints set to that location).
>>
>> from an implementation point of view, the is a BP  <1--------n>
>> BpLocation relation, even if two locations have the same address.
>>
>> Also, I think that the end-users won't have problems to understand
>> these BpLocations, as they're already exposed with "info breakpoints":
>
> I was purely talking from a scripting point of view, but ok.  My view
> is, if the user has to run experiments to collect data on the actual
> behavior, then our documentation needs to be a little better.

yes, no doubt about that!

> You could just put in that "from an implementation point of view" paragraph in
> the documentation somewhere?

I've added it to 'BpLocation.owner' description:

@defvar BpLocation.owner
This attribute holds a reference to the @code{gdb.Breakpoint} object which
owns this location. This attribute is not writable. From an implementation
point of view, there is a @code{1 ... n} relation between a breakpoint and
its locations, even if two breakpoints are set on at same address.

>>>> +struct bploc_object
>>>> +{
>>>> +  PyObject_HEAD
>>>> +
>>>> +  /* The location corresponding to this py object.  NULL is the location
>>>> +     has been deleted.  */
>>>> +  struct bp_location *loc;
>>>
>>> Typo in the comment "NULL is the location has been deleted.".  Also nit
>>> pick shouldn't it be "The location corresponding to a gdb.Breakpoint
>>> object"?
>>
>> typo fixed, but not the nit: it's the same idea as breakpoint,
>> if the 'backend' breakpoint is deleted, 'struct breakpoint_object . bp' is NULL,
>> if the 'backend' location is deleted, 'struct bploc_object . loc' is
>> NULL
>
> My objection was to "py object", I would just find it clearer to name
> the object.  Also, the explanation above would go a long way for future
> hackers to understand the relationship.  Maybe add that to the code too?

You're right, it looks obvious to me, but it would take time to figure
out by itself.
Here are a few more bits of comments:

/* The location corresponding to this gdb.BpLocation object. It's the same
idea as gdb.Breakpoint, if the 'backend' location is deleted, LOC is
set to NULL. No reference to the location is owned here (in terms of
ref. counting) in order not to change the internal behavior. */
struct bp_location *loc;

>>
>>>> +
>>>> +  /* 1 if the owner BP has been deleted, 0 otherwise.  */
>>>> +  int invalid_owner;
>>>> +
>>>> +  /* Cache for the gdb.Value object corresponding to loc->address.  */
>>>> +  PyObject *py_address;
>>>> +};
>>>
>>> I'm not sure if breakpoint locations can change.  I do not think so, but
>>> why do we need to cache loc->address?
>>
>> I assumed that it can't change, but didn't actually investigate all
>> the implementation.
>> My feeling is "caching is easy, creating a Py object has a cost, so
>> let's cache!", but I've got no idea about the actual cost, so I'll
>> change it if someone insists [and convinces me] :)
>
> I don't know enough about breakpoint locations to be able to advise you,
> but what does update_global_location_list do?  Just shuffle add/delete
> locations to the list?  I am not against caching.  Just not sure why we
> need it.  I tend towards the conservative with things like these, if
> only purely because we keep missing reference counts in the existing
> code.

I somehow managed to convinced myself to remove this address caching,
1/I'm not sure about the actual py object creation, certainly quite low,
2/BpLocation objects have a short lifespan, so the cached addresses
would be discarded quite frequently
so all in all, I can't see anymore any relevant reason to cache it :)

>>>> +
>>>> +/* Require that LOCATION be a valid bp_location; throw a Python
>>>> +   exception if it is invalid.  */
>>>> +#define BPLOCPY_REQUIRE_VALID(Location)                                 \
>>>> +    do {                                                                \
>>>> +      if ((Location)->loc == NULL)                                      \
>>>> +        return PyErr_Format (PyExc_RuntimeError,                        \
>>>> +                             _("BpLocation invalid."));                 \
>>>> +    } while (0)
>>>
>>> I prefer error messages a little more descriptive.  That is just a
>>> personal thing of mine, but "BpLocation invalid" would seem cryptic when
>>> thrown in the context of a script.
>>
>> I'm not sure how I could improve it, at this point (checking validity
>> at the beginning of all the attributes), we just know that the backend
>> location has been freed, but we don't know why.
>
> "BpLocation object, breakpoint location is NULL". Or something like that.

I'm still not sure, I think that when I read "BpLocation invalid", I'd
take a look at the 'is_valid' method description, and understand
that "A @code{gdb.BpLocation} object may be invalidated by GDB at any
moment for internal reasons."
'breakpoint location is NULL' sounds like an internal error to me ...

Maybe "BpLocation invalid: internal location object freed up/removed by GDB." ?

>>>> +PyObject *
>>>> +bplocation_to_bplocation_object (struct bp_location *loc)
>>>> +{
>>>> +  bploc_object *bploc_obj;
>>>> +
>>>> +  gdb_assert (loc != NULL);
>>>
>>> Is this a fatal error that we need to shutdown GDB for?  gdb_assert
>>> seems pretty strong from an API point of view.
>>
>> hum, I'm not sure here,
>> I wrote this line to actually shutdown GDB instead of segfaulting on
>> the next line,
>> for instance 'py-pspace.c::pspace_to_pspace_object' and
>> 'py-objfile.c::objfile_to_objfile_object" will just segfault if
>> there're called with a NULL parameter.
>>
>> throwing an Python/GDB exception wouldn't make much sense to me
>> either, as there end-user won't be able to deal with it
>
> Neither would killing GDB.  I almost never use gdb_assert because of the
> possible negative user implications.  Can you discern that GDB has
> entered an unstable state because a function has received a NULL
> argument for a bp_location?  If you cannot answer "yes" with a high
> degree of certainty, it is my opinion we should raise an exception and
> return NULL.  We can make sure in the Python API that, if this is the
> case, we will return the exception to the script boundary for the script
> to deal with.  From an API point-of-view I think we should defer to
> script authors the opportunity to deal with and decide what to do in
> this scenario.  If GDB is truly unstable, then I agree, assert is
> correct.  I think the other two cases are bugs.

I get your point and quite agree with you, the only thing is that
we're talking about 'gdb_assert' and not 'assert' itself.

> Neither would killing GDB.
and here's my point, 'gdb_assert' does't kill GDB, but raises a
warning message asking the user if he wants to quit or continue.
My feeling about *assert is that it's a development/beta-test
features, which might/should be turned into noop for production.

> From an API point-of-view I think we should defer to script authors the opportunity to deal with
I don't think so, because (from my developer point of view) I see it
as very improbable that this situation would occur, or even impossible
if I were a good programmer!
I don't think that API should cover internal 'buggy' situations,
otherwise they would be endless!

> The Maintainers may differ on this, wait for their opinion.
yes, I'm curious about different point of views

>>>> +bplocpy_get_address (PyObject *self, void *closure)
>>>> +{
>>>> +  bploc_object *self_bploc = (bploc_object *) self;
>>>> +
>>>> +  BPLOCPY_REQUIRE_VALID (self_bploc);
>>>> +
>>>> +  if (!self_bploc->py_address)
>>>> +    {
>>>> +      /* Get the address Value object as a void *.  */
>>>> +      volatile struct gdb_exception except;
>>>> +      struct type *void_ptr_type;
>>>> +      struct value *val = NULL ; /* Initialize to appease gcc warning.  */
>>>> +
>>>> +      TRY_CATCH (except, RETURN_MASK_ALL)
>>>> +        {
>>>> +          void_ptr_type = lookup_pointer_type (
>>>> +              builtin_type (python_gdbarch)->builtin_void);
>>>> +          val = value_from_pointer (void_ptr_type, self_bploc->loc->address);
>>>> +        }
>>>> +      GDB_PY_HANDLE_EXCEPTION (except);
>>>> +
>>>> +      self_bploc->py_address = value_to_value_object (val);
>>>> +      Py_XINCREF (self_bploc->py_address);
>>>> +    }
>>>> +  Py_XINCREF (self_bploc->py_address);
>>>
>>> I don't really mind it, but I prefer explicit return NULL when dealing
>>> with cases of exceptions.  I find the other logic hard to read.  This is
>>> not a request for a change. Is there a case where py_address will be
>>> NULL?  Yes, there is, value_to_value_object can return NULL.  If it
>>> returns NULL, then there is an exception set.  I much prefer to exit
>>> then and there, other the conditional XINCREF step, and returning at the
>>> function exit.  Still, this is just a stylistic thing, and probably
>>> personal thing.  The second XINCREF can just be a plain old INCREF as we
>>> already tested for NULL.
>>
>> makes sense to me, my "single return point" idea doesn't stand against
>> error handling
>
> Note this is purely a personal style issue, there was nothing wrong with
> the correctness of your code.

sure, I try to make it clear when I don't agree with the suggestion;
this one was perfectly fine,
I think that's actually the way I wrote it at the first time :)

>>> This brings me to the address cache argument.  Is it worthwhile managing
>>> the cache increment counts instead of just returning the address each
>>> time?  I ask as I am not sure if you have done any metrics that indicate
>>> this is a slow operation.
>>
>> no, I didn't run any measurement so far; I'll wait for a maintainer
>> point of view about that
>
> Ok.
>
>>> We need to make sure that the this is not a watchpoint.
>>
>> why not?
>> I don't know much about watchpoints, but as per my quick
>> investigations, they share the same high-level structure (struct
>> breakpoint), and they seem to use bp_location the same way ?
>
> I'm not sure as watchpoints are based on expressions.  Maybe it is ok,
> but note that watchpoints are removed and reinserted on scope of the
> expression.  It might be fine.  A few tests would prove the issue.

I'll try to build some test to ensure that it's fine

thanks for your comments and reviews,

Kevin

--

2011-12-13 Kevin Pouget <kevin.pouget@st.com>

Add bp_location to Python interface
* Makefile.in (SUBDIR_PYTHON_OBS): Add py-bploc.o
(SUBDIR_PYTHON_SRCS): Add python/py-bploc.c
Add build rule for this file.
* breakpoint.h (struct bploc_object): Forward declaration.
(struct bp_location): Add py_bploc_obj.
* breakpoint.c (free_bp_location): Call gdbpy_bplocation_free.
* python/py-bploc.c: New file.
* python/py-breakpoint.c (bppy_locations): New function.
(breakpoint_object_methods): New method binding: locations().
* python/python-internal.h (bploc_object): New typedef.
(bplocation_to_bplocation_object): New prototype.
(gdbpy_initialize_bplocation): Likewise.
* python/python.c (gdbpy_bplocation_free): New empty stub.
(_initialize_python): Call gdbpy_initialize_bplocation.
* python/python.h (gdbpy_bplocation_free): New prototype.

doc/
Add bp_location to Python interface
* gdb.texinfo (Breakpoints In Python): Document
gdb.Breakpoint.locations and gdb.BpLocation.

testsuite/
Add bp_location to Python interface
* gdb.python/py-breakpoint.exp: Test gdb.BpLocation.

[-- Attachment #2: 0001-Add-bp_location-to-Python-interface.patch --]
[-- Type: text/x-patch, Size: 22545 bytes --]

From e4f3e7af4686b196ce2d4cbe6d8c7ef00915d352 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Wed, 18 May 2011 10:02:23 -0400
Subject: [PATCH] Add bp_location to Python interface

---
gdb/Makefile.in | 6 +
gdb/NEWS | 8 +
gdb/breakpoint.c | 3 +
gdb/breakpoint.h | 6 +
gdb/doc/gdb.texinfo | 44 ++++
gdb/python/py-bploc.c | 302 ++++++++++++++++++++++++++++
gdb/python/py-breakpoint.c | 39 ++++
gdb/python/python-internal.h | 6 +
gdb/python/python.c | 7 +
gdb/python/python.h | 2 +
gdb/testsuite/gdb.python/py-breakpoint.exp | 52 +++++
11 files changed, 475 insertions(+), 0 deletions(-)
create mode 100644 gdb/python/py-bploc.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b71db33..58b4910 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -280,6 +280,7 @@ SUBDIR_PYTHON_OBS = \
py-auto-load.o \
py-block.o \
py-bpevent.o \
+ py-bploc.o \
py-breakpoint.o \
py-cmd.o \
py-continueevent.o \
@@ -311,6 +312,7 @@ SUBDIR_PYTHON_SRCS = \
python/py-auto-load.c \
python/py-block.c \
python/py-bpevent.c \
+ python/py-bploc.c \
python/py-breakpoint.c \
python/py-cmd.c \
python/py-continueevent.c \
@@ -2081,6 +2083,10 @@ py-bpevent.o: $(srcdir)/python/py-bpevent.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-bpevent.c
$(POSTCOMPILE)

+py-bploc.o: $(srcdir)/python/py-bploc.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-bploc.c
+ $(POSTCOMPILE)
+
py-breakpoint.o: $(srcdir)/python/py-breakpoint.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-breakpoint.c
$(POSTCOMPILE)
diff --git a/gdb/NEWS b/gdb/NEWS
index 42782ce..4344fc9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -1,6 +1,14 @@
What has changed in GDB?
(Organized release by release)

+*** Changes since GDB 7.4
+
+* Python scripting
+
+ ** A new method "gdb.Breakpoint.locations" has been added, as well as
+ the class gdb.BpLocation to provide further details about breakpoint
+ locations.
+
*** Changes since GDB 7.3.1

* GDB now handles ambiguous linespecs more consistently; the existing
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0686587..2f1357a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5681,6 +5681,9 @@ static void
free_bp_location (struct bp_location *loc)
{
loc->ops->dtor (loc);
+
+ gdbpy_bplocation_free (loc);
+
xfree (loc);
}

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ddf1881..5a8a1af 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -28,6 +28,7 @@
struct value;
struct block;
struct breakpoint_object;
+struct bploc_object;
struct get_number_or_range_state;
struct thread_info;
struct bpstats;
@@ -405,6 +406,11 @@ struct bp_location
/* Source file name of this address. */

char *source_file;
+
+ /* Python object associated with this location. May be NULL if the location
+ is not yet exported to Python. */
+
+ struct bploc_object *py_bploc_obj;
};

/* This structure is a collection of function pointers that, if available,
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 50c299e..4665772 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24335,6 +24335,50 @@ commands, separated by newlines. If there are no commands, this
attribute is @code{None}. This attribute is not writable.
@end defvar

+@findex gdb.locations
+@defun gdb.locations ()
+Return a tuple containing a sequence of @code{gdb.BpLocation} objects
+(see below) associated with this breakpoint. A breakpoint with no location
+is a pending breakpoint (@xref{Set Breaks, , pending breakpoints}).
+@end defun
+
+A breakpoint location is represented with a @code{gdb.BpLocation} object,
+which offers the following attributes (all read only) and methods.
+Please note that breakpoint locations are very transient entities in
+@value{GDBN}, so one should avoid keeping references to them.
+
+@defvar BpLocation.owner
+This attribute holds a reference to the @code{gdb.Breakpoint} object which
+owns this location. This attribute is not writable. From an implementation
+point of view, there is a @code{1 ... n} relation between a breakpoint and
+its locations, even if two breakpoints are set on at same address.
+
+@end defvar
+
+@defvar BpLocation.enabled
+This attribute indicates whether this location is currently enabled or not.
+This attribute is not writable.
+@end defvar
+
+@defvar BpLocation.inferior
+This attribute holds a reference to the @code{gdb.Inferior} inferior object
+in which this breakpoint location has been inserted. The value will be
+@code{None} if there is no inferior associated with this location. This
+attribute is not writable.
+@end defvar
+
+@defvar BpLocation.address
+This attribute holds a @code{gdb.Value} object corresponding to the address
+at which the breakpoint has been inserted. This attribute is not writable.
+@end defvar
+
+@defun BpLocation.is_valid ()
+Returns @code{True} if the @code{gdb.BpLocation} object is valid,
+@code{False} if not. A @code{gdb.BpLocation} object may be invalidated by
+GDB at any moment for internal reasons. All other @code{gdb.BpLocation}
+methods and attributes will throw an exception if the object is invalid.
+@end defun
+
@node Lazy Strings In Python
@subsubsection Python representation of lazy strings.

diff --git a/gdb/python/py-bploc.c b/gdb/python/py-bploc.c
new file mode 100644
index 0000000..5ba3948
--- /dev/null
+++ b/gdb/python/py-bploc.c
@@ -0,0 +1,302 @@
+/* Python interface to breakpoint locations.
+
+ Copyright (C) 2011 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+
+#include "defs.h"
+#include "inferior.h"
+#include "python-internal.h"
+#include "observer.h"
+#include "gdbarch.h"
+
+struct bploc_object
+{
+ PyObject_HEAD
+
+ /* The location corresponding to this gdb.BpLocation object. It's the same
+ idea as gdb.Breakpoint, if the 'backend' location is deleted, LOC is
+ set to NULL. No reference to the location is owned here (in terms of
+ ref. counting) in order not to change the internal behavior. */
+ struct bp_location *loc;
+
+ /* 1 if the owner BP has been deleted, 0 otherwise. */
+ int invalid_owner;
+};
+
+/* Require that LOCATION be a valid bp_location; throw a Python
+ exception if it is invalid. */
+#define BPLOCPY_REQUIRE_VALID(Location) \
+ do { \
+ if ((Location)->loc == NULL) \
+ return PyErr_Format (PyExc_RuntimeError, \
+ _("BpLocation invalid.")); \
+ } while (0)
+
+static PyTypeObject bploc_object_type;
+
+/* Call by free_bp_location when loc is about to be freed. */
+
+void
+gdbpy_bplocation_free (struct bp_location *loc)
+{
+ if (loc->py_bploc_obj)
+ {
+ loc->py_bploc_obj->loc = NULL;
+ Py_DECREF (loc->py_bploc_obj);
+ }
+}
+
+/* Dissociate the bp_location from the Python object. */
+
+static void
+bplocpy_dealloc (PyObject *self)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+
+ if (self_bploc->loc != NULL)
+ self_bploc->loc->py_bploc_obj = NULL;
+
+ self->ob_type->tp_free (self);
+}
+
+/* Create or acquire a ref to the bp_location object (gdb.BpLocation)
+ that encapsulates the struct bp_location from GDB. */
+
+PyObject *
+bplocation_to_bplocation_object (struct bp_location *loc)
+{
+ bploc_object *bploc_obj;
+
+ gdb_assert (loc != NULL);
+ if (loc->py_bploc_obj)
+ {
+ Py_INCREF (loc->py_bploc_obj);
+ return (PyObject *) loc->py_bploc_obj;
+ }
+
+ bploc_obj = PyObject_New (bploc_object, &bploc_object_type);
+ if (!bploc_obj)
+ return NULL;
+
+ bploc_obj->loc = loc;
+ bploc_obj->invalid_owner = 0;
+ Py_INCREF (bploc_obj);
+ loc->py_bploc_obj = bploc_obj;
+
+ return (PyObject *) bploc_obj;
+}
+
+/* Python function to get the BP owning this location, if any. */
+
+static PyObject *
+bplocpy_get_owner (PyObject *self, void *closure)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+
+ BPLOCPY_REQUIRE_VALID (self_bploc);
+
+ if (self_bploc->invalid_owner)
+ Py_RETURN_NONE;
+
+ if (self_bploc->loc->owner
+ && self_bploc->loc->owner->py_bp_object)
+ {
+ Py_INCREF (self_bploc->loc->owner->py_bp_object);
+ return (PyObject *) self_bploc->loc->owner->py_bp_object;
+ }
+
+ Py_RETURN_NONE;
+}
+
+/* Python function to test whether or not this breakpoint location is
+ enabled. */
+
+static PyObject *
+bplocpy_get_enabled (PyObject *self, void *closure)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+
+ BPLOCPY_REQUIRE_VALID (self_bploc);
+
+ if (self_bploc->loc->enabled)
+ Py_RETURN_TRUE;
+
+ Py_RETURN_FALSE;
+}
+
+/* Python function to get the address of this breakpoint location. The
+ gdb.Value object will be cached if this is the first access. Returns
+ NULL in case of failure, with a Python exception set. */
+
+static PyObject *
+bplocpy_get_address (PyObject *self, void *closure)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+ volatile struct gdb_exception except;
+ struct type *void_ptr_type;
+ struct value *val = NULL; /* Initialize to appease gcc warning. */
+ PyObject *py_address;
+
+ BPLOCPY_REQUIRE_VALID (self_bploc);
+
+ /* Get the address Value object as a void * value. */
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ void_ptr_type = lookup_pointer_type (
+ builtin_type (python_gdbarch)->builtin_void);
+ val = value_from_pointer (void_ptr_type, self_bploc->loc->address);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
+
+ py_address = value_to_value_object (val);
+ if (!py_address)
+ return NULL;
+
+ return py_address;
+}
+
+/* Python function to get the inferior hosting this breakpoint location.
+ Return Py_None if there is no inferior associated with the program space of
+ this location, or NULL in case of failure, with a python exception set. */
+
+static PyObject *
+bplocpy_get_inferior (PyObject *self, void *closure)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+ struct inferior *inf;
+ PyObject *infobj;
+
+ BPLOCPY_REQUIRE_VALID (self_bploc);
+
+ inf = find_inferior_for_program_space (self_bploc->loc->pspace);
+ if (!inf)
+ Py_RETURN_NONE;
+
+ infobj = inferior_to_inferior_object (inf);
+ Py_XINCREF (infobj);
+
+ return infobj;
+}
+
+/* Python function which checks the validity of a bp location object. */
+
+static PyObject *
+bplocpy_is_valid (PyObject *self, PyObject *args)
+{
+ bploc_object *self_bploc = (bploc_object *) self;
+
+ if (self_bploc->loc)
+ Py_RETURN_TRUE;
+ Py_RETURN_FALSE;
+}
+
+/* Callback triggered when a breakpoint is deleted. This will invalidate
+ the corresponding bp_location Python object owners. */
+
+static void
+bplocpy_breakpoint_deleted (struct breakpoint *b) {
+ struct bp_location *loc;
+
+ for (loc = b->loc; loc; loc = loc->next)
+ {
+ if (loc->py_bploc_obj)
+ loc->py_bploc_obj->invalid_owner = 1;
+ }
+}
+
+/* Initialize the Python bp_location code. */
+
+void
+gdbpy_initialize_bplocation (void)
+{
+ if (PyType_Ready (&bploc_object_type) < 0)
+ return;
+
+ Py_INCREF (&bploc_object_type);
+ if (PyModule_AddObject (gdb_module, "BpLocation",
+ (PyObject *) &bploc_object_type) < 0)
+ return;
+
+ observer_attach_breakpoint_deleted (bplocpy_breakpoint_deleted);
+}
+
+static PyGetSetDef bploc_object_getset[] =
+{
+ { "owner", bplocpy_get_owner, NULL,
+ "Each breakpoint location must belong to exactly one higher-level \
+breakpoint. This pointer is NULL iff this bp_location is no \
+longer attached to a breakpoint (read-only).",
+ NULL },
+ { "enabled", bplocpy_get_enabled, NULL,
+ "Is this particular location enabled.", NULL },
+ { "address", bplocpy_get_address, NULL,
+ "The address at which the breakpoint has been set.", NULL },
+ { "inferior", bplocpy_get_inferior, NULL,
+ "The inferior in which this breakpoint location has been set.", NULL },
+ { NULL } /* Sentinel. */
+};
+
+
+static PyMethodDef bploc_object_methods[] =
+{
+ { "is_valid", bplocpy_is_valid, METH_NOARGS,
+ "Return true if this breakpoint location is valid, false if not." },
+ { NULL } /* Sentinel. */
+};
+
+static PyTypeObject bploc_object_type =
+{
+ PyObject_HEAD_INIT (NULL)
+ 0, /* ob_size */
+ "gdb.BpLocation", /* tp_name */
+ sizeof (bploc_object), /* tp_basicsize */
+ 0, /* tp_itemsize */
+ bplocpy_dealloc, /* tp_dealloc */
+ 0, /* tp_print */
+ 0, /* tp_getattr */
+ 0, /* tp_setattr */
+ 0, /* tp_compare */
+ 0, /* tp_repr */
+ 0, /* tp_as_number */
+ 0, /* tp_as_sequence */
+ 0, /* tp_as_mapping */
+ 0, /* tp_hash */
+ 0, /* tp_call */
+ 0, /* tp_str */
+ 0, /* tp_getattro */
+ 0, /* tp_setattro */
+ 0, /* tp_as_buffer */
+ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
+ "GDB breakpoint location object", /* tp_doc */
+ 0, /* tp_traverse */
+ 0, /* tp_clear */
+ 0, /* tp_richcompare */
+ 0, /* tp_weaklistoffset */
+ 0, /* tp_iter */
+ 0, /* tp_iternext */
+ bploc_object_methods, /* tp_methods */
+ 0, /* tp_members */
+ bploc_object_getset, /* tp_getset */
+ 0, /* tp_base */
+ 0, /* tp_dict */
+ 0, /* tp_descr_get */
+ 0, /* tp_descr_set */
+ 0, /* tp_dictoffset */
+ 0, /* tp_init */
+ 0 /* tp_alloc */
+};
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 11d60fe..cccbf0e 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -620,6 +620,43 @@ bppy_get_ignore_count (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->ignore_count);
}

+
+/* Python function which returns the BpLocation objects associated
+ with this breakpoint. */
+
+static PyObject *
+bppy_locations (PyObject *self, PyObject *args)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+ PyObject *list, *tuple;
+ struct bp_location *loc;
+ int err;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ list = PyList_New (0);
+ if (!list)
+ return NULL;
+
+ err = 0;
+ for (loc = self_bp->bp->loc; loc; loc = loc->next)
+ {
+ PyObject *loc_obj = bplocation_to_bplocation_object (loc);
+ err = PyList_Append (list, loc_obj);
+ if (err == -1)
+ {
+ Py_DECREF (list);
+ return NULL;
+ }
+ Py_DECREF (loc_obj);
+ }
+
+ tuple = PyList_AsTuple (list);
+ Py_DECREF (list);
+
+ return tuple;
+}
+
/* Python function to create a new breakpoint. */
static int
bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
@@ -1003,6 +1040,8 @@ static PyMethodDef breakpoint_object_methods[] =
"Return true if this breakpoint is valid, false if not." },
{ "delete", bppy_delete_breakpoint, METH_NOARGS,
"Delete the underlying GDB breakpoint." },
+ { "locations", bppy_locations, METH_NOARGS,
+ "Get a list of gdb.BpLocation objects associated with this breakpoint." },
{ NULL } /* Sentinel. */
};

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index ef39d5d..49bb32e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -124,6 +124,9 @@ extern PyTypeObject stop_event_object_type;
/* Defined in py-breakpoint.c */
typedef struct breakpoint_object breakpoint_object;

+/* Defined in py-bploc.c */
+typedef struct bploc_object bploc_object;
+
typedef struct
{
PyObject_HEAD
@@ -176,6 +179,8 @@ PyObject *pspy_get_printers (PyObject *, void *);
PyObject *objfile_to_objfile_object (struct objfile *);
PyObject *objfpy_get_printers (PyObject *, void *);

+PyObject *bplocation_to_bplocation_object (struct bp_location *loc);
+
thread_object *create_thread_object (struct thread_info *tp);
thread_object *find_thread_object (ptid_t ptid);
PyObject *find_inferior_object (int pid);
@@ -202,6 +207,7 @@ void gdbpy_initialize_functions (void);
void gdbpy_initialize_pspace (void);
void gdbpy_initialize_objfile (void);
void gdbpy_initialize_breakpoints (void);
+void gdbpy_initialize_bplocation (void);
void gdbpy_initialize_lazy_string (void);
void gdbpy_initialize_parameters (void);
void gdbpy_initialize_thread (void);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index b0b9a9c..de6b51f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1058,6 +1058,12 @@ gdbpy_breakpoint_has_py_cond (struct breakpoint_object *bp_obj)
"scripting is not supported."));
}

+void
+gdbpy_bplocation_free (struct breakpoint_object *bp_obj)
+{
+ return;
+}
+
#endif /* HAVE_PYTHON */

\f
@@ -1247,6 +1253,7 @@ Enables or disables printing of Python stack traces."),
gdbpy_initialize_pspace ();
gdbpy_initialize_objfile ();
gdbpy_initialize_breakpoints ();
+ gdbpy_initialize_bplocation ();
gdbpy_initialize_lazy_string ();
gdbpy_initialize_thread ();
gdbpy_initialize_inferior ();
diff --git a/gdb/python/python.h b/gdb/python/python.h
index ae55cc2..cee8a6b 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -47,4 +47,6 @@ int gdbpy_should_stop (struct breakpoint_object *bp_obj);

int gdbpy_breakpoint_has_py_cond (struct breakpoint_object *bp_obj);

+void gdbpy_bplocation_free (struct bp_location *loc);
+
#endif /* GDB_PYTHON_H */
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index e0dd087..d946d7b 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -301,3 +301,55 @@ gdb_py_test_silent_cmd "python wp1 = wp_eval (\"result\", type=gdb.BP_WATCHPOIN
gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value =.*New value = 788.*" "Test watchpoint write"
gdb_test "python print never_eval_bp1.count" "0" \
"Check that this unrelated breakpoints eval function was never called."
+
+# gdb.BpLocation
+
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+delete_breakpoints
+gdb_test_no_output "set detach-on-fork off" "don't detach on fork"
+gdb_test "call fork()" "New process .*" "create a second inferior"
+
+gdb_breakpoint "main"
+gdb_test "py print len(gdb.breakpoints())" "1" "ensure that threre is only one BP"
+gdb_test_no_output {py bp0 = gdb.breakpoints()[0]} "save breakpoint 0"
+gdb_test "py print len(bp0.locations())" "2" "ensure that threre are 2 locations"
+
+gdb_test_no_output {py loc0 = bp0.locations()[0]} "save location 0"
+gdb_test_no_output {py loc1 = bp0.locations()[1]} "save location 1"
+
+gdb_test "py print loc0.owner == loc1.owner == bp0" "True" "verify ownership"
+gdb_test "py print loc0.address == loc1.address " "True" "verify addresses are identical"
+# how to check address location ? != address(main)
+
+gdb_test {py print loc0.inferior == gdb.inferiors()[0]} "True" "verify inferior for loc 0" #inf 2
+gdb_test {py print loc1.inferior == gdb.inferiors()[1]} "True" "verify inferior for loc 1" #inf 1
+
+gdb_test "py print loc0.enabled == loc1.enabled == True" "True" "verify that locations are enabled"
+
+gdb_test "py print loc0.inferior.num" "2" "ensure that loc0 is on inferior 2"
+
+gdb_test "detach inferior 2" "Detaching from program:.*" "detach inferior 2"
+gdb_test "inferior 1" "Switching to inferior .*" "switch to inferior 1"
+gdb_test_no_output "remove-inferiors 2" "remove inferior 2"
+gdb_test "py print loc0.inferior" "None" "removed inferior set to None"
+
+delete_breakpoints
+gdb_test "py print bp0.is_valid()" "False" "verify that BP has been invalidated"
+gdb_test "py bp0.locations()" ".*RuntimeError: Breakpoint .* is invalid.*"\
+ "verify that locations can't accessed on an invalid breakpoint"
+
+gdb_test "py print loc0.is_valid()" "False" "verify that location is invalid"
+gdb_test "py print loc0.owner" ".*RuntimeError: BpLocation invalid.*"\
+ "verify that owner can't be accessed"
+gdb_test "py print loc0.enabled" ".*RuntimeError: BpLocation invalid.*"\
+ "verify that location can't be accessed"
+gdb_test "py print loc0.address" ".*RuntimeError: BpLocation invalid.*"\
+ "verify that location can't be accessed"
+gdb_test "py print loc0.inferior" ".*RuntimeError: BpLocation invalid.*"\
+ "verify that inferior can't be accessed"
--
1.7.6.4