* [patch] Add visible flag to breakpoints.
@ 2010-09-30 16:28 Phil Muldoon
2010-09-30 16:41 ` Daniel Jacobowitz
` (3 more replies)
0 siblings, 4 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-09-30 16:28 UTC (permalink / raw)
To: gdb-patches
This patch allows breakpoints to be become invisible to the user.
This is part of a larger effort to improve the Python breakpoint support
within GDB. One of the use-cases we have in Python is for a command to
set (maybe a large) number of breakpoints to catch predefined
behavior. However we do not want this to impact the usability or
readability of the existing 'info breakpoints' output. This patch fixes
that by allowing breakpoints to become invisible to the user.
One of the initial approaches we took was to create our own kind of
Python breakpoint. This would be much like all of the special case
internal breakpoints that currently exist within GDB. But these are
too special purpose -- we lose all the general accessibility and
edge-cases behavior within general purpose breakpoints. And,
ultimately we would have to put all of that code back in, causing code
duplication. So we decided to add a visible flag.
Currently this visibility flag is only accessible through the Python
breakpoint code. If the visible keyword is set when the breakpoint is
created, it will not be mentioned (only the new breakpoint observer will
be called), and the breakpoint will not be enumerated via 'info
breakpoints'. The visibility of a breakpoint can subsequently be
altered via the 'visible' attribute of the Python object which will flip
the visible flag in the breakpoint struct. Subsequent calls to 'info
breakpoints' will then show the breakpoint. There are several
assumptions I have made in this patch.
If when the breakpoint it created, the visible flag is set to False:
-- GDB will not mention the breakpoint creation, but will notify any
observers.
-- The breakpoint will not be enumerated in the 'info breakpoint' list.
-- The breakpoint whether invisible or not, will always be enumerated in
'maint info breakpoints'
-- The internalvar bpnum is always set to the breakpoint just created,
regardless of whether the visibility flag is set.
I think these are sane defaults, but I'm open to changing them as we
need too.
What do you think?
Cheers,
Phil
--
2010-09-30 Phil Muldoon <pmuldoon@redhat.com>
* python/py-breakpoint.c (bppy_get_visibility): New function.
(bppy_set_visibility): Ditto.
(bppy_new): Add visible keyword and pass it as a parameter to
create_new_breakpoint.
* breakpoint.h (struct breakpoint): Add visible flag.
(create_new_breakpoint): Declare.
* breakpoint.c (breakpoint_visible): New function.
(breakpoint_1): Check if breakpoint is visible.
(set_raw_breakpoint_without_location): Set visible to 1.
(create_breakpoint_sal): Add visible parameter and set visibility.
Only call mention if breakpoint is visible, else just notify the
observer.
(create_breakpoint_sals): Ditto.
(create_new_breakpoint): Renamed from create_new_breakpoint. Add
visible parameter.
(create_breakpoint): Wrap create_new_breakpoint with default
visibility set to on.
2010-09-30 Phil Muldoon <pmuldoon@redhat.com>
* gdb.texinfo (Breakpoints In Python): Document optional visible
flag. Document visible attribute.
2010-09-30 Phil Muldoon <pmuldoon@redhat.com>
* gdb.python/py-breakpoint.exp: Add breakpoint visibility tests.
--
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4502e7..26cab2c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4907,6 +4907,14 @@ user_settable_breakpoint (const struct breakpoint *b)
|| is_tracepoint (b)
|| is_watchpoint (b));
}
+
+/* Return non-zero if B is visible to the user. */
+
+static int
+breakpoint_visible (const struct breakpoint *b)
+{
+ return b->visible;
+}
/* Print information on user settable breakpoint (watchpoint, etc)
number BNUM. If BNUM is -1 print all user-settable breakpoints.
@@ -4939,7 +4947,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
if (filter && !filter (b))
continue;
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && breakpoint_visible (b)))
{
int addr_bit, type_len;
@@ -5007,7 +5016,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
/* We only print out user settable breakpoints unless the
allflag is set. */
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && breakpoint_visible (b)))
print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
}
}
@@ -5456,6 +5466,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
b->syscalls_to_be_caught = NULL;
b->ops = NULL;
b->condition_not_parsed = 0;
+ b->visible = 1;
/* Add this breakpoint to the end of the chain
so that a list of breakpoints will come out in order
@@ -6914,7 +6925,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
char *cond_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- struct breakpoint_ops *ops, int from_tty, int enabled)
+ struct breakpoint_ops *ops, int from_tty,
+ int enabled, int visible)
{
struct breakpoint *b = NULL;
int i;
@@ -6961,6 +6973,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->disposition = disposition;
b->pspace = sals.sals[0].pspace;
+ b->visible = visible;
if (type == bp_static_tracepoint)
{
@@ -7034,7 +7047,12 @@ Couldn't determine the static tracepoint marker to probe"));
= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
b->ops = ops;
- mention (b);
+ if (visible)
+ mention (b);
+ else
+ /* Just notify observers without printing. */
+ observer_notify_breakpoint_created (b->number);
+
}
/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
@@ -7190,7 +7208,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
struct breakpoint_ops *ops, int from_tty,
- int enabled)
+ int enabled, int visible)
{
int i;
@@ -7201,7 +7219,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
create_breakpoint_sal (gdbarch, expanded, addr_string[i],
cond_string, type, disposition,
- thread, task, ignore_count, ops, from_tty, enabled);
+ thread, task, ignore_count, ops,
+ from_tty, enabled, visible);
}
}
@@ -7474,15 +7493,16 @@ decode_static_tracepoint_spec (char **arg_p)
was created; false otherwise. */
int
-create_breakpoint (struct gdbarch *gdbarch,
- char *arg, char *cond_string, int thread,
- int parse_condition_and_thread,
- int tempflag, enum bptype type_wanted,
- int ignore_count,
- enum auto_boolean pending_break_support,
- struct breakpoint_ops *ops,
- int from_tty,
- int enabled)
+create_new_breakpoint (struct gdbarch *gdbarch,
+ char *arg, char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag, enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int visible)
{
struct gdb_exception e;
struct symtabs_and_lines sals;
@@ -7658,7 +7678,7 @@ create_breakpoint (struct gdbarch *gdbarch,
cond_string, type_wanted,
tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops,
- from_tty, enabled);
+ from_tty, enabled, visible);
do_cleanups (old_chain);
@@ -7679,7 +7699,7 @@ create_breakpoint (struct gdbarch *gdbarch,
create_breakpoints_sal (gdbarch, sals, addr_string, cond_string,
type_wanted, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, from_tty,
- enabled);
+ enabled, visible);
}
else
{
@@ -7699,13 +7719,18 @@ create_breakpoint (struct gdbarch *gdbarch,
b->ops = ops;
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->pspace = current_program_space;
+ b->visible = visible;
if (enabled && b->pspace->executing_startup
&& (b->type == bp_breakpoint
|| b->type == bp_hardware_breakpoint))
b->enable_state = bp_startup_disabled;
- mention (b);
+ if (visible)
+ mention (b);
+ else
+ /* Just notify observers without printing. */
+ observer_notify_breakpoint_created (b->number);
}
if (sals.nelts > 1)
@@ -7727,6 +7752,34 @@ create_breakpoint (struct gdbarch *gdbarch,
return 1;
}
+
+/* Set a breakpoint. This function is shared between CLI and MI
+ functions for setting a breakpoint. This function has two major
+ modes of operations, selected by the PARSE_CONDITION_AND_THREAD
+ parameter. If non-zero, the function will parse arg, extracting
+ breakpoint location, address and thread. Otherwise, ARG is just the
+ location of breakpoint, with condition and thread specified by the
+ COND_STRING and THREAD parameters. Returns true if any breakpoint
+ was created; false otherwise. */
+
+int
+create_breakpoint (struct gdbarch *gdbarch,
+ char *arg, char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag, enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled)
+{
+ return create_new_breakpoint (gdbarch, arg, cond_string, thread,
+ parse_condition_and_thread, tempflag,
+ type_wanted, ignore_count,
+ pending_break_support,
+ ops, from_tty, enabled, 1);
+}
+
/* Set a breakpoint.
ARG is a string describing breakpoint address,
condition, and thread.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9f7600a..3c3b5d0 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -557,6 +557,13 @@ struct breakpoint
breakpoints, we will use this index to try to find the same
marker again. */
int static_trace_marker_id_idx;
+
+ /* Python breakpoints are allowed to specify whether the
+ breakpoint will be visible to the user via 'info breakpoints'.
+ This is to enable Python scripts to set many breakpoints in
+ commands/functions without impacting the readability of the
+ 'info breakpoints' command. */
+ int visible;
};
typedef struct breakpoint *breakpoint_p;
@@ -870,6 +877,18 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
int from_tty,
int enabled);
+extern int create_new_breakpoint (struct gdbarch *gdbarch, char *arg,
+ char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag,
+ enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int visible);
+
extern void insert_breakpoints (void);
extern int remove_breakpoints (void);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0b24718..1fa6faa 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22552,7 +22552,7 @@ Return the symbol table's source absolute file name.
Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
class.
-@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]}
+@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}visible@r{]}
Create a new breakpoint. @var{spec} is a string naming the
location of the breakpoint, or an expression that defines a
watchpoint. The contents can be any location recognized by the
@@ -22560,7 +22560,12 @@ watchpoint. The contents can be any location recognized by the
command. The optional @var{type} denotes the breakpoint to create
from the types defined later in this chapter. This argument can be
either: @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}. @var{type}
-defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
+defaults to @code{BP_BREAKPOINT}. The optional @var{visible} argument
+allows the breakpoint to become invisible to the user. The breakpoint
+will neither be reported when created, or will it be enumerated in the
+output from @samp{info breakpoints} (but will be enumerated with the
+@samp{maint info breakpoints} command). The @var{visible} argument
+has no effect with watchpoints. The optional @var{wp_class}
argument defines the class of watchpoint to create, if @var{type} is
defined as @code{BP_WATCHPOINT}. If a watchpoint class is not
provided, it is assumed to be a @var{WP_WRITE} class.
@@ -22638,6 +22643,12 @@ determine the actual breakpoint type or use-case. This attribute is not
writable.
@end defivar
+@defivar Breakpoint visible
+This attribute holds the breakpoint's visibility flag --- the identifier used to
+determine whether the breakpoint is visible to the user when set, or
+when the @samp{info breakpoints} command is run. This attribute is writable.
+@end defivar
+
The available types are represented by constants defined in the @code{gdb}
module:
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 0c70cbf..acbaab2 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -500,6 +500,40 @@ bppy_get_type (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->type);
}
+/* Python function to get the visibility state of the breakpoint. */
+static PyObject *
+bppy_get_visibility (PyObject *self, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ return PyInt_FromLong (self_bp->bp->visible);
+}
+
+/* Python function to set the visibility state of a breakpoint. */
+static int
+bppy_set_visibility (PyObject *self, PyObject *newvalue, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_SET_REQUIRE_VALID (self_bp);
+
+ if (newvalue == NULL || ! PyBool_Check (newvalue))
+ {
+ PyErr_SetString (PyExc_TypeError,
+ _("Value must be True or False."));
+ return -1;
+ }
+
+ if (newvalue == Py_True)
+ self_bp->bp->visible = 1;
+ else
+ self_bp->bp->visible = 0;
+
+ return 0;
+}
+
/* Python function to get the breakpoint's number. */
static PyObject *
bppy_get_number (PyObject *self, void *closure)
@@ -566,14 +600,15 @@ static PyObject *
bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
PyObject *result;
- static char *keywords[] = { "spec", "type", "wp_class", NULL };
+ static char *keywords[] = { "spec", "type", "wp_class", "visible", NULL };
char *spec;
int type = bp_breakpoint;
int access_type = hw_write;
+ int visible = 1;
volatile struct gdb_exception except;
- if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|ii", keywords,
- &spec, &type, &access_type))
+ if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iii", keywords,
+ &spec, &type, &access_type, &visible))
return NULL;
result = subtype->tp_alloc (subtype, 0);
@@ -589,13 +624,13 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
case bp_breakpoint:
{
- create_breakpoint (python_gdbarch,
- spec, NULL, -1,
- 0,
- 0, bp_breakpoint,
- 0,
- AUTO_BOOLEAN_TRUE,
- NULL, 0, 1);
+ create_new_breakpoint (python_gdbarch,
+ spec, NULL, -1,
+ 0,
+ 0, bp_breakpoint,
+ 0,
+ AUTO_BOOLEAN_TRUE,
+ NULL, 0, 1, visible);
break;
}
case bp_watchpoint:
@@ -816,6 +851,9 @@ or None if no condition set."},
"Commands of the breakpoint, as specified by the user."},
{ "type", bppy_get_type, NULL,
"Type of breakpoint."},
+ { "visible", bppy_get_visibility, bppy_set_visibility,
+ "Whether the breakpoint is visible to the user."},
+
{ NULL } /* Sentinel. */
};
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 07a7362..dc78a92 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -126,6 +126,30 @@ gdb_test "end"
gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" "Get Breakpoint List" 0
gdb_test "python print blist\[len(blist)-1\].commands" "print \"Command for breakpoint has been executed.\".*print result"
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+
+# Test invisible breakpooints.
+delete_breakpoints
+set ibp_location [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", visible=False)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "0" "Check breakpoint visibility"
+gdb_test "python print ilist\[0\].number == gdb.parse_and_eval(\"\$bpnum\")" "True" "Check that bpnum has been updated"
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check maint info breakpoints shows invisible breakpoints"
+gdb_py_test_silent_cmd "python ilist\[0\].visible = True" "Set visibility to True" 0
+gdb_test "info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check info breakpoints shows visible breakpoints"
+gdb_py_test_silent_cmd "python ilist\[0\].visible = False" "Set visibility to True" 0
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+
# Watchpoints
# Start with a fresh gdb.
clean_restart ${testfile}
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 16:28 [patch] Add visible flag to breakpoints Phil Muldoon
@ 2010-09-30 16:41 ` Daniel Jacobowitz
2010-09-30 17:51 ` Phil Muldoon
2010-09-30 17:04 ` Pedro Alves
` (2 subsequent siblings)
3 siblings, 1 reply; 31+ messages in thread
From: Daniel Jacobowitz @ 2010-09-30 16:41 UTC (permalink / raw)
To: Phil Muldoon; +Cc: gdb-patches
On Thu, Sep 30, 2010 at 03:02:14PM +0100, Phil Muldoon wrote:
> This is part of a larger effort to improve the Python breakpoint support
> within GDB. One of the use-cases we have in Python is for a command to
> set (maybe a large) number of breakpoints to catch predefined
> behavior. However we do not want this to impact the usability or
> readability of the existing 'info breakpoints' output. This patch fixes
> that by allowing breakpoints to become invisible to the user.
How does this interact with GDB-internal breakpoints (those with
negative numbers)? If you didn't need to switch visibility at
runtime, you could just create these as internal.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 16:28 [patch] Add visible flag to breakpoints Phil Muldoon
2010-09-30 16:41 ` Daniel Jacobowitz
@ 2010-09-30 17:04 ` Pedro Alves
2010-09-30 17:55 ` Phil Muldoon
2010-09-30 17:51 ` Eli Zaretskii
2010-10-05 22:28 ` Tom Tromey
3 siblings, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2010-09-30 17:04 UTC (permalink / raw)
To: gdb-patches, pmuldoon
On Thursday 30 September 2010 15:02:14, Phil Muldoon wrote:
> This is part of a larger effort to improve the Python breakpoint support
> within GDB. One of the use-cases we have in Python is for a command to
> set (maybe a large) number of breakpoints to catch predefined
> behavior. However we do not want this to impact the usability or
> readability of the existing 'info breakpoints' output. This patch fixes
> that by allowing breakpoints to become invisible to the user.
(...)
> Currently this visibility flag is only accessible through the Python
> breakpoint code. If the visible keyword is set when the breakpoint is
> created, it will not be mentioned (only the new breakpoint observer will
> be called), and the breakpoint will not be enumerated via 'info
> breakpoints'. The visibility of a breakpoint can subsequently be
> altered via the 'visible' attribute of the Python object which will flip
> the visible flag in the breakpoint struct. Subsequent calls to 'info
> breakpoints' will then show the breakpoint. There are several
> assumptions I have made in this patch.
Can you give an example of a use case where you would want to be
able to show/hide breakpoints from the user _after_ they've been
created? This looks like something that has potential to confuse
users. E.g., "my gdb sometimes creates breakpoint 10 and then
skips to create breakpoint 20, what gives? where are 11-19?".
If toggling the new visible attribute isn't really necessary,
did you consider instead a new "internal-or-not-internal" flag to
the breakpoint constructor? An internal breakpoint is just a
breakpoint with number < 0, and as such is not visible to
the user.
?
As is, your patch does not consider for example the
"delete" command --- it wipes all non-internal breakpoints, even
if hidden! That's potential for breaking the python client
code that creates and manages such breakpoints. I suggest
you go through breakpoint.c and inspect all checks against
b->number < 0 or b->number >= 0.
> (create_new_breakpoint): Renamed from create_new_breakpoint. Add
> visible parameter.
Renamed from create_breakpoint.
> +# Test invisible breakpooints.
Typo.
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 16:41 ` Daniel Jacobowitz
@ 2010-09-30 17:51 ` Phil Muldoon
2010-09-30 17:55 ` Pedro Alves
0 siblings, 1 reply; 31+ messages in thread
From: Phil Muldoon @ 2010-09-30 17:51 UTC (permalink / raw)
To: gdb-patches; +Cc: dan
Daniel Jacobowitz <dan@codesourcery.com> writes:
> On Thu, Sep 30, 2010 at 03:02:14PM +0100, Phil Muldoon wrote:
>> This is part of a larger effort to improve the Python breakpoint support
>> within GDB. One of the use-cases we have in Python is for a command to
>> set (maybe a large) number of breakpoints to catch predefined
>> behavior. However we do not want this to impact the usability or
>> readability of the existing 'info breakpoints' output. This patch fixes
>> that by allowing breakpoints to become invisible to the user.
>
> How does this interact with GDB-internal breakpoints (those with
> negative numbers)? If you didn't need to switch visibility at
> runtime, you could just create these as internal.
The original patch I wrote did use negative numbers for bp_breakpoint
type (in fact that patch is a commit in the archer branch:
archer-pmuldoon-python-breakpoints). But normal bp_breakpoints with a
negative number are still displayed with 'info breakpoints'. Currently
the visibility of breakpoints is not decided on their number but their
type. breakpoint_1 tests for these in user_settable_breakpoint. All I
did in that aspect was to add an additional check for
breakpoint_visible. I decided that if displaying the breakpoint was
just an arbitrary check that I would introduce a visible flag and avoid
all the re-plumbing of numbers/negative numbers in create_breakpoint.
The plumbing would have been necessary because ...
I also did look at create_internal_breakpoint, but those work on
a single address. We would either have the user translate the breakpoint to an
address with calls to decode_line or do it ourselves. I felt that a
flag would be better here than to duplicate this effort. Also there are
several cases/detections in generic bp_breakpoints that might be
useful for users of Python breakpoints. And finally to substantially change the
breakpoint mechanics underneath the call for a Python breakpoint purely
because one was created visible and the other not seemed counter-intuitive.
Cheers
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 16:28 [patch] Add visible flag to breakpoints Phil Muldoon
2010-09-30 16:41 ` Daniel Jacobowitz
2010-09-30 17:04 ` Pedro Alves
@ 2010-09-30 17:51 ` Eli Zaretskii
2010-10-05 22:28 ` Tom Tromey
3 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2010-09-30 17:51 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches
> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Thu, 30 Sep 2010 15:02:14 +0100
>
> Currently this visibility flag is only accessible through the Python
> breakpoint code. If the visible keyword is set when the breakpoint is
> created, it will not be mentioned (only the new breakpoint observer will
> be called), and the breakpoint will not be enumerated via 'info
> breakpoints'.
What will happen if there's some low-level failure in inserting such a
breakpoint? Won't GDB display an error message citing a breakpoint
that "doesn't exist", as far as the user is concerned?
> -defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
> +defaults to @code{BP_BREAKPOINT}. The optional @var{visible} argument
> +allows the breakpoint to become invisible to the user. The breakpoint
> +will neither be reported when created, or will it be enumerated in the
^^
"nor" should be used here.
Also, consider using "listed" or "shown" instead of "enumerated". I
think the former 2 alternatives make the text easier to read without
losing anything.
> +output from @samp{info breakpoints} (but will be enumerated with the
> +@samp{maint info breakpoints} command).
I believe we use @code markup for commands, not @samp.
> +@defivar Breakpoint visible
> +This attribute holds the breakpoint's visibility flag --- the identifier used to
^^^^^
No spaces are needed around the em-dash.
Okay with those changes.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 17:04 ` Pedro Alves
@ 2010-09-30 17:55 ` Phil Muldoon
0 siblings, 0 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-09-30 17:55 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <pedro@codesourcery.com> writes:
> Can you give an example of a use case where you would want to be
> able to show/hide breakpoints from the user _after_ they've been
> created? This looks like something that has potential to confuse
> users. E.g., "my gdb sometimes creates breakpoint 10 and then
> skips to create breakpoint 20, what gives? where are 11-19?".
I don't have one. I purely created the API for scripting utility, I have no
objections whatsoever to removing the setter.
> If toggling the new visible attribute isn't really necessary,
> did you consider instead a new "internal-or-not-internal" flag to
> the breakpoint constructor? An internal breakpoint is just a
> breakpoint with number < 0, and as such is not visible to
> the user.
I replied to Dan on this. Could I ask you to read my reply there?
Thanks!
> As is, your patch does not consider for example the
> "delete" command --- it wipes all non-internal breakpoints, even
> if hidden! That's potential for breaking the python client
> code that creates and manages such breakpoints. I suggest
> you go through breakpoint.c and inspect all checks against
> b->number < 0 or b->number >= 0.
'delete breakpoints' have always been likely to confuse python scripts
that set breakpoints (now, and in this new visibility patch). I did the
checks you mentioned. I'm just not sure how to work around it given my
objections to using internal breakpoints. What do you think?
>
>> (create_new_breakpoint): Renamed from create_new_breakpoint. Add
>> visible parameter.
>
> Renamed from create_breakpoint.
>
>> +# Test invisible breakpooints.
Thanks
Cheers,
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 17:51 ` Phil Muldoon
@ 2010-09-30 17:55 ` Pedro Alves
2010-09-30 18:12 ` Phil Muldoon
2010-10-08 12:51 ` Phil Muldoon
0 siblings, 2 replies; 31+ messages in thread
From: Pedro Alves @ 2010-09-30 17:55 UTC (permalink / raw)
To: gdb-patches, pmuldoon; +Cc: dan
On Thursday 30 September 2010 17:18:15, Phil Muldoon wrote:
> The original patch I wrote did use negative numbers for bp_breakpoint
> type (in fact that patch is a commit in the archer branch:
> archer-pmuldoon-python-breakpoints). But normal bp_breakpoints with a
> negative number are still displayed with 'info breakpoints'. Currently
> the visibility of breakpoints is not decided on their number but their
> type.
> breakpoint_1 tests for these in user_settable_breakpoint.
I think that's just cruft and can be replaced by a b->number < 0 check?
> All I
> did in that aspect was to add an additional check for
> breakpoint_visible. I decided that if displaying the breakpoint was
> just an arbitrary check that I would introduce a visible flag and avoid
> all the re-plumbing of numbers/negative numbers in create_breakpoint.
> The plumbing would have been necessary because ...
>
> I also did look at create_internal_breakpoint, but those work on
> a single address. We would either have the user translate the breakpoint to an
> address with calls to decode_line or do it ourselves. I felt that a
> flag would be better here than to duplicate this effort. Also there are
> several cases/detections in generic bp_breakpoints that might be
> useful for users of Python breakpoints. And finally to substantially change the
> breakpoint mechanics underneath the call for a Python breakpoint purely
> because one was created visible and the other not seemed counter-intuitive.
I'm not sure what large effort you're thinking this entails.
You've carried the "visible" flag as argument all the way down to
create_breakpoint_sal already. So instead of:
@@ -6961,6 +6973,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->disposition = disposition;
b->pspace = sals.sals[0].pspace;
+ b->visible = visible;
You change this:
b = set_raw_breakpoint (gdbarch, sal, type);
set_breakpoint_count (breakpoint_count + 1);
b->number = breakpoint_count;
to:
b = set_raw_breakpoint (gdbarch, sal, type);
if (visible /* or some other name, user? !internal? */)
{
set_breakpoint_count (breakpoint_count + 1);
b->number = breakpoint_count;
}
else
{
b->number = internal_breakpoint_number--;
}
and you're golden. What am I missing?
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 17:55 ` Pedro Alves
@ 2010-09-30 18:12 ` Phil Muldoon
2010-10-08 12:51 ` Phil Muldoon
1 sibling, 0 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-09-30 18:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, dan
Pedro Alves <pedro@codesourcery.com> writes:
> On Thursday 30 September 2010 17:18:15, Phil Muldoon wrote:
>> The original patch I wrote did use negative numbers for bp_breakpoint
>> type (in fact that patch is a commit in the archer branch:
>> archer-pmuldoon-python-breakpoints). But normal bp_breakpoints with a
>> negative number are still displayed with 'info breakpoints'. Currently
>> the visibility of breakpoints is not decided on their number but their
>> type.
>> breakpoint_1 tests for these in user_settable_breakpoint.
>
> I think that's just cruft and can be replaced by a b->number < 0 check?
Ah, there is the crux. I presumed (from the code) that there was a
scenario where negative numbered breakpoints were valid to be printed.
But looks like not. We can use negative numbers if the < 0 printing
check is ok and do away with the visible flag.
> I'm not sure what large effort you're thinking this entails.
> You've carried the "visible" flag as argument all the way down to
> create_breakpoint_sal already. So instead of:
>
> @@ -6961,6 +6973,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
> b->enable_state = enabled ? bp_enabled : bp_disabled;
> b->disposition = disposition;
> b->pspace = sals.sals[0].pspace;
> + b->visible = visible;
>
> You change this:
>
> b = set_raw_breakpoint (gdbarch, sal, type);
> set_breakpoint_count (breakpoint_count + 1);
> b->number = breakpoint_count;
>
> to:
>
> b = set_raw_breakpoint (gdbarch, sal, type);
> if (visible /* or some other name, user? !internal? */)
> {
> set_breakpoint_count (breakpoint_count + 1);
> b->number = breakpoint_count;
> }
> else
> {
> b->number = internal_breakpoint_number--;
> }
>
> and you're golden. What am I missing?
I have a patch in my archer branch that does just this. The original
patch I wrote did this ;)
There is some work to be done on the Python bookkeeping side as breakpoint
references are stored via breakpoint number (in a vector), so the negative
numbers would be painful there. Luckily in the aforementioned patch I
already changed this bookkeeping over to a single linked-list.
Looks like my first intentions were best then. I'll reconstitute the
patch from previous commits and resubmit (happily this is super easy
with git)
Cheers
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 16:28 [patch] Add visible flag to breakpoints Phil Muldoon
` (2 preceding siblings ...)
2010-09-30 17:51 ` Eli Zaretskii
@ 2010-10-05 22:28 ` Tom Tromey
3 siblings, 0 replies; 31+ messages in thread
From: Tom Tromey @ 2010-10-05 22:28 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Phil> This patch allows breakpoints to be become invisible to the user.
I have a couple additional notes on this patch.
Phil> +static PyObject *
Phil> +bppy_get_visibility (PyObject *self, void *closure)
[...]
I think it would be handy to keep this attribute around in the next
revision of the patch, even if it is not writeable.
Phil> + if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iii", keywords,
Phil> + &spec, &type, &access_type, &visible))
I think this should be "s|iiO", and then use PyObject_IsTrue to
determine whether the argument is true or false. I think this is more
idiomatic than an integer argument. Also, I think the documentation
should be updated to indicate that this argument is a boolean.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-09-30 17:55 ` Pedro Alves
2010-09-30 18:12 ` Phil Muldoon
@ 2010-10-08 12:51 ` Phil Muldoon
2010-10-08 13:35 ` Pedro Alves
2010-10-08 18:40 ` Tom Tromey
1 sibling, 2 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-10-08 12:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, dan
Pedro Alves <pedro@codesourcery.com> writes:
> On Thursday 30 September 2010 17:18:15, Phil Muldoon wrote:
>> The original patch I wrote did use negative numbers for bp_breakpoint
>> type (in fact that patch is a commit in the archer branch:
>> archer-pmuldoon-python-breakpoints). But normal bp_breakpoints with a
>> negative number are still displayed with 'info breakpoints'. Currently
>> the visibility of breakpoints is not decided on their number but their
>> type.
>> breakpoint_1 tests for these in user_settable_breakpoint.
>
> I think that's just cruft and can be replaced by a b->number < 0 check?
Here is a rewritten version of the b->number == internal_breakpoint_count
patch. There are a bits in py-breakpoint.c I had to re-architect to
facilitate this. The ChangeLog entries remain largely the same: I will
rewrite them on patch acceptance.
Tested on X86-64 Fedora/Linux with no regressions
Cheers
Phil
--
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4502e7..1516989 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4939,7 +4939,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
if (filter && !filter (b))
continue;
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
{
int addr_bit, type_len;
@@ -5007,7 +5008,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
/* We only print out user settable breakpoints unless the
allflag is set. */
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
}
}
@@ -6914,7 +6916,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
char *cond_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- struct breakpoint_ops *ops, int from_tty, int enabled)
+ struct breakpoint_ops *ops, int from_tty,
+ int enabled, int internal)
{
struct breakpoint *b = NULL;
int i;
@@ -6951,8 +6954,13 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
if (i == 0)
{
b = set_raw_breakpoint (gdbarch, sal, type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = thread;
b->task = task;
@@ -7034,7 +7042,12 @@ Couldn't determine the static tracepoint marker to probe"));
= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
b->ops = ops;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
@@ -7190,7 +7203,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
struct breakpoint_ops *ops, int from_tty,
- int enabled)
+ int enabled, int internal)
{
int i;
@@ -7201,7 +7214,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
create_breakpoint_sal (gdbarch, expanded, addr_string[i],
cond_string, type, disposition,
- thread, task, ignore_count, ops, from_tty, enabled);
+ thread, task, ignore_count, ops,
+ from_tty, enabled, internal);
}
}
@@ -7464,25 +7478,24 @@ decode_static_tracepoint_spec (char **arg_p)
return sals;
}
-/* Set a breakpoint. This function is shared between CLI and MI
- functions for setting a breakpoint. This function has two major
- modes of operations, selected by the PARSE_CONDITION_AND_THREAD
- parameter. If non-zero, the function will parse arg, extracting
- breakpoint location, address and thread. Otherwise, ARG is just the
- location of breakpoint, with condition and thread specified by the
- COND_STRING and THREAD parameters. Returns true if any breakpoint
- was created; false otherwise. */
+/* Set a breakpoint. This function has two major modes of operations,
+ selected by the PARSE_CONDITION_AND_THREAD parameter. If non-zero,
+ the function will parse arg, extracting breakpoint location,
+ address and thread. Otherwise, ARG is just the location of
+ breakpoint, with condition and thread specified by the COND_STRING
+ and THREAD parameters. If INTERNAL is non-zero, the breakpoint
+ number will be allocated from the internal breakpoint count.
+ Returns true if any breakpoint was created; false otherwise. */
int
-create_breakpoint (struct gdbarch *gdbarch,
+create_new_breakpoint (struct gdbarch *gdbarch,
char *arg, char *cond_string, int thread,
int parse_condition_and_thread,
int tempflag, enum bptype type_wanted,
int ignore_count,
enum auto_boolean pending_break_support,
struct breakpoint_ops *ops,
- int from_tty,
- int enabled)
+ int from_tty, int enabled, int internal)
{
struct gdb_exception e;
struct symtabs_and_lines sals;
@@ -7658,12 +7671,15 @@ create_breakpoint (struct gdbarch *gdbarch,
cond_string, type_wanted,
tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops,
- from_tty, enabled);
+ from_tty, enabled, internal);
do_cleanups (old_chain);
/* Get the tracepoint we just created. */
- tp = get_breakpoint (breakpoint_count);
+ if (internal)
+ tp = get_breakpoint (internal_breakpoint_number);
+ else
+ tp = get_breakpoint (breakpoint_count);
gdb_assert (tp != NULL);
/* Given that its possible to have multiple markers with
@@ -7679,7 +7695,7 @@ create_breakpoint (struct gdbarch *gdbarch,
create_breakpoints_sal (gdbarch, sals, addr_string, cond_string,
type_wanted, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, from_tty,
- enabled);
+ enabled, internal);
}
else
{
@@ -7688,8 +7704,13 @@ create_breakpoint (struct gdbarch *gdbarch,
make_cleanup (xfree, copy_arg);
b = set_raw_breakpoint_without_location (gdbarch, type_wanted);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = -1;
b->addr_string = addr_string[0];
b->cond_string = NULL;
@@ -7705,7 +7726,12 @@ create_breakpoint (struct gdbarch *gdbarch,
|| b->type == bp_hardware_breakpoint))
b->enable_state = bp_startup_disabled;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
if (sals.nelts > 1)
@@ -7727,6 +7753,31 @@ create_breakpoint (struct gdbarch *gdbarch,
return 1;
}
+
+/* Set a breakpoint. This function is shared between CLI and MI
+ functions for setting a breakpoint. It wraps create_new_breakpoint
+ and never asks for an internal breakpoint number to be allocated
+ against the breakpoint. Returns true if any breakpoint was
+ created; false otherwise. */
+
+int
+create_breakpoint (struct gdbarch *gdbarch,
+ char *arg, char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag, enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled)
+{
+ return create_new_breakpoint (gdbarch, arg, cond_string, thread,
+ parse_condition_and_thread, tempflag,
+ type_wanted, ignore_count,
+ pending_break_support,
+ ops, from_tty, enabled, 0);
+}
+
/* Set a breakpoint.
ARG is a string describing breakpoint address,
condition, and thread.
@@ -11372,7 +11423,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11410,7 +11461,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9f7600a..d6cd6c6 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -870,6 +870,18 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
int from_tty,
int enabled);
+extern int create_new_breakpoint (struct gdbarch *gdbarch, char *arg,
+ char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag,
+ enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int internal);
+
extern void insert_breakpoints (void);
extern int remove_breakpoints (void);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0b24718..415efc0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22552,7 +22552,7 @@ Return the symbol table's source absolute file name.
Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
class.
-@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]}
+@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}internal@r{]}
Create a new breakpoint. @var{spec} is a string naming the
location of the breakpoint, or an expression that defines a
watchpoint. The contents can be any location recognized by the
@@ -22560,7 +22560,12 @@ watchpoint. The contents can be any location recognized by the
command. The optional @var{type} denotes the breakpoint to create
from the types defined later in this chapter. This argument can be
either: @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}. @var{type}
-defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
+defaults to @code{BP_BREAKPOINT}. The optional @var{internal} argument
+allows the breakpoint to become invisible to the user. The breakpoint
+will neither be reported when created, nor will it be listed in the
+output from @code{info breakpoints} (but will be listed with the
+@code{maint info breakpoints} command). The @var{internal} argument
+has no effect with watchpoints. The optional @var{wp_class}
argument defines the class of watchpoint to create, if @var{type} is
defined as @code{BP_WATCHPOINT}. If a watchpoint class is not
provided, it is assumed to be a @var{WP_WRITE} class.
@@ -22638,6 +22643,13 @@ determine the actual breakpoint type or use-case. This attribute is not
writable.
@end defivar
+@defivar Breakpoint visible
+This attribute holds the breakpoint's visibility flag---the identifier used to
+determine whether the breakpoint is visible to the user when set, or
+when the @samp{info breakpoints} command is run. This attribute is
+not writable.
+@end defivar
+
The available types are represented by constants defined in the @code{gdb}
module:
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 0c70cbf..6381a19 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -34,17 +34,6 @@ typedef struct breakpoint_object breakpoint_object;
static PyTypeObject breakpoint_object_type;
-/* A dynamically allocated vector of breakpoint objects. Each
- breakpoint has a number. A breakpoint is valid if its slot in this
- vector is non-null. When a breakpoint is deleted, we drop our
- reference to it and zero its slot; this is how we let the Python
- object have a lifetime which is independent from that of the gdb
- breakpoint. */
-static breakpoint_object **bppy_breakpoints;
-
-/* Number of slots in bppy_breakpoints. */
-static int bppy_slots;
-
/* Number of live breakpoints. */
static int bppy_live;
@@ -62,13 +51,20 @@ struct breakpoint_object
/* The gdb breakpoint object, or NULL if the breakpoint has been
deleted. */
struct breakpoint *bp;
+ struct breakpoint_object *next;
};
+/* Top and bottom of the breakpoint chain. We store bottom so we do
+ not have to constantly iterate through the chain when we add a new
+ breakpoint. */
+struct breakpoint_object *top;
+struct breakpoint_object *bottom;
+
/* Require that BREAKPOINT be a valid breakpoint ID; throw a Python
exception if it is invalid. */
#define BPPY_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if (Breakpoint == NULL) \
return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
} while (0)
@@ -77,7 +73,7 @@ struct breakpoint_object
exception if it is invalid. This macro is for use in setter functions. */
#define BPPY_SET_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if (Breakpoint == NULL) \
{ \
PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
@@ -85,6 +81,9 @@ struct breakpoint_object
} \
} while (0)
+/* Iterator for Python breakpoints linked-list. */
+#define ALL_PY_BREAKPOINTS(B) for (B = top; B; B = B->next)
+
/* This is used to initialize various gdb.bp_* constants. */
struct pybp_code
{
@@ -115,18 +114,6 @@ static struct pybp_code pybp_watch_types[] =
{NULL} /* Sentinel. */
};
-/* Evaluate to true if the breakpoint NUM is valid, false otherwise. */
-static int
-bpnum_is_valid (int num)
-{
- if (num >=0
- && num < bppy_slots
- && bppy_breakpoints[num] != NULL)
- return 1;
-
- return 0;
-}
-
/* Python function which checks the validity of a breakpoint object. */
static PyObject *
bppy_is_valid (PyObject *self, PyObject *args)
@@ -500,6 +487,20 @@ bppy_get_type (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->type);
}
+/* Python function to get the visibility of the breakpoint. */
+static PyObject *
+bppy_get_visibility (PyObject *self, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ if (self_bp->bp->number < 0)
+ Py_RETURN_FALSE;
+
+ Py_RETURN_TRUE;
+}
+
/* Python function to get the breakpoint's number. */
static PyObject *
bppy_get_number (PyObject *self, void *closure)
@@ -566,16 +567,29 @@ static PyObject *
bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
PyObject *result;
- static char *keywords[] = { "spec", "type", "wp_class", NULL };
+ static char *keywords[] = { "spec", "type", "wp_class", "internal", NULL };
char *spec;
int type = bp_breakpoint;
int access_type = hw_write;
+ PyObject *internal = NULL;
+ int internal_bp = 0;
volatile struct gdb_exception except;
- if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|ii", keywords,
- &spec, &type, &access_type))
+ if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords,
+ &spec, &type, &access_type, &internal))
return NULL;
+ if (internal)
+ {
+ if (! PyBool_Check (internal))
+ {
+ PyErr_SetString (PyExc_TypeError,
+ _("The value of `internal' keyword must be a boolean."));
+ return NULL;
+ }
+ if (internal == Py_True)
+ internal_bp = 1;
+ }
result = subtype->tp_alloc (subtype, 0);
if (! result)
return NULL;
@@ -583,19 +597,22 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
bppy_pending_object->number = -1;
bppy_pending_object->bp = NULL;
+ /* We do not add the breakpoint to the linked-list yet; GDB
+ has not created the breakpoint. */
+ bppy_pending_object->next = NULL;
TRY_CATCH (except, RETURN_MASK_ALL)
{
switch (type)
{
case bp_breakpoint:
{
- create_breakpoint (python_gdbarch,
- spec, NULL, -1,
- 0,
- 0, bp_breakpoint,
- 0,
- AUTO_BOOLEAN_TRUE,
- NULL, 0, 1);
+ create_new_breakpoint (python_gdbarch,
+ spec, NULL, -1,
+ 0,
+ 0, bp_breakpoint,
+ 0,
+ AUTO_BOOLEAN_TRUE,
+ NULL, 0, 1, internal_bp);
break;
}
case bp_watchpoint:
@@ -634,6 +651,7 @@ PyObject *
gdbpy_breakpoints (PyObject *self, PyObject *args)
{
PyObject *result;
+ breakpoint_object *b;
if (bppy_live == 0)
Py_RETURN_NONE;
@@ -641,16 +659,13 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
result = PyTuple_New (bppy_live);
if (result)
{
- int i, out = 0;
-
- for (i = 0; out < bppy_live; ++i)
- {
- if (! bppy_breakpoints[i])
- continue;
- Py_INCREF (bppy_breakpoints[i]);
- PyTuple_SetItem (result, out, (PyObject *) bppy_breakpoints[i]);
- ++out;
- }
+ int i = 0;
+ ALL_PY_BREAKPOINTS (b)
+ {
+ Py_INCREF (b);
+ PyTuple_SetItem (result, i, (PyObject *) b);
+ ++i;
+ }
}
return result;
}
@@ -668,9 +683,6 @@ gdbpy_breakpoint_created (int num)
struct breakpoint *bp = NULL;
PyGILState_STATE state;
- if (num < 0)
- return;
-
bp = get_breakpoint (num);
if (! bp)
return;
@@ -682,21 +694,6 @@ gdbpy_breakpoint_created (int num)
&& bp->type != bp_access_watchpoint)
return;
- if (num >= bppy_slots)
- {
- int old = bppy_slots;
-
- bppy_slots = bppy_slots * 2 + 10;
- bppy_breakpoints
- = (breakpoint_object **) xrealloc (bppy_breakpoints,
- (bppy_slots
- * sizeof (breakpoint_object *)));
- memset (&bppy_breakpoints[old], 0,
- (bppy_slots - old) * sizeof (PyObject *));
- }
-
- ++bppy_live;
-
state = PyGILState_Ensure ();
if (bppy_pending_object)
@@ -710,9 +707,27 @@ gdbpy_breakpoint_created (int num)
{
newbp->number = num;
newbp->bp = bp;
- bppy_breakpoints[num] = newbp;
+ newbp->next = NULL;
Py_INCREF (newbp);
}
+ else
+ {
+ PyErr_SetString (PyExc_RuntimeError,
+ _("Error while creating breakpoint from GDB."));
+ return;
+ }
+
+ ++bppy_live;
+
+ /* If this is the first breakpoint created, set the top and bottom
+ of the linked list. */
+ if (top == NULL)
+ top = newbp;
+
+ if (bottom)
+ bottom->next = newbp;
+
+ bottom = newbp;
/* Just ignore errors here. */
PyErr_Clear ();
@@ -726,15 +741,28 @@ static void
gdbpy_breakpoint_deleted (int num)
{
PyGILState_STATE state;
+ breakpoint_object *b = NULL;
+ breakpoint_object *prev = NULL;
state = PyGILState_Ensure ();
- if (bpnum_is_valid (num))
- {
- bppy_breakpoints[num]->bp = NULL;
- Py_DECREF (bppy_breakpoints[num]);
- bppy_breakpoints[num] = NULL;
- --bppy_live;
- }
+ ALL_PY_BREAKPOINTS (b)
+ {
+ if (b->number == num)
+ {
+ if (b == top)
+ top = b->next;
+ else
+ prev->next = b->next;
+ if (b == bottom)
+ bottom = prev;
+
+ b->bp = NULL;
+ Py_DECREF (b);
+ --bppy_live;
+ break;
+ }
+ prev = b;
+ }
PyGILState_Release (state);
}
@@ -754,6 +782,9 @@ gdbpy_initialize_breakpoints (void)
PyModule_AddObject (gdb_module, "Breakpoint",
(PyObject *) &breakpoint_object_type);
+ top = NULL;
+ bottom = NULL;
+
observer_attach_breakpoint_created (gdbpy_breakpoint_created);
observer_attach_breakpoint_deleted (gdbpy_breakpoint_deleted);
@@ -816,6 +847,8 @@ or None if no condition set."},
"Commands of the breakpoint, as specified by the user."},
{ "type", bppy_get_type, NULL,
"Type of breakpoint."},
+ { "visible", bppy_get_visibility, NULL,
+ "Whether the breakpoint is visible to the user."},
{ NULL } /* Sentinel. */
};
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 7da94c4..bdf8300 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -117,6 +117,32 @@ gdb_test "end"
gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" "Get Breakpoint List" 0
gdb_test "python print blist\[len(blist)-1\].commands" "print \"Command for breakpoint has been executed.\".*print result"
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+
+# Test invisible breakpooints.
+delete_breakpoints
+set ibp_location [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "True" "Check breakpoint visibility"
+gdb_test "info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check info breakpoints shows visible breakpoints"
+delete_breakpoints
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=True)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "False" "Check breakpoint visibility"
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check maint info breakpoints shows invisible breakpoints"
+
# Watchpoints
# Start with a fresh gdb.
clean_restart ${testfile}
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 12:51 ` Phil Muldoon
@ 2010-10-08 13:35 ` Pedro Alves
2010-10-08 14:04 ` Phil Muldoon
2010-10-08 18:40 ` Tom Tromey
1 sibling, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2010-10-08 13:35 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches, dan
On Friday 08 October 2010 13:50:34, Phil Muldoon wrote:
> The @var{internal} argument has no effect with watchpoints.
Should it be an error instead (or made to work)?
The non-python bits looked fine. I'd prefer Tom or someone
else to look and approve those. I did notice:
> #define BPPY_REQUIRE_VALID(Breakpoint) \
> do { \
> - if (! bpnum_is_valid ((Breakpoint)->number)) \
> + if (Breakpoint == NULL) \
> return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
> (Breakpoint)->number); \
'(Breakpoint)->number' when Breakpoint is NULL is not going to work.
Also, missing ()s:
if ((Breakpoint) == NULL)
I would also suggest using a VEC for the breakpoints instead
of a linked list.
(Also, the old code appeared to have been designed for O(1) access to
the python breakpoint objects given a breakpoing number: I have no
clue it if that matters but you could preserve that easily with two
VECs or bppy_breakpoints arrays.)
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 13:35 ` Pedro Alves
@ 2010-10-08 14:04 ` Phil Muldoon
2010-10-08 18:44 ` Tom Tromey
2010-10-16 19:03 ` Pedro Alves
0 siblings, 2 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-10-08 14:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, dan
Pedro Alves <pedro@codesourcery.com> writes:
> On Friday 08 October 2010 13:50:34, Phil Muldoon wrote:
>> The @var{internal} argument has no effect with watchpoints.
>
> Should it be an error instead (or made to work)?
I can make it an error. I decided not to do watchpoints. The
consequences of setting hidden watchpoints from a script that could turn
out to be software watchpoints seemed a little troubling.
> The non-python bits looked fine. I'd prefer Tom or someone
> else to look and approve those. I did notice:
>
>> #define BPPY_REQUIRE_VALID(Breakpoint) \
>> do { \
>> - if (! bpnum_is_valid ((Breakpoint)->number)) \
>> + if (Breakpoint == NULL) \
>> return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
>> (Breakpoint)->number); \
> '(Breakpoint)->number' when Breakpoint is NULL is not going to work.
Oops, thanks for catching that. (Breakpoint)->number is left over from
the old macro. Will fix.
> Also, missing ()s:
>
> if ((Breakpoint) == NULL)
Thanks.
> I would also suggest using a VEC for the breakpoints instead
> of a linked list.
>
> (Also, the old code appeared to have been designed for O(1) access to
> the python breakpoint objects given a breakpoing number: I have no
> clue it if that matters but you could preserve that easily with two
> VECs or bppy_breakpoints arrays.)
Yeah the old code used vectors, and I converted it to a linked list to
cope with negative breakpoint numbers. I did consider keeping two
vectors. But I wasn't sure if the 0(1) design choice was conscious for
performance or whether it was just done that way. FWIW we only track
user watchpoints and breakpoints (and now the special breakpoint we have
introduced). Even in a potentially heavy situation I would expect the
number of user breakpoints to be < 1000? Is it really necessary? It's a
trade-off of for keeping two reallocated vectors over the lifetime to
maintaining a linked list. I'm ambivalent to how we decide to do it. I
just wanted to explain my rationale.
Cheers,
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 12:51 ` Phil Muldoon
2010-10-08 13:35 ` Pedro Alves
@ 2010-10-08 18:40 ` Tom Tromey
1 sibling, 0 replies; 31+ messages in thread
From: Tom Tromey @ 2010-10-08 18:40 UTC (permalink / raw)
To: pmuldoon; +Cc: Pedro Alves, gdb-patches, dan
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Phil> +/* Top and bottom of the breakpoint chain. We store bottom so we do
Phil> + not have to constantly iterate through the chain when we add a new
Phil> + breakpoint. */
Phil> +struct breakpoint_object *top;
Phil> +struct breakpoint_object *bottom;
These should be static.
Phil> + if (Breakpoint == NULL)
Pedro pointed out the problem here. I suspect you want ((Breakpoint)->bp)
instead.
There are 2 instances of this.
Phil> +/* Python function to get the visibility of the breakpoint. */
Phil> +static PyObject *
Phil> +bppy_get_visibility (PyObject *self, void *closure)
Blank line between the comment & function.
We're trying to enforce this now.
Phil> + if (internal)
Phil> + {
Phil> + if (! PyBool_Check (internal))
Phil> + {
Phil> + PyErr_SetString (PyExc_TypeError,
Phil> + _("The value of `internal' keyword must be a boolean."));
Use PyObject_IsTrue, don't type-check for a boolean.
Phil> + int i = 0;
Phil> + ALL_PY_BREAKPOINTS (b)
Phil> + {
Phil> + Py_INCREF (b);
Phil> + PyTuple_SetItem (result, i, (PyObject *) b);
Phil> + ++i;
I know this is copied code, as it were, but I think this needs an error
check on PyTuple_SetItem.
Phil> + PyErr_SetString (PyExc_RuntimeError,
Phil> + _("Error while creating breakpoint from GDB."));
Phil> + return;
Phil> + }
I don't think it is ok to do this here. I think it will just confuse
some future call into Python.
I am not sure what is the best thing to do here. We can't throw a gdb
exception, because we are in an observer.
Maybe setting the python error, as above, then immediately calling
gdbpy_print_stack. This is sort of weird, but at least it properly
handles the user's settings.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 14:04 ` Phil Muldoon
@ 2010-10-08 18:44 ` Tom Tromey
2010-10-12 20:25 ` Phil Muldoon
2010-10-16 19:03 ` Pedro Alves
1 sibling, 1 reply; 31+ messages in thread
From: Tom Tromey @ 2010-10-08 18:44 UTC (permalink / raw)
To: pmuldoon; +Cc: Pedro Alves, gdb-patches, dan
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Pedro> I would also suggest using a VEC for the breakpoints instead
Pedro> of a linked list.
Pedro> (Also, the old code appeared to have been designed for O(1) access to
Pedro> the python breakpoint objects given a breakpoing number: I have no
Pedro> clue it if that matters but you could preserve that easily with two
Pedro> VECs or bppy_breakpoints arrays.)
Phil> Yeah the old code used vectors, and I converted it to a linked list to
Phil> cope with negative breakpoint numbers. I did consider keeping two
Phil> vectors. But I wasn't sure if the 0(1) design choice was conscious for
Phil> performance or whether it was just done that way.
It was, but I am not sure that it matters.
Phil> FWIW we only track
Phil> user watchpoints and breakpoints (and now the special breakpoint we have
Phil> introduced). Even in a potentially heavy situation I would expect the
Phil> number of user breakpoints to be < 1000? Is it really necessary? It's a
Phil> trade-off of for keeping two reallocated vectors over the lifetime to
Phil> maintaining a linked list. I'm ambivalent to how we decide to do it. I
Phil> just wanted to explain my rationale.
Why don't we just put a reference to the PyObject into struct
breakpoint? This solves the problem completely, or at least pushes it
up to whatever algorithms are used in breakpoint.c. This can easily be
done in a way that does not break the Python-less case (see varobj.c for
an example).
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 18:44 ` Tom Tromey
@ 2010-10-12 20:25 ` Phil Muldoon
2010-10-12 21:34 ` Tom Tromey
0 siblings, 1 reply; 31+ messages in thread
From: Phil Muldoon @ 2010-10-12 20:25 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches, dan
Tom Tromey <tromey@redhat.com> writes:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> Why don't we just put a reference to the PyObject into struct
> breakpoint? This solves the problem completely, or at least pushes it
> up to whatever algorithms are used in breakpoint.c. This can easily be
> done in a way that does not break the Python-less case (see varobj.c for
> an example).
In this patch iteration I removed any form of storage/tracking of
breakpoints. Instead, as above, I placed a reference in the breakpoint
struct.
There are two disadvantages that stem from this:
* I had to extern breakpoint_chain
* I had to move the ALL_BREAKPOINTS macro to breakpoint.h
I'm not sure how to get around those, or, if indeed they are perceived
as disadvantages. Anyway, please take a look and let me know what you
think.
I will rewrite the ChangeLogs on patch approval.
Tested with no regressions on x86-64.
Cheers,
Phil
--
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4502e7..47d5bf2 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -345,12 +345,6 @@ static int executing_breakpoint_commands;
/* Are overlay event breakpoints enabled? */
static int overlay_events_enabled;
-/* Walk the following statement or block through all breakpoints.
- ALL_BREAKPOINTS_SAFE does so even if the statment deletes the current
- breakpoint. */
-
-#define ALL_BREAKPOINTS(B) for (B = breakpoint_chain; B; B = B->next)
-
#define ALL_BREAKPOINTS_SAFE(B,TMP) \
for (B = breakpoint_chain; \
B ? (TMP=B->next, 1): 0; \
@@ -4939,7 +4933,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
if (filter && !filter (b))
continue;
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
{
int addr_bit, type_len;
@@ -5007,7 +5002,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
/* We only print out user settable breakpoints unless the
allflag is set. */
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
}
}
@@ -5456,6 +5452,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
b->syscalls_to_be_caught = NULL;
b->ops = NULL;
b->condition_not_parsed = 0;
+ b->py_bp_object = NULL;
/* Add this breakpoint to the end of the chain
so that a list of breakpoints will come out in order
@@ -6914,7 +6911,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
char *cond_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- struct breakpoint_ops *ops, int from_tty, int enabled)
+ struct breakpoint_ops *ops, int from_tty,
+ int enabled, int internal)
{
struct breakpoint *b = NULL;
int i;
@@ -6951,8 +6949,13 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
if (i == 0)
{
b = set_raw_breakpoint (gdbarch, sal, type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = thread;
b->task = task;
@@ -7034,7 +7037,12 @@ Couldn't determine the static tracepoint marker to probe"));
= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
b->ops = ops;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
@@ -7190,7 +7198,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
struct breakpoint_ops *ops, int from_tty,
- int enabled)
+ int enabled, int internal)
{
int i;
@@ -7201,7 +7209,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
create_breakpoint_sal (gdbarch, expanded, addr_string[i],
cond_string, type, disposition,
- thread, task, ignore_count, ops, from_tty, enabled);
+ thread, task, ignore_count, ops,
+ from_tty, enabled, internal);
}
}
@@ -7464,25 +7473,24 @@ decode_static_tracepoint_spec (char **arg_p)
return sals;
}
-/* Set a breakpoint. This function is shared between CLI and MI
- functions for setting a breakpoint. This function has two major
- modes of operations, selected by the PARSE_CONDITION_AND_THREAD
- parameter. If non-zero, the function will parse arg, extracting
- breakpoint location, address and thread. Otherwise, ARG is just the
- location of breakpoint, with condition and thread specified by the
- COND_STRING and THREAD parameters. Returns true if any breakpoint
- was created; false otherwise. */
+/* Set a breakpoint. This function has two major modes of operations,
+ selected by the PARSE_CONDITION_AND_THREAD parameter. If non-zero,
+ the function will parse arg, extracting breakpoint location,
+ address and thread. Otherwise, ARG is just the location of
+ breakpoint, with condition and thread specified by the COND_STRING
+ and THREAD parameters. If INTERNAL is non-zero, the breakpoint
+ number will be allocated from the internal breakpoint count.
+ Returns true if any breakpoint was created; false otherwise. */
int
-create_breakpoint (struct gdbarch *gdbarch,
+create_new_breakpoint (struct gdbarch *gdbarch,
char *arg, char *cond_string, int thread,
int parse_condition_and_thread,
int tempflag, enum bptype type_wanted,
int ignore_count,
enum auto_boolean pending_break_support,
struct breakpoint_ops *ops,
- int from_tty,
- int enabled)
+ int from_tty, int enabled, int internal)
{
struct gdb_exception e;
struct symtabs_and_lines sals;
@@ -7658,12 +7666,15 @@ create_breakpoint (struct gdbarch *gdbarch,
cond_string, type_wanted,
tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops,
- from_tty, enabled);
+ from_tty, enabled, internal);
do_cleanups (old_chain);
/* Get the tracepoint we just created. */
- tp = get_breakpoint (breakpoint_count);
+ if (internal)
+ tp = get_breakpoint (internal_breakpoint_number);
+ else
+ tp = get_breakpoint (breakpoint_count);
gdb_assert (tp != NULL);
/* Given that its possible to have multiple markers with
@@ -7679,7 +7690,7 @@ create_breakpoint (struct gdbarch *gdbarch,
create_breakpoints_sal (gdbarch, sals, addr_string, cond_string,
type_wanted, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, from_tty,
- enabled);
+ enabled, internal);
}
else
{
@@ -7688,8 +7699,13 @@ create_breakpoint (struct gdbarch *gdbarch,
make_cleanup (xfree, copy_arg);
b = set_raw_breakpoint_without_location (gdbarch, type_wanted);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = -1;
b->addr_string = addr_string[0];
b->cond_string = NULL;
@@ -7699,13 +7715,19 @@ create_breakpoint (struct gdbarch *gdbarch,
b->ops = ops;
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->pspace = current_program_space;
+ b->py_bp_object = NULL;
if (enabled && b->pspace->executing_startup
&& (b->type == bp_breakpoint
|| b->type == bp_hardware_breakpoint))
b->enable_state = bp_startup_disabled;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
if (sals.nelts > 1)
@@ -7727,6 +7749,31 @@ create_breakpoint (struct gdbarch *gdbarch,
return 1;
}
+
+/* Set a breakpoint. This function is shared between CLI and MI
+ functions for setting a breakpoint. It wraps create_new_breakpoint
+ and never asks for an internal breakpoint number to be allocated
+ against the breakpoint. Returns true if any breakpoint was
+ created; false otherwise. */
+
+int
+create_breakpoint (struct gdbarch *gdbarch,
+ char *arg, char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag, enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled)
+{
+ return create_new_breakpoint (gdbarch, arg, cond_string, thread,
+ parse_condition_and_thread, tempflag,
+ type_wanted, ignore_count,
+ pending_break_support,
+ ops, from_tty, enabled, 0);
+}
+
/* Set a breakpoint.
ARG is a string describing breakpoint address,
condition, and thread.
@@ -11372,7 +11419,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11410,7 +11457,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9f7600a..da4461a 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -24,9 +24,21 @@
#include "value.h"
#include "vec.h"
+#if HAVE_PYTHON
+#include "python/python.h"
+#include "python/python-internal.h"
+#endif
+
struct value;
struct block;
+/* Walk the following statement or block through all breakpoints.
+ ALL_BREAKPOINTS_SAFE does so even if the statment deletes the current
+ breakpoint. */
+
+extern struct breakpoint *breakpoint_chain;
+#define ALL_BREAKPOINTS(B) for (B = breakpoint_chain; B; B = B->next)
+
/* This is the maximum number of bytes a breakpoint instruction can take.
Feel free to increase it. It's just used in a few places to size
arrays that should be independent of the target architecture. */
@@ -557,7 +569,14 @@ struct breakpoint
breakpoints, we will use this index to try to find the same
marker again. */
int static_trace_marker_id_idx;
- };
+
+ /* With a Python scripting enabled GDB, store a reference to the
+ Python object that has been associated with this breakpoint.
+ This is always NULL for a GDB that is not script enabled. It
+ can sometimes be NULL for enabled GDBs as not all breakpoint
+ types are tracked by the Python scripting API. */
+ PyObject *py_bp_object;
+};
typedef struct breakpoint *breakpoint_p;
DEF_VEC_P(breakpoint_p);
@@ -870,6 +889,18 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
int from_tty,
int enabled);
+extern int create_new_breakpoint (struct gdbarch *gdbarch, char *arg,
+ char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag,
+ enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int internal);
+
extern void insert_breakpoints (void);
extern int remove_breakpoints (void);
diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..643338e 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1240,4 +1240,8 @@ void dummy_obstack_deallocate (void *object, void *data);
extern void initialize_progspace (void);
extern void initialize_inferiors (void);
+#ifndef HAVE_PYTHON
+typedef int PyObject;
+#endif
+
#endif /* #ifndef DEFS_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0b24718..415efc0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22552,7 +22552,7 @@ Return the symbol table's source absolute file name.
Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
class.
-@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]}
+@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}internal@r{]}
Create a new breakpoint. @var{spec} is a string naming the
location of the breakpoint, or an expression that defines a
watchpoint. The contents can be any location recognized by the
@@ -22560,7 +22560,12 @@ watchpoint. The contents can be any location recognized by the
command. The optional @var{type} denotes the breakpoint to create
from the types defined later in this chapter. This argument can be
either: @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}. @var{type}
-defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
+defaults to @code{BP_BREAKPOINT}. The optional @var{internal} argument
+allows the breakpoint to become invisible to the user. The breakpoint
+will neither be reported when created, nor will it be listed in the
+output from @code{info breakpoints} (but will be listed with the
+@code{maint info breakpoints} command). The @var{internal} argument
+has no effect with watchpoints. The optional @var{wp_class}
argument defines the class of watchpoint to create, if @var{type} is
defined as @code{BP_WATCHPOINT}. If a watchpoint class is not
provided, it is assumed to be a @var{WP_WRITE} class.
@@ -22638,6 +22643,13 @@ determine the actual breakpoint type or use-case. This attribute is not
writable.
@end defivar
+@defivar Breakpoint visible
+This attribute holds the breakpoint's visibility flag---the identifier used to
+determine whether the breakpoint is visible to the user when set, or
+when the @samp{info breakpoints} command is run. This attribute is
+not writable.
+@end defivar
+
The available types are represented by constants defined in the @code{gdb}
module:
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 0c70cbf..a9a02f7 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -34,17 +34,6 @@ typedef struct breakpoint_object breakpoint_object;
static PyTypeObject breakpoint_object_type;
-/* A dynamically allocated vector of breakpoint objects. Each
- breakpoint has a number. A breakpoint is valid if its slot in this
- vector is non-null. When a breakpoint is deleted, we drop our
- reference to it and zero its slot; this is how we let the Python
- object have a lifetime which is independent from that of the gdb
- breakpoint. */
-static breakpoint_object **bppy_breakpoints;
-
-/* Number of slots in bppy_breakpoints. */
-static int bppy_slots;
-
/* Number of live breakpoints. */
static int bppy_live;
@@ -68,7 +57,7 @@ struct breakpoint_object
exception if it is invalid. */
#define BPPY_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
} while (0)
@@ -77,7 +66,7 @@ struct breakpoint_object
exception if it is invalid. This macro is for use in setter functions. */
#define BPPY_SET_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
{ \
PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
@@ -115,18 +104,6 @@ static struct pybp_code pybp_watch_types[] =
{NULL} /* Sentinel. */
};
-/* Evaluate to true if the breakpoint NUM is valid, false otherwise. */
-static int
-bpnum_is_valid (int num)
-{
- if (num >=0
- && num < bppy_slots
- && bppy_breakpoints[num] != NULL)
- return 1;
-
- return 0;
-}
-
/* Python function which checks the validity of a breakpoint object. */
static PyObject *
bppy_is_valid (PyObject *self, PyObject *args)
@@ -500,6 +477,21 @@ bppy_get_type (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->type);
}
+/* Python function to get the visibility of the breakpoint. */
+
+static PyObject *
+bppy_get_visibility (PyObject *self, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ if (self_bp->bp->number < 0)
+ Py_RETURN_FALSE;
+
+ Py_RETURN_TRUE;
+}
+
/* Python function to get the breakpoint's number. */
static PyObject *
bppy_get_number (PyObject *self, void *closure)
@@ -566,16 +558,21 @@ static PyObject *
bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
PyObject *result;
- static char *keywords[] = { "spec", "type", "wp_class", NULL };
+ static char *keywords[] = { "spec", "type", "wp_class", "internal", NULL };
char *spec;
int type = bp_breakpoint;
int access_type = hw_write;
+ PyObject *internal = NULL;
+ int internal_bp = 0;
volatile struct gdb_exception except;
- if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|ii", keywords,
- &spec, &type, &access_type))
+ if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords,
+ &spec, &type, &access_type, &internal))
return NULL;
+ if (internal && PyObject_IsTrue (internal))
+ internal_bp = 1;
+
result = subtype->tp_alloc (subtype, 0);
if (! result)
return NULL;
@@ -589,13 +586,13 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
case bp_breakpoint:
{
- create_breakpoint (python_gdbarch,
- spec, NULL, -1,
- 0,
- 0, bp_breakpoint,
- 0,
- AUTO_BOOLEAN_TRUE,
- NULL, 0, 1);
+ create_new_breakpoint (python_gdbarch,
+ spec, NULL, -1,
+ 0,
+ 0, bp_breakpoint,
+ 0,
+ AUTO_BOOLEAN_TRUE,
+ NULL, 0, 1, internal_bp);
break;
}
case bp_watchpoint:
@@ -634,6 +631,7 @@ PyObject *
gdbpy_breakpoints (PyObject *self, PyObject *args)
{
PyObject *result;
+ struct breakpoint *b;
if (bppy_live == 0)
Py_RETURN_NONE;
@@ -641,16 +639,24 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
result = PyTuple_New (bppy_live);
if (result)
{
- int i, out = 0;
-
- for (i = 0; out < bppy_live; ++i)
- {
- if (! bppy_breakpoints[i])
- continue;
- Py_INCREF (bppy_breakpoints[i]);
- PyTuple_SetItem (result, out, (PyObject *) bppy_breakpoints[i]);
- ++out;
- }
+ int i = 0;
+ ALL_BREAKPOINTS (b)
+ {
+ /* Not all breakpoints will have a companion Python object.
+ Only breakpoints that were created via bppy_new, or
+ breakpoints that were created externally and are tracked by
+ the Python Scripting API. */
+ if (b->py_bp_object)
+ {
+ if (PyTuple_SetItem (result, i, (PyObject *) b->py_bp_object) != 0)
+ {
+ Py_DECREF (result);
+ return NULL;
+ }
+ Py_INCREF (b->py_bp_object);
+ ++i;
+ }
+ }
}
return result;
}
@@ -667,9 +673,7 @@ gdbpy_breakpoint_created (int num)
breakpoint_object *newbp;
struct breakpoint *bp = NULL;
PyGILState_STATE state;
-
- if (num < 0)
- return;
+ int error = 0;
bp = get_breakpoint (num);
if (! bp)
@@ -682,21 +686,6 @@ gdbpy_breakpoint_created (int num)
&& bp->type != bp_access_watchpoint)
return;
- if (num >= bppy_slots)
- {
- int old = bppy_slots;
-
- bppy_slots = bppy_slots * 2 + 10;
- bppy_breakpoints
- = (breakpoint_object **) xrealloc (bppy_breakpoints,
- (bppy_slots
- * sizeof (breakpoint_object *)));
- memset (&bppy_breakpoints[old], 0,
- (bppy_slots - old) * sizeof (PyObject *));
- }
-
- ++bppy_live;
-
state = PyGILState_Ensure ();
if (bppy_pending_object)
@@ -710,9 +699,19 @@ gdbpy_breakpoint_created (int num)
{
newbp->number = num;
newbp->bp = bp;
- bppy_breakpoints[num] = newbp;
+ newbp->bp->py_bp_object = (PyObject *)newbp;
Py_INCREF (newbp);
}
+ else
+ {
+ PyErr_SetString (PyExc_RuntimeError,
+ _("Error while creating breakpoint from GDB."));
+ gdbpy_print_stack ();
+ error = 1;
+ }
+
+ if (! error)
+ ++bppy_live;
/* Just ignore errors here. */
PyErr_Clear ();
@@ -726,15 +725,25 @@ static void
gdbpy_breakpoint_deleted (int num)
{
PyGILState_STATE state;
+ struct breakpoint *b = NULL;
state = PyGILState_Ensure ();
- if (bpnum_is_valid (num))
- {
- bppy_breakpoints[num]->bp = NULL;
- Py_DECREF (bppy_breakpoints[num]);
- bppy_breakpoints[num] = NULL;
- --bppy_live;
- }
+ ALL_BREAKPOINTS (b)
+ {
+ if (b->number == num)
+ {
+ breakpoint_object *bp_obj =
+ ((breakpoint_object *)b->py_bp_object);
+
+ if (bp_obj)
+ {
+ bp_obj->bp = NULL;
+ --bppy_live;
+ Py_DECREF (bp_obj);
+ }
+ break;
+ }
+ }
PyGILState_Release (state);
}
@@ -816,6 +825,8 @@ or None if no condition set."},
"Commands of the breakpoint, as specified by the user."},
{ "type", bppy_get_type, NULL,
"Type of breakpoint."},
+ { "visible", bppy_get_visibility, NULL,
+ "Whether the breakpoint is visible to the user."},
{ NULL } /* Sentinel. */
};
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 7da94c4..9acc433 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -117,6 +117,33 @@ gdb_test "end"
gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" "Get Breakpoint List" 0
gdb_test "python print blist\[len(blist)-1\].commands" "print \"Command for breakpoint has been executed.\".*print result"
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+
+# Test invisible breakpooints.
+delete_breakpoints
+set ibp_location [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "True" "Check breakpoint visibility"
+gdb_test "info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check info breakpoints shows visible breakpoints"
+delete_breakpoints
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=True)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "False" "Check breakpoint visibility"
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check maint info breakpoints shows invisible breakpoints"
+
+
# Watchpoints
# Start with a fresh gdb.
clean_restart ${testfile}
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 56cb8ae..2f2e469 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -39,8 +39,6 @@
#if HAVE_PYTHON
#include "python/python.h"
#include "python/python-internal.h"
-#else
-typedef int PyObject;
#endif
/* Non-zero if we want to see trace of varobj level stuff. */
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-12 20:25 ` Phil Muldoon
@ 2010-10-12 21:34 ` Tom Tromey
2010-10-13 12:45 ` Phil Muldoon
0 siblings, 1 reply; 31+ messages in thread
From: Tom Tromey @ 2010-10-12 21:34 UTC (permalink / raw)
To: pmuldoon; +Cc: Pedro Alves, gdb-patches, dan
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Phil> In this patch iteration I removed any form of storage/tracking of
Phil> breakpoints. Instead, as above, I placed a reference in the breakpoint
Phil> struct.
I like this better.
Phil> There are two disadvantages that stem from this:
Phil> * I had to extern breakpoint_chain
Phil> * I had to move the ALL_BREAKPOINTS macro to breakpoint.h
Phil> I'm not sure how to get around those, or, if indeed they are perceived
Phil> as disadvantages.
It is definitely bad to do this. I think it is fixable, though.
It is a little odd that the observer passes the number and not the
breakpoint itself.
Phil> + if (internal && PyObject_IsTrue (internal))
Phil> + internal_bp = 1;
PyObject_IsTrue can return -1 on failure, so this code must account for
that.
I see we have a case where we don't check this in py-prettyprint.c :(
I will fix that.
Phil> + int i = 0;
Phil> + ALL_BREAKPOINTS (b)
This instance of ALL_BREAKPOINTS should probably be replaced with some
kind of callback API, like the other iterate_over_* functions in gdb.
Phil> + {
Phil> + /* Not all breakpoints will have a companion Python object.
Phil> + Only breakpoints that were created via bppy_new, or
Phil> + breakpoints that were created externally and are tracked by
Phil> + the Python Scripting API. */
Phil> + if (b->py_bp_object)
Phil> + {
Phil> + if (PyTuple_SetItem (result, i, (PyObject *) b->py_bp_object) != 0)
I think that cast to PyObject is unnecessary.
Phil> + {
Phil> + Py_DECREF (result);
Phil> + return NULL;
Phil> + }
Phil> + Py_INCREF (b->py_bp_object);
This incref should come before the call to PyTuple_SetItem.
Phil> @@ -667,9 +673,7 @@ gdbpy_breakpoint_created (int num)
Phil> breakpoint_object *newbp;
Phil> struct breakpoint *bp = NULL;
Phil> PyGILState_STATE state;
Phil> -
Phil> - if (num < 0)
Phil> - return;
Phil> + int error = 0;
I am not certain that we want a new breakpoint object to be created for
internal breakpoints set by other modules. It seems potentially
harmful.
I think one way to do this would be to rephrase this check as
if (num < 0 && bppy_pending_object == NULL)
Phil> + newbp->bp->py_bp_object = (PyObject *)newbp;
No cast.
Phil> + if (! error)
Phil> + ++bppy_live;
You can get rid of the 'error' local and just push this back up into the
"ok" branch of the preceding 'if'.
Phil> /* Just ignore errors here. */
Phil> PyErr_Clear ();
I think this is redundant now.
Phil> + ALL_BREAKPOINTS (b)
Phil> + {
Phil> + if (b->number == num)
Phil> + {
Phil> + breakpoint_object *bp_obj =
Phil> + ((breakpoint_object *)b->py_bp_object);
I think you can replace this ALL_BREAKPOINTS with a call to
get_breakpoint.
Space after the close paren in the cast.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-12 21:34 ` Tom Tromey
@ 2010-10-13 12:45 ` Phil Muldoon
2010-10-13 13:06 ` Phil Muldoon
` (2 more replies)
0 siblings, 3 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-10-13 12:45 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches, dan
Tom Tromey <tromey@redhat.com> writes:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> In this patch iteration I removed any form of storage/tracking of
> Phil> breakpoints. Instead, as above, I placed a reference in the breakpoint
> Phil> struct.
>
> I like this better.
>
> Phil> + if (internal && PyObject_IsTrue (internal))
> Phil> + internal_bp = 1;
>
> PyObject_IsTrue can return -1 on failure, so this code must account for
> that.
Oops done. Thanks.
> Phil> + int i = 0;
> Phil> + ALL_BREAKPOINTS (b)
>
> This instance of ALL_BREAKPOINTS should probably be replaced with some
> kind of callback API, like the other iterate_over_* functions in gdb.
I wrote an iterate function for this, much like the inferior iterator.
I noticed in py-inferior.c we do not account for an error when adding to a
list. (I'm pretty sure I wrote that too :( I'll fix this later.
> Phil> + {
> Phil> + /* Not all breakpoints will have a companion Python object.
> Phil> + Only breakpoints that were created via bppy_new, or
> Phil> + breakpoints that were created externally and are tracked by
> Phil> + the Python Scripting API. */
> Phil> + if (b->py_bp_object)
> Phil> + {
> Phil> + if (PyTuple_SetItem (result, i, (PyObject *) b->py_bp_object) != 0)
>
> I think that cast to PyObject is unnecessary.
As we use an iterator, we now set a list then convert it to an iterator
later. This is like how we do it with inferior lists. (And it moves
away from reference stealing that tuples do :(
> Phil> @@ -667,9 +673,7 @@ gdbpy_breakpoint_created (int num)
> Phil> breakpoint_object *newbp;
> Phil> struct breakpoint *bp = NULL;
> Phil> PyGILState_STATE state;
> Phil> -
> Phil> - if (num < 0)
> Phil> - return;
> Phil> + int error = 0;
>
> I am not certain that we want a new breakpoint object to be created for
> internal breakpoints set by other modules. It seems potentially
> harmful.
I agree, fixed.
> Phil> + newbp->bp->py_bp_object = (PyObject *)newbp;
>
> No cast.
If I do not cast this, the compiler errors with:
../../archer/gdb/python/py-breakpoint.c: In function gdbpy_breakpoint_created:
../../archer/gdb/python/py-breakpoint.c:719: error: assignment from incompatible pointer type
> Phil> + if (! error)
> Phil> + ++bppy_live;
>
> You can get rid of the 'error' local and just push this back up into the
> "ok" branch of the preceding 'if'.
Ok.
> Phil> /* Just ignore errors here. */
> Phil> PyErr_Clear ();
>
> I think this is redundant now.
Ok.
> Phil> + ALL_BREAKPOINTS (b)
> Phil> + {
> Phil> + if (b->number == num)
> Phil> + {
> Phil> + breakpoint_object *bp_obj =
> Phil> + ((breakpoint_object *)b->py_bp_object);
>
> I think you can replace this ALL_BREAKPOINTS with a call to
> get_breakpoint.
Done.
Latest patched attached. What do you think?
Cheers,
Phil
--
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4502e7..7ef97ec 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4939,7 +4939,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
if (filter && !filter (b))
continue;
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
{
int addr_bit, type_len;
@@ -5007,7 +5008,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
/* We only print out user settable breakpoints unless the
allflag is set. */
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
}
}
@@ -5456,6 +5458,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
b->syscalls_to_be_caught = NULL;
b->ops = NULL;
b->condition_not_parsed = 0;
+ b->py_bp_object = NULL;
/* Add this breakpoint to the end of the chain
so that a list of breakpoints will come out in order
@@ -6914,7 +6917,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
char *cond_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- struct breakpoint_ops *ops, int from_tty, int enabled)
+ struct breakpoint_ops *ops, int from_tty,
+ int enabled, int internal)
{
struct breakpoint *b = NULL;
int i;
@@ -6951,8 +6955,13 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
if (i == 0)
{
b = set_raw_breakpoint (gdbarch, sal, type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = thread;
b->task = task;
@@ -7034,7 +7043,12 @@ Couldn't determine the static tracepoint marker to probe"));
= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
b->ops = ops;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
@@ -7190,7 +7204,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
struct breakpoint_ops *ops, int from_tty,
- int enabled)
+ int enabled, int internal)
{
int i;
@@ -7201,7 +7215,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
create_breakpoint_sal (gdbarch, expanded, addr_string[i],
cond_string, type, disposition,
- thread, task, ignore_count, ops, from_tty, enabled);
+ thread, task, ignore_count, ops,
+ from_tty, enabled, internal);
}
}
@@ -7464,25 +7479,24 @@ decode_static_tracepoint_spec (char **arg_p)
return sals;
}
-/* Set a breakpoint. This function is shared between CLI and MI
- functions for setting a breakpoint. This function has two major
- modes of operations, selected by the PARSE_CONDITION_AND_THREAD
- parameter. If non-zero, the function will parse arg, extracting
- breakpoint location, address and thread. Otherwise, ARG is just the
- location of breakpoint, with condition and thread specified by the
- COND_STRING and THREAD parameters. Returns true if any breakpoint
- was created; false otherwise. */
+/* Set a breakpoint. This function has two major modes of operations,
+ selected by the PARSE_CONDITION_AND_THREAD parameter. If non-zero,
+ the function will parse arg, extracting breakpoint location,
+ address and thread. Otherwise, ARG is just the location of
+ breakpoint, with condition and thread specified by the COND_STRING
+ and THREAD parameters. If INTERNAL is non-zero, the breakpoint
+ number will be allocated from the internal breakpoint count.
+ Returns true if any breakpoint was created; false otherwise. */
int
-create_breakpoint (struct gdbarch *gdbarch,
+create_new_breakpoint (struct gdbarch *gdbarch,
char *arg, char *cond_string, int thread,
int parse_condition_and_thread,
int tempflag, enum bptype type_wanted,
int ignore_count,
enum auto_boolean pending_break_support,
struct breakpoint_ops *ops,
- int from_tty,
- int enabled)
+ int from_tty, int enabled, int internal)
{
struct gdb_exception e;
struct symtabs_and_lines sals;
@@ -7658,12 +7672,15 @@ create_breakpoint (struct gdbarch *gdbarch,
cond_string, type_wanted,
tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops,
- from_tty, enabled);
+ from_tty, enabled, internal);
do_cleanups (old_chain);
/* Get the tracepoint we just created. */
- tp = get_breakpoint (breakpoint_count);
+ if (internal)
+ tp = get_breakpoint (internal_breakpoint_number);
+ else
+ tp = get_breakpoint (breakpoint_count);
gdb_assert (tp != NULL);
/* Given that its possible to have multiple markers with
@@ -7679,7 +7696,7 @@ create_breakpoint (struct gdbarch *gdbarch,
create_breakpoints_sal (gdbarch, sals, addr_string, cond_string,
type_wanted, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, from_tty,
- enabled);
+ enabled, internal);
}
else
{
@@ -7688,8 +7705,13 @@ create_breakpoint (struct gdbarch *gdbarch,
make_cleanup (xfree, copy_arg);
b = set_raw_breakpoint_without_location (gdbarch, type_wanted);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
b->thread = -1;
b->addr_string = addr_string[0];
b->cond_string = NULL;
@@ -7699,13 +7721,19 @@ create_breakpoint (struct gdbarch *gdbarch,
b->ops = ops;
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->pspace = current_program_space;
+ b->py_bp_object = NULL;
if (enabled && b->pspace->executing_startup
&& (b->type == bp_breakpoint
|| b->type == bp_hardware_breakpoint))
b->enable_state = bp_startup_disabled;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
if (sals.nelts > 1)
@@ -7727,6 +7755,31 @@ create_breakpoint (struct gdbarch *gdbarch,
return 1;
}
+
+/* Set a breakpoint. This function is shared between CLI and MI
+ functions for setting a breakpoint. It wraps create_new_breakpoint
+ and never asks for an internal breakpoint number to be allocated
+ against the breakpoint. Returns true if any breakpoint was
+ created; false otherwise. */
+
+int
+create_breakpoint (struct gdbarch *gdbarch,
+ char *arg, char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag, enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled)
+{
+ return create_new_breakpoint (gdbarch, arg, cond_string, thread,
+ parse_condition_and_thread, tempflag,
+ type_wanted, ignore_count,
+ pending_break_support,
+ ops, from_tty, enabled, 0);
+}
+
/* Set a breakpoint.
ARG is a string describing breakpoint address,
condition, and thread.
@@ -11372,7 +11425,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11410,7 +11463,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11627,6 +11680,21 @@ save_command (char *arg, int from_tty)
help_list (save_cmdlist, "save ", -1, gdb_stdout);
}
+struct breakpoint *
+iterate_over_breakpoints (int (*callback) (struct breakpoint *, void *),
+ void *data)
+{
+ struct breakpoint *b, *temp;
+
+ ALL_BREAKPOINTS_SAFE (b, temp)
+ {
+ if ((*callback) (b, data))
+ return b;
+ }
+
+ return NULL;
+}
+
void
_initialize_breakpoint (void)
{
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9f7600a..d3fbc6a 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -24,6 +24,11 @@
#include "value.h"
#include "vec.h"
+#if HAVE_PYTHON
+#include "python/python.h"
+#include "python/python-internal.h"
+#endif
+
struct value;
struct block;
@@ -557,7 +562,14 @@ struct breakpoint
breakpoints, we will use this index to try to find the same
marker again. */
int static_trace_marker_id_idx;
- };
+
+ /* With a Python scripting enabled GDB, store a reference to the
+ Python object that has been associated with this breakpoint.
+ This is always NULL for a GDB that is not script enabled. It
+ can sometimes be NULL for enabled GDBs as not all breakpoint
+ types are tracked by the Python scripting API. */
+ PyObject *py_bp_object;
+};
typedef struct breakpoint *breakpoint_p;
DEF_VEC_P(breakpoint_p);
@@ -870,6 +882,18 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
int from_tty,
int enabled);
+extern int create_new_breakpoint (struct gdbarch *gdbarch, char *arg,
+ char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag,
+ enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int internal);
+
extern void insert_breakpoints (void);
extern int remove_breakpoints (void);
@@ -1101,4 +1125,15 @@ extern void check_tracepoint_command (char *line, void *closure);
extern void start_rbreak_breakpoints (void);
extern void end_rbreak_breakpoints (void);
+/* Breakpoint iterator function.
+
+ Calls a callback function once for each breakpoint, so long as the
+ callback function returns false. If the callback function returns
+ true, the iteration will end and the current breakpoint will be
+ returned. This can be useful for implementing a search for a
+ breakpoint with arbitrary attributes, or for applying an operation
+ to every breakpoint. */
+extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *,
+ void *), void *);
+
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..643338e 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1240,4 +1240,8 @@ void dummy_obstack_deallocate (void *object, void *data);
extern void initialize_progspace (void);
extern void initialize_inferiors (void);
+#ifndef HAVE_PYTHON
+typedef int PyObject;
+#endif
+
#endif /* #ifndef DEFS_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c73e2f3..632d657 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22552,7 +22552,7 @@ Return the symbol table's source absolute file name.
Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
class.
-@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]}
+@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}internal@r{]}
Create a new breakpoint. @var{spec} is a string naming the
location of the breakpoint, or an expression that defines a
watchpoint. The contents can be any location recognized by the
@@ -22560,7 +22560,12 @@ watchpoint. The contents can be any location recognized by the
command. The optional @var{type} denotes the breakpoint to create
from the types defined later in this chapter. This argument can be
either: @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}. @var{type}
-defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
+defaults to @code{BP_BREAKPOINT}. The optional @var{internal} argument
+allows the breakpoint to become invisible to the user. The breakpoint
+will neither be reported when created, nor will it be listed in the
+output from @code{info breakpoints} (but will be listed with the
+@code{maint info breakpoints} command). The @var{internal} argument
+has no effect with watchpoints. The optional @var{wp_class}
argument defines the class of watchpoint to create, if @var{type} is
defined as @code{BP_WATCHPOINT}. If a watchpoint class is not
provided, it is assumed to be a @var{WP_WRITE} class.
@@ -22638,6 +22643,13 @@ determine the actual breakpoint type or use-case. This attribute is not
writable.
@end defivar
+@defivar Breakpoint visible
+This attribute holds the breakpoint's visibility flag---the identifier used to
+determine whether the breakpoint is visible to the user when set, or
+when the @samp{info breakpoints} command is run. This attribute is
+not writable.
+@end defivar
+
The available types are represented by constants defined in the @code{gdb}
module:
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 0c70cbf..2eaeb07 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -34,17 +34,6 @@ typedef struct breakpoint_object breakpoint_object;
static PyTypeObject breakpoint_object_type;
-/* A dynamically allocated vector of breakpoint objects. Each
- breakpoint has a number. A breakpoint is valid if its slot in this
- vector is non-null. When a breakpoint is deleted, we drop our
- reference to it and zero its slot; this is how we let the Python
- object have a lifetime which is independent from that of the gdb
- breakpoint. */
-static breakpoint_object **bppy_breakpoints;
-
-/* Number of slots in bppy_breakpoints. */
-static int bppy_slots;
-
/* Number of live breakpoints. */
static int bppy_live;
@@ -68,7 +57,7 @@ struct breakpoint_object
exception if it is invalid. */
#define BPPY_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
} while (0)
@@ -77,7 +66,7 @@ struct breakpoint_object
exception if it is invalid. This macro is for use in setter functions. */
#define BPPY_SET_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
{ \
PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
@@ -115,18 +104,6 @@ static struct pybp_code pybp_watch_types[] =
{NULL} /* Sentinel. */
};
-/* Evaluate to true if the breakpoint NUM is valid, false otherwise. */
-static int
-bpnum_is_valid (int num)
-{
- if (num >=0
- && num < bppy_slots
- && bppy_breakpoints[num] != NULL)
- return 1;
-
- return 0;
-}
-
/* Python function which checks the validity of a breakpoint object. */
static PyObject *
bppy_is_valid (PyObject *self, PyObject *args)
@@ -500,6 +477,21 @@ bppy_get_type (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->type);
}
+/* Python function to get the visibility of the breakpoint. */
+
+static PyObject *
+bppy_get_visibility (PyObject *self, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ if (self_bp->bp->number < 0)
+ Py_RETURN_FALSE;
+
+ Py_RETURN_TRUE;
+}
+
/* Python function to get the breakpoint's number. */
static PyObject *
bppy_get_number (PyObject *self, void *closure)
@@ -566,16 +558,25 @@ static PyObject *
bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
PyObject *result;
- static char *keywords[] = { "spec", "type", "wp_class", NULL };
+ static char *keywords[] = { "spec", "type", "wp_class", "internal", NULL };
char *spec;
int type = bp_breakpoint;
int access_type = hw_write;
+ PyObject *internal = NULL;
+ int internal_bp = 0;
volatile struct gdb_exception except;
- if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|ii", keywords,
- &spec, &type, &access_type))
+ if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords,
+ &spec, &type, &access_type, &internal))
return NULL;
+ if (internal)
+ {
+ internal_bp = PyObject_IsTrue (internal);
+ if (internal_bp == -1)
+ return NULL;
+ }
+
result = subtype->tp_alloc (subtype, 0);
if (! result)
return NULL;
@@ -589,13 +590,13 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
case bp_breakpoint:
{
- create_breakpoint (python_gdbarch,
- spec, NULL, -1,
- 0,
- 0, bp_breakpoint,
- 0,
- AUTO_BOOLEAN_TRUE,
- NULL, 0, 1);
+ create_new_breakpoint (python_gdbarch,
+ spec, NULL, -1,
+ 0,
+ 0, bp_breakpoint,
+ 0,
+ AUTO_BOOLEAN_TRUE,
+ NULL, 0, 1, internal_bp);
break;
}
case bp_watchpoint:
@@ -628,31 +629,50 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
\f
+static int
+build_bp_list (struct breakpoint *b, void *arg)
+{
+ PyObject *list = arg;
+ PyObject *bp = b->py_bp_object;
+ int iserr = 0;
+
+ /* Not all breakpoints will have a companion Python object.
+ Only breakpoints that were created via bppy_new, or
+ breakpoints that were created externally and are tracked by
+ the Python Scripting API. */
+ if (bp)
+ iserr = PyList_Append (list, bp);
+
+ if (iserr == -1)
+ return 1;
+
+ return 0;
+}
+
/* Static function to return a tuple holding all breakpoints. */
PyObject *
gdbpy_breakpoints (PyObject *self, PyObject *args)
{
- PyObject *result;
+ PyObject *list;
if (bppy_live == 0)
Py_RETURN_NONE;
- result = PyTuple_New (bppy_live);
- if (result)
- {
- int i, out = 0;
+ list = PyList_New (0);
+ if (!list)
+ return NULL;
- for (i = 0; out < bppy_live; ++i)
- {
- if (! bppy_breakpoints[i])
- continue;
- Py_INCREF (bppy_breakpoints[i]);
- PyTuple_SetItem (result, out, (PyObject *) bppy_breakpoints[i]);
- ++out;
- }
+ /* If iteratre_over_breakpoints returns non NULL it signals an error
+ condition in our case. In that case abandon building the list and
+ return NULL. */
+ if (iterate_over_breakpoints (build_bp_list, list) != NULL)
+ {
+ Py_DECREF (list);
+ return NULL;
}
- return result;
+
+ return PyList_AsTuple (list);
}
\f
@@ -668,13 +688,13 @@ gdbpy_breakpoint_created (int num)
struct breakpoint *bp = NULL;
PyGILState_STATE state;
- if (num < 0)
- return;
-
bp = get_breakpoint (num);
if (! bp)
return;
+ if (num < 0 && bppy_pending_object == NULL)
+ return;
+
if (bp->type != bp_breakpoint
&& bp->type != bp_watchpoint
&& bp->type != bp_hardware_watchpoint
@@ -682,21 +702,6 @@ gdbpy_breakpoint_created (int num)
&& bp->type != bp_access_watchpoint)
return;
- if (num >= bppy_slots)
- {
- int old = bppy_slots;
-
- bppy_slots = bppy_slots * 2 + 10;
- bppy_breakpoints
- = (breakpoint_object **) xrealloc (bppy_breakpoints,
- (bppy_slots
- * sizeof (breakpoint_object *)));
- memset (&bppy_breakpoints[old], 0,
- (bppy_slots - old) * sizeof (PyObject *));
- }
-
- ++bppy_live;
-
state = PyGILState_Ensure ();
if (bppy_pending_object)
@@ -710,12 +715,16 @@ gdbpy_breakpoint_created (int num)
{
newbp->number = num;
newbp->bp = bp;
- bppy_breakpoints[num] = newbp;
+ newbp->bp->py_bp_object = (PyObject *) newbp;
Py_INCREF (newbp);
+ ++bppy_live;
+ }
+ else
+ {
+ PyErr_SetString (PyExc_RuntimeError,
+ _("Error while creating breakpoint from GDB."));
+ gdbpy_print_stack ();
}
-
- /* Just ignore errors here. */
- PyErr_Clear ();
PyGILState_Release (state);
}
@@ -726,14 +736,20 @@ static void
gdbpy_breakpoint_deleted (int num)
{
PyGILState_STATE state;
+ struct breakpoint *bp = NULL;
+ breakpoint_object *bp_obj;
state = PyGILState_Ensure ();
- if (bpnum_is_valid (num))
+ bp = get_breakpoint (num);
+ if (! bp)
+ return;
+
+ bp_obj = ((breakpoint_object *) bp->py_bp_object);
+ if (bp_obj)
{
- bppy_breakpoints[num]->bp = NULL;
- Py_DECREF (bppy_breakpoints[num]);
- bppy_breakpoints[num] = NULL;
+ bp_obj->bp = NULL;
--bppy_live;
+ Py_DECREF (bp_obj);
}
PyGILState_Release (state);
}
@@ -816,6 +832,8 @@ or None if no condition set."},
"Commands of the breakpoint, as specified by the user."},
{ "type", bppy_get_type, NULL,
"Type of breakpoint."},
+ { "visible", bppy_get_visibility, NULL,
+ "Whether the breakpoint is visible to the user."},
{ NULL } /* Sentinel. */
};
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 7da94c4..9acc433 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -117,6 +117,33 @@ gdb_test "end"
gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" "Get Breakpoint List" 0
gdb_test "python print blist\[len(blist)-1\].commands" "print \"Command for breakpoint has been executed.\".*print result"
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+
+# Test invisible breakpooints.
+delete_breakpoints
+set ibp_location [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "True" "Check breakpoint visibility"
+gdb_test "info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check info breakpoints shows visible breakpoints"
+delete_breakpoints
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=True)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "False" "Check breakpoint visibility"
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check maint info breakpoints shows invisible breakpoints"
+
+
# Watchpoints
# Start with a fresh gdb.
clean_restart ${testfile}
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 56cb8ae..2f2e469 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -39,8 +39,6 @@
#if HAVE_PYTHON
#include "python/python.h"
#include "python/python-internal.h"
-#else
-typedef int PyObject;
#endif
/* Non-zero if we want to see trace of varobj level stuff. */
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-13 12:45 ` Phil Muldoon
@ 2010-10-13 13:06 ` Phil Muldoon
2010-10-13 15:36 ` Tom Tromey
2010-10-16 18:42 ` Pedro Alves
2 siblings, 0 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-10-13 13:06 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches, dan
Phil Muldoon <pmuldoon@redhat.com> writes:
> Tom Tromey <tromey@redhat.com> writes:
>
>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>
>> Phil> In this patch iteration I removed any form of storage/tracking of
>> Phil> breakpoints. Instead, as above, I placed a reference in the breakpoint
>> Phil> struct.
>>
>> I like this better.
>>
>> Phil> + if (internal && PyObject_IsTrue (internal))
>> Phil> + internal_bp = 1;
>>
>> PyObject_IsTrue can return -1 on failure, so this code must account for
>> that.
>
> Oops done. Thanks.
>
>> Phil> + int i = 0;
>> Phil> + ALL_BREAKPOINTS (b)
>>
>> This instance of ALL_BREAKPOINTS should probably be replaced with some
>> kind of callback API, like the other iterate_over_* functions in gdb.
>
> I wrote an iterate function for this, much like the inferior iterator.
> I noticed in py-inferior.c we do not account for an error when adding to a
> list. (I'm pretty sure I wrote that too :( I'll fix this later.
>
>> Phil> + {
>> Phil> + /* Not all breakpoints will have a companion Python object.
>> Phil> + Only breakpoints that were created via bppy_new, or
>> Phil> + breakpoints that were created externally and are tracked by
>> Phil> + the Python Scripting API. */
>> Phil> + if (b->py_bp_object)
>> Phil> + {
>> Phil> + if (PyTuple_SetItem (result, i, (PyObject *) b->py_bp_object) != 0)
>>
>> I think that cast to PyObject is unnecessary.
>
> As we use an iterator, we now set a list then convert it to an iterator
> later. ^^^^^^^^
I mean tuple here. Sorry for the confusion!
Cheers,
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-13 12:45 ` Phil Muldoon
2010-10-13 13:06 ` Phil Muldoon
@ 2010-10-13 15:36 ` Tom Tromey
2010-10-16 18:42 ` Pedro Alves
2 siblings, 0 replies; 31+ messages in thread
From: Tom Tromey @ 2010-10-13 15:36 UTC (permalink / raw)
To: pmuldoon; +Cc: Pedro Alves, gdb-patches, dan
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Phil> Latest patched attached. What do you think?
The Python parts look good to me.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-13 12:45 ` Phil Muldoon
2010-10-13 13:06 ` Phil Muldoon
2010-10-13 15:36 ` Tom Tromey
@ 2010-10-16 18:42 ` Pedro Alves
2 siblings, 0 replies; 31+ messages in thread
From: Pedro Alves @ 2010-10-16 18:42 UTC (permalink / raw)
To: pmuldoon; +Cc: Tom Tromey, gdb-patches, dan
The non-python bits are fine, pending a couple of small issues
I neglected mentioning before. Sorry about that.
> +
> +/* Set a breakpoint. This function is shared between CLI and MI
> + functions for setting a breakpoint. It wraps create_new_breakpoint
> + and never asks for an internal breakpoint number to be allocated
> + against the breakpoint. Returns true if any breakpoint was
> + created; false otherwise. */
> +
> +int
> +create_breakpoint (struct gdbarch *gdbarch,
> + char *arg, char *cond_string, int thread,
> + int parse_condition_and_thread,
> + int tempflag, enum bptype type_wanted,
> + int ignore_count,
> + enum auto_boolean pending_break_support,
> + struct breakpoint_ops *ops,
> + int from_tty,
> + int enabled)
> +{
> + return create_new_breakpoint (gdbarch, arg, cond_string, thread,
> + parse_condition_and_thread, tempflag,
> + type_wanted, ignore_count,
> + pending_break_support,
> + ops, from_tty, enabled, 0);
> +}
Having both create_breakpoint and create_new_breakpoint exported
in breakpoint.h is quite reduntant API-wise, and by just looking at
the .h even confusing, given that "_new_" does not describe
anything that create_breakpoint does not do as well (create_breakpoint
creates a breakpoint, so of course it also creates a _new_ breakpoint!).
Please just add the new flag to create_breakpoint, and do the
trivial update to all its callers. There are only 3 outside
breakpoint.c, I think.
> - set_breakpoint_count (breakpoint_count + 1);
> - b->number = breakpoint_count;
> + if (internal)
> + b->number = internal_breakpoint_number--;
> + else
> + {
> + set_breakpoint_count (breakpoint_count + 1);
> + b->number = breakpoint_count;
> + }
It would be nice if this pattern that now appears several times
in breakpoint.c would be abstrated out into a small helper function.
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-08 14:04 ` Phil Muldoon
2010-10-08 18:44 ` Tom Tromey
@ 2010-10-16 19:03 ` Pedro Alves
2010-10-18 16:09 ` Tom Tromey
1 sibling, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2010-10-16 19:03 UTC (permalink / raw)
To: gdb-patches, pmuldoon; +Cc: dan
Below's just for the record. I'm okay with whatever you guys
have decided. ;-)
On Friday 08 October 2010 15:04:37, Phil Muldoon wrote:
> Pedro Alves <pedro@codesourcery.com> writes:
>
> > On Friday 08 October 2010 13:50:34, Phil Muldoon wrote:
> >> The @var{internal} argument has no effect with watchpoints.
> >
> > Should it be an error instead (or made to work)?
>
> I can make it an error. I decided not to do watchpoints.
IMO, either of these would be better API than a silent ignore.
If you go with error, the client of the API can
retry a non-internal breakpoint after e.g., printing a warning,
say. As is, if you ever add support for internal watchpoints,
then a script has no way to tell whether such internal breakpoints
are actually supported by the gdb at hand until trying: if the flag
is just ignored, too late, the user has already seen the
breakpoint being created...
> The
> consequences of setting hidden watchpoints from a script that could turn
> out to be software watchpoints seemed a little troubling.
IMO, it should the script writers' responsibility to weight that,
but it's also okay to not support it until someone asks for it
(though that someone would be happier if she didn't have to,
obviously :-) ).
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-16 19:03 ` Pedro Alves
@ 2010-10-18 16:09 ` Tom Tromey
2010-10-22 21:05 ` Phil Muldoon
0 siblings, 1 reply; 31+ messages in thread
From: Tom Tromey @ 2010-10-18 16:09 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, pmuldoon, dan
>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
Pedro> Should it be an error instead (or made to work)?
Phil> I can make it an error. I decided not to do watchpoints.
Pedro> IMO, either of these would be better API than a silent ignore.
I agree.
Phil> The consequences of setting hidden watchpoints from a script that
Phil> could turn out to be software watchpoints seemed a little
Phil> troubling.
Pedro> IMO, it should the script writers' responsibility to weight that,
Pedro> but it's also okay to not support it until someone asks for it
Pedro> (though that someone would be happier if she didn't have to,
Pedro> obviously :-) ).
Yes, I agree with this as well.
Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-18 16:09 ` Tom Tromey
@ 2010-10-22 21:05 ` Phil Muldoon
2010-10-22 21:31 ` Eli Zaretskii
2010-10-31 21:07 ` Pedro Alves
0 siblings, 2 replies; 31+ messages in thread
From: Phil Muldoon @ 2010-10-22 21:05 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches, dan
Tom Tromey <tromey@redhat.com> writes:
>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>
> Pedro> Should it be an error instead (or made to work)?
>
> Phil> I can make it an error. I decided not to do watchpoints.
>
> Pedro> IMO, either of these would be better API than a silent ignore.
>
> I agree.
>
> Phil> The consequences of setting hidden watchpoints from a script that
> Phil> could turn out to be software watchpoints seemed a little
> Phil> troubling.
>
> Pedro> IMO, it should the script writers' responsibility to weight that,
> Pedro> but it's also okay to not support it until someone asks for it
> Pedro> (though that someone would be happier if she didn't have to,
> Pedro> obviously :-) ).
>
> Yes, I agree with this as well.
I decided to add internal watchpoint support. I've also added the other
changes requested.
OK?
Cheers,
Phil
--
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4502e7..e68c5c8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2068,6 +2068,24 @@ reattach_breakpoints (int pid)
static int internal_breakpoint_number = -1;
+/* Set the breakpoint number of B, depending on the value of INTERNAL.
+ If INTERNAL is non-zero, the breakpoint number will be populated
+ from internal_breakpoint_number and that variable decremented.
+ Otherwise the breakpoint number will be populated from
+ breakpoint_count and that value incremented. Internal breakpoints
+ do not set the internal var bpnum. */
+static void
+set_breakpoint_number (int internal, struct breakpoint *b)
+{
+ if (internal)
+ b->number = internal_breakpoint_number--;
+ else
+ {
+ set_breakpoint_count (breakpoint_count + 1);
+ b->number = breakpoint_count;
+ }
+}
+
static struct breakpoint *
create_internal_breakpoint (struct gdbarch *gdbarch,
CORE_ADDR address, enum bptype type)
@@ -4939,7 +4957,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
if (filter && !filter (b))
continue;
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
{
int addr_bit, type_len;
@@ -5007,7 +5026,8 @@ breakpoint_1 (int bnum, int allflag, int (*filter) (const struct breakpoint *))
/* We only print out user settable breakpoints unless the
allflag is set. */
- if (allflag || user_settable_breakpoint (b))
+ if (allflag || (user_settable_breakpoint (b)
+ && b->number > 0))
print_one_breakpoint (b, &last_loc, print_address_bits, allflag);
}
}
@@ -5456,6 +5476,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
b->syscalls_to_be_caught = NULL;
b->ops = NULL;
b->condition_not_parsed = 0;
+ b->py_bp_object = NULL;
/* Add this breakpoint to the end of the chain
so that a list of breakpoints will come out in order
@@ -6914,7 +6935,8 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
char *cond_string,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
- struct breakpoint_ops *ops, int from_tty, int enabled)
+ struct breakpoint_ops *ops, int from_tty,
+ int enabled, int internal)
{
struct breakpoint *b = NULL;
int i;
@@ -6951,8 +6973,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
if (i == 0)
{
b = set_raw_breakpoint (gdbarch, sal, type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ set_breakpoint_number (internal, b);
b->thread = thread;
b->task = task;
@@ -7034,7 +7055,12 @@ Couldn't determine the static tracepoint marker to probe"));
= xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
b->ops = ops;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
/* Remove element at INDEX_TO_REMOVE from SAL, shifting other
@@ -7190,7 +7216,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
enum bptype type, enum bpdisp disposition,
int thread, int task, int ignore_count,
struct breakpoint_ops *ops, int from_tty,
- int enabled)
+ int enabled, int internal)
{
int i;
@@ -7201,7 +7227,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
create_breakpoint_sal (gdbarch, expanded, addr_string[i],
cond_string, type, disposition,
- thread, task, ignore_count, ops, from_tty, enabled);
+ thread, task, ignore_count, ops,
+ from_tty, enabled, internal);
}
}
@@ -7464,15 +7491,16 @@ decode_static_tracepoint_spec (char **arg_p)
return sals;
}
-/* Set a breakpoint. This function is shared between CLI and MI
+/* Set a breakpoint. This function is shared between CLI and MI
functions for setting a breakpoint. This function has two major
modes of operations, selected by the PARSE_CONDITION_AND_THREAD
parameter. If non-zero, the function will parse arg, extracting
breakpoint location, address and thread. Otherwise, ARG is just the
location of breakpoint, with condition and thread specified by the
- COND_STRING and THREAD parameters. Returns true if any breakpoint
- was created; false otherwise. */
-
+ COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
+ breakpoint number will be allocated from the internal breakpoint
+ count. Returns true if any breakpoint was created; false
+ otherwise. */
int
create_breakpoint (struct gdbarch *gdbarch,
char *arg, char *cond_string, int thread,
@@ -7481,8 +7509,7 @@ create_breakpoint (struct gdbarch *gdbarch,
int ignore_count,
enum auto_boolean pending_break_support,
struct breakpoint_ops *ops,
- int from_tty,
- int enabled)
+ int from_tty, int enabled, int internal)
{
struct gdb_exception e;
struct symtabs_and_lines sals;
@@ -7658,12 +7685,15 @@ create_breakpoint (struct gdbarch *gdbarch,
cond_string, type_wanted,
tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops,
- from_tty, enabled);
+ from_tty, enabled, internal);
do_cleanups (old_chain);
/* Get the tracepoint we just created. */
- tp = get_breakpoint (breakpoint_count);
+ if (internal)
+ tp = get_breakpoint (internal_breakpoint_number);
+ else
+ tp = get_breakpoint (breakpoint_count);
gdb_assert (tp != NULL);
/* Given that its possible to have multiple markers with
@@ -7679,7 +7709,7 @@ create_breakpoint (struct gdbarch *gdbarch,
create_breakpoints_sal (gdbarch, sals, addr_string, cond_string,
type_wanted, tempflag ? disp_del : disp_donttouch,
thread, task, ignore_count, ops, from_tty,
- enabled);
+ enabled, internal);
}
else
{
@@ -7688,8 +7718,7 @@ create_breakpoint (struct gdbarch *gdbarch,
make_cleanup (xfree, copy_arg);
b = set_raw_breakpoint_without_location (gdbarch, type_wanted);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ set_breakpoint_number (internal, b);
b->thread = -1;
b->addr_string = addr_string[0];
b->cond_string = NULL;
@@ -7699,13 +7728,19 @@ create_breakpoint (struct gdbarch *gdbarch,
b->ops = ops;
b->enable_state = enabled ? bp_enabled : bp_disabled;
b->pspace = current_program_space;
+ b->py_bp_object = NULL;
if (enabled && b->pspace->executing_startup
&& (b->type == bp_breakpoint
|| b->type == bp_hardware_breakpoint))
b->enable_state = bp_startup_disabled;
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
}
if (sals.nelts > 1)
@@ -7750,7 +7785,8 @@ break_command_1 (char *arg, int flag, int from_tty)
pending_break_support,
NULL /* breakpoint_ops */,
from_tty,
- 1 /* enabled */);
+ 1 /* enabled */,
+ 0 /* internal */);
}
@@ -8017,7 +8053,8 @@ watchpoint_exp_is_const (const struct expression *exp)
hw_read: watch read,
hw_access: watch access (read or write) */
static void
-watch_command_1 (char *arg, int accessflag, int from_tty, int just_location)
+watch_command_1 (char *arg, int accessflag, int from_tty,
+ int just_location, int internal)
{
struct breakpoint *b, *scope_breakpoint = NULL;
struct expression *exp;
@@ -8225,8 +8262,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty, int just_location)
/* Now set up the breakpoint. */
b = set_raw_breakpoint_without_location (NULL, bp_type);
- set_breakpoint_count (breakpoint_count + 1);
- b->number = breakpoint_count;
+ set_breakpoint_number (internal, b);
b->thread = thread;
b->disposition = disp_donttouch;
b->exp = exp;
@@ -8285,8 +8321,12 @@ watch_command_1 (char *arg, int accessflag, int from_tty, int just_location)
/* Finally update the new watchpoint. This creates the locations
that should be inserted. */
update_watchpoint (b, 1);
-
- mention (b);
+ if (internal)
+ /* Do not mention breakpoints with a negative number, but do
+ notify observers. */
+ observer_notify_breakpoint_created (b->number);
+ else
+ mention (b);
update_global_location_list (1);
}
@@ -8370,9 +8410,9 @@ can_use_hardware_watchpoint (struct value *v)
}
void
-watch_command_wrapper (char *arg, int from_tty)
+watch_command_wrapper (char *arg, int from_tty, int internal)
{
- watch_command_1 (arg, hw_write, from_tty, 0);
+ watch_command_1 (arg, hw_write, from_tty, 0, internal);
}
/* A helper function that looks for an argument at the start of a
@@ -8408,7 +8448,7 @@ watch_maybe_just_location (char *arg, int accessflag, int from_tty)
just_location = 1;
}
- watch_command_1 (arg, accessflag, from_tty, just_location);
+ watch_command_1 (arg, accessflag, from_tty, just_location, 0);
}
static void
@@ -8418,9 +8458,9 @@ watch_command (char *arg, int from_tty)
}
void
-rwatch_command_wrapper (char *arg, int from_tty)
+rwatch_command_wrapper (char *arg, int from_tty, int internal)
{
- watch_command_1 (arg, hw_read, from_tty, 0);
+ watch_command_1 (arg, hw_read, from_tty, 0, internal);
}
static void
@@ -8430,9 +8470,9 @@ rwatch_command (char *arg, int from_tty)
}
void
-awatch_command_wrapper (char *arg, int from_tty)
+awatch_command_wrapper (char *arg, int from_tty, int internal)
{
- watch_command_1 (arg, hw_access, from_tty, 0);
+ watch_command_1 (arg, hw_access, from_tty, 0, internal);
}
static void
@@ -8790,7 +8830,8 @@ handle_gnu_v3_exceptions (int tempflag, char *cond_string,
0,
AUTO_BOOLEAN_TRUE /* pending */,
&gnu_v3_exception_catchpoint_ops, from_tty,
- 1 /* enabled */);
+ 1 /* enabled */,
+ 0 /* internal */);
return 1;
}
@@ -11010,7 +11051,8 @@ trace_command (char *arg, int from_tty)
pending_break_support,
NULL,
from_tty,
- 1 /* enabled */))
+ 1 /* enabled */,
+ 0 /* internal */))
set_tracepoint_count (breakpoint_count);
}
@@ -11026,7 +11068,8 @@ ftrace_command (char *arg, int from_tty)
pending_break_support,
NULL,
from_tty,
- 1 /* enabled */))
+ 1 /* enabled */,
+ 0 /* internal */))
set_tracepoint_count (breakpoint_count);
}
@@ -11044,7 +11087,8 @@ strace_command (char *arg, int from_tty)
pending_break_support,
NULL,
from_tty,
- 1 /* enabled */))
+ 1 /* enabled */,
+ 0 /* internal */))
set_tracepoint_count (breakpoint_count);
}
@@ -11106,7 +11150,8 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
pending_break_support,
NULL,
0 /* from_tty */,
- utp->enabled /* enabled */))
+ utp->enabled /* enabled */,
+ 0 /* internal */))
return NULL;
set_tracepoint_count (breakpoint_count);
@@ -11372,7 +11417,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11410,7 +11455,7 @@ save_breakpoints (char *filename, int from_tty,
ALL_BREAKPOINTS (tp)
{
/* Skip internal and momentary breakpoints. */
- if (!user_settable_breakpoint (tp))
+ if (!user_settable_breakpoint (tp) || tp->number < 0)
continue;
/* If we have a filter, only save the breakpoints it accepts. */
@@ -11627,6 +11672,21 @@ save_command (char *arg, int from_tty)
help_list (save_cmdlist, "save ", -1, gdb_stdout);
}
+struct breakpoint *
+iterate_over_breakpoints (int (*callback) (struct breakpoint *, void *),
+ void *data)
+{
+ struct breakpoint *b, *temp;
+
+ ALL_BREAKPOINTS_SAFE (b, temp)
+ {
+ if ((*callback) (b, data))
+ return b;
+ }
+
+ return NULL;
+}
+
void
_initialize_breakpoint (void)
{
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9f7600a..925193b 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -24,6 +24,11 @@
#include "value.h"
#include "vec.h"
+#if HAVE_PYTHON
+#include "python/python.h"
+#include "python/python-internal.h"
+#endif
+
struct value;
struct block;
@@ -557,7 +562,14 @@ struct breakpoint
breakpoints, we will use this index to try to find the same
marker again. */
int static_trace_marker_id_idx;
- };
+
+ /* With a Python scripting enabled GDB, store a reference to the
+ Python object that has been associated with this breakpoint.
+ This is always NULL for a GDB that is not script enabled. It
+ can sometimes be NULL for enabled GDBs as not all breakpoint
+ types are tracked by the Python scripting API. */
+ PyObject *py_bp_object;
+};
typedef struct breakpoint *breakpoint_p;
DEF_VEC_P(breakpoint_p);
@@ -855,20 +867,22 @@ extern void break_command (char *, int);
extern void hbreak_command_wrapper (char *, int);
extern void thbreak_command_wrapper (char *, int);
extern void rbreak_command_wrapper (char *, int);
-extern void watch_command_wrapper (char *, int);
-extern void awatch_command_wrapper (char *, int);
-extern void rwatch_command_wrapper (char *, int);
+extern void watch_command_wrapper (char *, int, int);
+extern void awatch_command_wrapper (char *, int, int);
+extern void rwatch_command_wrapper (char *, int, int);
extern void tbreak_command (char *, int);
extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
- char *cond_string, int thread,
- int parse_condition_and_thread,
- int tempflag, enum bptype wanted_type,
- int ignore_count,
- enum auto_boolean pending_break_support,
- struct breakpoint_ops *ops,
- int from_tty,
- int enabled);
+ char *cond_string, int thread,
+ int parse_condition_and_thread,
+ int tempflag,
+ enum bptype type_wanted,
+ int ignore_count,
+ enum auto_boolean pending_break_support,
+ struct breakpoint_ops *ops,
+ int from_tty,
+ int enabled,
+ int internal);
extern void insert_breakpoints (void);
@@ -1101,4 +1115,15 @@ extern void check_tracepoint_command (char *line, void *closure);
extern void start_rbreak_breakpoints (void);
extern void end_rbreak_breakpoints (void);
+/* Breakpoint iterator function.
+
+ Calls a callback function once for each breakpoint, so long as the
+ callback function returns false. If the callback function returns
+ true, the iteration will end and the current breakpoint will be
+ returned. This can be useful for implementing a search for a
+ breakpoint with arbitrary attributes, or for applying an operation
+ to every breakpoint. */
+extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *,
+ void *), void *);
+
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..643338e 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -1240,4 +1240,8 @@ void dummy_obstack_deallocate (void *object, void *data);
extern void initialize_progspace (void);
extern void initialize_inferiors (void);
+#ifndef HAVE_PYTHON
+typedef int PyObject;
+#endif
+
#endif /* #ifndef DEFS_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9446932..d0db93a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22556,7 +22556,7 @@ Return the symbol table's source absolute file name.
Python code can manipulate breakpoints via the @code{gdb.Breakpoint}
class.
-@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]}
+@defmethod Breakpoint __init__ spec @r{[}type@r{]} @r{[}wp_class@r{]} @r{[}internal@r{]}
Create a new breakpoint. @var{spec} is a string naming the
location of the breakpoint, or an expression that defines a
watchpoint. The contents can be any location recognized by the
@@ -22564,7 +22564,11 @@ watchpoint. The contents can be any location recognized by the
command. The optional @var{type} denotes the breakpoint to create
from the types defined later in this chapter. This argument can be
either: @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}. @var{type}
-defaults to @code{BP_BREAKPOINT}. The optional @var{wp_class}
+defaults to @code{BP_BREAKPOINT}. The optional @var{internal} argument
+allows the breakpoint to become invisible to the user. The breakpoint
+will neither be reported when created, nor will it be listed in the
+output from @code{info breakpoints} (but will be listed with the
+@code{maint info breakpoints} command). The optional @var{wp_class}
argument defines the class of watchpoint to create, if @var{type} is
defined as @code{BP_WATCHPOINT}. If a watchpoint class is not
provided, it is assumed to be a @var{WP_WRITE} class.
@@ -22642,6 +22646,13 @@ determine the actual breakpoint type or use-case. This attribute is not
writable.
@end defivar
+@defivar Breakpoint visible
+This attribute holds the breakpoint's visibility flag---the identifier used to
+determine whether the breakpoint is visible to the user when set, or
+when the @samp{info breakpoints} command is run. This attribute is
+not writable.
+@end defivar
+
The available types are represented by constants defined in the @code{gdb}
module:
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 3408244..2aad420 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -169,7 +169,7 @@ mi_cmd_break_insert (char *command, char **argv, int argc)
temp_p, type_wanted,
ignore_count,
pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
- NULL, 0, enabled);
+ NULL, 0, enabled, 0);
do_cleanups (back_to);
}
@@ -259,13 +259,13 @@ mi_cmd_break_watch (char *command, char **argv, int argc)
switch (type)
{
case REG_WP:
- watch_command_wrapper (expr, FROM_TTY);
+ watch_command_wrapper (expr, FROM_TTY, 0);
break;
case READ_WP:
- rwatch_command_wrapper (expr, FROM_TTY);
+ rwatch_command_wrapper (expr, FROM_TTY, 0);
break;
case ACCESS_WP:
- awatch_command_wrapper (expr, FROM_TTY);
+ awatch_command_wrapper (expr, FROM_TTY, 0);
break;
default:
error (_("mi_cmd_break_watch: Unknown watchpoint type."));
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index b18f7f3..36153ad 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -34,17 +34,6 @@ typedef struct breakpoint_object breakpoint_object;
static PyTypeObject breakpoint_object_type;
-/* A dynamically allocated vector of breakpoint objects. Each
- breakpoint has a number. A breakpoint is valid if its slot in this
- vector is non-null. When a breakpoint is deleted, we drop our
- reference to it and zero its slot; this is how we let the Python
- object have a lifetime which is independent from that of the gdb
- breakpoint. */
-static breakpoint_object **bppy_breakpoints;
-
-/* Number of slots in bppy_breakpoints. */
-static int bppy_slots;
-
/* Number of live breakpoints. */
static int bppy_live;
@@ -68,7 +57,7 @@ struct breakpoint_object
exception if it is invalid. */
#define BPPY_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
return PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
} while (0)
@@ -77,7 +66,7 @@ struct breakpoint_object
exception if it is invalid. This macro is for use in setter functions. */
#define BPPY_SET_REQUIRE_VALID(Breakpoint) \
do { \
- if (! bpnum_is_valid ((Breakpoint)->number)) \
+ if ((Breakpoint)->bp == NULL) \
{ \
PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
(Breakpoint)->number); \
@@ -115,18 +104,6 @@ static struct pybp_code pybp_watch_types[] =
{NULL} /* Sentinel. */
};
-/* Evaluate to true if the breakpoint NUM is valid, false otherwise. */
-static int
-bpnum_is_valid (int num)
-{
- if (num >=0
- && num < bppy_slots
- && bppy_breakpoints[num] != NULL)
- return 1;
-
- return 0;
-}
-
/* Python function which checks the validity of a breakpoint object. */
static PyObject *
bppy_is_valid (PyObject *self, PyObject *args)
@@ -503,6 +480,21 @@ bppy_get_type (PyObject *self, void *closure)
return PyInt_FromLong (self_bp->bp->type);
}
+/* Python function to get the visibility of the breakpoint. */
+
+static PyObject *
+bppy_get_visibility (PyObject *self, void *closure)
+{
+ breakpoint_object *self_bp = (breakpoint_object *) self;
+
+ BPPY_REQUIRE_VALID (self_bp);
+
+ if (self_bp->bp->number < 0)
+ Py_RETURN_FALSE;
+
+ Py_RETURN_TRUE;
+}
+
/* Python function to get the breakpoint's number. */
static PyObject *
bppy_get_number (PyObject *self, void *closure)
@@ -569,16 +561,25 @@ static PyObject *
bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
{
PyObject *result;
- static char *keywords[] = { "spec", "type", "wp_class", NULL };
+ static char *keywords[] = { "spec", "type", "wp_class", "internal", NULL };
char *spec;
int type = bp_breakpoint;
int access_type = hw_write;
+ PyObject *internal = NULL;
+ int internal_bp = 0;
volatile struct gdb_exception except;
- if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|ii", keywords,
- &spec, &type, &access_type))
+ if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiO", keywords,
+ &spec, &type, &access_type, &internal))
return NULL;
+ if (internal)
+ {
+ internal_bp = PyObject_IsTrue (internal);
+ if (internal_bp == -1)
+ return NULL;
+ }
+
result = subtype->tp_alloc (subtype, 0);
if (! result)
return NULL;
@@ -593,22 +594,22 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
case bp_breakpoint:
{
create_breakpoint (python_gdbarch,
- spec, NULL, -1,
- 0,
- 0, bp_breakpoint,
- 0,
- AUTO_BOOLEAN_TRUE,
- NULL, 0, 1);
+ spec, NULL, -1,
+ 0,
+ 0, bp_breakpoint,
+ 0,
+ AUTO_BOOLEAN_TRUE,
+ NULL, 0, 1, internal_bp);
break;
}
case bp_watchpoint:
{
if (access_type == hw_write)
- watch_command_wrapper (spec, 0);
+ watch_command_wrapper (spec, 0, internal_bp);
else if (access_type == hw_access)
- awatch_command_wrapper (spec, 0);
+ awatch_command_wrapper (spec, 0, internal_bp);
else if (access_type == hw_read)
- rwatch_command_wrapper (spec, 0);
+ rwatch_command_wrapper (spec, 0, internal_bp);
else
error(_("Cannot understand watchpoint access type."));
break;
@@ -631,31 +632,50 @@ bppy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
\f
+static int
+build_bp_list (struct breakpoint *b, void *arg)
+{
+ PyObject *list = arg;
+ PyObject *bp = b->py_bp_object;
+ int iserr = 0;
+
+ /* Not all breakpoints will have a companion Python object.
+ Only breakpoints that were created via bppy_new, or
+ breakpoints that were created externally and are tracked by
+ the Python Scripting API. */
+ if (bp)
+ iserr = PyList_Append (list, bp);
+
+ if (iserr == -1)
+ return 1;
+
+ return 0;
+}
+
/* Static function to return a tuple holding all breakpoints. */
PyObject *
gdbpy_breakpoints (PyObject *self, PyObject *args)
{
- PyObject *result;
+ PyObject *list;
if (bppy_live == 0)
Py_RETURN_NONE;
- result = PyTuple_New (bppy_live);
- if (result)
- {
- int i, out = 0;
+ list = PyList_New (0);
+ if (!list)
+ return NULL;
- for (i = 0; out < bppy_live; ++i)
- {
- if (! bppy_breakpoints[i])
- continue;
- Py_INCREF (bppy_breakpoints[i]);
- PyTuple_SetItem (result, out, (PyObject *) bppy_breakpoints[i]);
- ++out;
- }
+ /* If iteratre_over_breakpoints returns non NULL it signals an error
+ condition. In that case abandon building the list and return
+ NULL. */
+ if (iterate_over_breakpoints (build_bp_list, list) != NULL)
+ {
+ Py_DECREF (list);
+ return NULL;
}
- return result;
+
+ return PyList_AsTuple (list);
}
\f
@@ -671,13 +691,13 @@ gdbpy_breakpoint_created (int num)
struct breakpoint *bp = NULL;
PyGILState_STATE state;
- if (num < 0)
- return;
-
bp = get_breakpoint (num);
if (! bp)
return;
+ if (num < 0 && bppy_pending_object == NULL)
+ return;
+
if (bp->type != bp_breakpoint
&& bp->type != bp_watchpoint
&& bp->type != bp_hardware_watchpoint
@@ -685,21 +705,6 @@ gdbpy_breakpoint_created (int num)
&& bp->type != bp_access_watchpoint)
return;
- if (num >= bppy_slots)
- {
- int old = bppy_slots;
-
- bppy_slots = bppy_slots * 2 + 10;
- bppy_breakpoints
- = (breakpoint_object **) xrealloc (bppy_breakpoints,
- (bppy_slots
- * sizeof (breakpoint_object *)));
- memset (&bppy_breakpoints[old], 0,
- (bppy_slots - old) * sizeof (PyObject *));
- }
-
- ++bppy_live;
-
state = PyGILState_Ensure ();
if (bppy_pending_object)
@@ -713,12 +718,16 @@ gdbpy_breakpoint_created (int num)
{
newbp->number = num;
newbp->bp = bp;
- bppy_breakpoints[num] = newbp;
+ newbp->bp->py_bp_object = (PyObject *) newbp;
Py_INCREF (newbp);
+ ++bppy_live;
+ }
+ else
+ {
+ PyErr_SetString (PyExc_RuntimeError,
+ _("Error while creating breakpoint from GDB."));
+ gdbpy_print_stack ();
}
-
- /* Just ignore errors here. */
- PyErr_Clear ();
PyGILState_Release (state);
}
@@ -729,14 +738,20 @@ static void
gdbpy_breakpoint_deleted (int num)
{
PyGILState_STATE state;
+ struct breakpoint *bp = NULL;
+ breakpoint_object *bp_obj;
state = PyGILState_Ensure ();
- if (bpnum_is_valid (num))
+ bp = get_breakpoint (num);
+ if (! bp)
+ return;
+
+ bp_obj = ((breakpoint_object *) bp->py_bp_object);
+ if (bp_obj)
{
- bppy_breakpoints[num]->bp = NULL;
- Py_DECREF (bppy_breakpoints[num]);
- bppy_breakpoints[num] = NULL;
+ bp_obj->bp = NULL;
--bppy_live;
+ Py_DECREF (bp_obj);
}
PyGILState_Release (state);
}
@@ -819,6 +834,8 @@ or None if no condition set."},
"Commands of the breakpoint, as specified by the user."},
{ "type", bppy_get_type, NULL,
"Type of breakpoint."},
+ { "visible", bppy_get_visibility, NULL,
+ "Whether the breakpoint is visible to the user."},
{ NULL } /* Sentinel. */
};
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index b34833c..0e73d92 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1895,7 +1895,8 @@ spu_catch_start (struct objfile *objfile)
bp_breakpoint /* type_wanted */,
0 /* ignore_count */,
AUTO_BOOLEAN_FALSE /* pending_break_support */,
- NULL /* ops */, 0 /* from_tty */, 1 /* enabled */);
+ NULL /* ops */, 0 /* from_tty */, 1 /* enabled */,
+ 0 /* internal */););
}
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 7da94c4..d030b55 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -117,6 +117,33 @@ gdb_test "end"
gdb_py_test_silent_cmd "python blist = gdb.breakpoints()" "Get Breakpoint List" 0
gdb_test "python print blist\[len(blist)-1\].commands" "print \"Command for breakpoint has been executed.\".*print result"
+# Start with a fresh gdb.
+clean_restart ${testfile}
+
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+
+# Test invisible breakpooints.
+delete_breakpoints
+set ibp_location [gdb_get_line_number "Break at multiply."]
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=False)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "True" "Check breakpoint visibility"
+gdb_test "info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check info breakpoints shows visible breakpoints"
+delete_breakpoints
+gdb_py_test_silent_cmd "python ibp = gdb.Breakpoint(\"$ibp_location\", internal=True)" "Set invisible breakpoint" 0
+gdb_py_test_silent_cmd "python ilist = gdb.breakpoints()" "Get Breakpoint List" 0
+gdb_test "python print ilist\[0\]" "<gdb.Breakpoint object at $hex>" "Check invisible bp obj exists"
+gdb_test "python print ilist\[0\].location" "py-breakpoint\.c:$ibp_location*" "Check breakpoint location"
+gdb_test "python print ilist\[0\].visible" "False" "Check breakpoint visibility"
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" "py-breakpoint\.c:$ibp_location.*" "Check maint info breakpoints shows invisible breakpoints"
+
+
# Watchpoints
# Start with a fresh gdb.
clean_restart ${testfile}
@@ -134,5 +161,17 @@ if ![runto_main] then {
gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE )" "Set watchpoint" 0
gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*main.*" "Test watchpoint write"
+# Internal breakpoints.
+# Start with a fresh gdb.
+clean_restart ${testfile}
+if ![runto_main] then {
+ fail "Cannot run to main."
+ return 0
+}
+delete_breakpoints
+gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, internal=True )" "Set watchpoint" 0
+gdb_test "info breakpoints" "No breakpoints or watchpoints.*" "Check info breakpoints does not show invisible breakpoints"
+gdb_test "maint info breakpoints" ".*hw watchpoint.*result.*" "Check maint info breakpoints shows invisible breakpoints"
+gdb_test "continue" ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*" "Test watchpoint write"
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 09f91eb..9cff971 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -39,8 +39,6 @@
#if HAVE_PYTHON
#include "python/python.h"
#include "python/python-internal.h"
-#else
-typedef int PyObject;
#endif
/* Non-zero if we want to see trace of varobj level stuff. */
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-22 21:05 ` Phil Muldoon
@ 2010-10-22 21:31 ` Eli Zaretskii
2010-10-22 21:37 ` Phil Muldoon
2010-10-31 21:07 ` Pedro Alves
1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2010-10-22 21:31 UTC (permalink / raw)
To: pmuldoon; +Cc: tromey, pedro, gdb-patches, dan
> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org, dan@codesourcery.com
> Date: Fri, 22 Oct 2010 22:05:30 +0100
>
> I decided to add internal watchpoint support. I've also added the other
> changes requested.
>
> OK?
Thanks.
> -/* Set a breakpoint. This function is shared between CLI and MI
> +/* Set a breakpoint. This function is shared between CLI and MI
Why did you need to remove one of the two spaces between these
sentences?
> functions for setting a breakpoint. This function has two major
> modes of operations, selected by the PARSE_CONDITION_AND_THREAD
> parameter. If non-zero, the function will parse arg, extracting
> breakpoint location, address and thread. Otherwise, ARG is just the
> location of breakpoint, with condition and thread specified by the
> - COND_STRING and THREAD parameters. Returns true if any breakpoint
> - was created; false otherwise. */
> -
> + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
> + breakpoint number will be allocated from the internal breakpoint
> + count. Returns true if any breakpoint was created; false
^^^
And here you have 3 instead of 2.
> +@code{maint info breakpoints} command). The optional @var{wp_class}
> argument defines the class of watchpoint to create, if @var{type} is
> defined as @code{BP_WATCHPOINT}.
"if @var{type} is @code{BP_WATCHPOINT}" is simpler and more clear, IMO.
> +@defivar Breakpoint visible
> +This attribute holds the breakpoint's visibility flag---the identifier used to
> +determine whether the breakpoint is visible to the user when set, or
> +when the @samp{info breakpoints} command is run.
There's no need to go through identifiers. How about this rewording?
This attribute tells whether the breakpoint is visible to the user
when set, or when the @samp{info breakpoints} command is run.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-22 21:31 ` Eli Zaretskii
@ 2010-10-22 21:37 ` Phil Muldoon
2010-10-23 9:07 ` Eli Zaretskii
0 siblings, 1 reply; 31+ messages in thread
From: Phil Muldoon @ 2010-10-22 21:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: tromey, pedro, gdb-patches, dan
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Phil Muldoon <pmuldoon@redhat.com>
>> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org, dan@codesourcery.com
>> Date: Fri, 22 Oct 2010 22:05:30 +0100
>>
>> I decided to add internal watchpoint support. I've also added the other
>> changes requested.
>>
>> OK?
>
> Thanks.
>
>> -/* Set a breakpoint. This function is shared between CLI and MI
>> +/* Set a breakpoint. This function is shared between CLI and MI
>
> Why did you need to remove one of the two spaces between these
> sentences?
I didn't, this was an error. Thanks for catching.
>
>> functions for setting a breakpoint. This function has two major
>> modes of operations, selected by the PARSE_CONDITION_AND_THREAD
>> parameter. If non-zero, the function will parse arg, extracting
>> breakpoint location, address and thread. Otherwise, ARG is just the
>> location of breakpoint, with condition and thread specified by the
>> - COND_STRING and THREAD parameters. Returns true if any breakpoint
>> - was created; false otherwise. */
>> -
>> + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
>> + breakpoint number will be allocated from the internal breakpoint
>> + count. Returns true if any breakpoint was created; false
> ^^^
> And here you have 3 instead of 2.
Thanks
>> +@code{maint info breakpoints} command). The optional @var{wp_class}
>> argument defines the class of watchpoint to create, if @var{type} is
>> defined as @code{BP_WATCHPOINT}.
>
> "if @var{type} is @code{BP_WATCHPOINT}" is simpler and more clear, IMO.
Thanks.
>> +@defivar Breakpoint visible
>> +This attribute holds the breakpoint's visibility flag---the identifier used to
>> +determine whether the breakpoint is visible to the user when set, or
>> +when the @samp{info breakpoints} command is run.
>
> There's no need to go through identifiers. How about this rewording?
>
> This attribute tells whether the breakpoint is visible to the user
> when set, or when the @samp{info breakpoints} command is run.
>
Works for me.
Cheers,
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-22 21:37 ` Phil Muldoon
@ 2010-10-23 9:07 ` Eli Zaretskii
0 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2010-10-23 9:07 UTC (permalink / raw)
To: pmuldoon; +Cc: tromey, pedro, gdb-patches, dan
> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: tromey@redhat.com, pedro@codesourcery.com, gdb-patches@sourceware.org,
> dan@codesourcery.com
> Date: Fri, 22 Oct 2010 22:37:19 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Phil Muldoon <pmuldoon@redhat.com>
> >> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org, dan@codesourcery.com
> >> Date: Fri, 22 Oct 2010 22:05:30 +0100
> >>
> >> I decided to add internal watchpoint support. I've also added the other
> >> changes requested.
> >>
> >> OK?
> >
> > Thanks.
> >
> >> -/* Set a breakpoint. This function is shared between CLI and MI
> >> +/* Set a breakpoint. This function is shared between CLI and MI
> >
> > Why did you need to remove one of the two spaces between these
> > sentences?
>
> I didn't, this was an error. Thanks for catching.
>
> >
> >> functions for setting a breakpoint. This function has two major
> >> modes of operations, selected by the PARSE_CONDITION_AND_THREAD
> >> parameter. If non-zero, the function will parse arg, extracting
> >> breakpoint location, address and thread. Otherwise, ARG is just the
> >> location of breakpoint, with condition and thread specified by the
> >> - COND_STRING and THREAD parameters. Returns true if any breakpoint
> >> - was created; false otherwise. */
> >> -
> >> + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
> >> + breakpoint number will be allocated from the internal breakpoint
> >> + count. Returns true if any breakpoint was created; false
> > ^^^
> > And here you have 3 instead of 2.
>
> Thanks
>
> >> +@code{maint info breakpoints} command). The optional @var{wp_class}
> >> argument defines the class of watchpoint to create, if @var{type} is
> >> defined as @code{BP_WATCHPOINT}.
> >
> > "if @var{type} is @code{BP_WATCHPOINT}" is simpler and more clear, IMO.
>
> Thanks.
>
> >> +@defivar Breakpoint visible
> >> +This attribute holds the breakpoint's visibility flag---the identifier used to
> >> +determine whether the breakpoint is visible to the user when set, or
> >> +when the @samp{info breakpoints} command is run.
> >
> > There's no need to go through identifiers. How about this rewording?
> >
> > This attribute tells whether the breakpoint is visible to the user
> > when set, or when the @samp{info breakpoints} command is run.
> >
>
> Works for me.
Then the documentation patch is okay with these changes.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-22 21:05 ` Phil Muldoon
2010-10-22 21:31 ` Eli Zaretskii
@ 2010-10-31 21:07 ` Pedro Alves
2010-11-11 14:36 ` Phil Muldoon
1 sibling, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2010-10-31 21:07 UTC (permalink / raw)
To: gdb-patches, pmuldoon; +Cc: Tom Tromey, dan
On Friday 22 October 2010 22:05:30, Phil Muldoon wrote:
> -/* Set a breakpoint. This function is shared between CLI and MI
> +/* Set a breakpoint. This function is shared between CLI and MI
Spurious, and incorrect whitespace change.
> functions for setting a breakpoint. This function has two major
> modes of operations, selected by the PARSE_CONDITION_AND_THREAD
> parameter. If non-zero, the function will parse arg, extracting
> breakpoint location, address and thread. Otherwise, ARG is just the
> location of breakpoint, with condition and thread specified by the
> - COND_STRING and THREAD parameters. Returns true if any breakpoint
> - was created; false otherwise. */
> -
> + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
> + breakpoint number will be allocated from the internal breakpoint
> + count. Returns true if any breakpoint was created; false
> + otherwise. */
> int
Two spaces after period, not three. Don't lose the empty line
between comment and function.
> create_breakpoint (struct gdbarch *gdbarch,
> @@ -7750,7 +7785,8 @@ break_command_1 (char *arg, int flag, int from_tty)
> pending_break_support,
> NULL /* breakpoint_ops */,
> from_tty,
> - 1 /* enabled */);
> + 1 /* enabled */,
> + 0 /* internal */);
> }
>
Inconsistent whitespace with the other cases. Please fix that
in all the several places this appears.
> int static_trace_marker_id_idx;
> - };
> +
> + /* With a Python scripting enabled GDB, store a reference to the
> + Python object that has been associated with this breakpoint.
> + This is always NULL for a GDB that is not script enabled. It
> + can sometimes be NULL for enabled GDBs as not all breakpoint
> + types are tracked by the Python scripting API. */
> + PyObject *py_bp_object;
> +};
Is the indentation correct here? Looks two spaces too much to the right.
> + if (internal)
> + /* Do not mention breakpoints with a negative number, but do
> + notify observers. */
> + observer_notify_breakpoint_created (b->number);
> + else
> + mention (b);
Same here. Please double check your patch for formatting -- there may be
other cases I missed.
> extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
> - char *cond_string, int thread,
> - int parse_condition_and_thread,
> - int tempflag, enum bptype wanted_type,
> - int ignore_count,
> - enum auto_boolean pending_break_support,
> - struct breakpoint_ops *ops,
> - int from_tty,
> - int enabled);
> + char *cond_string, int thread,
> + int parse_condition_and_thread,
> + int tempflag,
> + enum bptype type_wanted,
> + int ignore_count,
> + enum auto_boolean pending_break_support,
> + struct breakpoint_ops *ops,
> + int from_tty,
> + int enabled,
> + int internal);
Leftover reindent from "create_new_breakpoint"? Doesn't
look necessary or correct? Please undo.
> create_breakpoint (python_gdbarch,
> - spec, NULL, -1,
> - 0,
> - 0, bp_breakpoint,
> - 0,
> - AUTO_BOOLEAN_TRUE,
> - NULL, 0, 1);
> + spec, NULL, -1,
> + 0,
> + 0, bp_breakpoint,
> + 0,
> + AUTO_BOOLEAN_TRUE,
> + NULL, 0, 1, internal_bp);
Same here?
Otherwise looks fine to me. Thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-10-31 21:07 ` Pedro Alves
@ 2010-11-11 14:36 ` Phil Muldoon
2010-11-12 12:43 ` Ken Werner
0 siblings, 1 reply; 31+ messages in thread
From: Phil Muldoon @ 2010-11-11 14:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Tom Tromey, dan, eliz
> Pedro Alves <pedro@codesourcery.com> writes:
> Otherwise looks fine to me. Thanks.
Thanks I fixed up the changes. Apologies for that spurious change
noise.
> Eli Zaretskii <eliz@gnu.org> writes:
> Then the documentation patch is okay with these changes.
Thanks I fixed the documentation issues you noted.
> Tom Tromey <tromey@redhat.com> writes:
> The Python parts look good to me.
I added the changes that Pedro requested too.
So committed, thanks
Cheers,
Phil
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-11-11 14:36 ` Phil Muldoon
@ 2010-11-12 12:43 ` Ken Werner
2010-11-12 12:49 ` Pedro Alves
0 siblings, 1 reply; 31+ messages in thread
From: Ken Werner @ 2010-11-12 12:43 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches, Pedro Alves, Tom Tromey, dan, eliz
[-- Attachment #1: Type: Text/Plain, Size: 276 bytes --]
On Thursday, November 11, 2010 3:36:21 pm Phil Muldoon wrote:
> So committed, thanks
This change introduces a small typo in the spu-tdep.c file which prevents the
gdb from compiling if --target=spu is enabled. The attached patch fixes this
issue. Ok to apply?
Regards
Ken
[-- Attachment #2: typo.patch --]
[-- Type: text/x-patch, Size: 660 bytes --]
ChangeLog:
2010-11-12 Ken Werner <ken.werner@de.ibm.com>
* spu-tdep.c (spu_catch_start): Fix typo.
Index: gdb/spu-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.c,v
retrieving revision 1.63
diff -p -u -r1.63 spu-tdep.c
--- gdb/spu-tdep.c 11 Nov 2010 14:11:52 -0000 1.63
+++ gdb/spu-tdep.c 12 Nov 2010 12:37:49 -0000
@@ -1896,7 +1896,7 @@ spu_catch_start (struct objfile *objfile
0 /* ignore_count */,
AUTO_BOOLEAN_FALSE /* pending_break_support */,
NULL /* ops */, 0 /* from_tty */, 1 /* enabled */,
- 0 /* internal */););
+ 0 /* internal */);
}
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-11-12 12:43 ` Ken Werner
@ 2010-11-12 12:49 ` Pedro Alves
2010-11-12 12:58 ` Ken Werner
0 siblings, 1 reply; 31+ messages in thread
From: Pedro Alves @ 2010-11-12 12:49 UTC (permalink / raw)
To: Ken Werner; +Cc: pmuldoon, gdb-patches, Tom Tromey, dan, eliz
On Friday 12 November 2010 12:43:27, Ken Werner wrote:
> On Thursday, November 11, 2010 3:36:21 pm Phil Muldoon wrote:
> > So committed, thanks
>
> This change introduces a small typo in the spu-tdep.c file which prevents the
> gdb from compiling if --target=spu is enabled. The attached patch fixes this
> issue. Ok to apply?
Sure, thanks. FYI, obvious patches don't really require approval.
--
Pedro Alves
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [patch] Add visible flag to breakpoints.
2010-11-12 12:49 ` Pedro Alves
@ 2010-11-12 12:58 ` Ken Werner
0 siblings, 0 replies; 31+ messages in thread
From: Ken Werner @ 2010-11-12 12:58 UTC (permalink / raw)
To: Pedro Alves; +Cc: pmuldoon, gdb-patches, Tom Tromey, dan, eliz, Jan Kratochvil
On Friday, November 12, 2010 1:49:25 pm Pedro Alves wrote:
> On Friday 12 November 2010 12:43:27, Ken Werner wrote:
> > On Thursday, November 11, 2010 3:36:21 pm Phil Muldoon wrote:
> > > So committed, thanks
> >
> > This change introduces a small typo in the spu-tdep.c file which prevents
> > the gdb from compiling if --target=spu is enabled. The attached patch
> > fixes this issue. Ok to apply?
>
> Sure, thanks. FYI, obvious patches don't really require approval.
I was just about committing the change and saw that Jan Kratochvil already
fixed this a minute ago: http://sourceware.org/ml/gdb-
cvs/2010-11/msg00055.html
Thanks
Ken
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2010-11-12 12:58 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-30 16:28 [patch] Add visible flag to breakpoints Phil Muldoon
2010-09-30 16:41 ` Daniel Jacobowitz
2010-09-30 17:51 ` Phil Muldoon
2010-09-30 17:55 ` Pedro Alves
2010-09-30 18:12 ` Phil Muldoon
2010-10-08 12:51 ` Phil Muldoon
2010-10-08 13:35 ` Pedro Alves
2010-10-08 14:04 ` Phil Muldoon
2010-10-08 18:44 ` Tom Tromey
2010-10-12 20:25 ` Phil Muldoon
2010-10-12 21:34 ` Tom Tromey
2010-10-13 12:45 ` Phil Muldoon
2010-10-13 13:06 ` Phil Muldoon
2010-10-13 15:36 ` Tom Tromey
2010-10-16 18:42 ` Pedro Alves
2010-10-16 19:03 ` Pedro Alves
2010-10-18 16:09 ` Tom Tromey
2010-10-22 21:05 ` Phil Muldoon
2010-10-22 21:31 ` Eli Zaretskii
2010-10-22 21:37 ` Phil Muldoon
2010-10-23 9:07 ` Eli Zaretskii
2010-10-31 21:07 ` Pedro Alves
2010-11-11 14:36 ` Phil Muldoon
2010-11-12 12:43 ` Ken Werner
2010-11-12 12:49 ` Pedro Alves
2010-11-12 12:58 ` Ken Werner
2010-10-08 18:40 ` Tom Tromey
2010-09-30 17:04 ` Pedro Alves
2010-09-30 17:55 ` Phil Muldoon
2010-09-30 17:51 ` Eli Zaretskii
2010-10-05 22:28 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).