[RFA/RFC] Clarify contents of NEWS entry re: Python "rbreak" (waa: "Re: [python][patch] Python rbreak")

Joel Brobecker
2018-02-01T09:47:48Z

[hi Eli -- not sure if the attached patch is obvious or not, so I will
wait for your approval, assuming Phil confirms. Thank you!]

Hi Phil,

Can you clarify something for me?

> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index 2bad096a86..1d26ea4af7 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -24,6 +24,10 @@
> > gdb.new_thread are emitted. See the manual for further
> > description of these.
> >
> > + ** A new command, "rbreak" has been added to the Python API. This
> > + command allows the setting of a large number of breakpoints via a
> > + regex pattern in Python. See the manual for further details.

Based on the implementation, I'm pretty sure that it's actualy a new
function, rather than a new command; is that correct?

If I am, I would like to suggest the following change to the NEWS file,
to make it clearer.

gdb/ChangeLog:

* NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
Python function, rather than a new command.

No further comments from me past this point, but still keeping the rest
of the email to provide the "diff" as the context, in case it is
helpful.

Thank you!

> > +
> > * New features in the GDB remote stub, GDBserver
> >
> > ** GDBserver is now able to start inferior processes with a
> > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> > index f661e489bb..f411f60d7e 100644
> > --- a/gdb/doc/python.texi
> > +++ b/gdb/doc/python.texi
> > @@ -243,6 +243,23 @@ were no breakpoints. This peculiarity was subsequently fixed, and now
> > @code{gdb.breakpoints} returns an empty sequence in this case.
> > @end defun
> >
> > +@defun gdb.rbreak (regex @r{[}, minsyms @r{[}, throttle, @r{[}, symtabs @r{]]]})
> > +Return a Python list holding a collection of newly set
> > +@code{gdb.Breakpoint} objects matching function names defined by the
> > +@var{regex} pattern. If the @var{minsyms} keyword is @code{True}, all
> > +system functions (those not explicitly defined in the inferior) will
> > +also be included in the match. The @var{throttle} keyword takes an
> > +integer that defines the maximum number of pattern matches for
> > +functions matched by the @var{regex} pattern. If the number of
> > +matches exceeds the integer value of @var{throttle}, a
> > +@code{RuntimeError} will be raised and no breakpoints will be created.
> > +If @var{throttle} is not defined then there is no imposed limit on the
> > +maximum number of matches and breakpoints to be created. The
> > +@var{symtabs} keyword takes a Python iterable that yields a collection
> > +of @code{gdb.Symtab} objects and will restrict the search to those
> > +functions only contained within the @code{gdb.Symtab} objects.
> > +@end defun
> > +
> > @findex gdb.parameter
> > @defun gdb.parameter (parameter)
> > Return the value of a @value{GDBN} @var{parameter} given by its name,
> > diff --git a/gdb/python/python.c b/gdb/python/python.c
> > index b04057ec4a..a044b8ff8b 100644
> > --- a/gdb/python/python.c
> > +++ b/gdb/python/python.c
> > @@ -642,6 +642,190 @@ gdbpy_solib_name (PyObject *self, PyObject *args)
> > return str_obj;
> > }
> >
> > +/* Implementation of Python rbreak command. Take a REGEX and
> > + optionally a MINSYMS, THROTTLE and SYMTABS keyword and return a
> > + Python list that contains newly set breakpoints that match that
> > + criteria. REGEX refers to a GDB format standard regex pattern of
> > + symbols names to search; MINSYMS is an optional boolean (default
> > + False) that indicates if the function should search GDB's minimal
> > + symbols; THROTTLE is an optional integer (default unlimited) that
> > + indicates the maximum amount of breakpoints allowable before the
> > + function exits (note, if the throttle bound is passed, no
> > + breakpoints will be set and a runtime error returned); SYMTABS is
> > + an optional Python iterable that contains a set of gdb.Symtabs to
> > + constrain the search within. */
> > +
> > +static PyObject *
> > +gdbpy_rbreak (PyObject *self, PyObject *args, PyObject *kw)
> > +{
> > + /* A simple type to ensure clean up of a vector of allocated strings
> > + when a C interface demands a const char *array[] type
> > + interface. */
> > + struct symtab_list_type
> > + {
> > + ~symtab_list_type ()
> > + {
> > + for (const char *elem: vec)
> > + xfree ((void *) elem);
> > + }
> > + std::vector<const char *> vec;
> > + };
> > +
> > + char *regex = NULL;
> > + std::vector<symbol_search> symbols;
> > + unsigned long count = 0;
> > + PyObject *symtab_list = NULL;
> > + PyObject *minsyms_p_obj = NULL;
> > + int minsyms_p = 0;
> > + unsigned int throttle = 0;
> > + static const char *keywords[] = {"regex","minsyms", "throttle",
> > + "symtabs", NULL};
> > + symtab_list_type symtab_paths;
> > +
> > + if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!IO", keywords,
> > + &regex, &PyBool_Type,
> > + &minsyms_p_obj, &throttle,
> > + &symtab_list))
> > + return NULL;
> > +
> > + /* Parse minsyms keyword. */
> > + if (minsyms_p_obj != NULL)
> > + {
> > + int cmp = PyObject_IsTrue (minsyms_p_obj);
> > + if (cmp < 0)
> > + return NULL;
> > + minsyms_p = cmp;
> > + }
> > +
> > + /* The "symtabs" keyword is any Python iterable object that returns
> > + a gdb.Symtab on each iteration. If specified, iterate through
> > + the provided gdb.Symtabs and extract their full path. As
> > + python_string_to_target_string returns a
> > + gdb::unique_xmalloc_ptr<char> and a vector containing these types
> > + cannot be coerced to a const char **p[] via the vector.data call,
> > + release the value from the unique_xmalloc_ptr and place it in a
> > + simple type symtab_list_type (which holds the vector and a
> > + destructor that frees the contents of the allocated strings. */
> > + if (symtab_list != NULL)
> > + {
> > + gdbpy_ref<> iter (PyObject_GetIter (symtab_list));
> > +
> > + if (iter == NULL)
> > + return NULL;
> > +
> > + while (true)
> > + {
> > + gdbpy_ref<> next (PyIter_Next (iter.get ()));
> > +
> > + if (next == NULL)
> > + {
> > + if (PyErr_Occurred ())
> > + return NULL;
> > + break;
> > + }
> > +
> > + gdbpy_ref<> obj_name (PyObject_GetAttrString (next.get (),
> > + "filename"));
> > +
> > + if (obj_name == NULL)
> > + return NULL;
> > +
> > + /* Is the object file still valid? */
> > + if (obj_name == Py_None)
> > + continue;
> > +
> > + gdb::unique_xmalloc_ptr<char> filename =
> > + python_string_to_target_string (obj_name.get ());
> > +
> > + if (filename == NULL)
> > + return NULL;
> > +
> > + /* Make sure there is a definite place to store the value of
> > + s before it is released. */
> > + symtab_paths.vec.push_back (nullptr);
> > + symtab_paths.vec.back () = filename.release ();
> > + }
> > + }
> > +
> > + if (symtab_list)
> > + {
> > + const char **files = symtab_paths.vec.data ();
> > +
> > + symbols = search_symbols (regex, FUNCTIONS_DOMAIN,
> > + symtab_paths.vec.size (), files);
> > + }
> > + else
> > + symbols = search_symbols (regex, FUNCTIONS_DOMAIN, 0, NULL);
> > +
> > + /* Count the number of symbols (both symbols and optionally minimal
> > + symbols) so we can correctly check the throttle limit. */
> > + for (const symbol_search &p : symbols)
> > + {
> > + /* Minimal symbols included? */
> > + if (minsyms_p)
> > + {
> > + if (p.msymbol.minsym != NULL)
> > + count++;
> > + }
> > +
> > + if (p.symbol != NULL)
> > + count++;
> > + }
> > +
> > + /* Check throttle bounds and exit if in excess. */
> > + if (throttle != 0 && count > throttle)
> > + {
> > + PyErr_SetString (PyExc_RuntimeError,
> > + _("Number of breakpoints exceeds throttled maximum."));
> > + return NULL;
> > + }
> > +
> > + gdbpy_ref<> return_list (PyList_New (0));
> > +
> > + if (return_list == NULL)
> > + return NULL;
> > +
> > + /* Construct full path names for symbols and call the Python
> > + breakpoint constructor on the resulting names. Be tolerant of
> > + individual breakpoint failures. */
> > + for (const symbol_search &p : symbols)
> > + {
> > + std::string symbol_name;
> > +
> > + /* Skipping minimal symbols? */
> > + if (minsyms_p == 0)
> > + if (p.msymbol.minsym != NULL)
> > + continue;
> > +
> > + if (p.msymbol.minsym == NULL)
> > + {
> > + struct symtab *symtab = symbol_symtab (p.symbol);
> > + const char *fullname = symtab_to_fullname (symtab);
> > +
> > + symbol_name = fullname;
> > + symbol_name += ":";
> > + symbol_name += SYMBOL_LINKAGE_NAME (p.symbol);
> > + }
> > + else
> > + symbol_name = MSYMBOL_LINKAGE_NAME (p.msymbol.minsym);
> > +
> > + gdbpy_ref<> argList (Py_BuildValue("(s)", symbol_name.c_str ()));
> > + gdbpy_ref<> obj (PyObject_CallObject ((PyObject *)
> > + &breakpoint_object_type,
> > + argList.get ()));
> > +
> > + /* Tolerate individual breakpoint failures. */
> > + if (obj == NULL)
> > + gdbpy_print_stack ();
> > + else
> > + {
> > + if (PyList_Append (return_list.get (), obj.get ()) == -1)
> > + return NULL;
> > + }
> > + }
> > + return return_list.release ();
> > +}
> > +
> > /* A Python function which is a wrapper for decode_line_1. */
> >
> > static PyObject *
> > @@ -1912,7 +2096,9 @@ Return the name of the current target charset." },
> > { "target_wide_charset", gdbpy_target_wide_charset, METH_NOARGS,
> > "target_wide_charset () -> string.\n\
> > Return the name of the current target wide charset." },
> > -
> > + { "rbreak", (PyCFunction) gdbpy_rbreak, METH_VARARGS | METH_KEYWORDS,
> > + "rbreak (Regex) -> List.\n\
> > +Return a Tuple containing gdb.Breakpoint objects that match the given Regex." },
> > { "string_to_argv", gdbpy_string_to_argv, METH_VARARGS,
> > "string_to_argv (String) -> Array.\n\
> > Parse String and return an argv-like array.\n\
> > diff --git a/gdb/testsuite/gdb.python/py-rbreak-func2.c b/gdb/testsuite/gdb.python/py-rbreak-func2.c
> > new file mode 100644
> > index 0000000000..2d24b6b557
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.python/py-rbreak-func2.c
> > @@ -0,0 +1,34 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > + Copyright 2017 Free Software Foundation, Inc.
> > +
> > + This program is free software; you can redistribute it and/or modify
> > + it under the terms of the GNU General Public License as published by
> > + the Free Software Foundation; either version 3 of the License, or
> > + (at your option) any later version.
> > +
> > + This program is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + GNU General Public License for more details.
> > +
> > + You should have received a copy of the GNU General Public License
> > + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> > +
> > +int
> > +efunc1 ()
> > +{
> > + return 1;
> > +}
> > +
> > +int
> > +efunc2 ()
> > +{
> > + return 2;
> > +}
> > +
> > +int
> > +efunc3 ()
> > +{
> > + return 3;
> > +}
> > diff --git a/gdb/testsuite/gdb.python/py-rbreak.c b/gdb/testsuite/gdb.python/py-rbreak.c
> > new file mode 100644
> > index 0000000000..e79d2a34ae
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.python/py-rbreak.c
> > @@ -0,0 +1,70 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > + Copyright 2013-2017 Free Software Foundation, Inc.
> > +
> > + This program is free software; you can redistribute it and/or modify
> > + it under the terms of the GNU General Public License as published by
> > + the Free Software Foundation; either version 3 of the License, or
> > + (at your option) any later version.
> > +
> > + This program is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + GNU General Public License for more details.
> > +
> > + You should have received a copy of the GNU General Public License
> > + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> > +
> > +int
> > +func1 ()
> > +{
> > + return 1;
> > +}
> > +
> > +int
> > +func2 ()
> > +{
> > + return 2;
> > +}
> > +
> > +int
> > +func3 ()
> > +{
> > + return 3;
> > +}
> > +
> > +int
> > +func4 ()
> > +{
> > + return 4;
> > +}
> > +
> > +int
> > +func5 ()
> > +{
> > + return 5;
> > +}
> > +
> > +void
> > +func6 ()
> > +{
> > + return;
> > +}
> > +
> > +void
> > +outside_scope ()
> > +{
> > + return;
> > +}
> > +
> > +int
> > +main()
> > +{
> > + func1 (); /* Break func1. */
> > + func2 ();
> > + func3 ();
> > + func4 ();
> > + func5 ();
> > + func6 ();
> > + outside_scope ();
> > +}
> > diff --git a/gdb/testsuite/gdb.python/py-rbreak.exp b/gdb/testsuite/gdb.python/py-rbreak.exp
> > new file mode 100644
> > index 0000000000..5aaf2975c9
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.python/py-rbreak.exp
> > @@ -0,0 +1,61 @@
> > +# Copyright (C) 2017 Free Software Foundation, Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# This file is part of the GDB testsuite. It tests the mechanism
> > +# exposing values to Python.
> > +
> > +load_lib gdb-python.exp
> > +
> > +standard_testfile py-rbreak.c py-rbreak-func2.c
> > +
> > +if {[prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $srcfile2]] } {
> > + return 1
> > +}
> > +
> > +# Skip all tests if Python scripting is not enabled.
> > +if { [skip_python_tests] } { continue }
> > +
> > +if ![runto_main] then {
> > + fail "can't run to main"
> > + return 0
> > +}
> > +
> > +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"\",minsyms=False)" \
> > + "get all function breakpoints" 0
> > +gdb_test "py print(len(sl))" "11" \
> > + "check number of returned breakpoints is 11"
> > +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"main\.\*\",minsyms=False)" \
> > + "get main function breakpoint" 0
> > +gdb_test "py print(len(sl))" "1" \
> > + "check number of returned breakpoints is 1"
> > +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"func\.\*\",minsyms=False,throttle=10)" \
> > + "get functions matching func.*" 0
> > +gdb_test "py print(len(sl))" "9" \
> > + "check number of returned breakpoints is 9"
> > +gdb_test "py gdb.rbreak(\"func\.\*\",minsyms=False,throttle=5)" \
> > + "Number of breakpoints exceeds throttled maximum.*" \
> > + "check throttle errors on too many breakpoints"
> > +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"func1\",minsyms=True)" \
> > + "including minimal symbols, get functions matching func.*" 0
> > +gdb_test "py print(len(sl))" "2" \
> > + "check number of returned breakpoints is 2"
> > +gdb_py_test_silent_cmd "python sym = gdb.lookup_symbol(\"efunc1\")" \
> > + "find a symbol in objfile" 1
> > +gdb_py_test_silent_cmd "python symtab = sym\[0\].symtab" \
> > + "get backing symbol table" 1
> > +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"func\.\*\",minsyms=False,throttle=10,symtabs=\[symtab\])" \
> > + "get functions matching func.* in one symtab only" 0
> > +gdb_test "py print(len(sl))" "3" \
> > + "check number of returned breakpoints is 3"
> >

--
Joel