Re: [RFC v5 14/18] gdb/python: add add_symbol () method to gdb.Block

[Andrew Burgess <aburgess@redhat.com>]

Jan Vrany <jan.vrany@labware.com> writes:

> This commit adds new method add_symbol () to gdb.Block objects.
> A typical use of it is to add previously instantiated gdb.Symbol object
> to block when interfacing with JIT compiler.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/doc/python.texi                   |  5 +++
>  gdb/python/py-block.c                 | 52 +++++++++++++++++++++++----
>  gdb/testsuite/gdb.python/py-block.exp | 20 +++++++++++
>  3 files changed, 71 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 709ef7c9d1a..15f851e5fd0 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -6180,6 +6180,11 @@ The new block's @var{start}--@var{end} range must be within superblock's
>  range and must not overlap with any block already contained in superblock.
>  @end defun
>  
> +@defun Block.add_symbol (symbol)
> +Add @var{symbol} to this block.  Both the block and the @var{symbol} must
> +belong to the same compunit (@pxref{Compunits In Python}).
> +@end defun
> +
>  @defun Block.is_valid ()
>  Returns @code{True} if the @code{gdb.Block} object is valid,
>  @code{False} if not.  A block object can become invalid if the block it
> diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
> index 43b15b6013e..7926808d4db 100644
> --- a/gdb/python/py-block.c
> +++ b/gdb/python/py-block.c
> @@ -263,6 +263,43 @@ blpy_is_static (PyObject *self, void *closure)
>    Py_RETURN_FALSE;
>  }
>  
> +/* Implementation of gdb.Block.add_symbol (self, symbol).
> +   Adds SYMBOL to this block.  */
> +
> +static PyObject *
> +blpy_add_symbol (PyObject *self, PyObject *symbol_obj)
> +{
> +  const struct block *block;
> +
> +  BLPY_REQUIRE_VALID (self, block);
> +
> +  struct symbol *symbol = symbol_object_to_symbol (symbol_obj);
> +  if (symbol == nullptr)
> +    {
> +      return PyErr_Format (PyExc_TypeError,
> +			   _("The symbol argument is not valid gdb.Symbol"));
> +    }
> +
> +  if (symbol->symtab ()->compunit() != block->global_block ()->compunit ())
> +    {
> +      return PyErr_Format (PyExc_TypeError,
> +			   _("The symbol argument belongs to different "
> +			     "compunit than block"));
> +    }

In both these `if` blocks, the '{' and '}' are not needed.  In the
second one, is TypeError the best choice?  Would ValueError?  Or
RuntimeError? be better?

Thanks,
Andrew

> +
> +  multidictionary *dict = block->multidict ();
> +  if (dict == nullptr)
> +    {
> +      auto_obstack *obstack =
> +	&(block->global_block ()->compunit ()->objfile ()->objfile_obstack);
> +      dict = mdict_create_linear (obstack, nullptr);
> +      const_cast<struct block *>(block)->set_multidict (dict);
> +    }
> +
> +  mdict_add_symbol (dict, symbol);
> +  Py_RETURN_NONE;
> +}
> +
>  /* Given a string, returns the gdb.Symbol representing that symbol in this
>     block.  If such a symbol does not exist, returns NULL with a Python
>     exception.  */
> @@ -360,7 +397,7 @@ blpy_init (PyObject *zelf, PyObject *args, PyObject *kw)
>        return -1;
>      }
>  
> -  static const char *keywords[] = { "superblock", "start", "end", NULL };
> +  static const char *keywords[] = { "superblock", "start", "end", nullptr };
>    PyObject *superblock_obj;
>    uint64_t start;
>    uint64_t end;
> @@ -400,9 +437,9 @@ blpy_init (PyObject *zelf, PyObject *args, PyObject *kw)
>  
>    /* Check that start-end range does not overlap with any
>       "sibling" blocks' range.  */
> -  auto cu = superblock->global_block ()->compunit ();
> +  compunit_symtab *cu = superblock->global_block ()->compunit ();
>  
> -  for (auto each : cu->blockvector ()->blocks ())
> +  for (const struct block *each : cu->blockvector ()->blocks ())
>      {
>        if (each->superblock () == superblock)
>  	{
> @@ -417,11 +454,11 @@ blpy_init (PyObject *zelf, PyObject *args, PyObject *kw)
>  	}
>      }
>  
> -  auto obstack = &(cu->objfile ()->objfile_obstack);
> -  auto blk = new (obstack) block ();
> +  auto_obstack *obstack = &(cu->objfile ()->objfile_obstack);
> +  struct block *blk = new (obstack) block ();
>  
>    blk->set_superblock (superblock);
> -  blk->set_multidict (mdict_create_linear (obstack, NULL));
> +  blk->set_multidict (mdict_create_linear (obstack, nullptr));
>    blk->set_start ((CORE_ADDR) start);
>    blk->set_end ((CORE_ADDR) end);
>  
> @@ -653,6 +690,9 @@ static PyMethodDef block_object_methods[] = {
>    { "is_valid", blpy_is_valid, METH_NOARGS,
>      "is_valid () -> Boolean.\n\
>  Return true if this block is valid, false if not." },
> +  { "add_symbol", blpy_add_symbol, METH_O,
> +    "add_symbol (symbol) -> None.\n\
> +Add given symbol to the block." },
>    {NULL}  /* Sentinel */
>  };
>  
> diff --git a/gdb/testsuite/gdb.python/py-block.exp b/gdb/testsuite/gdb.python/py-block.exp
> index afa306be00f..b1cb93ca718 100644
> --- a/gdb/testsuite/gdb.python/py-block.exp
> +++ b/gdb/testsuite/gdb.python/py-block.exp
> @@ -138,6 +138,26 @@ gdb_test "python print ( gdb.Block(cu.static_block(), 160, 170))" \
>  	 "<gdb.Block <anonymous> \{.*\}>" \
>  	 "Create sibling block"
>  
> +# Test adding symbols to a block.
> +gdb_py_test_silent_cmd "python symtab = gdb.Symtab(\"some_file.txt\", cu)" \
> +		       "Create new symtab" 1
> +gdb_py_test_silent_cmd "python typ = gdb.selected_inferior().architecture().integer_type(0).function()" \
> +		       "Create type of new symbol" 1
> +gdb_py_test_silent_cmd "python sym = gdb.Symbol(\"static_block\", symtab, typ, gdb.SYMBOL_FUNCTION_DOMAIN, gdb.SYMBOL_LOC_BLOCK, cu.static_block() )" \
> +		       "Create new symbol" 1
> +gdb_test "python print ( sym in list(cu.global_block()) )" \
> +	 "False" \
> +	 "Symbol is not in global block"
> +gdb_py_test_silent_cmd "python cu.global_block().add_symbol(sym)" \
> +		       "Add new symbol to block" 1
> +gdb_test "python print ( sym in list(cu.global_block()) )" \
> +		       "True" \
> +		       "Symbol is in global block"
> +gdb_test "python print ( cu.global_block().add_symbol(cu))" \
> +	 "TypeError.*:.*" \
> +	 "Add non-symbol to block"
> +
> +
>  # Test Block is_valid.  This must always be the last test in this
>  # testcase as it unloads the object file.
>  delete_breakpoints
> -- 
> 2.47.2