This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch][python] Add symbol, symbol table and frame block support to GDB API
- From: Tom Tromey <tromey at redhat dot com>
- To: Phil Muldoon <pmuldoon at redhat dot com>
- Cc: Eli Zaretskii <eliz at gnu dot org>, gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Wed, 17 Feb 2010 19:49:56 -0700
- Subject: Re: [patch][python] Add symbol, symbol table and frame block support to GDB API
- References: <4B66DA35.7080701@redhat.com> <m3r5p0inxy.fsf@fleche.redhat.com> <4B796506.7010909@redhat.com>
- Reply-to: tromey at redhat dot com
Phil> I've added life-cycle support for symbols, symbol tables, symbol tables
Phil> and line, and blocks. This was a little more involved than I first
Phil> thought. I've also added the other changes you requested
Thanks for doing this.
I think this could use a NEWS entry. I suggest waiting until my earlier
patch is approved, then adding items to that block... assuming your
patch goes in before 7.1. Otherwise, add a new Python block in the
post-7.1 changes.
Phil> +/* Require a valid block. All access to block_object->block should be
Phil> + gated by this call. This must be called inside a TRY_CATCH, or
Phil> + another context in which a gdb exception is allowed. */
Phil> +#define BLPY_REQUIRE_VALID(block_obj, block) \
Phil> + do { \
Phil> + block = block_object_to_block (block_obj); \
Phil> + if (block == NULL) \
Phil> + error (_("Block is invalid.")); \
Phil> + } while (0)
All the uses of this macro appear in a TRY_CATCH, which is then followed
by GDB_PY_HANDLE_EXCEPTION. That is quite roundabout; it would be
better to bypass the middle man and just set the Python exception
directly and `return NULL'.
Phil> +static PyObject *
Phil> +blpy_block_syms_iternext (PyObject *self)
[...]
Phil> + return (sym == NULL) ? NULL : symbol_to_symbol_object (sym);
If you return NULL you must set the Python exception.
I think in this case you want to raise StopIteration.
Phil> +/* Require a valid symbol. All access to symbol_object->symbol should be
Phil> + gated by this call. This must be called inside a TRY_CATCH, or
Phil> + another context in which a gdb exception is allowed. */
Phil> +#define SYMPY_REQUIRE_VALID(symbol_obj, symbol) \
Phil> + do { \
Phil> + symbol = symbol_object_to_symbol (symbol_obj); \
Phil> + if (symbol == NULL) \
Phil> + error (_("Symbol is invalid.")); \
Phil> + } while (0)
The comment that applied to BLPY_REQUIRE_VALID applies here.
Phil> +static PyObject *
Phil> +sympy_str (PyObject *self)
Phil> +{
Phil> + char *s;
Phil> + PyObject *result;
Phil> + struct symbol *symbol = NULL;
Phil> + volatile struct gdb_exception except;
Phil> +
Phil> + TRY_CATCH (except, RETURN_MASK_ALL)
Phil> + {
Phil> + SYMPY_REQUIRE_VALID (self, symbol);
Phil> + }
Phil> + GDB_PY_HANDLE_EXCEPTION (except);
Phil> +
Phil> + s = xstrprintf ("symbol for %s",
Phil> + SYMBOL_PRINT_NAME (symbol));
Why not just have this be SYMBOL_PRINT_NAME?
I forgot the difference between the various python to-string methods.
Maybe this one is supposed to be more verbose? Let me know.
Phil> + obj->next = objfile_data (symbol->symtab->objfile, sympy_objfile_data_key);
Use the SYMBOL_SYMTAB macro here, and anywhere you have to refer to the
symbol's symtab.
Phil> + set_objfile_data (sym_obj->symbol->symtab->objfile, sympy_objfile_data_key, sym_obj->next);
This wraps, I think a couple of other lines nearby do too.
Line break after a comma somewhere.
Phil> +/* Require a valid symbol table. All access to symtab_object->symtab
Phil> + should be gated by this call. This must be called inside a
Phil> + TRY_CATCH, or another context in which a gdb exception is
Phil> + allowed. */
Phil> +#define STPY_REQUIRE_VALID(symtab_obj, symtab) \
Phil> + do { \
Phil> + symtab = symtab_object_to_symtab (symtab_obj); \
Phil> + if (symtab == NULL) \
Phil> + error (_("Symbol table is invalid.")); \
Phil> + } while (0)
As above.
Also SALPY_REQUIRE_VALID.
Phil> + s = xstrprintf ("symbol table for %s",
Phil> + symtab->filename);
Also as above.
Phil> + result = objfile_to_objfile_object (symtab->objfile);
Phil> + Py_INCREF (result);
Phil> + return result;
objfile_to_objfile_object can return NULL, which would make
that incref crash.
Phil> + s = xstrprintf ("symbol and line for %s, line %d", filename,
Phil> + sal->line);
As above.
Phil> +static void
Phil> +set_sal (sal_object *sal_obj, struct symtab_and_line sal)
Phil> +{
Phil> +
Spurious blank line.
Phil> + if (symtab_obj == NULL)
Phil> + {
Phil> + Py_DECREF (sal_obj);
Phil> + return;
Phil> + }
This does not make sense to me.
This destroys the sal_obj but the caller then proceeds to return it.
Phil> + sizeof(struct symtab_and_line));
Space before paren.
Phil> +PyObject *
Phil> +symtab_and_line_to_sal_object (struct symtab_and_line sal)
Phil> +
Phil> +{
Phil> + sal_object *sal_obj;
Phil> + symtab_object *symtab_obj;
Phil> +
Phil> + sal_obj = PyObject_New (sal_object, &sal_object_type);
Phil> +
Phil> + if (sal_obj)
Phil> + set_sal (sal_obj, sal);
Phil> +
Phil> + return (PyObject *) sal_obj;
Here's the bad return. If set_sal destroy sal_obj, gdb will crash.
Phil> +static PyMethodDef symtab_object_methods[] = {
Phil> + { "fullname", stpy_fullname, METH_NOARGS,
Phil> + "Return the symtab's full source filename." },
For methods we're trying to put the type info into the help.
I forgot to check the other new methods in this patch, but please do
this. See the existing python.c for examples.
Phil> "lookup_type (name [, block]) -> type\n\
Phil> Return a Type corresponding to the given name." },
It is funny that this "[, block]" help is still there.
I must have forgot to remove this when I stripped the block stuff from a
patch I upstreamed.
Anyway, I was going to mention this ... could you put the block code
back into lookup_type? It is fine by me if you'd rather do this as a
second patch.
There may be some other block-related addition on the branch that should
go in now. I forget.
thanks,
Tom