diff mbox

New python function gdb.lookup_objfile.

Message ID yjt2sigp4kd3.fsf@ruffy.mtv.corp.google.com
State New
Headers show

Commit Message

Doug Evans Dec. 9, 2014, 2:39 a.m. UTC
Hi.

This patch implements a new python function gdb.lookup_objfile.
It provides the ability to look up an objfile given a file name
or build id.

While one could call gdb.objfiles() and then scan over that,
it requires construction of the list, and with *lots* of objfiles
it is a bit cumbersome.

Regression tested on amd64-linux.

2014-12-08  Doug Evans  <dje@google.com>

	* NEWS: Mention gdb.lookup_objfile.
	* python/python.c (GdbMethods): Add lookup_objfile.
	* python/python-internal.h (gdbpy_lookup_objfile): Declare.
	* python/py-objfile.c: #include "symtab.h".
	(objfpy_build_id_ok, objfpy_build_id_matches): New functions.
	(objfpy_lookup_objfile_by_name): New function.
	(objfpy_lookup_objfile_by_build_id): New function.
	(gdbpy_lookup_objfile): New function.

	doc/
	* python.texi (Objfiles In Python): Document gdb.lookup_objfile.

	testsuite/
	* lib/gdb-python.exp (get_python_valueof): New function.
	* gdb.python/py-objfile.exp: Add tests for gdb.lookup_objfile.

Comments

Phil Muldoon Dec. 9, 2014, 10:49 a.m. UTC | #1
On 09/12/14 02:39, Doug Evans wrote:
> Hi.
>
> This patch implements a new python function gdb.lookup_objfile.
> It provides the ability to look up an objfile given a file name
> or build id.
>
> While one could call gdb.objfiles() and then scan over that,
> it requires construction of the list, and with *lots* of objfiles
> it is a bit cumbersome.
>
> Regression tested on amd64-linux.

Thanks.


> +@findex gdb.lookup_objfile
> +@defun gdb.lookup_objfile (name @r{[}, by_build_id{]})
> +Look up @var{name}, which is a file name, in the list of objfiles
> +for the current program space (@pxref{Progspaces In Python}).
> +If the objfile is not found then a Python @code{ValueError} exception
> +is thrown.


Do wildcard or partial matches work? It would be excellent if they
did, and if so, a note in the documentation.  If not, how much work do
you think it would be to have this functionality?

A soft objection.  Not a fan of functions that return an Exception on
"item not found" personally.  I prefer to return None, and save
exceptions for errors.  But I understand that in some projects this
paradigm is used.

> +{
> +  static char *keywords[] = { "name", "by_build_id", NULL };
> +  const char *name;
> +  PyObject *by_build_id_obj = NULL;
> +  int by_build_id;
> +  struct objfile *objfile;
> +
> +  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!", keywords,
> +                     &name, &PyBool_Type, &by_build_id_obj))
> +    return NULL;

If "name" is an empty string should we just immediately return an
exception here?  There are some operations later that depend on
strlen.  I had a quick look, and it seems none of the functions have a
real hard issue with zero string lookups, but it might be best to
check here first.

Cheers

Phil
Eli Zaretskii Dec. 9, 2014, 4:21 p.m. UTC | #2
> From: Doug Evans <dje@google.com>
> Date: Mon, 08 Dec 2014 18:39:36 -0800
> 
> This patch implements a new python function gdb.lookup_objfile.
> It provides the ability to look up an objfile given a file name
> or build id.
> 
> While one could call gdb.objfiles() and then scan over that,
> it requires construction of the list, and with *lots* of objfiles
> it is a bit cumbersome.
> 
> Regression tested on amd64-linux.
> 
> 2014-12-08  Doug Evans  <dje@google.com>
> 
> 	* NEWS: Mention gdb.lookup_objfile.
> 	* python/python.c (GdbMethods): Add lookup_objfile.
> 	* python/python-internal.h (gdbpy_lookup_objfile): Declare.
> 	* python/py-objfile.c: #include "symtab.h".
> 	(objfpy_build_id_ok, objfpy_build_id_matches): New functions.
> 	(objfpy_lookup_objfile_by_name): New function.
> 	(objfpy_lookup_objfile_by_build_id): New function.
> 	(gdbpy_lookup_objfile): New function.
> 
> 	doc/
> 	* python.texi (Objfiles In Python): Document gdb.lookup_objfile.
> 
> 	testsuite/
> 	* lib/gdb-python.exp (get_python_valueof): New function.
> 	* gdb.python/py-objfile.exp: Add tests for gdb.lookup_objfile.
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 6a2cb9b..93e06d4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -28,6 +28,7 @@
>    ** A new event "gdb.clear_objfiles" has been added, triggered when
>       selecting a new file to debug.
>    ** You can now add attributes to gdb.Objfile and gdb.Progspace objects.
> +  ** New function gdb.lookup_objfile.

OK for NEWS.

> +@findex gdb.lookup_objfile
> +@defun gdb.lookup_objfile (name @r{[}, by_build_id{]})
> +Look up @var{name}, which is a file name, in the list of objfiles
> +for the current program space (@pxref{Progspaces In Python}).

  "Look up @var{name}, a file name or a build ID, in the list of
  objfiles for the current program space."

> +If the objfile is not found then a Python @code{ValueError} exception
> +is thrown.

"... throw the Python ValueError exception" is better, because it
avoids the passive tense.

> +If @var{by_build_id} is provided and is @code{True} then @var{name}
> +is the build ID of the objfile.

I would add "Otherwise, @var{name} is a file name."

The documentation changes are OK with those fixed.

Thanks.
Doug Evans Dec. 9, 2014, 5:23 p.m. UTC | #3
On Tue, Dec 9, 2014 at 2:49 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> On 09/12/14 02:39, Doug Evans wrote:
>> Hi.
>>
>> This patch implements a new python function gdb.lookup_objfile.
>> It provides the ability to look up an objfile given a file name
>> or build id.
>>
>> While one could call gdb.objfiles() and then scan over that,
>> it requires construction of the list, and with *lots* of objfiles
>> it is a bit cumbersome.
>>
>> Regression tested on amd64-linux.
>
> Thanks.
>
>
>> +@findex gdb.lookup_objfile
>> +@defun gdb.lookup_objfile (name @r{[}, by_build_id{]})
>> +Look up @var{name}, which is a file name, in the list of objfiles
>> +for the current program space (@pxref{Progspaces In Python}).
>> +If the objfile is not found then a Python @code{ValueError} exception
>> +is thrown.
>
>
> Do wildcard or partial matches work? It would be excellent if they
> did, and if so, a note in the documentation.  If not, how much work do
> you think it would be to have this functionality?

I'm using gdb's standard routine for filename matches, so yeah partial
names work.
I'll spiff up the docs a bit (hopefully just a cut-n-paste-n-tweak of
existing docs on the subject ... :-))

> A soft objection.  Not a fan of functions that return an Exception on
> "item not found" personally.  I prefer to return None, and save
> exceptions for errors.  But I understand that in some projects this
> paradigm is used.

I'm kinda indifferent, and was just "going with the flow" here.
Some existing lookup routines throw an exception and some return None.
Throwing an exception seemed more Pythonic.

>> +{
>> +  static char *keywords[] = { "name", "by_build_id", NULL };
>> +  const char *name;
>> +  PyObject *by_build_id_obj = NULL;
>> +  int by_build_id;
>> +  struct objfile *objfile;
>> +
>> +  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!", keywords,
>> +                     &name, &PyBool_Type, &by_build_id_obj))
>> +    return NULL;
>
> If "name" is an empty string should we just immediately return an
> exception here?  There are some operations later that depend on
> strlen.  I had a quick look, and it seems none of the functions have a
> real hard issue with zero string lookups, but it might be best to
> check here first.

Yeah, but it's more code, and Consistency Is Good would then make me
want to do an initial check for empty strings in every lookup routine.
diff mbox

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 6a2cb9b..93e06d4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -28,6 +28,7 @@ 
   ** A new event "gdb.clear_objfiles" has been added, triggered when
      selecting a new file to debug.
   ** You can now add attributes to gdb.Objfile and gdb.Progspace objects.
+  ** New function gdb.lookup_objfile.
 
 * New Python-based convenience functions:
 
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index efd258d..b54d341 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3488,6 +3488,22 @@  Return a sequence of all the objfiles current known to @value{GDBN}.
 @xref{Objfiles In Python}.
 @end defun
 
+@findex gdb.lookup_objfile
+@defun gdb.lookup_objfile (name @r{[}, by_build_id{]})
+Look up @var{name}, which is a file name, in the list of objfiles
+for the current program space (@pxref{Progspaces In Python}).
+If the objfile is not found then a Python @code{ValueError} exception
+is thrown.
+
+If @var{by_build_id} is provided and is @code{True} then @var{name}
+is the build ID of the objfile.
+This is supported only on some operating systems, notably those which use
+the ELF format for binary files and the @sc{gnu} Binutils.  For more details
+about this feature, see the description of the @option{--build-id}
+command-line option in @ref{Options, , Command Line Options, ld.info,
+The GNU Linker}.
+@end defun
+
 Each objfile is represented by an instance of the @code{gdb.Objfile}
 class.
 
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 51cf47c..e78ceba 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -24,6 +24,7 @@ 
 #include "language.h"
 #include "build-id.h"
 #include "elf-bfd.h"
+#include "symtab.h"
 
 typedef struct
 {
@@ -380,6 +385,147 @@  objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
   Py_RETURN_NONE;
 }
 
+/* Subroutine of gdbpy_lookup_objfile_by_build_id to simplify it.
+   Return non-zero if STRING is a potentially valid build id.  */
+
+static int
+objfpy_build_id_ok (const char *string)
+{
+  size_t i, n = strlen (string);
+
+  if (n % 2 != 0)
+    return 0;
+  for (i = 0; i < n; ++i)
+    {
+      if (!isxdigit (string[i]))
+	return 0;
+    }
+  return 1;
+}
+
+/* Subroutine of gdbpy_lookup_objfile_by_build_id to simplify it.
+   Returns non-zero if BUILD_ID matches STRING.
+   It is assumed that objfpy_build_id_ok (string) returns TRUE.  */
+
+static int
+objfpy_build_id_matches (const struct elf_build_id *build_id,
+			 const char *string)
+{
+  size_t i;
+
+  if (strlen (string) != 2 * build_id->size)
+    return 0;
+
+  for (i = 0; i < build_id->size; ++i)
+    {
+      char c1 = string[i * 2], c2 = string[i * 2 + 1];
+      int byte = (host_hex_value (c1) << 4) | host_hex_value (c2);
+
+      if (byte != build_id->data[i])
+	return 0;
+    }
+
+  return 1;
+}
+
+/* Subroutine of gdbpy_lookup_objfile to simplify it.
+   Look up an objfile by its file name.  */
+
+static struct objfile *
+objfpy_lookup_objfile_by_name (const char *name)
+{
+  struct objfile *objfile;
+
+  ALL_OBJFILES (objfile)
+    {
+      if ((objfile->flags & OBJF_NOT_FILENAME) != 0)
+	continue;
+      /* Don't return separate debug files.  */
+      if (objfile->separate_debug_objfile_backlink != NULL)
+	continue;
+      if (compare_filenames_for_search (objfile_name (objfile), name))
+	return objfile;
+    }
+
+  return NULL;
+}
+
+/* Subroutine of gdbpy_lookup_objfile to simplify it.
+   Look up an objfile by its build id.  */
+
+static struct objfile *
+objfpy_lookup_objfile_by_build_id (const char *build_id)
+{
+  struct objfile *objfile;
+
+  ALL_OBJFILES (objfile)
+    {
+      const struct elf_build_id *obfd_build_id;
+
+      if (objfile->obfd == NULL)
+	continue;
+      /* Don't return separate debug files.  */
+      if (objfile->separate_debug_objfile_backlink != NULL)
+	continue;
+      obfd_build_id = build_id_bfd_get (objfile->obfd);
+      if (obfd_build_id == NULL)
+	continue;
+      if (objfpy_build_id_matches (obfd_build_id, build_id))
+	return objfile;
+    }
+
+  return NULL;
+}
+
+/* Implementation of gdb.lookup_objfile.  */
+
+PyObject *
+gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw)
+{
+  static char *keywords[] = { "name", "by_build_id", NULL };
+  const char *name;
+  PyObject *by_build_id_obj = NULL;
+  int by_build_id;
+  struct objfile *objfile;
+
+  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!", keywords,
+				     &name, &PyBool_Type, &by_build_id_obj))
+    return NULL;
+
+  by_build_id = 0;
+  if (by_build_id_obj != NULL)
+    {
+      int cmp = PyObject_IsTrue (by_build_id_obj);
+
+      if (cmp < 0)
+	return NULL;
+      by_build_id = cmp;
+    }
+
+  if (by_build_id)
+    {
+      if (!objfpy_build_id_ok (name))
+	{
+	  PyErr_SetString (PyExc_TypeError, _("Not a valid build id."));
+	  return NULL;
+	}
+      objfile = objfpy_lookup_objfile_by_build_id (name);
+    }
+  else
+    objfile = objfpy_lookup_objfile_by_name (name);
+
+  if (objfile != NULL)
+    {
+      PyObject *result = objfile_to_objfile_object (objfile);
+
+      Py_XINCREF (result);
+      return result;
+    }
+
+  PyErr_SetString (PyExc_ValueError, _("Objfile not found."));
+  return NULL;
+}
+
 
 
 /* Clear the OBJFILE pointer in an Objfile object and remove the
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 716c0de..544fe93 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -393,6 +393,7 @@  PyObject *objfile_to_objfile_object (struct objfile *)
 PyObject *objfpy_get_printers (PyObject *, void *);
 PyObject *objfpy_get_frame_filters (PyObject *, void *);
 PyObject *objfpy_get_xmethods (PyObject *, void *);
+PyObject *gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw);
 
 PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1362bd2..b1d8283 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1956,6 +1956,14 @@  a boolean indicating if name is a field of the current implied argument\n\
     METH_VARARGS | METH_KEYWORDS,
     "lookup_global_symbol (name [, domain]) -> symbol\n\
 Return the symbol corresponding to the given name (or None)." },
+
+  { "lookup_objfile", (PyCFunction) gdbpy_lookup_objfile,
+    METH_VARARGS | METH_KEYWORDS,
+    "lookup_objfile (name, [by_build_id]) -> objfile\n\
+Look up the specified objfile.\n\
+If by_build_id is True, the objfile is looked up by using name\n\
+as its build id." },
+
   { "block_for_pc", gdbpy_block_for_pc, METH_VARARGS,
     "Return the block containing the given pc value, or None." },
   { "solib_name", gdbpy_solib_name, METH_VARARGS,
diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
index f3a8a6c..a56bc95 100644
--- a/gdb/testsuite/gdb.python/py-objfile.exp
+++ b/gdb/testsuite/gdb.python/py-objfile.exp
@@ -32,23 +32,39 @@  if ![runto_main] then {
     return 0
 }
 
+set python_error_text "Error while executing Python code\\."
+
 gdb_py_test_silent_cmd "python sym = gdb.lookup_symbol(\"some_var\")" \
     "Find a symbol in objfile" 1
 gdb_py_test_silent_cmd "python objfile = sym\[0\].symtab.objfile" \
     "Get backing object file" 1
 
-gdb_test "python print (objfile.filename)" ".*py-objfile.*" \
+gdb_test "python print (objfile.filename)" ".*${testfile}" \
   "Get objfile file name"
 
+gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").filename)" \
+    "${testfile}"
+gdb_test "python print (gdb.lookup_objfile (\"junk\"))" \
+    "Objfile not found\\.\r\n${python_error_text}"
+
 set binfile_build_id [get_build_id $binfile]
 if [string compare $binfile_build_id ""] {
     verbose -log "binfile_build_id = $binfile_build_id"
     gdb_test "python print (objfile.build_id)" "$binfile_build_id" \
     "Get objfile build id"
+    gdb_test "python print (gdb.lookup_objfile (\"$binfile_build_id\", by_build_id=True).filename)" \
+	"${testfile}"
 } else {
     unsupported "build-id is not supported by the compiler"
 }
 
+# Other lookup_objfile_by_build_id tests we can do, even if compiler doesn't
+# support them.
+gdb_test "python print (gdb.lookup_objfile (\"foo\", by_build_id=True))" \
+    "Not a valid build id\\.\r\n${python_error_text}"
+gdb_test "python print (gdb.lookup_objfile (\"1234abcdef\", by_build_id=True))" \
+    "Objfile not found\\.\r\n${python_error_text}"
+
 gdb_test "python print (objfile.progspace)" "<gdb\.Progspace object at .*>" \
   "Get objfile program space"
 gdb_test "python print (objfile.is_valid())" "True" \
@@ -93,3 +109,9 @@  gdb_test "python print (sep_objfile.owner.filename)" "${testfile}2" \
 
 gdb_test "p main" "= {int \\(\\)} $hex <main>" \
     "print main with debug info"
+
+# Separate debug files are not findable.
+if { [get_python_valueof "sep_objfile.build_id" "None"] != "None" } {
+    gdb_test "python print (gdb.lookup_objfile (sep_objfile.build_id, by_build_id=True))" \
+	"Objfile not found\\.\r\n${python_error_text}"
+}
diff --git a/gdb/testsuite/lib/gdb-python.exp b/gdb/testsuite/lib/gdb-python.exp
index d5e7928..eefff73 100644
--- a/gdb/testsuite/lib/gdb-python.exp
+++ b/gdb/testsuite/lib/gdb-python.exp
@@ -45,3 +45,24 @@  proc gdb_py_test_multiple { name args } {
     }
     return 0
 }
+
+# Return the result of python expression EXPR.
+# DEFAULT is returned if there's an error.
+# This is modelled after get_integer_valueof.
+
+proc get_python_valueof { exp default } {
+    global gdb_prompt
+
+    set test "get python valueof \"${exp}\""
+    set val ${default}
+    gdb_test_multiple "python print (\"valueof: %s\" % (${exp}))" "$test" {
+	-re "valueof: (\[^\r\n\]*)\[\r\n\]*$gdb_prompt $" {
+	    set val $expect_out(1,string)
+	    pass "$test ($val)"
+	}
+	timeout {
+	    fail "$test (timeout)"
+	}
+    }
+    return ${val}
+}