[6/7] py-minsymbol: Add interface to access minimal_symbols

  From: Jeff Mahoney <jeffm@suse.com>

This patch adds a new gdb.MinSymbol object to export the minimal_symbol
interface.  These objects can be resolved using a new
gdb.lookup_minimal_symbol call.
---
 gdb/Makefile.in              |   6 +
 gdb/python/py-minsymbol.c    | 390 +++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |   6 +
 gdb/python/python.c          |   5 +
 4 files changed, 407 insertions(+)
 create mode 100644 gdb/python/py-minsymbol.c
  

On 04/02/16 17:29, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com> > > This patch adds a new gdb.MinSymbol object to export the minimal_symbol > interface.  These objects can be resolved using a new > gdb.lookup_minimal_symbol call. > --- >  gdb/Makefile.in              |   6 + >  gdb/python/py-minsymbol.c    | 390 +++++++++++++++++++++++++++++++++++++++++++ >  gdb/python/python-internal.h |   6 + >  gdb/python/python.c          |   5 + >  4 files changed, 407 insertions(+) >  create mode 100644 gdb/python/py-minsymbol.c > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index 3eadbbc..c2f7eef 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -400,6 +400,7 @@ SUBDIR_PYTHON_OBS = \ >      py-infthread.o \ >      py-lazy-string.o \ >      py-linetable.o \ > +    py-minsymbol.o \ >      py-newobjfileevent.o \ >      py-objfile.o \ >      py-param.o \ > @@ -440,6 +441,7 @@ SUBDIR_PYTHON_SRCS = \ >      python/py-infthread.c \ >      python/py-lazy-string.c \ >      python/py-linetable.c \ > +    python/py-minsymbol.c \ >     
python/py-newobjfileevent.c \ >      python/py-objfile.c \ >      python/py-param.c \ > @@ -2635,6 +2637,10 @@ py-linetable.o: $(srcdir)/python/py-linetable.c >      $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-linetable.c >      $(POSTCOMPILE) >  > +py-minsymbol.o: $(srcdir)/python/py-minsymbol.c > +    $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-minsymbol.c > +    $(POSTCOMPILE) > + >  py-newobjfileevent.o: $(srcdir)/python/py-newobjfileevent.c >      $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-newobjfileevent.c >      $(POSTCOMPILE) > diff --git a/gdb/python/py-minsymbol.c b/gdb/python/py-minsymbol.c > new file mode 100644 > index 0000000..d8b56bc > --- /dev/null > +++ b/gdb/python/py-minsymbol.c > @@ -0,0 +1,390 @@ > +/* Python interface to minsymbols. > + > +   Copyright (C) 2008-2013 Free Software Foundation, Inc.

Wrong copyright date.

> + > +   This file is part of GDB. > + > +   This program is free software; you can redistribute it and/or modify > +   it under the terms of the GNU General Public License as published by > +   the Free Software Foundation; either version 3 of the License, or > +   (at your option) any later version. > + > +   This program is distributed in the hope that it will be useful, > +   but WITHOUT ANY WARRANTY; without even the implied warranty of > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the > +   GNU General Public License for more details. > + > +   You should have received a copy of the GNU General Public License > +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */ > + > +#include "defs.h" > +#include "block.h" > +#include "exceptions.h" > +#include "frame.h" > +#include "symtab.h" > +#include "python-internal.h" > +#include "objfiles.h" > +#include "value.h" > + > +typedef struct msympy_symbol_object { > +  PyObject_HEAD > +  /*
The GDB minimal_symbol structure this object is wrapping.  */ > +  struct minimal_symbol *minsym; > +  struct objfile *objfile; > + > +  struct type *type; > +  /* A symbol object is associated with an objfile, so keep track with > +     doubly-linked list, rooted in the objfile.  This lets us > +     invalidate the underlying struct minimal_symbol when the objfile is > +     deleted.  */ > +  struct msympy_symbol_object *prev; > +  struct msympy_symbol_object *next; > +} minsym_object;

Can you please document all elements in the struct.

> +/* Return the symbol that is wrapped by this symbol object.  */ > +static struct minimal_symbol * > +minsym_object_to_minsym (PyObject *obj) > +{ > +  if (! PyObject_TypeCheck (obj, &minsym_object_type)) > +    return NULL; > +  return ((minsym_object *) obj)->minsym; > +} > + > +static struct objfile * > +minsym_object_to_objfile (PyObject *obj) > +{ > +  if (! PyObject_TypeCheck (obj, &minsym_object_type)) > +    return NULL; > +  return ((minsym_object *) obj)->objfile; > +} > + > +/* Require a valid symbol.  All access to minsym_object->symbol should be > +   gated by this call.  */ > +#define MSYMPY_REQUIRE_VALID(minsym_obj, minsym)    \ > +  do {                            \ > +    minsym = minsym_object_to_minsym (minsym_obj);    \ > +    if (minsym == NULL)                \ > +      {                            \ > +    PyErr_SetString (PyExc_RuntimeError,        \ > +             _("MinSymbol is invalid."));    \ > +    return NULL;                    \ > +      }                            \ > +  } while (0) > + > +#define
MSYMPY_REQUIRE_VALID_BOUND(minsym_obj, minsym, objfile)    \ > +  do {                                \ > +    minsym = minsym_object_to_minsym (minsym_obj);        \ > +    objfile = minsym_object_to_objfile (minsym_obj);        \ > +    if (minsym == NULL || objfile == NULL)            \ > +      {                                \ > +    PyErr_SetString (PyExc_RuntimeError,            \ > +             _("MinSymbol is invalid."));        \ > +    return NULL;                        \ > +      }                                \ > +  } while (0) > + > +static PyObject * > +msympy_str (PyObject *self) > +{ > +  PyObject *result; > +  struct minimal_symbol *minsym = NULL; > + > +  MSYMPY_REQUIRE_VALID (self, minsym); > + > +  result = PyString_FromString (MSYMBOL_PRINT_NAME (minsym)); > + > +  return result; > +} > + > +static PyObject * > +msympy_get_name (PyObject *self, void *closure) > +{ > +  struct minimal_symbol *minsym = NULL; > + > +  MSYMPY_REQUIRE_VALID (self,
minsym); > + > +  return PyString_FromString (MSYMBOL_NATURAL_NAME (minsym)); > +} > + > +static PyObject * > +msympy_get_file_name (PyObject *self, void *closure) > +{ > +  struct minimal_symbol *minsym = NULL; > + > +  MSYMPY_REQUIRE_VALID (self, minsym); > + > +  return PyString_FromString (minsym->filename); > +} > + > +static PyObject * > +msympy_get_linkage_name (PyObject *self, void *closure) > +{ > +  struct minimal_symbol *minsym = NULL; > + > +  MSYMPY_REQUIRE_VALID (self, minsym); > + > +  return PyString_FromString (MSYMBOL_LINKAGE_NAME (minsym)); > +} > + > +static PyObject * > +msympy_get_print_name (PyObject *self, void *closure) > +{ > +  struct minimal_symbol *minsym = NULL; > + > +  MSYMPY_REQUIRE_VALID (self, minsym); > + > +  return msympy_str (self); > +} > + > +static PyObject * > +msympy_get_section (PyObject *self, void *closure) > +{ > +  struct minimal_symbol *minsym = NULL; > +  struct objfile *objfile = NULL; > +  struct obj_section *section; > + 
const char *name; > + > +  MSYMPY_REQUIRE_VALID_BOUND (self, minsym, objfile); > + > +  section = MSYMBOL_OBJ_SECTION (objfile, minsym); > +  if (section) {

Newline before {.

> +    name = bfd_section_name (objfile->obfd, section->the_bfd_section); > +    if (name) > +      return PyString_FromString (name); > +  } > + > +  Py_RETURN_NONE; > + > +} > + > +static PyObject * > +msympy_is_valid (PyObject *self, PyObject *args) > +{ > +  struct minimal_symbol *minsym = NULL; > + > +  minsym = minsym_object_to_minsym (self); > +  if (minsym == NULL) > +    Py_RETURN_FALSE; > + > +  Py_RETURN_TRUE; > +} > + > +/* Implementation of gdb.MinSymbol.value (self) -> gdb.Value.  Returns > +   the value of the symbol, or an error in various circumstances.  */ > + > +static PyObject * > +msympy_value (PyObject *self, PyObject *args) > +{ > +  minsym_object *minsym_obj = (minsym_object *)self; > +  struct minimal_symbol *minsym = NULL; > +  struct value *value = NULL; > + > +  if (!PyArg_ParseTuple (args, "")) > +    return NULL; > + > +  MSYMPY_REQUIRE_VALID (self, minsym); > +  TRY > +    { > +      value = value_from_ulongest(minsym_obj->type, > +                  MSYMBOL_VALUE_RAW_ADDRESS(minsym));

Spaces in function name and (. This an others.

> +      if (value) > +    set_value_address(value, MSYMBOL_VALUE_RAW_ADDRESS(minsym)); > +    }

Ditto x 2.

> +  CATCH (except, RETURN_MASK_ALL) > +    { > +      GDB_PY_HANDLE_EXCEPTION (except); > +    } > +  END_CATCH > + > +  return value_to_value_object (value); > +} > + > +/* Given a symbol, and a minsym_object that has previously been > +   allocated and initialized, populate the minsym_object with the > +   struct minimal_symbol data.  Also, register the minsym_object life-cycle > +   with the life-cycle of the object file associated with this > +   symbol, if needed.  */ > +static void > +set_symbol (minsym_object *obj, struct bound_minimal_symbol *bound) > +{ > +  obj->minsym = bound->minsym; > +  obj->objfile = bound->objfile; > +  switch (obj->minsym->type) { > +  case mst_text: > +  case mst_solib_trampoline: > +  case mst_file_text: > +  case mst_text_gnu_ifunc: > +  case mst_slot_got_plt: > +    obj->type = builtin_type(python_gdbarch)->builtin_func_ptr; > +    break; > +

Ditto.

> +  case mst_data: > +  case mst_abs: > +  case mst_bss: > +  case mst_file_data: > +  case mst_file_bss: > +    obj->type = builtin_type(python_gdbarch)->builtin_data_ptr; > +    break; > +

Ditto.

> +  case mst_unknown: > +  default: > +    obj->type = builtin_type(python_gdbarch)->builtin_void; > +    break; > +  } > +

Ditto.
> +  obj->prev = NULL; > +  obj->next = NULL; > +} > + > +static PyObject * > +bound_minsym_to_minsym_object (struct bound_minimal_symbol *bound) > +{ > +  minsym_object *msym_obj; > + > +  msym_obj = PyObject_New (minsym_object, &minsym_object_type); > +  if (msym_obj) > +    set_symbol (msym_obj, bound); > + > +  return (PyObject *) msym_obj; > +} > + > +static void > +msympy_dealloc (PyObject *obj) > +{ > +  minsym_object *msym_obj = (minsym_object *) obj; > + > +  if (msym_obj->prev) > +    msym_obj->prev->next = msym_obj->next; > +  if (msym_obj->next) > +    msym_obj->next->prev = msym_obj->prev; > +  msym_obj->minsym = NULL; > +  msym_obj->objfile = NULL; > +} > + > +/* Implementation of > +   gdb.lookup_minimal_symbol (name) -> symbol or None.  */ > + > +PyObject * > +gdbpy_lookup_minimal_symbol (PyObject *self, PyObject *args, PyObject *kw) > +{ > +  int domain = VAR_DOMAIN; > +  const char *name; > +  static char *keywords[] = { "name", NULL }; > +  struct bound_minimal_symbol
bound_minsym = {}; > +  struct minimal_symbol *minsym = NULL; > +  PyObject *msym_obj = NULL; > + > +  if (!PyArg_ParseTupleAndKeywords (args, kw, "s|", keywords, &name)) > +    return NULL; > + > +  TRY > +    { > +      bound_minsym = lookup_minimal_symbol (name, NULL, NULL); > +    } > +  CATCH (except, RETURN_MASK_ALL) > +    { > +      GDB_PY_HANDLE_EXCEPTION (except); > +    } > +  END_CATCH > + > +  if (bound_minsym.minsym) > +      msym_obj = bound_minsym_to_minsym_object (&bound_minsym); > + > +  if (msym_obj) > +    return msym_obj; > + > +  Py_RETURN_NONE; > +} > + > +int > +gdbpy_initialize_minsymbols (void) > +{ > +  if (PyType_Ready (&minsym_object_type) < 0) > +    return -1; > + > +  if (PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_UNKNOWN", > +                   mst_unknown) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_TEXT", mst_text) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_TEXT_GNU_IFUNC", > +           
      mst_text_gnu_ifunc) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_SLOT_GOT_PLT", > +                  mst_slot_got_plt) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_DATA", mst_data) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_BSS", mst_bss) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_ABS", mst_abs) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_SOLIB_TRAMPOLINE", > +                  mst_solib_trampoline) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_FILE_TEXT", > +                  mst_file_text) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_FILE_DATA", > +                  mst_file_data) < 0 > +  || PyModule_AddIntConstant (gdb_module, "MINSYMBOL_TYPE_FILE_BSS", > +                  mst_file_bss) < 0) > +    return -1; > + > +  return gdb_pymodule_addobject (gdb_module, "MinSymbol", > +                 (PyObject *)
&minsym_object_type); > +} > + > + > + > +static PyGetSetDef minsym_object_getset[] = { > +  { "name", msympy_get_name, NULL, > +    "Name of the symbol, as it appears in the source code.", NULL }, > +  { "linkage_name", msympy_get_linkage_name, NULL, > +    "Name of the symbol, as used by the linker (i.e., may be mangled).", > +    NULL }, > +  { "filename", msympy_get_file_name, NULL, > +    "Name of source file that contains this symbol. Only applies for mst_file_*.", > +    NULL }, > +  { "print_name", msympy_get_print_name, NULL, > +    "Name of the symbol in a form suitable for output.\n\ > +This is either name or linkage_name, depending on whether the user asked GDB\n\ > +to display demangled or mangled names.", NULL }, > +  { "section", msympy_get_section, NULL, > +    "Section that contains this symbol, if any", NULL, }, > +  { NULL }  /* Sentinel */ > +}; > + > +static PyMethodDef minsym_object_methods[] = { > +  { "is_valid", msympy_is_valid, METH_NOARGS, > +   
"is_valid () -> Boolean.\n\ > +Return true if this symbol is valid, false if not." }, > +  { "value", msympy_value, METH_VARARGS, > +    "value ([frame]) -> gdb.Value\n\ > +Return the value of the symbol." }, > +  {NULL}  /* Sentinel */ > +}; > + > +PyTypeObject minsym_object_type = { > +  PyVarObject_HEAD_INIT (NULL, 0) > +  "gdb.MinSymbol",          /*tp_name*/ > +  sizeof (minsym_object),      /*tp_basicsize*/ > +  0,                  /*tp_itemsize*/ > +  msympy_dealloc,          /*tp_dealloc*/ > +  0,                  /*tp_print*/ > +  0,                  /*tp_getattr*/ > +  0,                  /*tp_setattr*/ > +  0,                  /*tp_compare*/ > +  0,                  /*tp_repr*/ > +  0,                  /*tp_as_number*/ > +  0,                  /*tp_as_sequence*/ > +  0,                  /*tp_as_mapping*/ > +  0,                  /*tp_hash */ > +  0,                  /*tp_call*/ > +  msympy_str,              /*tp_str*/ > +  0,                  /*tp_getattro*/ > + 
0,                  /*tp_setattro*/ > +  0,                  /*tp_as_buffer*/ > +  Py_TPFLAGS_DEFAULT,          /*tp_flags*/ > +  "GDB minimal symbol object",      /*tp_doc */ > +  0,                  /*tp_traverse */ > +  0,                  /*tp_clear */ > +  0,                  /*tp_richcompare */ > +  0,                  /*tp_weaklistoffset */ > +  0,                  /*tp_iter */ > +  0,                  /*tp_iternext */ > +  minsym_object_methods,      /*tp_methods */ > +  0,                  /*tp_members */ > +  minsym_object_getset          /*tp_getset */ > +}; > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h > index ee949b7..2fae31d 100644 > --- a/gdb/python/python-internal.h > +++ b/gdb/python/python-internal.h > @@ -241,6 +241,8 @@ extern PyTypeObject block_object_type >      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF("block_object"); >  extern PyTypeObject symbol_object_type >      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("symbol_object"); > +extern
PyTypeObject minsym_object_type; > +     CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("minsym_object"); >  extern PyTypeObject event_object_type >      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("event_object"); >  extern PyTypeObject stop_event_object_type > @@ -362,6 +364,8 @@ PyObject *gdbpy_frame_stop_reason_string (PyObject *, PyObject *); >  PyObject *gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw); >  PyObject *gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, >                        PyObject *kw); > +PyObject *gdbpy_lookup_minimal_symbol (PyObject *self, PyObject *args, > +                       PyObject *kw); >  PyObject *gdbpy_newest_frame (PyObject *self, PyObject *args); >  PyObject *gdbpy_selected_frame (PyObject *self, PyObject *args); >  PyObject *gdbpy_block_for_pc (PyObject *self, PyObject *args); > @@ -436,6 +440,8 @@ int gdbpy_initialize_commands (void) >    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; >  int gdbpy_initialize_symbols (void) >   
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; > +int gdbpy_initialize_minsymbols (void) > +  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; >  int gdbpy_initialize_symtabs (void) >    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; >  int gdbpy_initialize_blocks (void) > diff --git a/gdb/python/python.c b/gdb/python/python.c > index 6cbe5f0..30a86e1 100644 > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -1800,6 +1800,7 @@ message == an error message without a stack will be printed."), >        || gdbpy_initialize_frames () < 0 >        || gdbpy_initialize_commands () < 0 >        || gdbpy_initialize_symbols () < 0 > +      || gdbpy_initialize_minsymbols () < 0 >        || gdbpy_initialize_symtabs () < 0 >        || gdbpy_initialize_blocks () < 0 >        || gdbpy_initialize_functions () < 0 > @@ -2025,6 +2026,10 @@ 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_minimal_symbol", (PyCFunction) gdbpy_lookup_minimal_symbol, > +    METH_VARARGS | METH_KEYWORDS, > +    "lookup_minimal_symbol (name) -> minsym\n\ > +Return the symbol corresponding to the given name (or None)." }, >  >    { "lookup_objfile", (PyCFunction) gdbpy_lookup_objfile, >      METH_VARARGS | METH_KEYWORDS,


This needs a set of tests and documentation.

Cheers

Phil
  

On 04/02/16 17:29, jeffm@suse.com wrote:
> From: Jeff Mahoney <jeffm@suse.com>
>
> This patch adds a new gdb.MinSymbol object to export the minimal_symbol
> interface.  These objects can be resolved using a new
> gdb.lookup_minimal_symbol call.
> ---
>  gdb/Makefile.in              |   6 +
>  gdb/python/py-minsymbol.c    | 390 +++++++++++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h |   6 +
>  gdb/python/python.c          |   5 +
>  4 files changed, 407 insertions(+)
>  create mode 100644 gdb/python/py-minsymbol.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 3eadbbc..c2f7eef 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -400,6 +400,7 @@ SUBDIR_PYTHON_OBS = \
>  	py-infthread.o \
>  	py-lazy-string.o \
>  	py-linetable.o \
> +	py-minsymbol.o \
>  	py-newobjfileevent.o \
>  	py-objfile.o \
>  	py-param.o \
> @@ -440,6 +441,7 @@ SUBDIR_PYTHON_SRCS = \
>  	python/py-infthread.c \
>  	python/py-lazy-string.c \
>  	python/py-linetable.c \
> +	python/py-minsymbol.c \
>  	python/py-newobjfileevent.c \
>  	python/py-objfile.c \
>  	python/py-param.c \
> @@ -2635,6 +2637,10 @@ py-linetable.o: $(srcdir)/python/py-linetable.c
>  	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-linetable.c
>  	$(POSTCOMPILE)
>  
> +py-minsymbol.o: $(srcdir)/python/py-minsymbol.c
> +	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-minsymbol.c
> +	$(POSTCOMPILE)
> +
>  py-newobjfileevent.o: $(srcdir)/python/py-newobjfileevent.c
>  	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-newobjfileevent.c
>  	$(POSTCOMPILE)
> diff --git a/gdb/python/py-minsymbol.c b/gdb/python/py-minsymbol.c
> new file mode 100644
> index 0000000..d8b56bc
> --- /dev/null
> +++ b/gdb/python/py-minsymbol.c
> @@ -0,0 +1,390 @@
> +/* Python interface to minsymbols.
> +
> +   Copyright (C) 2008-2013 Free Software Foundation, Inc.
Wrong Copyright date.

> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "block.h"
> +#include "exceptions.h"
> +#include "frame.h"
> +#include "symtab.h"
> +#include "python-internal.h"
> +#include "objfiles.h"
> +#include "value.h"
> +
> +typedef struct msympy_symbol_object {
> +  PyObject_HEAD
> +  /* The GDB minimal_symbol structure this object is wrapping.  */
> +  struct minimal_symbol *minsym;
> +  struct objfile *objfile;
> +
> +  struct type *type;
> +  /* A symbol object is associated with an objfile, so keep track with
> +     doubly-linked list, rooted in the objfile.  This lets us
> +     invalidate the underlying struct minimal_symbol when the objfile is
> +     deleted.  */
> +  struct msympy_symbol_object *prev;
> +  struct msympy_symbol_object *next;
> +} minsym_object;
Can you please document all members of a struct here.
> +/* Return the symbol that is wrapped by this symbol object.  */
> +static struct minimal_symbol *
> +minsym_object_to_minsym (PyObject *obj)
> +{
> +  if (! PyObject_TypeCheck (obj, &minsym_object_type))
> +    return NULL;
> +  return ((minsym_object *) obj)->minsym;
> +}
> +
> +static struct objfile *
> +minsym_object_to_objfile (PyObject *obj)
> +{
> +  if (! PyObject_TypeCheck (obj, &minsym_object_type))
> +    return NULL;
> +  return ((minsym_object *) obj)->objfile;
> +}
> +
> +/* Require a valid symbol.  All access to minsym_object->symbol should be
> +   gated by this call.  */
> +#define MSYMPY_REQUIRE_VALID(minsym_obj, minsym)	\
> +  do {							\
> +    minsym = minsym_object_to_minsym (minsym_obj);	\
> +    if (minsym == NULL)				\
> +      {							\
> +	PyErr_SetString (PyExc_RuntimeError,		\
> +			 _("MinSymbol is invalid."));	\
> +	return NULL;					\
> +      }							\
> +  } while (0)
> +
> +#define MSYMPY_REQUIRE_VALID_BOUND(minsym_obj, minsym, objfile)	\
> +  do {								\
> +    minsym = minsym_object_to_minsym (minsym_obj);		\
> +    objfile = minsym_object_to_objfile (minsym_obj);		\
> +    if (minsym == NULL || objfile == NULL)			\
> +      {								\
> +	PyErr_SetString (PyExc_RuntimeError,			\
> +			 _("MinSymbol is invalid."));		\
> +	return NULL;						\
> +      }								\
> +  } while (0)
> +
> +static PyObject *
> +msympy_str (PyObject *self)
> +{
> +  PyObject *result;
> +  struct minimal_symbol *minsym = NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +
> +  result = PyString_FromString (MSYMBOL_PRINT_NAME (minsym));
> +
> +  return result;
> +}
> +
> +static PyObject *
> +msympy_get_name (PyObject *self, void *closure)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +
> +  return PyString_FromString (MSYMBOL_NATURAL_NAME (minsym));
> +}
> +
> +static PyObject *
> +msympy_get_file_name (PyObject *self, void *closure)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +
> +  return PyString_FromString (minsym->filename);
> +}
> +
> +static PyObject *
> +msympy_get_linkage_name (PyObject *self, void *closure)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +
> +  return PyString_FromString (MSYMBOL_LINKAGE_NAME (minsym));
> +}
> +
> +static PyObject *
> +msympy_get_print_name (PyObject *self, void *closure)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +
> +  return msympy_str (self);
> +}
> +
> +static PyObject *
> +msympy_get_section (PyObject *self, void *closure)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +  struct objfile *objfile = NULL;
> +  struct obj_section *section;
> +  const char *name;
> +
> +  MSYMPY_REQUIRE_VALID_BOUND (self, minsym, objfile);
> +
> +  section = MSYMBOL_OBJ_SECTION (objfile, minsym);
> +  if (section) 
The { on a newline.
> {
> +    name = bfd_section_name (objfile->obfd, section->the_bfd_section);
> +    if (name)
> +      return PyString_FromString (name);
> +  }
> +
> +  Py_RETURN_NONE;
> +
> +}
> +
> +static PyObject *
> +msympy_is_valid (PyObject *self, PyObject *args)
> +{
> +  struct minimal_symbol *minsym = NULL;
> +
> +  minsym = minsym_object_to_minsym (self);
> +  if (minsym == NULL)
> +    Py_RETURN_FALSE;
> +
> +  Py_RETURN_TRUE;
> +}
> +
> +/* Implementation of gdb.MinSymbol.value (self) -> gdb.Value.  Returns
> +   the value of the symbol, or an error in various circumstances.  */
> +
> +static PyObject *
> +msympy_value (PyObject *self, PyObject *args)
> +{
> +  minsym_object *minsym_obj = (minsym_object *)self;
> +  struct minimal_symbol *minsym = NULL;
> +  struct value *value = NULL;
> +
> +  if (!PyArg_ParseTuple (args, ""))
> +    return NULL;
> +
> +  MSYMPY_REQUIRE_VALID (self, minsym);
> +  TRY
> +    {
> +      value = value_from_ulongest(minsym_obj->type,
> +				  MSYMBOL_VALUE_RAW_ADDRESS(minsym));
Space after function name and before (. This and other places.

The patch series, in principle, looks fine to me other than the nits I've highlighted.

However it needs tests for the new functionality.  It would be helpful if you could indicate if you have run the testsuite
and checked for regressions.

It also needs documentation additions and changes, and a documentation review.

Also please wait for a maintainer to sign off on it, too

Cheers

Phil
  

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/5/16 11:43 AM, Phil Muldoon wrote:
> On 04/02/16 17:29, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>> 
>> This patch adds a new gdb.MinSymbol object to export the
>> minimal_symbol interface.  These objects can be resolved using a
>> new gdb.lookup_minimal_symbol call. --- +static PyObject * 
>> +msympy_get_file_name (PyObject *self, void *closure) +{ +
>> struct minimal_symbol *minsym = NULL; + +  MSYMPY_REQUIRE_VALID
>> (self, minsym); + +  return PyString_FromString
>> (minsym->filename); +}

In writing the test cases, I discovered that this value usually (but
not always) has a value but the value tends to be meaningless or at
least compiler dependent.  In my test program, it dumps asm("...")
code into crtstuff.c.  I tried using an assembly file instead (with
generic pseudoinstructions and labels) and it presented an empty
filename.  When I load up the kernel, it puts every minimal symbol in
csum-wrappers_64.c (at least for this kernel build).  Given the
questionable utility of this value, should I even keep it?

[Removed other details for review; I have addressed them in each patch.]

> The patch series, in principle, looks fine to me other than the
> nits I've highlighted.
> 
> However it needs tests for the new functionality.  It would be
> helpful if you could indicate if you have run the testsuite and
> checked for regressions.
> 
> It also needs documentation additions and changes, and a
> documentation review.
> 
> Also please wait for a maintainer to sign off on it, too

Thanks for the review.  I've added tests and documentation for all but
regcache (which I'm about to do) and it already sussed out several
bugs.  I ended up having to rework the gdb.lookup_symbol for static
symbols patch significantly since it did introduce regressions.  It's
a lot simpler now.  I'll post the updated set once I finish up regcache.

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJWuLl7AAoJEB57S2MheeWySVsQAICvXs3bfkMCTXa6pnWHra+z
s+tKiyC6gjOX2U2bDzZq3uxSdbArtB3Rgl4TTRs3BFAAOdVLwsMw0jCEZjaNZQO1
D0qwErfrD0E7RF+YKKslako8+Au+yAMcqlQt97XnQFURSLHQ3Dp/jGyRRH9qSXo4
kFatHeKUlhX8gV2nkc7xwKTddxGnUSSHCb02UWcZOZ7Mjaa4ORVpvmWTHp5KdLWJ
S4AGzR4G/JB90ew31aZriAFqQbQTHMcauOhoMSktLobAMsVRX/zX6/CLXNntOPoT
tI8YK834H+DMGJVYzqOx9uqaxtTo8E84IMVMG4X1XIkgd1au4qQeuEyyfGdwfE0Z
Iun5rgCVaGtg8VxCB66jfDloImlfYbwZgGnN/+oWHlhTHurko5ma7xgV/mDgoUUw
J0v/m0yZ8pmMYtVSK2eWBcrKQcqdoKRa+XGqwa69/vWTwD0LnxUA8tphA61XOn3m
x26tHsokAXGmXy+RBrCMYa+ABehBcB5AQFZYxTHCPFF5qJlJs5MYT29+HdxsS71N
A/iHoJWaPPiM8FqmqLGexnS1rxClwd4DK7hiwq43b5gbfw4axXPp3l6AC1nQ++4M
ZPHz18KmlVRkgiNcAxMXwMCxGW04cbWkD/VrhnMk8TnNWYDWZyabIAXJnKTeJKIU
0Qox9ZHBecYHXtL0ADCu
=1A6j
-----END PGP SIGNATURE-----