[python][patch] Python rbreak

[python][patch] Python rbreak

[Phil Muldoon]

This introduces a Python rbreak function to the Python API. Its
functionality was designed to closely match that of the console rbreak
but with a number of caveats.

That being said, there's a bit of preamble first.

My original design was to have the gdb.Breakpoint constructor take a
regex pattern. While breakpoints can, and do, have multiple locations
they are always tied in with one linespec. I looked at the
linespec/location code and the relationship is pretty firmly embedded
as a one to many relationship. One linespec can have multiple
locations but only one linespec can be represented in the location
object. (Or address or probe or whatever the location is
representing). A linespec has to be definitive; it has to point to one
symbol. I thought about modifying this to have multiple symbols and
multiple locations but quickly decided this was really going to
violate the design contract of linespecs/location. It would also be
considerable and disruptive work. I looked then, again, at a
breakpoint having multiple linespecs but again decided the amount of
disruption this would generate was not worth a Python rbreak returning
one breakpoint representing multiple symbols and multiple locations.

So, instead, I've made the Python rbreak functionality a little more
tuneable than the console command equivalent. The first tuneable is to
allow the user to exclude mini symbols from the pattern matching. A
loosely written pattern passed to the console rbreak command can
generate 5000+ breakpoints from a simple C "hello world"
program. rbreak "" will place a breakpoint on every single mini symbol
representing a function. I can't possibly see why this would be
desirable. In order to facilitate broadly-written patterns that seek to
place a breakpoint on every user-defined function, the equivalent
would be:

gdb.rbreak ("", minisyms=False)

That would place a breakpoint on all functions that are actually
defined in the inferior (and not those that are inserted by the
compiler, linker, etc). The default for this keyword is False.

The second tuneable is a throttle. Beyond the name (which I am unsure
about but could not think of a better one), this allows the user to
enter a fail-safe limit for breakpoint creation. So, for the following
example, an inferior with ten user provided functions:

gdb.rbreak ("", minisyms=False, throttle=5)

Would set no breakpoints and raise a runtime error. This functionality
is more aimed at those who want to utilise Python rbreak from a
toolset perspective (say for a GDB UI). The default for this keyword
is unlimited.

The last tuneable is the ability to limit the pattern matching to a
set of gdb.Symtab objects. So, given the example below:

gdb.rbreak ("", minisyms=False, throttle=5, symtabs=symTuple)

where "symTuple" is a tuple of gdb.Symtab objects, the search/pattern
matching would be restricted to those gdb.Symtab objects only. The
default for this keyword (i.e, no keyword provided) is to search all
symbol tables.

All of the examples above have "" as the search pattern but you can,
of course, specify any valid GDB regex there.

On a review note I seek a little advice on the interface to
search_symbols. If the user does provide a symtabs keyword, the code
collects the name of the file attached to each symtab specified. It
does that through the usual Python method
(python_string_to_target_string). That returns a
gdb::unique_xmalloc_ptr<char>. I planned to store these in a
std::vector, however, I was unable to coerce a
std::vector<gdb::unique_xmalloc_ptr<char>> into a char **p[]
representation the search_symbols function requires. I ended up
releasing the object (that results in a char *) and wrapping the
std::vector in a simple type that provides a destructor to properly
free the released strings when the object falls out of scope. I
thought about just changing the search_symbols interface to accept a
vector of (gdb) unique pointers but noted a C++ run had already been
performed over the function. Any thoughts here would be appreciated.

Long, windy, preamble over the patch follows at the end of the email.

Cheers

Phil

--

2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>

	* python/python.c (gdbpy_rbreak): New function.

2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-rbreak.exp: New file.
	* gdb.python/py-rbreak.c: New file.
	* gdb.python/py-rbreak-func2.c: New file.

2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>

	* python.texi (Basic Python): Add rbreak documentation.

--

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index f661e489bb..b2d4516886 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -243,6 +243,24 @@ were no breakpoints.  This peculiarity was subsequently fixed, and now
 @code{gdb.breakpoints} returns an empty sequence in this case.
 @end defun
 
+@findex gdb.rbreak
+@defun gdb.rbreak (regex @r{[}, minisyms @r{[}, throttle, @r{[}, symtabs @r{]]]})
+Return a Python tuple holding a collection of newly set
+@code{gdb.Breakpoint} objects matching function names defined by the
+@var{regex} pattern.  If the @var{minisyms} 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..4d591df30c 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -642,6 +642,191 @@ gdbpy_solib_name (PyObject *self, PyObject *args)
   return str_obj;
 }
 
+/* Implementation of Python rbreak command.  Take a REGEX and
+   optionally a MINISYMS, THROTTLE and SYMTABS keyword and return a
+   Python tuple that contains newly set breakpoints that match that
+   criteria.  REGEX refers to a GDB format standard regex pattern of
+   symbols names to search; MINISYMS 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 iterator 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 *minisyms_p_obj = NULL;
+  int minisyms_p = 0;
+  unsigned int throttle = 0;
+  static const char *keywords[] = {"regex","minisyms", "throttle", "symtabs", NULL};
+  symtab_list_type symtab_paths;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!IO", keywords,
+					&regex, &PyBool_Type,
+					&minisyms_p_obj, &throttle,
+					&symtab_list))
+    return NULL;
+
+  /* Parse minisyms keyword.  */
+  if (minisyms_p_obj != NULL)
+    {
+      int cmp = PyObject_IsTrue (minisyms_p_obj);
+      if (cmp < 0)
+	return NULL;
+      minisyms_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 mini
+     symbols) so we can correctly size the tuple.  */
+  for (const symbol_search &p : symbols)
+    {
+      /* Mini symbols included?  */
+      if (minisyms_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_tuple (PyTuple_New (count));
+
+  if (return_tuple == NULL)
+    return NULL;
+
+  count = 0;
+
+  /* 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 mini-symbols?  */
+      if (minisyms_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
+	{
+	  PyTuple_SET_ITEM (return_tuple.get (), count, obj.get ());
+	  count++;
+	}
+    }
+  return return_tuple.release ();
+}
+
 /* A Python function which is a wrapper for decode_line_1.  */
 
 static PyObject *
@@ -1912,7 +2097,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) -> Tuple.\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..1de8bfd5b2
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-rbreak-func2.c
@@ -0,0 +1,31 @@
+/* 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..287aba6c03
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-rbreak.c
@@ -0,0 +1,62 @@
+/* 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..9385aeae08
--- /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(\"\",minisyms=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\.\*\",minisyms=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\.\*\",minisyms=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\.\*\",minisyms=False,throttle=1)" \
+    "Number of breakpoints exceeds throttled maximum.*" \
+    "Check throttle errors on too many breakpoints."
+gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"func1\",minisyms=True)" \
+    "Including mini 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\.\*\",minisyms=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"

Re: [python][patch] Python rbreak

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Wed, 11 Oct 2017 12:30:17 +0100
> 
> 2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>
> 
> 	* python.texi (Basic Python): Add rbreak documentation.

This should go into a separate gdb/doc/ChangeLog file.

> +@findex gdb.rbreak
> +@defun gdb.rbreak (regex @r{[}, minisyms @r{[}, throttle, @r{[}, symtabs @r{]]]})

You don't need that @findex, since @defun automatically inserts the
function into the function index.

The documentation part is okay with this fixed, but I think we also
want a NEWS entry.

Thanks.

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 11/10/17 13:11, Eli Zaretskii wrote:
>> From: Phil Muldoon <pmuldoon@redhat.com>
>> Date: Wed, 11 Oct 2017 12:30:17 +0100
>>
>> 2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>
>>
>> 	* python.texi (Basic Python): Add rbreak documentation.
> 
> This should go into a separate gdb/doc/ChangeLog file.

It is locally. Not sure why the relative paths were not
included. Anyway, noted and it will be in the doc/ ChangeLog.

> 
>> +@findex gdb.rbreak
>> +@defun gdb.rbreak (regex @r{[}, minisyms @r{[}, throttle, @r{[}, symtabs @r{]]]})
> 
> You don't need that @findex, since @defun automatically inserts the
> function into the function index.

Thanks.

> The documentation part is okay with this fixed, but I think we also
> want a NEWS entry.

I'll add a NEWS entry and repost the doc part of the patch.

Cheers

Phil

Re: [python][patch] Python rbreak

[Kevin Buettner]

On Wed, 11 Oct 2017 12:30:17 +0100
Phil Muldoon <pmuldoon@redhat.com> wrote:

> So, instead, I've made the Python rbreak functionality a little more
> tuneable than the console command equivalent. The first tuneable is to
> allow the user to exclude mini symbols from the pattern matching. A

[...]

> gdb.rbreak ("", minisyms=False)

While reading through your preamble, I noticed some terminology with
which I was unfamiliar: "mini symbols" and "minisyms".

In a private discussion, you informed me that you actually meant
"minimal symbols" and "minsyms", which cleared things up for me.

But, you also asked me to remind you about this via a public reply, so
here it is...  :)

Kevin

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 11/10/17 17:19, Kevin Buettner wrote:
> On Wed, 11 Oct 2017 12:30:17 +0100
> Phil Muldoon <pmuldoon@redhat.com> wrote:
> 
>> So, instead, I've made the Python rbreak functionality a little more
>> tuneable than the console command equivalent. The first tuneable is to
>> allow the user to exclude mini symbols from the pattern matching. A
> 
> [...]
> 
>> gdb.rbreak ("", minisyms=False)
> 
> While reading through your preamble, I noticed some terminology with
> which I was unfamiliar: "mini symbols" and "minisyms".
> 
> In a private discussion, you informed me that you actually meant
> "minimal symbols" and "minsyms", which cleared things up for me.
> 
> But, you also asked me to remind you about this via a public reply, so
> here it is...  :)
> 
> Kevin

Kevin,

Thanks for catching the inconsistency in terminology! I'll clear it up
and post it in the version 2 of the patch.  I'll won't repost for now
to see if other reviewers have other changes to request.

Cheers

Phil

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 11/10/17 12:30, Phil Muldoon wrote:
> This introduces a Python rbreak function to the Python API. Its
> functionality was designed to closely match that of the console rbreak
> but with a number of caveats.

> +
> +      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
> +	{
> +	  PyTuple_SET_ITEM (return_tuple.get (), count, obj.get ());
> +	  count++;

Not sure how I missed this on the first patch run, but that obj.get ()
should be obj.release (). Apologies for noise and fixed locally.

Cheers

Phil

Re: [python][patch] Python rbreak

[Simon Marchi]

On 2017-10-11 07:30 AM, Phil Muldoon wrote:
> This introduces a Python rbreak function to the Python API. Its
> functionality was designed to closely match that of the console rbreak
> but with a number of caveats.
> 
> That being said, there's a bit of preamble first.
> 
> My original design was to have the gdb.Breakpoint constructor take a
> regex pattern. While breakpoints can, and do, have multiple locations
> they are always tied in with one linespec. I looked at the
> linespec/location code and the relationship is pretty firmly embedded
> as a one to many relationship. One linespec can have multiple
> locations but only one linespec can be represented in the location
> object. (Or address or probe or whatever the location is
> representing). A linespec has to be definitive; it has to point to one
> symbol. I thought about modifying this to have multiple symbols and
> multiple locations but quickly decided this was really going to
> violate the design contract of linespecs/location. It would also be
> considerable and disruptive work. I looked then, again, at a
> breakpoint having multiple linespecs but again decided the amount of
> disruption this would generate was not worth a Python rbreak returning
> one breakpoint representing multiple symbols and multiple locations.
> 
> So, instead, I've made the Python rbreak functionality a little more
> tuneable than the console command equivalent. The first tuneable is to
> allow the user to exclude mini symbols from the pattern matching. A
> loosely written pattern passed to the console rbreak command can
> generate 5000+ breakpoints from a simple C "hello world"
> program. rbreak "" will place a breakpoint on every single mini symbol
> representing a function. I can't possibly see why this would be
> desirable. In order to facilitate broadly-written patterns that seek to
> place a breakpoint on every user-defined function, the equivalent
> would be:
> 
> gdb.rbreak ("", minisyms=False)

Hi Phil,

As Kevin noted (IIUC), this should be "minsyms" if we want to follow standard
GDB terminology.

> 
> That would place a breakpoint on all functions that are actually
> defined in the inferior (and not those that are inserted by the
> compiler, linker, etc). The default for this keyword is False.
> 
> The second tuneable is a throttle. Beyond the name (which I am unsure
> about but could not think of a better one), this allows the user to
> enter a fail-safe limit for breakpoint creation. So, for the following
> example, an inferior with ten user provided functions:
> 
> gdb.rbreak ("", minisyms=False, throttle=5)

max_results? max_breakpoints?

> Would set no breakpoints and raise a runtime error. This functionality
> is more aimed at those who want to utilise Python rbreak from a
> toolset perspective (say for a GDB UI). The default for this keyword
> is unlimited.
> 
> The last tuneable is the ability to limit the pattern matching to a
> set of gdb.Symtab objects. So, given the example below:
> 
> gdb.rbreak ("", minisyms=False, throttle=5, symtabs=symTuple)
> 
> where "symTuple" is a tuple of gdb.Symtab objects, the search/pattern
> matching would be restricted to those gdb.Symtab objects only. The
> default for this keyword (i.e, no keyword provided) is to search all
> symbol tables.
> 
> All of the examples above have "" as the search pattern but you can,
> of course, specify any valid GDB regex there.
> 
> On a review note I seek a little advice on the interface to
> search_symbols. If the user does provide a symtabs keyword, the code
> collects the name of the file attached to each symtab specified. It
> does that through the usual Python method
> (python_string_to_target_string). That returns a
> gdb::unique_xmalloc_ptr<char>. I planned to store these in a
> std::vector, however, I was unable to coerce a
> std::vector<gdb::unique_xmalloc_ptr<char>> into a char **p[]
> representation the search_symbols function requires. I ended up
> releasing the object (that results in a char *) and wrapping the
> std::vector in a simple type that provides a destructor to properly
> free the released strings when the object falls out of scope. I
> thought about just changing the search_symbols interface to accept a
> vector of (gdb) unique pointers but noted a C++ run had already been
> performed over the function. Any thoughts here would be appreciated.
> 
> Long, windy, preamble over the patch follows at the end of the email.
> 
> Cheers
> 
> Phil
> 
> --
> 
> 2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>
> 
> 	* python/python.c (gdbpy_rbreak): New function.
> 
> 2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>
> 
> 	* gdb.python/py-rbreak.exp: New file.
> 	* gdb.python/py-rbreak.c: New file.
> 	* gdb.python/py-rbreak-func2.c: New file.
> 
> 2017-10-11  Phil Muldoon  <pmuldoon@redhat.com>
> 
> 	* python.texi (Basic Python): Add rbreak documentation.
> 
> --
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index f661e489bb..b2d4516886 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -243,6 +243,24 @@ were no breakpoints.  This peculiarity was subsequently fixed, and now
>  @code{gdb.breakpoints} returns an empty sequence in this case.
>  @end defun
>  
> +@findex gdb.rbreak
> +@defun gdb.rbreak (regex @r{[}, minisyms @r{[}, throttle, @r{[}, symtabs @r{]]]})
> +Return a Python tuple holding a collection of newly set
> +@code{gdb.Breakpoint} objects matching function names defined by the
> +@var{regex} pattern.  If the @var{minisyms} 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..4d591df30c 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -642,6 +642,191 @@ gdbpy_solib_name (PyObject *self, PyObject *args)
>    return str_obj;
>  }
>  
> +/* Implementation of Python rbreak command.  Take a REGEX and
> +   optionally a MINISYMS, THROTTLE and SYMTABS keyword and return a
> +   Python tuple that contains newly set breakpoints that match that
> +   criteria.  REGEX refers to a GDB format standard regex pattern of
> +   symbols names to search; MINISYMS 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 iterator that contains a set of gdb.Symtabs to

iterator or iterable?  It would make sense to be able to pass a list here,
for example.

> +   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 *minisyms_p_obj = NULL;
> +  int minisyms_p = 0;
> +  unsigned int throttle = 0;
> +  static const char *keywords[] = {"regex","minisyms", "throttle", "symtabs", NULL};

Nit: line too long and missing space.	

> +  symtab_list_type symtab_paths;
> +
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!IO", keywords,
> +					&regex, &PyBool_Type,
> +					&minisyms_p_obj, &throttle,
> +					&symtab_list))
> +    return NULL;
> +
> +  /* Parse minisyms keyword.  */
> +  if (minisyms_p_obj != NULL)
> +    {
> +      int cmp = PyObject_IsTrue (minisyms_p_obj);
> +      if (cmp < 0)
> +	return NULL;
> +      minisyms_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 mini
> +     symbols) so we can correctly size the tuple.  */
> +  for (const symbol_search &p : symbols)
> +    {
> +      /* Mini symbols included?  */
> +      if (minisyms_p)
> +	{
> +	  if (p.msymbol.minsym != NULL)
> +	    count++;
> +	}

Would it be easy to pass the minisyms_p flag to search_symbols, so that
we don't need to search in the minsym tables if we don't even care about
them?

> +
> +      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_tuple (PyTuple_New (count));
> +
> +  if (return_tuple == NULL)
> +    return NULL;

How do you decide if this function should return a tuple or a list?
Instinctively I would have returned a list, but I can't really explain
why.

> +
> +  count = 0;
> +
> +  /* 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 mini-symbols?  */
> +      if (minisyms_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
> +	{
> +	  PyTuple_SET_ITEM (return_tuple.get (), count, obj.get ());

The Python doc says that SET_ITEM steals a reference to obj.  Isn't it
a problem, because gdbpy_ref also keeps the reference?

> +	  count++;
> +	}
> +    }

Hmm maybe this is a reason to use a list?  If a breakpoint fails to
be created, the tuple will not be filled completely.  What happens
to tuple elements that were not set?

With the list, you can simply PyList_Append.

> +  return return_tuple.release ();
> +}
> +
>  /* A Python function which is a wrapper for decode_line_1.  */
>  
>  static PyObject *
> @@ -1912,7 +2097,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) -> Tuple.\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..1de8bfd5b2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-rbreak-func2.c
> @@ -0,0 +1,31 @@
> +/* 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..287aba6c03
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-rbreak.c
> @@ -0,0 +1,62 @@
> +/* 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 ()

As for GDB code, put the return type on its own line.

> +{
> +  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..9385aeae08
> --- /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(\"\",minisyms=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\.\*\",minisyms=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\.\*\",minisyms=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\.\*\",minisyms=False,throttle=1)" \
> +    "Number of breakpoints exceeds throttled maximum.*" \
> +    "Check throttle errors on too many breakpoints."
> +gdb_py_test_silent_cmd "py sl = gdb.rbreak(\"func1\",minisyms=True)" \
> +    "Including mini 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\.\*\",minisyms=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"
> 

I can't find a reference, but I think we want test names to start
with a lower case letter and not end with a dot.  I'll see if we
can add this to the testcase cookbook wiki page.

Simon

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 16/10/17 23:22, Simon Marchi wrote:
> On 2017-10-11 07:30 AM, Phil Muldoon wrote:

> Hi Phil,
> 
> As Kevin noted (IIUC), this should be "minsyms" if we want to follow standard
> GDB terminology.

I've already fixed this up locally after Kevin noted it.  So just
noting that here.

>> That would place a breakpoint on all functions that are actually
>> defined in the inferior (and not those that are inserted by the
>> compiler, linker, etc). The default for this keyword is False.
>>
>> The second tuneable is a throttle. Beyond the name (which I am unsure
>> about but could not think of a better one), this allows the user to
>> enter a fail-safe limit for breakpoint creation. So, for the following
>> example, an inferior with ten user provided functions:
>>
>> gdb.rbreak ("", minisyms=False, throttle=5)
> 
> max_results? max_breakpoints?

I've no preference. I tried to imply in the keyword that if the
maximum was reached no breakpoints would be set. max_breakpoints, I
thought, implies that "if the maximum is reached breakpoints would be
set up to that limit." I've no strong opinion on this name, so if you
do, let me know.

> +/* Implementation of Python rbreak command.  Take a REGEX and
>> +   optionally a MINISYMS, THROTTLE and SYMTABS keyword and return a
>> +   Python tuple that contains newly set breakpoints that match that
>> +   criteria.  REGEX refers to a GDB format standard regex pattern of
>> +   symbols names to search; MINISYMS 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 iterator that contains a set of gdb.Symtabs to
> 
> iterator or iterable?  It would make sense to be able to pass a list here,
> for example.

The Python function does not care what you pass it: tuple, list,
whatever, as long as it is iterable.  So iterable is probably right
here.

>> +  static const char *keywords[] = {"regex","minisyms", "throttle", "symtabs", NULL};
> 
> Nit: line too long and missing space.	

Thanks for catching both nits.

>> +  for (const symbol_search &p : symbols)
>> +    {
>> +      /* Mini symbols included?  */
>> +      if (minisyms_p)
>> +	{
>> +	  if (p.msymbol.minsym != NULL)
>> +	    count++;
>> +	}
> 
> Would it be easy to pass the minisyms_p flag to search_symbols, so that
> we don't need to search in the minsym tables if we don't even care about
> them?

I thought about it. But instead of refactoring search_symbols to be
more selective, I wanted this patch to focus on Pythonic rbreak and
the added functionality it provides. I can change search_symbols, I've
no problem with that, but in a separate, more focused patch?

>> +  gdbpy_ref<> return_tuple (PyTuple_New (count));
>> +
>> +  if (return_tuple == NULL)
>> +    return NULL;
> 
> How do you decide if this function should return a tuple or a list?
> Instinctively I would have returned a list, but I can't really explain
> why.

I tend to think any collection a Python function returns normally
should be a tuple. Tuple's are immutable. That's the only reason
why. We have to count the symbols anyway to check the "throttle"
feature and, as we know the size of the array, I thought we might as
well make it a tuple.

>> +      /* Tolerate individual breakpoint failures.  */
>> +      if (obj == NULL)
>> +	gdbpy_print_stack ();
>> +      else
>> +	{
>> +	  PyTuple_SET_ITEM (return_tuple.get (), count, obj.get ());
> 
> The Python doc says that SET_ITEM steals a reference to obj.  Isn't it
> a problem, because gdbpy_ref also keeps the reference?

Sorry for the noise. I already self-caught this and I'm puzzled how it
got through (really, the tests should have failed as the objects would
have been garbage collected). But, already fixed. See:

https://sourceware.org/ml/gdb-patches/2017-10/msg00341.html


> Hmm maybe this is a reason to use a list?  If a breakpoint fails to
> be created, the tuple will not be filled completely.  What happens
> to tuple elements that were not set?
> 
> With the list, you can simply PyList_Append.

That's a good reason. I remember in a lot of other functions I've
written in the past I used PyList_AsTuple. I'm a bit worried about
that, though, as we could be dealing with thousands of breakpoints.

>> +int func1 ()
> 
> As for GDB code, put the return type on its own line.

I'll change this, it's not a problem, but I thought there was a large
degree of largess granted to testcase files with the idea that "GDB
has to work on real life (often messy) code."

> I can't find a reference, but I think we want test names to start
> with a lower case letter and not end with a dot.  I'll see if we
> can add this to the testcase cookbook wiki page.

As I mentioned on IRC, I've not heard of it but will happily change
the names to comply.

Cheers,

Phil

Re: [python][patch] Python rbreak

[Simon Marchi]

On 2017-10-16 19:01, Phil Muldoon wrote:
>>> That would place a breakpoint on all functions that are actually
>>> defined in the inferior (and not those that are inserted by the
>>> compiler, linker, etc). The default for this keyword is False.
>>> 
>>> The second tuneable is a throttle. Beyond the name (which I am unsure
>>> about but could not think of a better one), this allows the user to
>>> enter a fail-safe limit for breakpoint creation. So, for the 
>>> following
>>> example, an inferior with ten user provided functions:
>>> 
>>> gdb.rbreak ("", minisyms=False, throttle=5)
>> 
>> max_results? max_breakpoints?
> 
> I've no preference. I tried to imply in the keyword that if the
> maximum was reached no breakpoints would be set. max_breakpoints, I
> thought, implies that "if the maximum is reached breakpoints would be
> set up to that limit." I've no strong opinion on this name, so if you
> do, let me know.

Doesn't throttle imply the same thing?  I understand it as something 
that caps at a certain level.  I don't have a strong opinion, it just 
struck me as a not very common name to use for these kinds of things.  
The important thing is that it's documented properly.

>>> +  for (const symbol_search &p : symbols)
>>> +    {
>>> +      /* Mini symbols included?  */
>>> +      if (minisyms_p)
>>> +	{
>>> +	  if (p.msymbol.minsym != NULL)
>>> +	    count++;
>>> +	}
>> 
>> Would it be easy to pass the minisyms_p flag to search_symbols, so 
>> that
>> we don't need to search in the minsym tables if we don't even care 
>> about
>> them?
> 
> I thought about it. But instead of refactoring search_symbols to be
> more selective, I wanted this patch to focus on Pythonic rbreak and
> the added functionality it provides. I can change search_symbols, I've
> no problem with that, but in a separate, more focused patch?

That's fine with me.

>>> +  gdbpy_ref<> return_tuple (PyTuple_New (count));
>>> +
>>> +  if (return_tuple == NULL)
>>> +    return NULL;
>> 
>> How do you decide if this function should return a tuple or a list?
>> Instinctively I would have returned a list, but I can't really explain
>> why.
> 
> I tend to think any collection a Python function returns normally
> should be a tuple. Tuple's are immutable. That's the only reason
> why. We have to count the symbols anyway to check the "throttle"
> feature and, as we know the size of the array, I thought we might as
> well make it a tuple.

Ok.

>>> +      /* Tolerate individual breakpoint failures.  */
>>> +      if (obj == NULL)
>>> +	gdbpy_print_stack ();
>>> +      else
>>> +	{
>>> +	  PyTuple_SET_ITEM (return_tuple.get (), count, obj.get ());
>> 
>> The Python doc says that SET_ITEM steals a reference to obj.  Isn't it
>> a problem, because gdbpy_ref also keeps the reference?
> 
> Sorry for the noise. I already self-caught this and I'm puzzled how it
> got through (really, the tests should have failed as the objects would
> have been garbage collected). But, already fixed. See:
> 
> https://sourceware.org/ml/gdb-patches/2017-10/msg00341.html

Ah sorry.  I read that message before reviewing the patch, but because I 
didn't have the context then, it just flew over my head.

>> Hmm maybe this is a reason to use a list?  If a breakpoint fails to
>> be created, the tuple will not be filled completely.  What happens
>> to tuple elements that were not set?
>> 
>> With the list, you can simply PyList_Append.
> 
> That's a good reason. I remember in a lot of other functions I've
> written in the past I used PyList_AsTuple. I'm a bit worried about
> that, though, as we could be dealing with thousands of breakpoints.

As a Python user, I would have no problem with the API returning a list. 
  'hello you'.split() returns a list, for example.

>>> +int func1 ()
>> 
>> As for GDB code, put the return type on its own line.
> 
> I'll change this, it's not a problem, but I thought there was a large
> degree of largess granted to testcase files with the idea that "GDB
> has to work on real life (often messy) code."

As with the other "rule" below, I don't think it's written anywhere, but 
that's what I have seen in reviews since I've started contributing to 
GDB.  I'll add this to the wiki as well.

>> I can't find a reference, but I think we want test names to start
>> with a lower case letter and not end with a dot.  I'll see if we
>> can add this to the testcase cookbook wiki page.
> 
> As I mentioned on IRC, I've not heard of it but will happily change
> the names to comply.
> 
> Cheers,
> 
> Phil

Thanks,

Simon

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 17/10/17 01:24, Simon Marchi wrote:
> On 2017-10-16 19:01, Phil Muldoon wrote:

 
>>> I can't find a reference, but I think we want test names to start
>>> with a lower case letter and not end with a dot.  I'll see if we
>>> can add this to the testcase cookbook wiki page.
>>
>> As I mentioned on IRC, I've not heard of it but will happily change
>> the names to comply.

Sorry this took a bit longer to get back out than I would have liked.
Modified patch follows. I believe I have incorporated yours, Eli's and
Kevin's comments.  ChangeLogs remain the same (other than the new NEWS
entry which I have added locally.)

Cheers

--

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.
+
 * 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"

Re: [python][patch] Python rbreak

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Fri, 3 Nov 2017 09:46:42 +0000
> 
> Sorry this took a bit longer to get back out than I would have liked.
> Modified patch follows. I believe I have incorporated yours, Eli's and
> Kevin's comments.  ChangeLogs remain the same (other than the new NEWS
> entry which I have added locally.)

Thanks, the documentation parts are OK.

Re: [python][patch] Python rbreak

[Phil Muldoon]

Ping

On 03/11/17 09:46, Phil Muldoon wrote:
> On 17/10/17 01:24, Simon Marchi wrote:
>> On 2017-10-16 19:01, Phil Muldoon wrote:
> 
>  
>>>> I can't find a reference, but I think we want test names to start
>>>> with a lower case letter and not end with a dot.  I'll see if we
>>>> can add this to the testcase cookbook wiki page.
>>>
>>> As I mentioned on IRC, I've not heard of it but will happily change
>>> the names to comply.
> 
> Sorry this took a bit longer to get back out than I would have liked.
> Modified patch follows. I believe I have incorporated yours, Eli's and
> Kevin's comments.  ChangeLogs remain the same (other than the new NEWS
> entry which I have added locally.)
> 
> Cheers
> 
> --
> 
> 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.
> +
>  * 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"
> 

Re: [python][patch] Python rbreak

[Simon Marchi]

On 2017-11-03 05:46 AM, Phil Muldoon wrote:
>>>> I can't find a reference, but I think we want test names to start
>>>> with a lower case letter and not end with a dot.  I'll see if we
>>>> can add this to the testcase cookbook wiki page.
>>>
>>> As I mentioned on IRC, I've not heard of it but will happily change
>>> the names to comply.
> 
> Sorry this took a bit longer to get back out than I would have liked.
> Modified patch follows. I believe I have incorporated yours, Eli's and
> Kevin's comments.  ChangeLogs remain the same (other than the new NEWS
> entry which I have added locally.)

Hi Phil,

Sorry for the wait, I had missed the update.  The patch looks good to me,
with just a nit below.

> +	  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.  */

"of s" -> "of filename" ?

Simon

Re: [python][patch] Python rbreak

[Phil Muldoon]

On 14/11/17 20:22, Simon Marchi wrote:
> On 2017-11-03 05:46 AM, Phil Muldoon wrote:
>>>>> I can't find a reference, but I think we want test names to start
>>>>> with a lower case letter and not end with a dot.  I'll see if we
>>>>> can add this to the testcase cookbook wiki page.
>>>>
>>>> As I mentioned on IRC, I've not heard of it but will happily change
>>>> the names to comply.
>>
>> Sorry this took a bit longer to get back out than I would have liked.
>> Modified patch follows. I believe I have incorporated yours, Eli's and
>> Kevin's comments.  ChangeLogs remain the same (other than the new NEWS
>> entry which I have added locally.)
> 
> Hi Phil,
> 
> Sorry for the wait, I had missed the update.  The patch looks good to me,
> with just a nit below.
> 
>> +	  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.  */
> 
> "of s" -> "of filename" ?
> 
> Simon

So committed, with that additional change.

commit d8ae99a7b08e29e31446aee1e47e59943d7d9926

Thanks for the review

Cheers

Phil

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

[Joel Brobecker]

[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

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

[Phil Muldoon]

On 01/02/18 09:45, Joel Brobecker wrote:
> [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.

It's a Python implementation of rbreak. I thought about making prbreak
as a Python implementation of a GDB command but decided to leave it as
an API call instead. However you want to word it is totally
fine. Sorry for any confusion introduced!

Cheers

Phil

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

[Eli Zaretskii]

> Date: Thu, 1 Feb 2018 13:45:59 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
> 
> [hi Eli -- not sure if the attached patch is obvious or not, so I will
> wait for your approval, assuming Phil confirms. Thank you!]

Are you asking about the NEWS entry or about the entire patch?  If
the latter, I don't think I saw it, and neither do I see it in the
list archives.  I think.  What am I missing?

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

[Joel Brobecker]

> > [hi Eli -- not sure if the attached patch is obvious or not, so I will
> > wait for your approval, assuming Phil confirms. Thank you!]
> 
> Are you asking about the NEWS entry or about the entire patch?  If
> the latter, I don't think I saw it, and neither do I see it in the
> list archives.  I think.  What am I missing?

Just asking whether you approve the patch I attached, or if you
have some comments.

Thank you!
-- 
Joel

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

[Eli Zaretskii]

> Date: Thu, 1 Feb 2018 21:32:11 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: pmuldoon@redhat.com, simon.marchi@polymtl.ca,
> 	gdb-patches@sourceware.org
> 
> > > [hi Eli -- not sure if the attached patch is obvious or not, so I will
> > > wait for your approval, assuming Phil confirms. Thank you!]
> > 
> > Are you asking about the NEWS entry or about the entire patch?  If
> > the latter, I don't think I saw it, and neither do I see it in the
> > list archives.  I think.  What am I missing?
> 
> Just asking whether you approve the patch I attached, or if you
> have some comments.

Ah, OK.  But where was it posted?  I don't seem to be able to find it,
neither in my inbox nor in the list archives.  How did I miss it?

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

[Joel Brobecker]

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

> > > > [hi Eli -- not sure if the attached patch is obvious or not, so I will
> > > > wait for your approval, assuming Phil confirms. Thank you!]
> > > 
> > > Are you asking about the NEWS entry or about the entire patch?  If
> > > the latter, I don't think I saw it, and neither do I see it in the
> > > list archives.  I think.  What am I missing?
> > 
> > Just asking whether you approve the patch I attached, or if you
> > have some comments.
> 
> Ah, OK.  But where was it posted?  I don't seem to be able to find it,
> neither in my inbox nor in the list archives.  How did I miss it?

I was posted ... in my head! Another example of the forgetting to
attach the patch classic. Sorry about that.

gdb/ChangeLog:

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

Thanks,
-- 
Joel

[-- Attachment #2: 0001-gdb-NEWS-Clarify-the-news-entry-for-rbreak-in-GDB-8..patch --]
[-- Type: text/x-diff, Size: 1214 bytes --]

From 1db141b40e47bd5fc19dc4ee2a4c6f9c4fe2881b Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 1 Feb 2018 13:41:52 +0400
Subject: [PATCH] gdb/NEWS: Clarify the news entry for "rbreak" in GDB 8.1

gdb/ChangeLog:

        * NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
        Python function, rather than a new command.
---
 gdb/NEWS | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9cd38f3d91..1767cef920 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -125,9 +125,9 @@
      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.
+  ** A new function, "gdb.rbreak" has been added to the Python API.
+     This function allows the setting of a large number of breakpoints
+     via a regex pattern in Python.  See the manual for further details.
 
   ** Python breakpoints can now accept explicit locations.  See the
      manual for a further description of this feature.
-- 
2.11.0

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

[Joel Brobecker]

Hi Eli,

> I was posted ... in my head! Another example of the forgetting to
> attach the patch classic. Sorry about that.
> 
> gdb/ChangeLog:
> 
>         * NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
>         Python function, rather than a new command.

I don't think this patch was reviewed; do you think the patch is OK?

Thank you!


> >From 1db141b40e47bd5fc19dc4ee2a4c6f9c4fe2881b Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Thu, 1 Feb 2018 13:41:52 +0400
> Subject: [PATCH] gdb/NEWS: Clarify the news entry for "rbreak" in GDB 8.1
> 
> gdb/ChangeLog:
> 
>         * NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
>         Python function, rather than a new command.
> ---
>  gdb/NEWS | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9cd38f3d91..1767cef920 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -125,9 +125,9 @@
>       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.
> +  ** A new function, "gdb.rbreak" has been added to the Python API.
> +     This function allows the setting of a large number of breakpoints
> +     via a regex pattern in Python.  See the manual for further details.
>  
>    ** Python breakpoints can now accept explicit locations.  See the
>       manual for a further description of this feature.
> -- 
> 2.11.0
> 


-- 
Joel

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

[Eli Zaretskii]

> Date: Fri, 9 Feb 2018 08:30:05 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: pmuldoon@redhat.com, simon.marchi@polymtl.ca,
> 	gdb-patches@sourceware.org
> 
> > gdb/ChangeLog:
> > 
> >         * NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
> >         Python function, rather than a new command.
> 
> I don't think this patch was reviewed; do you think the patch is OK?

The documentation part LGTM, thanks.

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

[Joel Brobecker]

> > > gdb/ChangeLog:
> > > 
> > >         * NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
> > >         Python function, rather than a new command.
> > 
> > I don't think this patch was reviewed; do you think the patch is OK?
> 
> The documentation part LGTM, thanks.

Thank you, Eli. Patch pushed to master and gdb-8.1-branch
(using GDB PR gdb/22824).

-- 
Joel