* [RFC - Python Scripting] New method gdb.Architecture.disassemble
@ 2013-02-04 14:09 Siva Chandra
2013-02-05 23:28 ` Doug Evans
2013-02-06 19:58 ` Tom Tromey
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-04 14:09 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 794 bytes --]
The attached patch adds a new method 'disassemble' to the class
gdb.Architecture. I have not yet added docs and tests, but will do so
after I get feedback that adding such a method is OK.
2013-02-04 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* Makefile.in: Add entries for the new file python/py-out.c
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
* python/py-out.c: Implementation of a Python ui_out.
* python/python-internal.h: Add declarations for new utility
functions.
* python/python.c (_initialize_python): Initialize Python
ui_out.
[-- Attachment #2: arch_disassemble_patch_v1.txt --]
[-- Type: text/plain, Size: 14042 bytes --]
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 68d545e..6be64cf 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -291,6 +291,7 @@ SUBDIR_PYTHON_OBS = \
py-lazy-string.o \
py-newobjfileevent.o \
py-objfile.o \
+ py-out.o \
py-param.o \
py-prettyprint.o \
py-progspace.o \
@@ -325,6 +326,7 @@ SUBDIR_PYTHON_SRCS = \
python/py-lazy-string.c \
python/py-newobjfileevent.c \
python/py-objfile.c \
+ python/py-out.c \
python/py-param.c \
python/py-prettyprint.c \
python/py-progspace.c \
@@ -2129,6 +2131,10 @@ py-objfile.o: $(srcdir)/python/py-objfile.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-objfile.c
$(POSTCOMPILE)
+py-out.o: $(srcdir)/python/py-out.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-out.c
+ $(POSTCOMPILE)
+
py-param.o: $(srcdir)/python/py-param.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-param.c
$(POSTCOMPILE)
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..b6f4f5a 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -19,6 +19,7 @@
#include "defs.h"
#include "gdbarch.h"
+#include "disasm.h"
#include "arch-utils.h"
#include "python-internal.h"
@@ -86,6 +87,44 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of Architecture.disassemble (low, high, [opcodes]) -> List.
+ Returns a list of instructions, each of which is a dictionary. */
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args)
+{
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+ CORE_ADDR low, high;
+ int opcodes = 0, flags = 0;
+ PyObject *result, *temp;
+ volatile struct gdb_exception except;
+
+ if (!PyArg_ParseTuple (args, "KK|i", &low, &high, &opcodes))
+ return NULL;
+
+ if (opcodes)
+ flags = DISASSEMBLY_RAW_INSN;
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ gdb_disassembly (gdbarch, py_out, NULL, flags, -1, low, high);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
+
+ temp = fetch_and_reset_py_out_object (py_out);
+ if (! (PyList_Check (temp) && PyList_Size (temp) > 0))
+ return NULL;
+
+ /* gdb_disassembly puts a list of lists in py_out with the higher level list
+ containing a single item which is itself a list of instructions. Hence,
+ return the first element of the higher level list. */
+ result = PyList_GetItem (temp, 0);
+ Py_XINCREF (result);
+ Py_XDECREF (temp);
+
+ return result;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +144,9 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", archpy_disassemble, METH_VARARGS,
+ "name (low, high, [opcodes]) -> List.\n\
+Return the list of instructions in the address range from LOW to HIGH." },
{NULL} /* Sentinel */
};
diff --git a/gdb/python/py-out.c b/gdb/python/py-out.c
new file mode 100644
index 0000000..d278bc2
--- /dev/null
+++ b/gdb/python/py-out.c
@@ -0,0 +1,259 @@
+/* Python ui_out implementation.
+
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+ 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 "ui-out.h"
+#include "python-internal.h"
+
+struct ui_out *py_out;
+
+struct row_data
+{
+ /* DATA is either a list of rows, or just a dict. */
+ PyObject *data;
+
+ /* The enclosing row for the above DATA. */
+ struct row_data *parent_row;
+};
+
+/* This data structure captures the Python version of ui_out. The Python
+ version is not used to display output to a user, but to capture the results
+ from GDB's internals in to a Python data structure. Hence, it does not have
+ any representation for table headers. However, it can be viewed as a
+ recursive table structure wherin the highest level is a list of rows. All
+ rows in this list can either be a list themselves, or all of them can be
+ dicts holding the table's fields. If they were lists, then they follow the
+ same recurrsive structure as the higher levels.
+
+ Example:
+
+ [ # Highest level list which has two lists for rows
+ [ # Inner level row which is a list of lists
+ [ # Inner level row which is a list of dicts
+ {'a': 1, 'b': 2}, # Leaf row which is a dict
+ {'a': 3, 'b': 4}, # Leaf row which is a dict
+ ],
+
+ [ # Inner level row which is a list of dicts
+ {'x': 5, 'y': 6}, # Leaf row which is a dict
+ {'x': 7, 'y': 8}, # Leaf row which is a dict
+ ],
+ ],
+
+ [ # Inner level row which is list of dicts
+ {'p': 1, 'q': 2}, # Leaf row which is a dict
+ {'p': 3, 'q': 4}, # Leaf row which is a dict
+ ],
+ ]
+*/
+
+struct py_out_data
+{
+ /* The highest level list of rows. */
+ struct row_data *table;
+
+ /* The current row that is being added to the table. */
+ struct row_data *current_row;
+};
+
+static struct row_data *
+new_row (struct row_data *parent)
+{
+ struct row_data *row;
+
+ row = (struct row_data *) xmalloc (sizeof (struct row_data));
+ row->data = NULL;
+ row->parent_row = parent;
+
+ return row;
+}
+
+PyObject *
+fetch_and_reset_py_out_object (struct ui_out *ui_out)
+{
+ PyObject *temp;
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ /* Ensure that the py_out object is complete. */
+ if (py_out_data->current_row != py_out_data->table)
+ internal_error (__FILE__, __LINE__,
+ _("Trying to fetch an incomplete Python ui_out object"));
+
+ temp = py_out_data->table->data;
+ py_out_data->table->data = PyList_New (0);
+
+ return temp;
+}
+
+static void
+py_out_row_begin (struct ui_out *ui_out, enum ui_out_type type, int level,
+ const char *id)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ if (py_out_data->current_row)
+ {
+ if (py_out_data->current_row->data)
+ {
+ if (PyDict_Check (py_out_data->current_row->data))
+ /* If the row has data, check that it is not a dict first. */
+ internal_error (__FILE__, __LINE__,
+ _("Trying to add a row to a row which has "
+ "fields."));
+ else if (PyList_Check (py_out_data->current_row->data))
+ {
+ /* If the row is already a list, add a new row. */
+ struct row_data *new_row_data;
+
+ new_row_data = new_row (py_out_data->current_row);
+ py_out_data->current_row = new_row_data;
+ }
+ else
+ /* If it is neither a list or a dict, then something has gone wrong
+ somewhere. */
+ internal_error (__FILE__, __LINE__,
+ _("Unexpected internal state in creating Python "
+ "ui_out object."));
+ }
+ else
+ {
+ /* Make the current row a list and add a new row. */
+ struct row_data *new_row_data;
+
+ py_out_data->current_row->data = PyList_New (0);
+ new_row_data = new_row (py_out_data->current_row);
+ py_out_data->current_row = new_row_data;
+ }
+ }
+ else
+ {
+ /* This should never happen. */
+ internal_error (__FILE__, __LINE__,
+ _("Unexpected internal state in creating Python ui_out "
+ "object."));
+ }
+}
+
+static void
+py_out_row_end (struct ui_out *ui_out, enum ui_out_type type, int level)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+ struct row_data *temp;
+
+ /* If nothing was added to current row, then make it Py_None. */
+ if (py_out_data->current_row->data == NULL)
+ {
+ Py_INCREF (Py_None);
+ py_out_data->current_row->data = Py_None;
+ }
+
+ /* Commit the row to the parent list. */
+ PyList_Append (py_out_data->current_row->parent_row->data,
+ py_out_data->current_row->data);
+
+ /* Move up a level by making the parent row as the current row and free the
+ row_data object corresponding to current_row. */
+ temp = py_out_data->current_row;
+ py_out_data->current_row = py_out_data->current_row->parent_row;
+ xfree (temp);
+}
+
+#define CHECK_AND_INIT_FIELD_ROW_DATA(data) \
+ do { \
+ if (!(data)) \
+ (data) = PyDict_New (); \
+ else \
+ { \
+ if (!PyDict_Check ((data))) \
+ internal_error (__FILE__, __LINE__, \
+ _("Adding fields to a row which is not a field " \
+ "row.")); \
+ } \
+ } while (0)
+
+static void
+py_out_field_int (struct ui_out * ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname, int value)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ PyInt_FromLong (value));
+}
+
+static void
+py_out_field_skip (struct ui_out *ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ Py_INCREF (Py_None);
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ Py_None);
+}
+
+static void
+py_out_field_string (struct ui_out * ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname, const char *str)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ PyString_FromString (str));
+}
+
+static struct ui_out_impl py_ui_out_impl =
+{
+ 0, /* table_begin */
+ 0, /* table_body */
+ 0, /* table_end */
+ 0, /* table_header */
+ py_out_row_begin, /* begin */
+ py_out_row_end, /* end */
+ py_out_field_int, /* field_int */
+ py_out_field_skip, /* field_skip */
+ py_out_field_string, /* field_string */
+ 0, /* field_fmt */
+ 0, /* space */
+ 0, /* text */
+ 0, /* message */
+ 0, /* wrap_hint */
+ 0, /* flush */
+ 0, /* redirect */
+ 0 /* is_mi_like_p */
+};
+
+void
+gdbpy_initialize_py_out (void)
+{
+ struct py_out_data *py_out_data;
+
+ py_out_data = (struct py_out_data *) xmalloc (sizeof (struct py_out_data));
+ py_out_data->table = new_row (NULL);
+ py_out_data->table->data = PyList_New (0);
+ py_out_data->current_row = py_out_data->table;
+
+ py_out = ui_out_new (&py_ui_out_impl, py_out_data, 0);
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8dff1d7..d852d03 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -143,6 +143,7 @@ struct language_defn;
struct program_space;
struct bpstats;
struct inferior;
+struct ui_out;
extern PyObject *gdb_module;
extern PyObject *gdb_python_module;
@@ -267,6 +268,8 @@ struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
+PyObject *fetch_and_reset_py_out_object (struct ui_out *);
+
void gdbpy_initialize_gdb_readline (void);
void gdbpy_initialize_auto_load (void);
void gdbpy_initialize_values (void);
@@ -297,6 +300,7 @@ void gdbpy_initialize_exited_event (void);
void gdbpy_initialize_thread_event (void);
void gdbpy_initialize_new_objfile_event (void);
void gdbpy_initialize_arch (void);
+void gdbpy_initialize_py_out (void);
struct cleanup *make_cleanup_py_decref (PyObject *py);
@@ -305,6 +309,7 @@ struct cleanup *ensure_python_env (struct gdbarch *gdbarch,
extern struct gdbarch *python_gdbarch;
extern const struct language_defn *python_language;
+extern struct ui_out *py_out;
/* Use this after a TRY_EXCEPT to throw the appropriate Python
exception. */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 53ddee9..3ab4b7c 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1621,6 +1621,7 @@ message == an error message without a stack will be printed."),
gdbpy_initialize_thread_event ();
gdbpy_initialize_new_objfile_event () ;
gdbpy_initialize_arch ();
+ gdbpy_initialize_py_out ();
observer_attach_before_prompt (before_prompt_hook);
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-04 14:09 [RFC - Python Scripting] New method gdb.Architecture.disassemble Siva Chandra
@ 2013-02-05 23:28 ` Doug Evans
2013-02-06 1:53 ` Siva Chandra
2013-02-06 19:58 ` Tom Tromey
1 sibling, 1 reply; 39+ messages in thread
From: Doug Evans @ 2013-02-05 23:28 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
Siva Chandra writes:
> The attached patch adds a new method 'disassemble' to the class
> gdb.Architecture. I have not yet added docs and tests, but will do so
> after I get feedback that adding such a method is OK.
Hi.
I like the idea, but for an API I wouldn't mind seeing something
a bit lower level. E.g., skip the higher level disassembler entry
points in gdb (mixed source/assembly support and all that), and provide
more direct access to the disassembler.
I didn't go through it with a fine toothed comb, but here are some questions.
1) Can we remove the py_out global?
2) It seems like this will export a lot of struct ui_out to the user.
I'd rather provide disassembly without having to commit to supporting
struct ui_out in Python.
Thoughts?
> 2013-02-04 Siva Chandra Reddy <sivachandra@google.com>
>
> Add a new method 'disassemble' to gdb.Architecture class.
> * Makefile.in: Add entries for the new file python/py-out.c
> * python/py-arch.c (archpy_disassmble): Implementation of the
> new method gdb.Architecture.disassemble.
> (arch_object_methods): Add entry for the new method.
> * python/py-out.c: Implementation of a Python ui_out.
> * python/python-internal.h: Add declarations for new utility
> functions.
> * python/python.c (_initialize_python): Initialize Python
> ui_out.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 68d545e..6be64cf 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -291,6 +291,7 @@ SUBDIR_PYTHON_OBS = \
> py-lazy-string.o \
> py-newobjfileevent.o \
> py-objfile.o \
> + py-out.o \
> py-param.o \
> py-prettyprint.o \
> py-progspace.o \
> @@ -325,6 +326,7 @@ SUBDIR_PYTHON_SRCS = \
> python/py-lazy-string.c \
> python/py-newobjfileevent.c \
> python/py-objfile.c \
> + python/py-out.c \
> python/py-param.c \
> python/py-prettyprint.c \
> python/py-progspace.c \
> @@ -2129,6 +2131,10 @@ py-objfile.o: $(srcdir)/python/py-objfile.c
> $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-objfile.c
> $(POSTCOMPILE)
>
> +py-out.o: $(srcdir)/python/py-out.c
> + $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-out.c
> + $(POSTCOMPILE)
> +
> py-param.o: $(srcdir)/python/py-param.c
> $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-param.c
> $(POSTCOMPILE)
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index edd508f..b6f4f5a 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -19,6 +19,7 @@
>
> #include "defs.h"
> #include "gdbarch.h"
> +#include "disasm.h"
> #include "arch-utils.h"
> #include "python-internal.h"
>
> @@ -86,6 +87,44 @@ archpy_name (PyObject *self, PyObject *args)
> return py_name;
> }
>
> +/* Implementation of Architecture.disassemble (low, high, [opcodes]) -> List.
> + Returns a list of instructions, each of which is a dictionary. */
> +
> +static PyObject *
> +archpy_disassemble (PyObject *self, PyObject *args)
> +{
> + struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
> + CORE_ADDR low, high;
> + int opcodes = 0, flags = 0;
> + PyObject *result, *temp;
> + volatile struct gdb_exception except;
> +
> + if (!PyArg_ParseTuple (args, "KK|i", &low, &high, &opcodes))
> + return NULL;
> +
> + if (opcodes)
> + flags = DISASSEMBLY_RAW_INSN;
> +
> + TRY_CATCH (except, RETURN_MASK_ALL)
> + {
> + gdb_disassembly (gdbarch, py_out, NULL, flags, -1, low, high);
> + }
> + GDB_PY_HANDLE_EXCEPTION (except);
> +
> + temp = fetch_and_reset_py_out_object (py_out);
> + if (! (PyList_Check (temp) && PyList_Size (temp) > 0))
> + return NULL;
> +
> + /* gdb_disassembly puts a list of lists in py_out with the higher level list
> + containing a single item which is itself a list of instructions. Hence,
> + return the first element of the higher level list. */
> + result = PyList_GetItem (temp, 0);
> + Py_XINCREF (result);
> + Py_XDECREF (temp);
> +
> + return result;
> +}
> +
> /* Initializes the Architecture class in the gdb module. */
>
> void
> @@ -105,6 +144,9 @@ static PyMethodDef arch_object_methods [] = {
> { "name", archpy_name, METH_NOARGS,
> "name () -> String.\n\
> Return the name of the architecture as a string value." },
> + { "disassemble", archpy_disassemble, METH_VARARGS,
> + "name (low, high, [opcodes]) -> List.\n\
> +Return the list of instructions in the address range from LOW to HIGH." },
> {NULL} /* Sentinel */
> };
>
> diff --git a/gdb/python/py-out.c b/gdb/python/py-out.c
> new file mode 100644
> index 0000000..d278bc2
> --- /dev/null
> +++ b/gdb/python/py-out.c
> @@ -0,0 +1,259 @@
> +/* Python ui_out implementation.
> +
> + Copyright (C) 2013 Free Software Foundation, Inc.
> +
> + 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 "ui-out.h"
> +#include "python-internal.h"
> +
> +struct ui_out *py_out;
> +
> +struct row_data
> +{
> + /* DATA is either a list of rows, or just a dict. */
> + PyObject *data;
> +
> + /* The enclosing row for the above DATA. */
> + struct row_data *parent_row;
> +};
> +
> +/* This data structure captures the Python version of ui_out. The Python
> + version is not used to display output to a user, but to capture the results
> + from GDB's internals in to a Python data structure. Hence, it does not have
> + any representation for table headers. However, it can be viewed as a
> + recursive table structure wherin the highest level is a list of rows. All
> + rows in this list can either be a list themselves, or all of them can be
> + dicts holding the table's fields. If they were lists, then they follow the
> + same recurrsive structure as the higher levels.
> +
> + Example:
> +
> + [ # Highest level list which has two lists for rows
> + [ # Inner level row which is a list of lists
> + [ # Inner level row which is a list of dicts
> + {'a': 1, 'b': 2}, # Leaf row which is a dict
> + {'a': 3, 'b': 4}, # Leaf row which is a dict
> + ],
> +
> + [ # Inner level row which is a list of dicts
> + {'x': 5, 'y': 6}, # Leaf row which is a dict
> + {'x': 7, 'y': 8}, # Leaf row which is a dict
> + ],
> + ],
> +
> + [ # Inner level row which is list of dicts
> + {'p': 1, 'q': 2}, # Leaf row which is a dict
> + {'p': 3, 'q': 4}, # Leaf row which is a dict
> + ],
> + ]
> +*/
> +
> +struct py_out_data
> +{
> + /* The highest level list of rows. */
> + struct row_data *table;
> +
> + /* The current row that is being added to the table. */
> + struct row_data *current_row;
> +};
> +
> +static struct row_data *
> +new_row (struct row_data *parent)
> +{
> + struct row_data *row;
> +
> + row = (struct row_data *) xmalloc (sizeof (struct row_data));
> + row->data = NULL;
> + row->parent_row = parent;
> +
> + return row;
> +}
> +
> +PyObject *
> +fetch_and_reset_py_out_object (struct ui_out *ui_out)
> +{
> + PyObject *temp;
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> +
> + /* Ensure that the py_out object is complete. */
> + if (py_out_data->current_row != py_out_data->table)
> + internal_error (__FILE__, __LINE__,
> + _("Trying to fetch an incomplete Python ui_out object"));
> +
> + temp = py_out_data->table->data;
> + py_out_data->table->data = PyList_New (0);
> +
> + return temp;
> +}
> +
> +static void
> +py_out_row_begin (struct ui_out *ui_out, enum ui_out_type type, int level,
> + const char *id)
> +{
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> +
> + if (py_out_data->current_row)
> + {
> + if (py_out_data->current_row->data)
> + {
> + if (PyDict_Check (py_out_data->current_row->data))
> + /* If the row has data, check that it is not a dict first. */
> + internal_error (__FILE__, __LINE__,
> + _("Trying to add a row to a row which has "
> + "fields."));
> + else if (PyList_Check (py_out_data->current_row->data))
> + {
> + /* If the row is already a list, add a new row. */
> + struct row_data *new_row_data;
> +
> + new_row_data = new_row (py_out_data->current_row);
> + py_out_data->current_row = new_row_data;
> + }
> + else
> + /* If it is neither a list or a dict, then something has gone wrong
> + somewhere. */
> + internal_error (__FILE__, __LINE__,
> + _("Unexpected internal state in creating Python "
> + "ui_out object."));
> + }
> + else
> + {
> + /* Make the current row a list and add a new row. */
> + struct row_data *new_row_data;
> +
> + py_out_data->current_row->data = PyList_New (0);
> + new_row_data = new_row (py_out_data->current_row);
> + py_out_data->current_row = new_row_data;
> + }
> + }
> + else
> + {
> + /* This should never happen. */
> + internal_error (__FILE__, __LINE__,
> + _("Unexpected internal state in creating Python ui_out "
> + "object."));
> + }
> +}
> +
> +static void
> +py_out_row_end (struct ui_out *ui_out, enum ui_out_type type, int level)
> +{
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> + struct row_data *temp;
> +
> + /* If nothing was added to current row, then make it Py_None. */
> + if (py_out_data->current_row->data == NULL)
> + {
> + Py_INCREF (Py_None);
> + py_out_data->current_row->data = Py_None;
> + }
> +
> + /* Commit the row to the parent list. */
> + PyList_Append (py_out_data->current_row->parent_row->data,
> + py_out_data->current_row->data);
> +
> + /* Move up a level by making the parent row as the current row and free the
> + row_data object corresponding to current_row. */
> + temp = py_out_data->current_row;
> + py_out_data->current_row = py_out_data->current_row->parent_row;
> + xfree (temp);
> +}
> +
> +#define CHECK_AND_INIT_FIELD_ROW_DATA(data) \
> + do { \
> + if (!(data)) \
> + (data) = PyDict_New (); \
> + else \
> + { \
> + if (!PyDict_Check ((data))) \
> + internal_error (__FILE__, __LINE__, \
> + _("Adding fields to a row which is not a field " \
> + "row.")); \
> + } \
> + } while (0)
> +
> +static void
> +py_out_field_int (struct ui_out * ui_out, int fldno, int width,
> + enum ui_align align, const char *fldname, int value)
> +{
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> +
> + CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
> +
> + PyDict_SetItemString (py_out_data->current_row->data, fldname,
> + PyInt_FromLong (value));
> +}
> +
> +static void
> +py_out_field_skip (struct ui_out *ui_out, int fldno, int width,
> + enum ui_align align, const char *fldname)
> +{
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> +
> + CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
> +
> + Py_INCREF (Py_None);
> + PyDict_SetItemString (py_out_data->current_row->data, fldname,
> + Py_None);
> +}
> +
> +static void
> +py_out_field_string (struct ui_out * ui_out, int fldno, int width,
> + enum ui_align align, const char *fldname, const char *str)
> +{
> + struct py_out_data *py_out_data = ui_out_data (ui_out);
> +
> + CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
> +
> + PyDict_SetItemString (py_out_data->current_row->data, fldname,
> + PyString_FromString (str));
> +}
> +
> +static struct ui_out_impl py_ui_out_impl =
> +{
> + 0, /* table_begin */
> + 0, /* table_body */
> + 0, /* table_end */
> + 0, /* table_header */
> + py_out_row_begin, /* begin */
> + py_out_row_end, /* end */
> + py_out_field_int, /* field_int */
> + py_out_field_skip, /* field_skip */
> + py_out_field_string, /* field_string */
> + 0, /* field_fmt */
> + 0, /* space */
> + 0, /* text */
> + 0, /* message */
> + 0, /* wrap_hint */
> + 0, /* flush */
> + 0, /* redirect */
> + 0 /* is_mi_like_p */
> +};
> +
> +void
> +gdbpy_initialize_py_out (void)
> +{
> + struct py_out_data *py_out_data;
> +
> + py_out_data = (struct py_out_data *) xmalloc (sizeof (struct py_out_data));
> + py_out_data->table = new_row (NULL);
> + py_out_data->table->data = PyList_New (0);
> + py_out_data->current_row = py_out_data->table;
> +
> + py_out = ui_out_new (&py_ui_out_impl, py_out_data, 0);
> +}
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index 8dff1d7..d852d03 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -143,6 +143,7 @@ struct language_defn;
> struct program_space;
> struct bpstats;
> struct inferior;
> +struct ui_out;
>
> extern PyObject *gdb_module;
> extern PyObject *gdb_python_module;
> @@ -267,6 +268,8 @@ struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
> struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
> struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
>
> +PyObject *fetch_and_reset_py_out_object (struct ui_out *);
> +
> void gdbpy_initialize_gdb_readline (void);
> void gdbpy_initialize_auto_load (void);
> void gdbpy_initialize_values (void);
> @@ -297,6 +300,7 @@ void gdbpy_initialize_exited_event (void);
> void gdbpy_initialize_thread_event (void);
> void gdbpy_initialize_new_objfile_event (void);
> void gdbpy_initialize_arch (void);
> +void gdbpy_initialize_py_out (void);
>
> struct cleanup *make_cleanup_py_decref (PyObject *py);
>
> @@ -305,6 +309,7 @@ struct cleanup *ensure_python_env (struct gdbarch *gdbarch,
>
> extern struct gdbarch *python_gdbarch;
> extern const struct language_defn *python_language;
> +extern struct ui_out *py_out;
>
> /* Use this after a TRY_EXCEPT to throw the appropriate Python
> exception. */
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 53ddee9..3ab4b7c 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1621,6 +1621,7 @@ message == an error message without a stack will be printed."),
> gdbpy_initialize_thread_event ();
> gdbpy_initialize_new_objfile_event () ;
> gdbpy_initialize_arch ();
> + gdbpy_initialize_py_out ();
>
> observer_attach_before_prompt (before_prompt_hook);
>
--
/dje
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-05 23:28 ` Doug Evans
@ 2013-02-06 1:53 ` Siva Chandra
2013-02-06 20:00 ` Tom Tromey
2013-02-08 18:05 ` Doug Evans
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-06 1:53 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]
On Tue, Feb 5, 2013 at 3:28 PM, Doug Evans <dje@google.com> wrote:
> I like the idea, but for an API I wouldn't mind seeing something
> a bit lower level. E.g., skip the higher level disassembler entry
> points in gdb (mixed source/assembly support and all that), and provide
> more direct access to the disassembler.
The only useful entry point currently available is gdb_disassembly and
I do not think it is a bad entry point. Other disassembly functions in
disasm.c are static. However, for the Python API, my patch provides
only one option of whether to include or exclude opcodes in the
disassembled output.
> I didn't go through it with a fine toothed comb, but here are some questions.
> 1) Can we remove the py_out global?
At what level do you not want this to be global? I have made it static
to the file in the attached patch.
> 2) It seems like this will export a lot of struct ui_out to the user.
> I'd rather provide disassembly without having to commit to supporting
> struct ui_out in Python.
I am not very sure I understand this point. My patch does not expose
anything about the struct ui_out to the user/Python API. Python ui_out
is only a way to get results from GDB internals into a Python data
structure. Also, this Python data structure does not depend on
gdb_disassembly's display format.
2013-02-05 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* Makefile.in: Add entries for the new file python/py-out.c
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
* python/py-out.c: Implementation of a Python ui_out.
* python/python-internal.h: Add declarations for new utility
functions.
* python/python.c (_initialize_python): Initialize Python
ui_out.
[-- Attachment #2: arch_disassemble_patch_v2.txt --]
[-- Type: text/plain, Size: 13894 bytes --]
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 68d545e..6be64cf 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -291,6 +291,7 @@ SUBDIR_PYTHON_OBS = \
py-lazy-string.o \
py-newobjfileevent.o \
py-objfile.o \
+ py-out.o \
py-param.o \
py-prettyprint.o \
py-progspace.o \
@@ -325,6 +326,7 @@ SUBDIR_PYTHON_SRCS = \
python/py-lazy-string.c \
python/py-newobjfileevent.c \
python/py-objfile.c \
+ python/py-out.c \
python/py-param.c \
python/py-prettyprint.c \
python/py-progspace.c \
@@ -2129,6 +2131,10 @@ py-objfile.o: $(srcdir)/python/py-objfile.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-objfile.c
$(POSTCOMPILE)
+py-out.o: $(srcdir)/python/py-out.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-out.c
+ $(POSTCOMPILE)
+
py-param.o: $(srcdir)/python/py-param.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-param.c
$(POSTCOMPILE)
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..b1608d7 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -19,6 +19,7 @@
#include "defs.h"
#include "gdbarch.h"
+#include "disasm.h"
#include "arch-utils.h"
#include "python-internal.h"
@@ -86,6 +87,45 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of Architecture.disassemble (low, high, [opcodes]) -> List.
+ Returns a list of instructions, each of which is a dictionary. */
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args)
+{
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+ CORE_ADDR low, high;
+ int opcodes = 0, flags = 0;
+ PyObject *result, *temp;
+ struct ui_out *py_out = py_ui_out ();
+ volatile struct gdb_exception except;
+
+ if (!PyArg_ParseTuple (args, "KK|i", &low, &high, &opcodes))
+ return NULL;
+
+ if (opcodes)
+ flags = DISASSEMBLY_RAW_INSN;
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ gdb_disassembly (gdbarch, py_out, NULL, flags, -1, low, high);
+ }
+ GDB_PY_HANDLE_EXCEPTION (except);
+
+ temp = fetch_and_reset_py_out_object (py_out);
+ if (! (PyList_Check (temp) && PyList_Size (temp) > 0))
+ return NULL;
+
+ /* gdb_disassembly puts a list of lists in py_out with the higher level list
+ containing a single item which is itself a list of instructions. Hence,
+ return the first element of the higher level list. */
+ result = PyList_GetItem (temp, 0);
+ Py_XINCREF (result);
+ Py_XDECREF (temp);
+
+ return result;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +145,9 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", archpy_disassemble, METH_VARARGS,
+ "name (low, high, [opcodes]) -> List.\n\
+Return the list of instructions in the address range from LOW to HIGH." },
{NULL} /* Sentinel */
};
diff --git a/gdb/python/py-out.c b/gdb/python/py-out.c
new file mode 100644
index 0000000..f6a1456
--- /dev/null
+++ b/gdb/python/py-out.c
@@ -0,0 +1,265 @@
+/* Python ui_out implementation.
+
+ Copyright (C) 2013 Free Software Foundation, Inc.
+
+ 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 "ui-out.h"
+#include "python-internal.h"
+
+static struct ui_out *py_out;
+
+struct row_data
+{
+ /* DATA is either a list of rows, or just a dict. */
+ PyObject *data;
+
+ /* The enclosing row for the above DATA. */
+ struct row_data *parent_row;
+};
+
+/* This data structure captures the Python version of ui_out. The Python
+ version is not used to display output to a user, but to capture the results
+ from GDB's internals in to a Python data structure. Hence, it does not have
+ any representation for table headers. However, it can be viewed as a
+ recursive table structure wherin the highest level is a list of rows. All
+ rows in this list can either be a list themselves, or all of them can be
+ dicts holding the table's fields. If they were lists, then they follow the
+ same recurrsive structure as the higher levels.
+
+ Example:
+
+ [ # Highest level list which has two lists for rows
+ [ # Inner level row which is a list of lists
+ [ # Inner level row which is a list of dicts
+ {'a': 1, 'b': 2}, # Leaf row which is a dict
+ {'a': 3, 'b': 4}, # Leaf row which is a dict
+ ],
+
+ [ # Inner level row which is a list of dicts
+ {'x': 5, 'y': 6}, # Leaf row which is a dict
+ {'x': 7, 'y': 8}, # Leaf row which is a dict
+ ],
+ ],
+
+ [ # Inner level row which is list of dicts
+ {'p': 1, 'q': 2}, # Leaf row which is a dict
+ {'p': 3, 'q': 4}, # Leaf row which is a dict
+ ],
+ ]
+*/
+
+struct py_out_data
+{
+ /* The highest level list of rows. */
+ struct row_data *table;
+
+ /* The current row that is being added to the table. */
+ struct row_data *current_row;
+};
+
+static struct row_data *
+new_row (struct row_data *parent)
+{
+ struct row_data *row;
+
+ row = (struct row_data *) xmalloc (sizeof (struct row_data));
+ row->data = NULL;
+ row->parent_row = parent;
+
+ return row;
+}
+
+PyObject *
+fetch_and_reset_py_out_object (struct ui_out *ui_out)
+{
+ PyObject *temp;
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ /* Ensure that the py_out object is complete. */
+ if (py_out_data->current_row != py_out_data->table)
+ internal_error (__FILE__, __LINE__,
+ _("Trying to fetch an incomplete Python ui_out object"));
+
+ temp = py_out_data->table->data;
+ py_out_data->table->data = PyList_New (0);
+
+ return temp;
+}
+
+static void
+py_out_row_begin (struct ui_out *ui_out, enum ui_out_type type, int level,
+ const char *id)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ if (py_out_data->current_row)
+ {
+ if (py_out_data->current_row->data)
+ {
+ if (PyDict_Check (py_out_data->current_row->data))
+ /* If the row has data, check that it is not a dict first. */
+ internal_error (__FILE__, __LINE__,
+ _("Trying to add a row to a row which has "
+ "fields."));
+ else if (PyList_Check (py_out_data->current_row->data))
+ {
+ /* If the row is already a list, add a new row. */
+ struct row_data *new_row_data;
+
+ new_row_data = new_row (py_out_data->current_row);
+ py_out_data->current_row = new_row_data;
+ }
+ else
+ /* If it is neither a list or a dict, then something has gone wrong
+ somewhere. */
+ internal_error (__FILE__, __LINE__,
+ _("Unexpected internal state in creating Python "
+ "ui_out object."));
+ }
+ else
+ {
+ /* Make the current row a list and add a new row. */
+ struct row_data *new_row_data;
+
+ py_out_data->current_row->data = PyList_New (0);
+ new_row_data = new_row (py_out_data->current_row);
+ py_out_data->current_row = new_row_data;
+ }
+ }
+ else
+ {
+ /* This should never happen. */
+ internal_error (__FILE__, __LINE__,
+ _("Unexpected internal state in creating Python ui_out "
+ "object."));
+ }
+}
+
+static void
+py_out_row_end (struct ui_out *ui_out, enum ui_out_type type, int level)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+ struct row_data *temp;
+
+ /* If nothing was added to current row, then make it Py_None. */
+ if (py_out_data->current_row->data == NULL)
+ {
+ Py_INCREF (Py_None);
+ py_out_data->current_row->data = Py_None;
+ }
+
+ /* Commit the row to the parent list. */
+ PyList_Append (py_out_data->current_row->parent_row->data,
+ py_out_data->current_row->data);
+
+ /* Move up a level by making the parent row as the current row and free the
+ row_data object corresponding to current_row. */
+ temp = py_out_data->current_row;
+ py_out_data->current_row = py_out_data->current_row->parent_row;
+ xfree (temp);
+}
+
+#define CHECK_AND_INIT_FIELD_ROW_DATA(data) \
+ do { \
+ if (!(data)) \
+ (data) = PyDict_New (); \
+ else \
+ { \
+ if (!PyDict_Check ((data))) \
+ internal_error (__FILE__, __LINE__, \
+ _("Adding fields to a row which is not a field " \
+ "row.")); \
+ } \
+ } while (0)
+
+static void
+py_out_field_int (struct ui_out * ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname, int value)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ PyInt_FromLong (value));
+}
+
+static void
+py_out_field_skip (struct ui_out *ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ Py_INCREF (Py_None);
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ Py_None);
+}
+
+static void
+py_out_field_string (struct ui_out * ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname, const char *str)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ PyString_FromString (str));
+}
+
+static struct ui_out_impl py_ui_out_impl =
+{
+ 0, /* table_begin */
+ 0, /* table_body */
+ 0, /* table_end */
+ 0, /* table_header */
+ py_out_row_begin, /* begin */
+ py_out_row_end, /* end */
+ py_out_field_int, /* field_int */
+ py_out_field_skip, /* field_skip */
+ py_out_field_string, /* field_string */
+ 0, /* field_fmt */
+ 0, /* space */
+ 0, /* text */
+ 0, /* message */
+ 0, /* wrap_hint */
+ 0, /* flush */
+ 0, /* redirect */
+ 0 /* is_mi_like_p */
+};
+
+void
+gdbpy_initialize_py_out (void)
+{
+ struct py_out_data *py_out_data;
+
+ py_out_data = (struct py_out_data *) xmalloc (sizeof (struct py_out_data));
+ py_out_data->table = new_row (NULL);
+ py_out_data->table->data = PyList_New (0);
+ py_out_data->current_row = py_out_data->table;
+
+ py_out = ui_out_new (&py_ui_out_impl, py_out_data, 0);
+}
+
+struct ui_out *
+py_ui_out (void)
+{
+ return py_out;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8dff1d7..cbc9f31 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -143,6 +143,7 @@ struct language_defn;
struct program_space;
struct bpstats;
struct inferior;
+struct ui_out;
extern PyObject *gdb_module;
extern PyObject *gdb_python_module;
@@ -267,6 +268,9 @@ struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
+PyObject *fetch_and_reset_py_out_object (struct ui_out *);
+struct ui_out *py_ui_out (void);
+
void gdbpy_initialize_gdb_readline (void);
void gdbpy_initialize_auto_load (void);
void gdbpy_initialize_values (void);
@@ -297,6 +301,7 @@ void gdbpy_initialize_exited_event (void);
void gdbpy_initialize_thread_event (void);
void gdbpy_initialize_new_objfile_event (void);
void gdbpy_initialize_arch (void);
+void gdbpy_initialize_py_out (void);
struct cleanup *make_cleanup_py_decref (PyObject *py);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 53ddee9..3ab4b7c 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1621,6 +1621,7 @@ message == an error message without a stack will be printed."),
gdbpy_initialize_thread_event ();
gdbpy_initialize_new_objfile_event () ;
gdbpy_initialize_arch ();
+ gdbpy_initialize_py_out ();
observer_attach_before_prompt (before_prompt_hook);
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-04 14:09 [RFC - Python Scripting] New method gdb.Architecture.disassemble Siva Chandra
2013-02-05 23:28 ` Doug Evans
@ 2013-02-06 19:58 ` Tom Tromey
2013-02-06 20:31 ` Phil Muldoon
` (2 more replies)
1 sibling, 3 replies; 39+ messages in thread
From: Tom Tromey @ 2013-02-06 19:58 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> The attached patch adds a new method 'disassemble' to the class
Siva> gdb.Architecture. I have not yet added docs and tests, but will do so
Siva> after I get feedback that adding such a method is OK.
Why is it a method on gdb.Architecture and not elsewhere?
For example, we have gdb.Inferior.read_memory.
Siva> + if (!PyArg_ParseTuple (args, "KK|i", &low, &high, &opcodes))
Usually if we support more than one argument, we allow keywords as well.
Siva> + if (opcodes)
Siva> + flags = DISASSEMBLY_RAW_INSN;
Why an int treated as a boolean rather than a real boolean?
Siva> + TRY_CATCH (except, RETURN_MASK_ALL)
Siva> + {
Siva> + gdb_disassembly (gdbarch, py_out, NULL, flags, -1, low, high);
Why is py_out a global instead of something created and destroyed here?
Siva> + if (! (PyList_Check (temp) && PyList_Size (temp) > 0))
Siva> + return NULL;
You can't return NULL without setting the exception.
Siva> +struct ui_out *py_out;
I'd rather avoid new globals, and I don't see what this one adds.
Siva> +/* This data structure captures the Python version of ui_out. The Python
Siva> + version is not used to display output to a user, but to capture the results
Siva> + from GDB's internals in to a Python data structure. Hence, it does not have
Siva> + any representation for table headers. However, it can be viewed as a
Siva> + recursive table structure wherin the highest level is a list of rows. All
Siva> + rows in this list can either be a list themselves, or all of them can be
Siva> + dicts holding the table's fields. If they were lists, then they follow the
Siva> + same recurrsive structure as the higher levels.
Typo, "recursive".
I like this -- I've wanted it before -- but I wonder whether it handles
all cases. See http://sourceware.org/bugzilla/show_bug.cgi?id=11688#c6
Siva> +static struct row_data *
Siva> +new_row (struct row_data *parent)
Siva> +{
Siva> + struct row_data *row;
Siva> +
Siva> + row = (struct row_data *) xmalloc (sizeof (struct row_data));
You don't need the cast.
Siva> +PyObject *
Siva> +fetch_and_reset_py_out_object (struct ui_out *ui_out)
All new functions should have an intro comment.
Siva> + temp = py_out_data->table->data;
Siva> + py_out_data->table->data = PyList_New (0);
What if PyList_New returns NULL here?
Then the exception will be set, making problems later.
Siva> + /* Commit the row to the parent list. */
Siva> + PyList_Append (py_out_data->current_row->parent_row->data,
Siva> + py_out_data->current_row->data);
What if this fails?
This applies to many calls into Python in the patch.
Siva> +static struct ui_out_impl py_ui_out_impl =
Siva> +{
Siva> + 0, /* table_begin */
Siva> + 0, /* table_body */
Siva> + 0, /* table_end */
Why are these NULL?
Siva> + 0 /* is_mi_like_p */
It seems to me that this should be 1.
(Well, really, this is all a hack that should go away, but I doubt I'll
live to see that.)
Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 1:53 ` Siva Chandra
@ 2013-02-06 20:00 ` Tom Tromey
2013-02-08 18:05 ` Doug Evans
1 sibling, 0 replies; 39+ messages in thread
From: Tom Tromey @ 2013-02-06 20:00 UTC (permalink / raw)
To: Siva Chandra; +Cc: Doug Evans, gdb-patches
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> The only useful entry point currently available is gdb_disassembly and
Siva> I do not think it is a bad entry point. Other disassembly functions in
Siva> disasm.c are static. However, for the Python API, my patch provides
Siva> only one option of whether to include or exclude opcodes in the
Siva> disassembled output.
Come to think of it, why have that knob at all?
It is easy enough to ignore fields in structured output.
Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 19:58 ` Tom Tromey
@ 2013-02-06 20:31 ` Phil Muldoon
2013-02-06 22:31 ` Matt Rice
2013-02-07 14:14 ` Siva Chandra
2 siblings, 0 replies; 39+ messages in thread
From: Phil Muldoon @ 2013-02-06 20:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: Siva Chandra, gdb-patches
On 02/06/2013 07:58 PM, Tom Tromey wrote:
>>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
>
> Siva> +struct ui_out *py_out;
>
> I'd rather avoid new globals, and I don't see what this one adds.
Perhaps it is global as ui_out structures are a finite resource? See
ui-out.c:
/* Maintain a stack so that the info applicable to the inner most list
is always available. Stack/nested level 0 is reserved for the
top-level result. */
enum { MAX_UI_OUT_LEVELS = 8 };
I think we already run into this with recursive inferior function
calls in a python pretty printer:
http://sourceware.org/bugzilla/show_bug.cgi?id=10344
But, caveats galore, I have no had time to truly look at the patch.
Just a passing thought.
Cheers,
Phil
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 19:58 ` Tom Tromey
2013-02-06 20:31 ` Phil Muldoon
@ 2013-02-06 22:31 ` Matt Rice
2013-02-06 23:19 ` Siva Chandra
2013-02-07 14:14 ` Siva Chandra
2 siblings, 1 reply; 39+ messages in thread
From: Matt Rice @ 2013-02-06 22:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: Siva Chandra, gdb-patches
On 2/6/13, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
> Siva> +/* This data structure captures the Python version of ui_out. The
> Python
> Siva> + version is not used to display output to a user, but to capture
> the results
> Siva> + from GDB's internals in to a Python data structure. Hence, it
> does not have
> Siva> + any representation for table headers. However, it can be viewed
> as a
> Siva> + recursive table structure wherin the highest level is a list of
> rows. All
> Siva> + rows in this list can either be a list themselves, or all of them
> can be
> Siva> + dicts holding the table's fields. If they were lists, then they
> follow the
> Siva> + same recurrsive structure as the higher levels.
>
> Typo, "recursive".
>
> I like this -- I've wanted it before -- but I wonder whether it handles
> all cases. See http://sourceware.org/bugzilla/show_bug.cgi?id=11688#c6
these may help, not sure i'll try to be a little bit more pointed in
identifying the sources of the problems disregarding the details of
that specific patches attempt to paper around them:
http://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
value ==>
const | tuple | list
result ==>
variable "=" value
list ==>
"[]" | "[" value ( "," value )* "]" | "[" result ( "," result )* "]"
Notes:
New gdb/mi commands should only output lists containing values.
---
so, list can look like:
[1, 2, 3] or ["a" = 1, "b" = 2, "c" = 3]
both are created with a ui_out_type_list
thus we don't know if to create a python dict or list, until something is added,
and the existence of a fldname
then a 2nd issue entirely separate is that:
["a" = 1, "a" = 2, "a" = 3] is also valid and seen in practice
so sanely working around those issues is where the thoughts on adding
new uiout types comes from.
+static void
+py_out_field_string (struct ui_out * ui_out, int fldno, int width,
+ enum ui_align align, const char *fldname, const char *str)
+{
+ struct py_out_data *py_out_data = ui_out_data (ui_out);
+
+ CHECK_AND_INIT_FIELD_ROW_DATA (py_out_data->current_row->data);
+
+ PyDict_SetItemString (py_out_data->current_row->data, fldname,
+ PyString_FromString (str));
+}
need to test that 'fldname' isn't null, this should happen for the
[value, value] type of list.
PyDict_SetItemString:
the key object is created using PyString_FromString(key)
PyString_FromString:
The parameter v must not be NULL
my main concern is to do our best to make the datastructure returned
compatible with some future implementation of py-out which can handle
everything uiout throws at it.
then ask when uiout throws some new found nonsensical junk at us how
do we handle it?
not to dissuade from using an incomplete solution.
I suppose i'd be happy if the user had to create a py-uiout object
(passing in a version to the creation function), then passed that to
the dissasemble() function, but I dunno if the user really would.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 22:31 ` Matt Rice
@ 2013-02-06 23:19 ` Siva Chandra
2013-02-07 1:11 ` Siva Chandra
[not found] ` <20130206235707.GA2353@klara.mpi.htwm.de>
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-06 23:19 UTC (permalink / raw)
To: Matt Rice; +Cc: Tom Tromey, gdb-patches
On Wed, Feb 6, 2013 at 2:31 PM, Matt Rice <ratmice@gmail.com> wrote:
> so, list can look like:
> [1, 2, 3] or ["a" = 1, "b" = 2, "c" = 3]
>
> both are created with a ui_out_type_list
> thus we don't know if to create a python dict or list, until something is
> added,
> and the existence of a fldname
>
> then a 2nd issue entirely separate is that:
> ["a" = 1, "a" = 2, "a" = 3] is also valid and seen in practice
>
> so sanely working around those issues is where the thoughts on adding
> new uiout types comes from.
Firstly, thanks a lot to everyone for the feedback. Due to my limited
knowledge of GDB and its internals, my patch implements a Python
ui_out which I reversed engineered based on how gdb_disassembly works
and what I could understand from ui-out.c. I was hoping to get this
this kind of feedback, and I have now got it :)
Based on what Matt says in his comments, it seems like a leaf row can
look like one of these 3 possibilities:
1. [1, 2, 3]
2. ['a' : 1, 'b': 2, 'c': 3]
3. ['a': 1, 'a': 2, 'a': 3]
1 and 2 can be encapsulated with fundamental Python data structures. I
am not aware of any fundamental Python data structure which can
capture 3. So, is using a helper class called LabelValuePair a good
idea? With this, all leaf rows can be lists whose elements are either
all values, or are all LabelValuePairs: [value, value, value] or
[LabelValuePair, LabelValuePair, LabelValuePair]. Does this sound
reasonable? We can always go in for a list of single element dicts,
but I think that kind of makes it ugly. LabelValuePair can look like
this (in its Python form):
class LabelValuePair(object):
def __init__(self, label, value):
self.label = label # not a writable attribute
self.value = value # not a writable attribute
About table headers; are they relevant when ui_out is not for display?
That is, is it necessary to provision for them in the Python ui_out
object?
Thanks,
Siva Chandra
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 23:19 ` Siva Chandra
@ 2013-02-07 1:11 ` Siva Chandra
2013-02-07 23:03 ` Matt Rice
[not found] ` <20130206235707.GA2353@klara.mpi.htwm.de>
1 sibling, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-07 1:11 UTC (permalink / raw)
To: Matt Rice; +Cc: Tom Tromey, gdb-patches
On Wed, Feb 6, 2013 at 3:18 PM, Siva Chandra <sivachandra@google.com> wrote:
> Based on what Matt says in his comments, it seems like a leaf row can
> look like one of these 3 possibilities:
>
> 1. [1, 2, 3]
> 2. ['a' : 1, 'b': 2, 'c': 3]
> 3. ['a': 1, 'a': 2, 'a': 3]
>
> 1 and 2 can be encapsulated with fundamental Python data structures. I
> am not aware of any fundamental Python data structure which can
> capture 3. So, is using a helper class called LabelValuePair a good
> idea? With this, all leaf rows can be lists whose elements are either
> all values, or are all LabelValuePairs: [value, value, value] or
> [LabelValuePair, LabelValuePair, LabelValuePair]. Does this sound
> reasonable? We can always go in for a list of single element dicts,
> but I think that kind of makes it ugly. LabelValuePair can look like
> this (in its Python form):
>
> class LabelValuePair(object):
> def __init__(self, label, value):
> self.label = label # not a writable attribute
> self.value = value # not a writable attribute
Or, could it be a named tuple:
http://docs.python.org/2.7/library/collections.html#collections.namedtuple
The down side is that they are available only on Python 2.4 and higher.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
[not found] ` <20130206235707.GA2353@klara.mpi.htwm.de>
@ 2013-02-07 1:18 ` Siva Chandra
0 siblings, 0 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-07 1:18 UTC (permalink / raw)
To: André Pönitz; +Cc: gdb-patches
On Wed, Feb 6, 2013 at 3:57 PM, André Pönitz
<andre.poenitz@mathematik.tu-chemnitz.de> wrote:
>> 1. [1, 2, 3]
>> 2. ['a' : 1, 'b': 2, 'c': 3]
>> 3. ['a': 1, 'a': 2, 'a': 3]
>>
>> 1 and 2 can be encapsulated with fundamental Python data structures. I
>> am not aware of any fundamental Python data structure which can
>> capture 3.
>
> Are you asking whether you need to create 3. or whether you need to
> be able to read it?
>
> I think _producing_ 3. is never really needed from a consumers point
> of view, but happens more or less accidentally for historic reason.
> So the consumer has to be aware of the possibility of it appearing
> but generally handles it similarly to 1.
>
> New commands on the producer side could restrict themselves to not
> producing 3, i.e. to use "normal" Python structures only.
Python ui_out is a way to get what a "producer" produces into Python.
Hence, as Matt and Tom say, we want to be able to entertain as large a
set of producers as possible. At least, that is what I understand from
their comments.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 19:58 ` Tom Tromey
2013-02-06 20:31 ` Phil Muldoon
2013-02-06 22:31 ` Matt Rice
@ 2013-02-07 14:14 ` Siva Chandra
2013-02-07 16:42 ` Tom Tromey
2 siblings, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-07 14:14 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, Feb 6, 2013 at 11:58 AM, Tom Tromey <tromey@redhat.com> wrote:
> Why is it a method on gdb.Architecture and not elsewhere?
> For example, we have gdb.Inferior.read_memory.
Based on a comment in this thread,
http://sourceware.org/ml/gdb-patches/2012-11/msg00052.html, I gather
that architecture could change between frames. Hence, would it not be
more appropriate to have this method on gdb.Architecture?
> Siva> +struct ui_out *py_out;
>
> I'd rather avoid new globals, and I don't see what this one adds.
The only reason I made it global is because there is no way currently
to free up a ui_out object from outside of ui-out.c. Do you think it
is OK to add a field ui_out_destroy_ftype to struct ui_out_impl? If
yes, then I think we can first add the support for this 'virtual
destructor' in a separate patch.
Thanks,
Siva Chandra
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-07 14:14 ` Siva Chandra
@ 2013-02-07 16:42 ` Tom Tromey
0 siblings, 0 replies; 39+ messages in thread
From: Tom Tromey @ 2013-02-07 16:42 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> Based on a comment in this thread,
Siva> http://sourceware.org/ml/gdb-patches/2012-11/msg00052.html, I gather
Siva> that architecture could change between frames. Hence, would it not be
Siva> more appropriate to have this method on gdb.Architecture?
Ok.
Siva> The only reason I made it global is because there is no way currently
Siva> to free up a ui_out object from outside of ui-out.c. Do you think it
Siva> is OK to add a field ui_out_destroy_ftype to struct ui_out_impl? If
Siva> yes, then I think we can first add the support for this 'virtual
Siva> destructor' in a separate patch.
I think this would be much nicer, thanks.
Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-07 1:11 ` Siva Chandra
@ 2013-02-07 23:03 ` Matt Rice
0 siblings, 0 replies; 39+ messages in thread
From: Matt Rice @ 2013-02-07 23:03 UTC (permalink / raw)
To: Siva Chandra; +Cc: Tom Tromey, gdb-patches
On Wed, Feb 6, 2013 at 5:11 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Wed, Feb 6, 2013 at 3:18 PM, Siva Chandra <sivachandra@google.com> wrote:
>> Based on what Matt says in his comments, it seems like a leaf row can
>> look like one of these 3 possibilities:
>>
>> 1. [1, 2, 3]
>> 2. ['a' : 1, 'b': 2, 'c': 3]
>> 3. ['a': 1, 'a': 2, 'a': 3]
>>
>> 1 and 2 can be encapsulated with fundamental Python data structures. I
>> am not aware of any fundamental Python data structure which can
>> capture 3. So, is using a helper class called LabelValuePair a good
>> idea? With this, all leaf rows can be lists whose elements are either
>> all values, or are all LabelValuePairs: [value, value, value] or
>> [LabelValuePair, LabelValuePair, LabelValuePair]. Does this sound
>> reasonable? We can always go in for a list of single element dicts,
>> but I think that kind of makes it ugly. LabelValuePair can look like
>> this (in its Python form):
>>
>> class LabelValuePair(object):
>> def __init__(self, label, value):
>> self.label = label # not a writable attribute
>> self.value = value # not a writable attribute
>
> Or, could it be a named tuple:
> http://docs.python.org/2.7/library/collections.html#collections.namedtuple
> The down side is that they are available only on Python 2.4 and higher.
I definitely agree that these make it possible to do the py-out stuff
without the headache of modifying existing ui-out callers which is
desirable,
and turning it into {'a': (1, 2, 3)} doesn't really achieve that due
to the single 'uiout_list_type' entry point currently available.
one nice thing about the latter is that any python version can parse
it via 'literal_eval'.
which means if someone wanted to replace mi with py-out as a wire protocol
they need to share either the named_tuple stuff, or the LabelValuePair
object code on both the gdb and py-out client side.
so, my preference had been to represent duplicate keys as dict with a
list value just because it makes parsing dead simple/can just use a
stock python, that said I definitely see the appeal of a custom class
or named tuple, so I'm sort of on the fence on this one.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-06 1:53 ` Siva Chandra
2013-02-06 20:00 ` Tom Tromey
@ 2013-02-08 18:05 ` Doug Evans
2013-02-09 17:55 ` Matt Rice
2013-02-12 14:56 ` Siva Chandra
1 sibling, 2 replies; 39+ messages in thread
From: Doug Evans @ 2013-02-08 18:05 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
On Tue, Feb 5, 2013 at 5:53 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Tue, Feb 5, 2013 at 3:28 PM, Doug Evans <dje@google.com> wrote:
>> I like the idea, but for an API I wouldn't mind seeing something
>> a bit lower level. E.g., skip the higher level disassembler entry
>> points in gdb (mixed source/assembly support and all that), and provide
>> more direct access to the disassembler.
>
> The only useful entry point currently available is gdb_disassembly and
> I do not think it is a bad entry point. Other disassembly functions in
> disasm.c are static.
Well, it begs the question what entrypoint into libopcodes does
gdb_disassembly use? :-)
Anyways, my point is gdb_disassembly is a bit high level for the
Python API for me.
> However, for the Python API, my patch provides
> only one option of whether to include or exclude opcodes in the
> disassembled output.
Today.
Someone will think "Oh look, I can just add a flag and get more features."
with no real consideration if that level in the API is the right place
for those features.
>> I didn't go through it with a fine toothed comb, but here are some questions.
>> 1) Can we remove the py_out global?
>
> At what level do you not want this to be global? I have made it static
> to the file in the attached patch.
This has been addressed elsewhere.
>> 2) It seems like this will export a lot of struct ui_out to the user.
>> I'd rather provide disassembly without having to commit to supporting
>> struct ui_out in Python.
>
> I am not very sure I understand this point. My patch does not expose
> anything about the struct ui_out to the user/Python API. Python ui_out
> is only a way to get results from GDB internals into a Python data
> structure. Also, this Python data structure does not depend on
> gdb_disassembly's display format.
Let's take a step back and assume one wants to add disassembly to Python,
without knowing anything about what gdb does and doesn't already provide.
If I ask to disassemble an instruction at a particular address I
expect to get back a string (with possibly an error thrown or some
such if the memory at that address isn't readable).
Let's see an enumeration of the other things one might want to get back.
- symbol name of a referenced address?
- or at least the address so that the caller can do the symbol
lookup, which might be preferable
- raw bytes of the instruction?
- length of the instruction?
- anything else?
Hmmm, another thing occurs to me:
Global State In An API Is Bad.
What are the other inputs to disassembly, and should they be parameters?
- syntax?
- for now we could use the CLI setting as the default
- architecture variant? (including endianness)
- all aspects of this are presumably encoded in the
gdb.Architecture class so no need for an explicit parameter, included
here for completeness sake
- anything else?
If not, we're forcing the caller to do a "save current global state,
change it, catch the call to the disassembler so we can properly
restore state, restore global state" dance.
We need to move away from this in APIs (we're suffering from aspects
of this in gdb's internal APIs, let's learn this lesson and apply it).
> 2013-02-05 Siva Chandra Reddy <sivachandra@google.com>
>
> Add a new method 'disassemble' to gdb.Architecture class.
> * Makefile.in: Add entries for the new file python/py-out.c
> * python/py-arch.c (archpy_disassmble): Implementation of the
> new method gdb.Architecture.disassemble.
> (arch_object_methods): Add entry for the new method.
> * python/py-out.c: Implementation of a Python ui_out.
> * python/python-internal.h: Add declarations for new utility
> functions.
> * python/python.c (_initialize_python): Initialize Python
> ui_out.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-08 18:05 ` Doug Evans
@ 2013-02-09 17:55 ` Matt Rice
2013-02-12 14:56 ` Siva Chandra
1 sibling, 0 replies; 39+ messages in thread
From: Matt Rice @ 2013-02-09 17:55 UTC (permalink / raw)
To: Doug Evans; +Cc: Siva Chandra, gdb-patches
On Fri, Feb 8, 2013 at 10:05 AM, Doug Evans <dje@google.com> wrote:
> Let's take a step back and assume one wants to add disassembly to Python,
> without knowing anything about what gdb does and doesn't already provide.
Thanks for bringing this up, all very good points.
> Global State In An API Is Bad.
definitely agree, part of why i was wondering if the ui-out object
shouldn't be passed in to the disassemble method was because it seemed
more pure
I suppose this leads me to consider that the "data-disassemble" command is
(untested though) exposed already via the 'mi_execute' functions in
the PR #11688
which already has generic conversion of arguments to and from mi commands.
to transcribe an example (untested) from the mi docs [1]
gdb.mi_execute("data-disassemble", ("-s", "$pc", "-e", "$pc+20", "--", 0))
This at least makes it rather apparent that we aren't working with a
real python API
and are using a python version of some gdb command interpreter (and
all that entails).
that at least gives us a stopgap that we can use while codifying a real API
does disassemble method vs generic mi method make any difference?
my main concern with the generic mi method is that it makes the coding
of any real
python API addressing your concerns somewhat less imminent
[1] http://sourceware.org/gdb/current/onlinedocs/gdb/GDB_002fMI-Data-Manipulation.html#GDB_002fMI-Data-Manipulation
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-08 18:05 ` Doug Evans
2013-02-09 17:55 ` Matt Rice
@ 2013-02-12 14:56 ` Siva Chandra
2013-02-12 21:18 ` Tom Tromey
1 sibling, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-12 14:56 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
On Fri, Feb 8, 2013 at 10:05 AM, Doug Evans <dje@google.com> wrote:
> On Tue, Feb 5, 2013 at 5:53 PM, Siva Chandra <sivachandra@google.com> wrote:
>> The only useful entry point currently available is gdb_disassembly and
>> I do not think it is a bad entry point. Other disassembly functions in
>> disasm.c are static.
>
> Well, it begs the question what entrypoint into libopcodes does
> gdb_disassembly use? :-)
> Anyways, my point is gdb_disassembly is a bit high level for the
> Python API for me.
Thanks for your detailed reply. I now have a patch which does not use
ui_out. Instead of calling gdb_disassembly, it essentially
re-implements disasm.c:dump_insns. The patch is attached.
2013-02-12 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
[-- Attachment #2: arch_disassemble_patch_v3.txt --]
[-- Type: text/plain, Size: 6001 bytes --]
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..dd50d78 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,137 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a dict object with the following string keys:
+
+ KEY VALUE TYPE DESCRIPTION
+ =============================================================================
+ 'addr' long integer The address of the instruction in target memory.
+ -----------------------------------------------------------------------------
+ 'asm' string The instruction depicted in its assembly
+ language. Will be set to 'unknown' if GDB is
+ unable to obtain this value.
+ -----------------------------------------------------------------------------
+ 'func' string Name of the function to which the instruction
+ belongs to. Will be set to '<unknown>' if GDB is
+ unable to obtain this value.
+ -----------------------------------------------------------------------------
+ 'length' integer Length of the instruction in bytes.
+ -----------------------------------------------------------------------------
+ 'offset' integer The byte offset of the instruction in the func
+ above. Will be set to -1 if GDB is unable to
+ obtain this value.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "low", "high", NULL };
+ CORE_ADDR low, high;
+ CORE_ADDR pc;
+ PyObject *result_list;
+ static char unknown_str[] = { '<', 'u', 'n', 'k', 'n', 'o', 'w', 'n', '>', '\0' };
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG GDB_PY_LLU_ARG,
+ keywords, &low, &high))
+ return NULL;
+
+ result_list = PyList_New (0);
+ if (!result_list)
+ {
+ PyErr_SetString (PyExc_MemoryError,
+ _("Unable to create a list of disassembled "
+ "instructions."));
+ return NULL;
+ }
+
+ for (pc = low; pc <= high;)
+ {
+ int line = -1, unmapped, offset = -1, insn_len = 0;
+ char *filename = NULL, *funcname = NULL, *asm_code = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (!insn_dict)
+ {
+ Py_DECREF (result_list);
+ PyErr_SetString (PyExc_MemoryError,
+ _("Unable to create a dict for instruction."));
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ /* Even though filename, line and unmapped are passed as arguments,
+ they do not give us any meaningful values currently. */
+ build_address_symbolic (gdbarch, pc, 0, &funcname, &offset, &filename,
+ &line, &unmapped);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+ if (funcname)
+ xfree (funcname);
+ if (filename)
+ xfree (filename);
+ if (asm_code)
+ xfree (asm_code);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ asm_code = ui_file_xstrdup (memfile, NULL);
+ if (!asm_code)
+ asm_code = unknown_str;
+ if (!funcname)
+ funcname = unknown_str;
+ if (!filename)
+ filename = unknown_str;
+
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (asm_code))
+ || PyDict_SetItemString (insn_dict, "func",
+ PyString_FromString (funcname))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len))
+ || PyDict_SetItemString (insn_dict, "offset",
+ PyInt_FromLong (offset)))
+ {
+ Py_DECREF (result_list);
+ PyErr_SetString (PyExc_MemoryError,
+ _("Unable to add fields to instruction dict."));
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ ui_file_delete (memfile);
+ if (funcname && funcname != unknown_str)
+ xfree (funcname);
+ if (filename && filename != unknown_str)
+ xfree (filename);
+ if (asm_code && asm_code != unknown_str)
+ xfree (asm_code);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +237,10 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (low, high) -> List.\n\
+Return the list of disassembled instructions from LOW to HIGH." },
{NULL} /* Sentinel */
};
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-12 14:56 ` Siva Chandra
@ 2013-02-12 21:18 ` Tom Tromey
2013-02-13 14:37 ` Siva Chandra
0 siblings, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2013-02-12 21:18 UTC (permalink / raw)
To: Siva Chandra; +Cc: Doug Evans, gdb-patches
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> Thanks for your detailed reply. I now have a patch which does not use
Siva> ui_out. Instead of calling gdb_disassembly, it essentially
Siva> re-implements disasm.c:dump_insns. The patch is attached.
I don't have any problem with this approach.
I do have some nits on the patch though.
Siva> +/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
Siva> + Returns a list of instructions in a memory address range. Each instruction
Siva> + in the list is a dict object with the following string keys:
Siva> +
You can write a shorter comment here and just put all the informative
stuff into the docs.
Siva> + static char unknown_str[] = { '<', 'u', 'n', 'k', 'n', 'o', 'w', 'n', '>', '\0' };
This line looks too long.
I think it isn't too hard to reorganize so that this can be an ordinary
const char *.
Siva> + result_list = PyList_New (0);
Siva> + if (!result_list)
Siva> + {
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to create a list of disassembled "
Siva> + "instructions."));
Siva> + return NULL;
You don't need PyErr_SetString here.
PyList_New will have done that already.
So just return NULL.
Siva> + struct ui_file *memfile = mem_fileopen ();
Siva> + PyObject *insn_dict = PyDict_New ();
Siva> + volatile struct gdb_exception except;
Siva> +
Siva> + if (!insn_dict)
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to create a dict for instruction."));
Siva> +
Siva> + return NULL;
Here too.
This leaks memfile on failure.
Siva> + }
Siva> + if (PyList_Append (result_list, insn_dict))
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + Py_DECREF (insn_dict);
Siva> +
Siva> + return NULL; /* PyList_Append Sets the exception. */
This leaks it too.
Siva> + if (funcname)
Siva> + xfree (funcname);
Siva> + if (filename)
Siva> + xfree (filename);
Siva> + if (asm_code)
Siva> + xfree (asm_code);
xfree handles NULL arguments fine, so remove the "if"s.
Siva> + if (PyDict_SetItemString (insn_dict, "addr",
Siva> + gdb_py_long_from_ulongest (pc))
Siva> + || PyDict_SetItemString (insn_dict, "asm",
Siva> + PyString_FromString (asm_code))
I think PyString_FromString handles const char * arguments fine.
So you can make this:
asm_code == NULL ? unknown_str : asm_code
and avoid some hair above and below, and fix up unknown_str as well.
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to add fields to instruction dict."));
Siva> +
Siva> + return NULL;
You don't need the PyErr_SetString.
And this leaks some memory.
Siva> + if (funcname && funcname != unknown_str)
Siva> + xfree (funcname);
Siva> + if (filename && filename != unknown_str)
Siva> + xfree (filename);
Siva> + if (asm_code && asm_code != unknown_str)
Siva> + xfree (asm_code);
No need to check nullity; but with the other changes this could all be
unconditional.
Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-12 21:18 ` Tom Tromey
@ 2013-02-13 14:37 ` Siva Chandra
2013-02-13 17:52 ` Eli Zaretskii
2013-02-13 18:03 ` Tom Tromey
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-13 14:37 UTC (permalink / raw)
To: Tom Tromey; +Cc: Doug Evans, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]
I have addressed all of Tom's comments and added docs and tests in the
attached patch. I did not add a NEWS entry as an entry already exists
for the new gdb.Architecture class.
2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
doc/
* gdb.texinfo (Architectures In Python): Add description about
the new method gdb.Architecture.disassemble.
testsuite/
* gdb.python/py-arch.c: New test case
* gdb.python/py-arch.exp: New tests to test
gdb.Architecture.disassemble
* gdb.python/Makefile.in: Add py-arch to the list of
EXECUTABLES.
I have a couple of questions for my knowledge:
1. docs.python.org says that PyList_Append sets an exception when it
fails. It does not say so for PyList_New() and PyDict_New(). However,
Tom commented that even these functions set exception on failure. In
general, how do we know if a certain Python C API function sets an
exception or not?
2. One of Tom's suggestion was to use something like this:
asm_code == NULL ? unknown_str : asm_code
I have modified my code accordingly, but the concern I have is that
asm_code is of type 'char *', while unknown_str if of type 'const char
*'. This means that the type of result of the above expression is
dynamic! Though the code compiles with GCC, it that standard?
Thanks,
Siva Chandra
[-- Attachment #2: arch_disassemble_patch_v4.txt --]
[-- Type: text/plain, Size: 9084 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e3f336e..b18c409 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26040,6 +26040,36 @@ A @code{gdb.Architecture} class has the following methods:
Return the name (string value) of the architecture.
@end defun
+@defun Architecture.disassemble (@var{low}, @var{high})
+Return a list of disassembled instructions in the memory address range
+@var{low} to @var{high}. Each element of the list is a Python
+@code{dict} with the following string keys:
+
+@table @asis
+
+@item @code{`addr'}
+The value corresponding to this key is a Python long integer capturing
+the memory address of the instruction.
+
+@item @code{`asm'}
+The value corresponding to this key is a string value capturing the
+assembly language code of the instruction.
+
+@item @code{`func'}
+The value corresponding to this key is the name of the function (string
+value) the instruction belongs to.
+
+@item @code{`length'}
+The value correspoding to this key is the length (integer value) of the
+instruction in bytes.
+
+@item @code{`offset'}
+The value corresponding to this key is the byte offset (integer value)
+of the instruction within the function it belongs to.
+
+@end table
+@end defun
+
@node Python Auto-loading
@subsection Python Auto-loading
@cindex Python auto-loading
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..1837bf3 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,104 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a Python dict object.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "low", "high", NULL };
+ CORE_ADDR low, high;
+ CORE_ADDR pc;
+ PyObject *result_list;
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG GDB_PY_LLU_ARG,
+ keywords, &low, &high))
+ return NULL;
+
+ result_list = PyList_New (0);
+ if (!result_list)
+ return NULL;
+
+ for (pc = low; pc <= high;)
+ {
+ int line = -1, unmapped, offset = -1, insn_len = 0;
+ char *filename = NULL, *fn = NULL, *as = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (!insn_dict)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+ ui_file_delete (memfile);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ /* Even though filename, line and unmapped are passed as arguments,
+ they do not give us any meaningful values currently. */
+ build_address_symbolic (gdbarch, pc, 0, &fn, &offset, &filename,
+ &line, &unmapped);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+ xfree (fn);
+ xfree (filename);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ as = ui_file_xstrdup (memfile, NULL);
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (as ? as : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "func",
+ PyString_FromString (fn ? fn : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len))
+ || PyDict_SetItemString (insn_dict, "offset",
+ PyInt_FromLong (offset)))
+ {
+ Py_DECREF (result_list);
+ PyErr_SetString (PyExc_MemoryError,
+ _("Unable to add fields to instruction dict."));
+
+ ui_file_delete (memfile);
+ xfree (as);
+ xfree (fn);
+ xfree (filename);
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ ui_file_delete (memfile);
+ xfree (as);
+ xfree (fn);
+ xfree (filename);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +204,10 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (low, high) -> List.\n\
+Return the list of disassembled instructions from LOW to HIGH." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
index 4e286b5..0b81507 100644
--- a/gdb/testsuite/gdb.python/Makefile.in
+++ b/gdb/testsuite/gdb.python/Makefile.in
@@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
py-shared python lib-types py-events py-evthreads py-frame \
py-mi py-pp-maint py-progspace py-section-script py-objfile \
py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
- py-explore-cc
+ py-explore-cc py-arch
MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
new file mode 100644
index 0000000..e2fe55c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 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
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
new file mode 100644
index 0000000..5104619
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -0,0 +1,45 @@
+# Copyright 2013 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
+gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
+gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
+gdb_py_test_silent_cmd "python insn_list = arch.disassemble(pc, pc)" "disassemble" 0
+
+gdb_test "python print len(insn_list)" "1" "test number of instructions"
+
+gdb_py_test_silent_cmd "python insn = insn_list\[0\]" "get instruction" 0
+
+gdb_test "python print \"addr\" in insn" "True" "test key addr"
+gdb_test "python print \"asm\" in insn" "True" "test key asm"
+gdb_test "python print \"func\" in insn" "True" "test key func"
+gdb_test "python print \"length\" in insn" "True" "test key length"
+gdb_test "python print \"offset\" in insn" "True" "test key offset"
+
+# Negative test
+gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-13 14:37 ` Siva Chandra
@ 2013-02-13 17:52 ` Eli Zaretskii
2013-02-13 18:03 ` Tom Tromey
1 sibling, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2013-02-13 17:52 UTC (permalink / raw)
To: Siva Chandra; +Cc: tromey, dje, gdb-patches
> Date: Wed, 13 Feb 2013 06:37:46 -0800
> From: Siva Chandra <sivachandra@google.com>
> Cc: Doug Evans <dje@google.com>, gdb-patches <gdb-patches@sourceware.org>
>
> I have addressed all of Tom's comments and added docs and tests in the
> attached patch. I did not add a NEWS entry as an entry already exists
> for the new gdb.Architecture class.
Thanks.
> +@defun Architecture.disassemble (@var{low}, @var{high})
> +Return a list of disassembled instructions in the memory address range
> +@var{low} to @var{high}.
Please document whether the range is inclusive or exclusive. Being
in doubt about these nits is one of my big annoyances with
documentation out there.
> +@table @asis
Since all of the @item's use @code, it is better to use
@table @code
> +@item @code{`addr'}
No need for another pair of quotes inside @code. (But if you want the
quotes in the printed version of the manual as well, use @samp
instead of @code.
> +@item @code{`asm'}
> +The value corresponding to this key is a string value capturing the
> +assembly language code of the instruction.
What is "assembly language code"? is it a mnemonic instruction name or
a numeric opcode?
Btw, an example would clarify these issues nicely.
OK with those changes.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-13 14:37 ` Siva Chandra
2013-02-13 17:52 ` Eli Zaretskii
@ 2013-02-13 18:03 ` Tom Tromey
2013-02-13 19:50 ` Siva Chandra
1 sibling, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2013-02-13 18:03 UTC (permalink / raw)
To: Siva Chandra; +Cc: Doug Evans, gdb-patches
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> 1. docs.python.org says that PyList_Append sets an exception when it
Siva> fails. It does not say so for PyList_New() and PyDict_New(). However,
Siva> Tom commented that even these functions set exception on failure. In
Siva> general, how do we know if a certain Python C API function sets an
Siva> exception or not?
The general rule in Python is that failures set the exception. I found
some text in the manual, from
http://docs.python.org/2/c-api/intro.html#exceptions
All functions in the Python/C API can raise exceptions, unless an
explicit claim is made otherwise in a function’s documentation. In
general, when a function encounters an error, it sets an exception,
discards any object references that it owns, and returns an error
indicator. If not documented otherwise, this indicator is either NULL
or -1, depending on the function’s return type. A few functions return
a Boolean true/false result, with false indicating an error. Very few
functions return no explicit error indicator or have an ambiguous
return value, and require explicit testing for errors with
PyErr_Occurred(). These exceptions are always explicitly documented.
Siva> 2. One of Tom's suggestion was to use something like this:
Siva> asm_code == NULL ? unknown_str : asm_code
Siva> I have modified my code accordingly, but the concern I have is that
Siva> asm_code is of type 'char *', while unknown_str if of type 'const char
Siva> *'. This means that the type of result of the above expression is
Siva> dynamic! Though the code compiles with GCC, it that standard?
It is. I'd have to dig through the standard to find the real language,
but basically the type of the "?:" expression is the result of applying
the appropriate kind of promotion to its arguments. Here the type is
"const char *".
Siva> + if (!result_list)
There was a recent agreement to use "result == NULL" for pointer types.
I'm sorry if I didn't catch this last time around, I'm having a little
difficulty adapting.
Siva> + if (!insn_dict)
Here too.
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to add fields to instruction dict."));
I think you don't need this.
Siva> +gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" "test exception"
I think this could use a line break somewhere.
Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-13 18:03 ` Tom Tromey
@ 2013-02-13 19:50 ` Siva Chandra
2013-02-13 20:42 ` Doug Evans
0 siblings, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-13 19:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Eli Zaretskii
[-- Attachment #1: Type: text/plain, Size: 783 bytes --]
Addressed all off Eli's and Tom's comments in the attached patch.
Thanks a lot Tom, for your detailed explanations.
2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
doc/
* gdb.texinfo (Architectures In Python): Add description about
the new method gdb.Architecture.disassemble.
testsuite/
* gdb.python/py-arch.c: New test case
* gdb.python/py-arch.exp: New tests to test
gdb.Architecture.disassemble
* gdb.python/Makefile.in: Add py-arch to the list of
EXECUTABLES.
[-- Attachment #2: arch_disassemble_patch_v5.txt --]
[-- Type: text/plain, Size: 8991 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e3f336e..8436781 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26040,6 +26040,37 @@ A @code{gdb.Architecture} class has the following methods:
Return the name (string value) of the architecture.
@end defun
+@defun Architecture.disassemble (@var{low}, @var{high})
+Return a list of disassembled instructions whose start address falls in
+the closed memory address interval from @var{low} to @var{high}. Each
+element of the list is a Python @code{dict} with the following string
+keys:
+
+@table @code
+
+@item addr
+The value corresponding to this key is a Python long integer capturing
+the memory address of the instruction.
+
+@item asm
+The value corresponding to this key is a string value which represents
+the instruction with assembly language mnemonics.
+
+@item func
+The value corresponding to this key is the name of the function (string
+value) the instruction belongs to.
+
+@item length
+The value correspoding to this key is the length (integer value) of the
+instruction in bytes.
+
+@item offset
+The value corresponding to this key is the byte offset (integer value)
+of the instruction within the function it belongs to.
+
+@end table
+@end defun
+
@node Python Auto-loading
@subsection Python Auto-loading
@cindex Python auto-loading
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..1da7b67 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,102 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a Python dict object.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "low", "high", NULL };
+ CORE_ADDR low, high;
+ CORE_ADDR pc;
+ PyObject *result_list;
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG GDB_PY_LLU_ARG,
+ keywords, &low, &high))
+ return NULL;
+
+ result_list = PyList_New (0);
+ if (result_list == NULL)
+ return NULL;
+
+ for (pc = low; pc <= high;)
+ {
+ int line = -1, unmapped, offset = -1, insn_len = 0;
+ char *filename = NULL, *fn = NULL, *as = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (insn_dict == NULL)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+ ui_file_delete (memfile);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ /* Even though filename, line and unmapped are passed as arguments,
+ they do not give us any meaningful values currently. */
+ build_address_symbolic (gdbarch, pc, 0, &fn, &offset, &filename,
+ &line, &unmapped);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+ xfree (fn);
+ xfree (filename);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ as = ui_file_xstrdup (memfile, NULL);
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (as ? as : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "func",
+ PyString_FromString (fn ? fn : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len))
+ || PyDict_SetItemString (insn_dict, "offset",
+ PyInt_FromLong (offset)))
+ {
+ Py_DECREF (result_list);
+
+ ui_file_delete (memfile);
+ xfree (as);
+ xfree (fn);
+ xfree (filename);
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ ui_file_delete (memfile);
+ xfree (as);
+ xfree (fn);
+ xfree (filename);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +202,10 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (low, high) -> List.\n\
+Return the list of disassembled instructions from LOW to HIGH." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
index 4e286b5..0b81507 100644
--- a/gdb/testsuite/gdb.python/Makefile.in
+++ b/gdb/testsuite/gdb.python/Makefile.in
@@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
py-shared python lib-types py-events py-evthreads py-frame \
py-mi py-pp-maint py-progspace py-section-script py-objfile \
py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
- py-explore-cc
+ py-explore-cc py-arch
MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
new file mode 100644
index 0000000..e2fe55c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 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
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
new file mode 100644
index 0000000..25b8b11
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -0,0 +1,47 @@
+# Copyright 2013 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
+gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
+gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
+gdb_py_test_silent_cmd "python insn_list = arch.disassemble(pc, pc)" \
+ "disassemble" 0
+
+gdb_test "python print len(insn_list)" "1" "test number of instructions"
+
+gdb_py_test_silent_cmd "python insn = insn_list\[0\]" "get instruction" 0
+
+gdb_test "python print \"addr\" in insn" "True" "test key addr"
+gdb_test "python print \"asm\" in insn" "True" "test key asm"
+gdb_test "python print \"func\" in insn" "True" "test key func"
+gdb_test "python print \"length\" in insn" "True" "test key length"
+gdb_test "python print \"offset\" in insn" "True" "test key offset"
+
+# Negative test
+gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
+ "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-13 19:50 ` Siva Chandra
@ 2013-02-13 20:42 ` Doug Evans
2013-02-14 22:46 ` Siva Chandra
0 siblings, 1 reply; 39+ messages in thread
From: Doug Evans @ 2013-02-13 20:42 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
Siva Chandra writes:
> Addressed all off Eli's and Tom's comments in the attached patch.
>
> Thanks a lot Tom, for your detailed explanations.
>
> 2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
>
> Add a new method 'disassemble' to gdb.Architecture class.
> * python/py-arch.c (archpy_disassmble): Implementation of the
> new method gdb.Architecture.disassemble.
> (arch_object_methods): Add entry for the new method.
>
> doc/
>
> * gdb.texinfo (Architectures In Python): Add description about
> the new method gdb.Architecture.disassemble.
>
> testsuite/
>
> * gdb.python/py-arch.c: New test case
> * gdb.python/py-arch.exp: New tests to test
> gdb.Architecture.disassemble
> * gdb.python/Makefile.in: Add py-arch to the list of
> EXECUTABLES.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e3f336e..8436781 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26040,6 +26040,37 @@ A @code{gdb.Architecture} class has the following methods:
> Return the name (string value) of the architecture.
> @end defun
>
> +@defun Architecture.disassemble (@var{low}, @var{high})
> +Return a list of disassembled instructions whose start address falls in
> +the closed memory address interval from @var{low} to @var{high}. Each
Reading beyond high seems counterintuitive, but I can understand why it's easiest if it works that way.
Also, I can imagine a user wanting to specify a count instead of high.
How about supporting both "high" and "count", with the default being high is unspecified and count=1?
> +element of the list is a Python @code{dict} with the following string
> +keys:
> +
> +@table @code
> +
> +@item addr
> +The value corresponding to this key is a Python long integer capturing
> +the memory address of the instruction.
> +
> +@item asm
> +The value corresponding to this key is a string value which represents
> +the instruction with assembly language mnemonics.
> +
> +@item func
> +The value corresponding to this key is the name of the function (string
> +value) the instruction belongs to.
> +
> +@item length
> +The value correspoding to this key is the length (integer value) of the
> +instruction in bytes.
> +
> +@item offset
> +The value corresponding to this key is the byte offset (integer value)
> +of the instruction within the function it belongs to.
> +
> +@end table
> +@end defun
> +
Including the function name and offset in the output feels wrong.
It's trying to make the API call do too much.
Plus, the documentation needs to specify what "flavor" is used (e.g. intel vs att).
I'd just add a line of texzt saying the flavor (or choose a better word) is the one specified by the CLI variable disassembly-flavor,
and leave for another day adding an input parameter to specify the flavor.
> @node Python Auto-loading
> @subsection Python Auto-loading
> @cindex Python auto-loading
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index edd508f..1da7b67 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -20,6 +20,7 @@
> #include "defs.h"
> #include "gdbarch.h"
> #include "arch-utils.h"
> +#include "disasm.h"
> #include "python-internal.h"
>
> typedef struct arch_object_type_object {
> @@ -86,6 +87,102 @@ archpy_name (PyObject *self, PyObject *args)
> return py_name;
> }
>
> +/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
> + Returns a list of instructions in a memory address range. Each instruction
> + in the list is a Python dict object.
> +*/
> +
> +static PyObject *
> +archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
> +{
> + static char *keywords[] = { "low", "high", NULL };
> + CORE_ADDR low, high;
> + CORE_ADDR pc;
> + PyObject *result_list;
> + struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
> +
> + if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG GDB_PY_LLU_ARG,
> + keywords, &low, &high))
> + return NULL;
> +
> + result_list = PyList_New (0);
> + if (result_list == NULL)
> + return NULL;
> +
> + for (pc = low; pc <= high;)
> + {
> + int line = -1, unmapped, offset = -1, insn_len = 0;
> + char *filename = NULL, *fn = NULL, *as = NULL;
> + struct ui_file *memfile = mem_fileopen ();
> + PyObject *insn_dict = PyDict_New ();
> + volatile struct gdb_exception except;
> +
> + if (insn_dict == NULL)
> + {
> + Py_DECREF (result_list);
> + ui_file_delete (memfile);
> +
> + return NULL;
> + }
> + if (PyList_Append (result_list, insn_dict))
> + {
> + Py_DECREF (result_list);
> + Py_DECREF (insn_dict);
> + ui_file_delete (memfile);
> +
> + return NULL; /* PyList_Append Sets the exception. */
> + }
> +
> + TRY_CATCH (except, RETURN_MASK_ALL)
> + {
> + insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
> + /* Even though filename, line and unmapped are passed as arguments,
> + they do not give us any meaningful values currently. */
> + build_address_symbolic (gdbarch, pc, 0, &fn, &offset, &filename,
> + &line, &unmapped);
Why call build_address_symbol here?
To me it's trying to make this API call do too much.
I'm not against the functionality, far from it.
I'd say if it's useful, provide another API call to get it:
the user may have a pc and want this info, and not want to have to
call disassemble to get it.
What to return for offset for discontiguous functions is an open question.
If there's no real need for offset at the moment, I'd say let's punt
and just provide a routine to return the function for any given pc
(assuming it's useful).
Another open question is what to return for inlined functions,
I can imagine both answers (inline function itself and its caller) being useful.
btw, what about filename,line isn't useful?
It's easy enough to imagine the user wanting to obtain file+line for any given pc
(and in fact there is gdbpy_find_pc_line).
So I can imagine 3 API routines here:
- disassemble (pc)
- get_function_and_offset (pc) [or some such]
- get_file_and_line (pc) [assuming gdbpy_find_pc_line doesn't always do what one wants]
> + }
> + if (except.reason < 0)
> + {
> + Py_DECREF (result_list);
> + ui_file_delete (memfile);
> + xfree (fn);
> + xfree (filename);
> +
> + return gdbpy_convert_exception (except);
> + }
> +
> + as = ui_file_xstrdup (memfile, NULL);
> + if (PyDict_SetItemString (insn_dict, "addr",
> + gdb_py_long_from_ulongest (pc))
> + || PyDict_SetItemString (insn_dict, "asm",
> + PyString_FromString (as ? as : "<unknown>"))
> + || PyDict_SetItemString (insn_dict, "func",
> + PyString_FromString (fn ? fn : "<unknown>"))
> + || PyDict_SetItemString (insn_dict, "length",
> + PyInt_FromLong (insn_len))
> + || PyDict_SetItemString (insn_dict, "offset",
> + PyInt_FromLong (offset)))
> + {
> + Py_DECREF (result_list);
> +
> + ui_file_delete (memfile);
> + xfree (as);
> + xfree (fn);
> + xfree (filename);
> +
> + return NULL;
> + }
> +
> + pc += insn_len;
> + ui_file_delete (memfile);
> + xfree (as);
> + xfree (fn);
> + xfree (filename);
> + }
> +
> + return result_list;
> +}
> +
> /* Initializes the Architecture class in the gdb module. */
>
> void
> @@ -105,6 +202,10 @@ static PyMethodDef arch_object_methods [] = {
> { "name", archpy_name, METH_NOARGS,
> "name () -> String.\n\
> Return the name of the architecture as a string value." },
> + { "disassemble", (PyCFunction) archpy_disassemble,
> + METH_VARARGS | METH_KEYWORDS,
> + "disassemble (low, high) -> List.\n\
> +Return the list of disassembled instructions from LOW to HIGH." },
> {NULL} /* Sentinel */
> };
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-13 20:42 ` Doug Evans
@ 2013-02-14 22:46 ` Siva Chandra
2013-02-15 6:43 ` Doug Evans
0 siblings, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-14 22:46 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
[-- Attachment #1: Type: text/plain, Size: 3149 bytes --]
I now have a patch (find it attached) which has been tweaked based on
comments from Doug.
Thanks Doug, for another round of detailed feedback.
2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
doc/
* gdb.texinfo (Architectures In Python): Add description about
the new method gdb.Architecture.disassemble.
testsuite/
* gdb.python/py-arch.c: New test case
* gdb.python/py-arch.exp: New tests to test
gdb.Architecture.disassemble
* gdb.python/Makefile.in: Add py-arch to the list of
EXECUTABLES.
On Wed, Feb 13, 2013 at 12:42 PM, Doug Evans <dje@google.com> wrote:
> Reading beyond high seems counterintuitive, but I can understand why it's easiest if it works that way.
I made it that way because I used the way 'disassemble' command works
as my guideline for this Python disassemble feature. However, I made
Python API to disassemble instructions which have a start address in
[start, end], while the disassemble command does the same in [start,
end).
> Also, I can imagine a user wanting to specify a count instead of high.
> How about supporting both "high" and "count", with the default being high is unspecified and count=1?
I have now added a count argument.
> Including the function name and offset in the output feels wrong.
> It's trying to make the API call do too much.
Removed in the attached patch.
As I have mentioned above, I used the 'functionality' of the
'disassemble' command as a guide. Hence, I had put in whatever it has.
For my needs, all I want are 'addr' and 'asm', and the rest were only
good to have.
> Plus, the documentation needs to specify what "flavor" is used (e.g. intel vs att).
> I'd just add a line of texzt saying the flavor (or choose a better word) is the one specified by the CLI variable disassembly-flavor,
> and leave for another day adding an input parameter to specify the flavor.
Added a note to the docs.
I am not very sure if we should (in future) add the 'flavor' as an
argument to the disassemble method. I prefer if it were a global
function in the 'gdb' module. Again, this is my opinion as I
personally do not see why one would want to have assembly code from
different architectures in different flavors.
> Why call build_address_symbol here?
> To me it's trying to make this API call do too much.
Removed now.
> btw, what about filename,line isn't useful?
It is not that they are not useful, but that build_address_symbol does
not return useful values. I have verified this with experiments but
have not digged into the code to figure out why. There is a similar
not in disasm.c:dump_insns.
> So I can imagine 3 API routines here:
> - disassemble (pc)
> - get_function_and_offset (pc) [or some such]
> - get_file_and_line (pc) [assuming gdbpy_find_pc_line doesn't always do what one wants]
I agree. I will add the last two to my TODO list.
Thanks,
Siva Chandra
[-- Attachment #2: arch_disassemble_patch_v6.txt --]
[-- Type: text/plain, Size: 10637 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e3f336e..cbb64c4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26040,6 +26040,38 @@ A @code{gdb.Architecture} class has the following methods:
Return the name (string value) of the architecture.
@end defun
+@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
+Return a list of utmost @var{count} number of disassembled instructions
+whose start address falls in the closed memory address interval from
+@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
+@var{count} is specified, then @var{count} number of instructions
+starting from the address @var{start_pc} are returned. If @var{count}
+is not specified but @var{end_pc} is specified, then all instructions
+whose start address falls in the closed memory address interval from
+@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
+nor @var{count} are specified, then a single instruction at
+@var{start_pc} is returned. For all of these cases, the elements of the
+returned list are a Python @code{dict} with the following string keys:
+
+@table @code
+
+@item addr
+The value corresponding to this key is a Python long integer capturing
+the memory address of the instruction.
+
+@item asm
+The value corresponding to this key is a string value which represents
+the instruction with assembly language mnemonics. The assembly
+language flavor used is the same as that specified by the current CLI
+variable @code{disassembly-flavor}. @xref{Machine Code}.
+
+@item length
+The value correspoding to this key is the length (integer value) of the
+instruction in bytes.
+
+@end table
+@end defun
+
@node Python Auto-loading
@subsection Python Auto-loading
@cindex Python auto-loading
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..7f3e85c 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,131 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of
+ gdb.Architecture.disassemble (self, start_pc [, end_pc [,count]]) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a Python dict object.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "start_pc", "end_pc", "count", NULL };
+ CORE_ADDR start, end = 0;
+ CORE_ADDR pc;
+ long count = 0, i = 1;
+ PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
+ &start, &end_obj, &count_obj))
+ return NULL;
+
+ if (end_obj)
+ {
+ if (PyObject_TypeCheck (end_obj, &PyInt_Type))
+ /* If the end_pc value is specified without a trailing 'L', end_obj will
+ an integer and not a long integer. */
+ end = PyInt_AsLong (end_obj);
+ else if (PyObject_TypeCheck (end_obj, &PyLong_Type))
+ end = PyLong_AsUnsignedLongLong (end_obj);
+ else
+ {
+ Py_DECREF (end_obj);
+ Py_XDECREF (count_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'end_pc' should be a (long) integer."));
+
+ return NULL;
+ }
+ }
+ if (count_obj)
+ {
+ count = PyInt_AsLong (count_obj);
+ if (PyErr_Occurred ())
+ {
+ Py_DECREF (count_obj);
+ Py_XDECREF (end_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'count' should be an integer."));
+
+ return NULL;
+ }
+ }
+
+ result_list = PyList_New (0);
+ if (result_list == NULL)
+ return NULL;
+
+ for (pc = start;
+ /* All args are specified. */
+ (end_obj && count_obj && pc <= end && i <= count)
+ /* end_pc is specified, but no count. */
+ || (end_obj && count_obj == NULL && pc <= end)
+ /* end_pc is not specified, but a count is. */
+ || (end_obj == NULL && count_obj && i <= count)
+ /* Both end_pc and count are not specified. */
+ || (end_obj == NULL && count_obj == NULL && pc <= start);)
+ {
+ int insn_len = 0;
+ char *as = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (insn_dict == NULL)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+ ui_file_delete (memfile);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ as = ui_file_xstrdup (memfile, NULL);
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (as ? as : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len)))
+ {
+ Py_DECREF (result_list);
+
+ ui_file_delete (memfile);
+ xfree (as);
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ i++;
+ ui_file_delete (memfile);
+ xfree (as);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +231,10 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (low, high) -> List.\n\
+Return the list of disassembled instructions from LOW to HIGH." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
index 4e286b5..0b81507 100644
--- a/gdb/testsuite/gdb.python/Makefile.in
+++ b/gdb/testsuite/gdb.python/Makefile.in
@@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
py-shared python lib-types py-events py-evthreads py-frame \
py-mi py-pp-maint py-progspace py-section-script py-objfile \
py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
- py-explore-cc
+ py-explore-cc py-arch
MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
new file mode 100644
index 0000000..e2fe55c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 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
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
new file mode 100644
index 0000000..4e736b8
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -0,0 +1,54 @@
+# Copyright 2013 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
+gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
+gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
+gdb_py_test_silent_cmd "python insn_list1 = arch.disassemble(pc, pc, 1)" \
+ "disassemble" 0
+gdb_py_test_silent_cmd "python insn_list2 = arch.disassemble(pc, pc)" \
+ "disassemble no count" 0
+gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
+ "disassemble no end" 0
+gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
+ "disassemble no end no count" 0
+
+gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
+gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
+gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
+gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+
+gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
+
+gdb_test "python print \"addr\" in insn" "True" "test key addr"
+gdb_test "python print \"asm\" in insn" "True" "test key asm"
+gdb_test "python print \"length\" in insn" "True" "test key length"
+
+# Negative test
+gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
+ "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-14 22:46 ` Siva Chandra
@ 2013-02-15 6:43 ` Doug Evans
2013-02-15 17:32 ` Doug Evans
2013-02-15 20:36 ` Siva Chandra
0 siblings, 2 replies; 39+ messages in thread
From: Doug Evans @ 2013-02-15 6:43 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
Hi.
Thanks for persevering!
Just nits at this point.
Siva Chandra writes:
> I now have a patch (find it attached) which has been tweaked based on
> comments from Doug.
>
> Thanks Doug, for another round of detailed feedback.
>
> 2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
>
> Add a new method 'disassemble' to gdb.Architecture class.
> * python/py-arch.c (archpy_disassmble): Implementation of the
> new method gdb.Architecture.disassemble.
> (arch_object_methods): Add entry for the new method.
>
> doc/
>
> * gdb.texinfo (Architectures In Python): Add description about
> the new method gdb.Architecture.disassemble.
>
> testsuite/
>
> * gdb.python/py-arch.c: New test case
> * gdb.python/py-arch.exp: New tests to test
> gdb.Architecture.disassemble
> * gdb.python/Makefile.in: Add py-arch to the list of
> EXECUTABLES.
>
> On Wed, Feb 13, 2013 at 12:42 PM, Doug Evans <dje@google.com> wrote:
> > Reading beyond high seems counterintuitive, but I can understand why it's easiest if it works that way.
>
> I made it that way because I used the way 'disassemble' command works
> as my guideline for this Python disassemble feature. However, I made
> Python API to disassemble instructions which have a start address in
> [start, end], while the disassemble command does the same in [start,
> end).
>
> > Also, I can imagine a user wanting to specify a count instead of high.
> > How about supporting both "high" and "count", with the default being high is unspecified and count=1?
>
> I have now added a count argument.
>
> > Including the function name and offset in the output feels wrong.
> > It's trying to make the API call do too much.
>
> Removed in the attached patch.
>
> As I have mentioned above, I used the 'functionality' of the
> 'disassemble' command as a guide. Hence, I had put in whatever it has.
> For my needs, all I want are 'addr' and 'asm', and the rest were only
> good to have.
>
> > Plus, the documentation needs to specify what "flavor" is used (e.g. intel vs att).
> > I'd just add a line of texzt saying the flavor (or choose a better word) is the one specified by the CLI variable disassembly-flavor,
> > and leave for another day adding an input parameter to specify the flavor.
>
> Added a note to the docs.
>
> I am not very sure if we should (in future) add the 'flavor' as an
> argument to the disassemble method. I prefer if it were a global
> function in the 'gdb' module. Again, this is my opinion as I
> personally do not see why one would want to have assembly code from
> different architectures in different flavors.
>
> > Why call build_address_symbol here?
> > To me it's trying to make this API call do too much.
>
> Removed now.
>
> > btw, what about filename,line isn't useful?
>
> It is not that they are not useful, but that build_address_symbol does
> not return useful values. I have verified this with experiments but
> have not digged into the code to figure out why. There is a similar
> not in disasm.c:dump_insns.
>
> > So I can imagine 3 API routines here:
> > - disassemble (pc)
> > - get_function_and_offset (pc) [or some such]
> > - get_file_and_line (pc) [assuming gdbpy_find_pc_line doesn't always do what one wants]
>
> I agree. I will add the last two to my TODO list.
>
> Thanks,
> Siva Chandra
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e3f336e..cbb64c4 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26040,6 +26040,38 @@ A @code{gdb.Architecture} class has the following methods:
> Return the name (string value) of the architecture.
> @end defun
>
> +@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
> +Return a list of utmost @var{count} number of disassembled instructions
s/utmost/at most/ ?
s/utmost/up to/ ?
I'd also be ok with deleting "number of".
> +whose start address falls in the closed memory address interval from
> +@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
> +@var{count} is specified, then @var{count} number of instructions
> +starting from the address @var{start_pc} are returned. If @var{count}
> +is not specified but @var{end_pc} is specified, then all instructions
> +whose start address falls in the closed memory address interval from
> +@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
> +nor @var{count} are specified, then a single instruction at
> +@var{start_pc} is returned. For all of these cases, the elements of the
> +returned list are a Python @code{dict} with the following string keys:
> +
> +@table @code
> +
> +@item addr
> +The value corresponding to this key is a Python long integer capturing
> +the memory address of the instruction.
> +
> +@item asm
> +The value corresponding to this key is a string value which represents
> +the instruction with assembly language mnemonics. The assembly
> +language flavor used is the same as that specified by the current CLI
> +variable @code{disassembly-flavor}. @xref{Machine Code}.
> +
> +@item length
> +The value correspoding to this key is the length (integer value) of the
typo: corresponding
I'd also be ok with "The value of this key ...".
> +instruction in bytes.
> +
> +@end table
> +@end defun
> +
> @node Python Auto-loading
> @subsection Python Auto-loading
> @cindex Python auto-loading
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index edd508f..7f3e85c 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -20,6 +20,7 @@
> #include "defs.h"
> #include "gdbarch.h"
> #include "arch-utils.h"
> +#include "disasm.h"
> #include "python-internal.h"
>
> typedef struct arch_object_type_object {
> @@ -86,6 +87,131 @@ archpy_name (PyObject *self, PyObject *args)
> return py_name;
> }
>
> +/* Implementation of
> + gdb.Architecture.disassemble (self, start_pc [, end_pc [,count]]) -> List.
> + Returns a list of instructions in a memory address range. Each instruction
> + in the list is a Python dict object.
> +*/
> +
> +static PyObject *
> +archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
> +{
> + static char *keywords[] = { "start_pc", "end_pc", "count", NULL };
> + CORE_ADDR start, end = 0;
> + CORE_ADDR pc;
> + long count = 0, i = 1;
> + PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
> + struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
> +
> + if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
> + &start, &end_obj, &count_obj))
> + return NULL;
Hmmm, what if we're on a host where longs are 32 bits but target addresses are 64?
I think start should be gdb_py_longest.
[Or you could keep start as a CORE_ADDR, and pass &tmp_start, or some such.
But the type you pass to match GDB_PY_LLU_ARG needs to be gdb_py_longest.]
> +
> + if (end_obj)
> + {
> + if (PyObject_TypeCheck (end_obj, &PyInt_Type))
> + /* If the end_pc value is specified without a trailing 'L', end_obj will
> + an integer and not a long integer. */
> + end = PyInt_AsLong (end_obj);
> + else if (PyObject_TypeCheck (end_obj, &PyLong_Type))
> + end = PyLong_AsUnsignedLongLong (end_obj);
> + else
> + {
> + Py_DECREF (end_obj);
> + Py_XDECREF (count_obj);
> + PyErr_SetString (PyExc_TypeError,
> + _("Argument 'end_pc' should be a (long) integer."));
> +
> + return NULL;
> + }
Verify end >= start?
> + }
> + if (count_obj)
> + {
> + count = PyInt_AsLong (count_obj);
> + if (PyErr_Occurred ())
> + {
> + Py_DECREF (count_obj);
> + Py_XDECREF (end_obj);
> + PyErr_SetString (PyExc_TypeError,
> + _("Argument 'count' should be an integer."));
> +
> + return NULL;
> + }
Verify count >= 0?
[As a degenerate case, count == 0 => empty result list is ok with me.]
> + }
It's easy enough to support long counts later, so the above is ok with me.
Later, if Python doesn't provide one, we should provide a utility that abstracts
away int vs long (seems like anywhere either an int or long is ok,
we should support both). You don't have to do that with this patch though.
> + result_list = PyList_New (0);
> + if (result_list == NULL)
> + return NULL;
> +
> + for (pc = start;
I'd initialize "i" here.
for (pc = start, i = 1;
[It's more conventional to use i = 0 and compare i < count, but ok.]
> + /* All args are specified. */
> + (end_obj && count_obj && pc <= end && i <= count)
> + /* end_pc is specified, but no count. */
> + || (end_obj && count_obj == NULL && pc <= end)
> + /* end_pc is not specified, but a count is. */
> + || (end_obj == NULL && count_obj && i <= count)
> + /* Both end_pc and count are not specified. */
> + || (end_obj == NULL && count_obj == NULL && pc <= start);)
One *could* :-) nitpick the absence of != NULL in tests of end_obj and count_obj,
but the current code is preferable to me.
The "pc <= start" test is a bit odd. pc == start?
> + {
> + int insn_len = 0;
> + char *as = NULL;
> + struct ui_file *memfile = mem_fileopen ();
> + PyObject *insn_dict = PyDict_New ();
> + volatile struct gdb_exception except;
> +
> + if (insn_dict == NULL)
> + {
> + Py_DECREF (result_list);
> + ui_file_delete (memfile);
> +
> + return NULL;
> + }
> + if (PyList_Append (result_list, insn_dict))
> + {
> + Py_DECREF (result_list);
> + Py_DECREF (insn_dict);
> + ui_file_delete (memfile);
> +
> + return NULL; /* PyList_Append Sets the exception. */
> + }
> +
> + TRY_CATCH (except, RETURN_MASK_ALL)
> + {
> + insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
> + }
> + if (except.reason < 0)
> + {
> + Py_DECREF (result_list);
> + ui_file_delete (memfile);
> +
> + return gdbpy_convert_exception (except);
> + }
> +
> + as = ui_file_xstrdup (memfile, NULL);
> + if (PyDict_SetItemString (insn_dict, "addr",
> + gdb_py_long_from_ulongest (pc))
> + || PyDict_SetItemString (insn_dict, "asm",
> + PyString_FromString (as ? as : "<unknown>"))
"as" can never be NULL.
(*as ? as : "<unknown>") ?
[I don't know if a disassembler could ever return "", but I like handling it.]
> + || PyDict_SetItemString (insn_dict, "length",
> + PyInt_FromLong (insn_len)))
> + {
> + Py_DECREF (result_list);
> +
> + ui_file_delete (memfile);
> + xfree (as);
> +
> + return NULL;
> + }
> +
> + pc += insn_len;
> + i++;
> + ui_file_delete (memfile);
> + xfree (as);
> + }
> +
> + return result_list;
> +}
> +
> /* Initializes the Architecture class in the gdb module. */
>
> void
> @@ -105,6 +231,10 @@ static PyMethodDef arch_object_methods [] = {
> { "name", archpy_name, METH_NOARGS,
> "name () -> String.\n\
> Return the name of the architecture as a string value." },
> + { "disassemble", (PyCFunction) archpy_disassemble,
> + METH_VARARGS | METH_KEYWORDS,
> + "disassemble (low, high) -> List.\n\
> +Return the list of disassembled instructions from LOW to HIGH." },
This needs to be updated to include "count", right?
> {NULL} /* Sentinel */
> };
>
> diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
> index 4e286b5..0b81507 100644
> --- a/gdb/testsuite/gdb.python/Makefile.in
> +++ b/gdb/testsuite/gdb.python/Makefile.in
> @@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
> py-shared python lib-types py-events py-evthreads py-frame \
> py-mi py-pp-maint py-progspace py-section-script py-objfile \
> py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
> - py-explore-cc
> + py-explore-cc py-arch
>
> MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
>
> diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
> new file mode 100644
> index 0000000..e2fe55c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-arch.c
> @@ -0,0 +1,23 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2013 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
> +main (void)
> +{
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
> new file mode 100644
> index 0000000..4e736b8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-arch.exp
> @@ -0,0 +1,54 @@
> +# Copyright 2013 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/>.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
> + return -1
> +}
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
> +gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
> +gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
> +gdb_py_test_silent_cmd "python insn_list1 = arch.disassemble(pc, pc, 1)" \
> + "disassemble" 0
> +gdb_py_test_silent_cmd "python insn_list2 = arch.disassemble(pc, pc)" \
> + "disassemble no count" 0
> +gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
> + "disassemble no end" 0
> +gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
> + "disassemble no end no count" 0
> +
> +gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
> +gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
> +gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
> +gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
> +
> +gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
> +
> +gdb_test "python print \"addr\" in insn" "True" "test key addr"
> +gdb_test "python print \"asm\" in insn" "True" "test key asm"
> +gdb_test "python print \"length\" in insn" "True" "test key length"
> +
> +# Negative test
> +gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
> + "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 6:43 ` Doug Evans
@ 2013-02-15 17:32 ` Doug Evans
2013-02-15 17:40 ` Siva Chandra
2013-02-15 20:36 ` Siva Chandra
1 sibling, 1 reply; 39+ messages in thread
From: Doug Evans @ 2013-02-15 17:32 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
On Thu, Feb 14, 2013 at 10:42 PM, Doug Evans <dje@google.com> wrote:
> Hi.
> Thanks for persevering!
> Just nits at this point.
>
> Siva Chandra writes:
> > I now have a patch (find it attached) which has been tweaked based on
> > comments from Doug.
> >
> > Thanks Doug, for another round of detailed feedback.
> >
> > 2013-02-13 Siva Chandra Reddy <sivachandra@google.com>
> >
> > Add a new method 'disassemble' to gdb.Architecture class.
> > * python/py-arch.c (archpy_disassmble): Implementation of the
> > new method gdb.Architecture.disassemble.
> > (arch_object_methods): Add entry for the new method.
> >
> > doc/
> >
> > * gdb.texinfo (Architectures In Python): Add description about
> > the new method gdb.Architecture.disassemble.
> >
> > testsuite/
> >
> > * gdb.python/py-arch.c: New test case
> > * gdb.python/py-arch.exp: New tests to test
> > gdb.Architecture.disassemble
> > * gdb.python/Makefile.in: Add py-arch to the list of
> > EXECUTABLES.
Oh, duh. Sorry for the followup.
This needs a NEWS entry.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 17:32 ` Doug Evans
@ 2013-02-15 17:40 ` Siva Chandra
2013-02-15 17:41 ` Siva Chandra
2013-02-15 18:57 ` Doug Evans
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-15 17:40 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
On Fri, Feb 15, 2013 at 9:32 AM, Doug Evans <dje@google.com> wrote:
> Oh, duh. Sorry for the followup.
> This needs a NEWS entry.
There is an entry in the NEWS file:
** New method Frame.architecture returns the gdb.Architecture object
corresponding to the frame's architecture.
Adding a new entry for this new method on gdb.Architecture might not
be good idea? I can of course modify to say, "... gdb.Architecture
object corresponding to the frame's architecture with methods 'name'
and 'disassemble'.", but it doesn't sound that very nice.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 17:40 ` Siva Chandra
@ 2013-02-15 17:41 ` Siva Chandra
2013-02-15 18:57 ` Doug Evans
1 sibling, 0 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-15 17:41 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
oops, sorry! I pasted the wrong new entry...
On Fri, Feb 15, 2013 at 9:40 AM, Siva Chandra <sivachandra@google.com> wrote:
> ** New method Frame.architecture returns the gdb.Architecture object
> corresponding to the frame's architecture.
The above should have been:
** New class gdb.Architecture exposes GDB's internal representation
of architecture in the Python API.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 17:40 ` Siva Chandra
2013-02-15 17:41 ` Siva Chandra
@ 2013-02-15 18:57 ` Doug Evans
1 sibling, 0 replies; 39+ messages in thread
From: Doug Evans @ 2013-02-15 18:57 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
On Fri, Feb 15, 2013 at 9:40 AM, Siva Chandra <sivachandra@google.com> wrote:
> On Fri, Feb 15, 2013 at 9:32 AM, Doug Evans <dje@google.com> wrote:
>> Oh, duh. Sorry for the followup.
>> This needs a NEWS entry.
>
> There is an entry in the NEWS file:
>
> ** New method Frame.architecture returns the gdb.Architecture object
> corresponding to the frame's architecture.
>
> Adding a new entry for this new method on gdb.Architecture might not
> be good idea? I can of course modify to say, "... gdb.Architecture
> object corresponding to the frame's architecture with methods 'name'
> and 'disassemble'.", but it doesn't sound that very nice.
Ah.
I guess whether to add another one for disassemble is debatable then.
Ok with me to leave out the NEWS entry then.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 6:43 ` Doug Evans
2013-02-15 17:32 ` Doug Evans
@ 2013-02-15 20:36 ` Siva Chandra
2013-02-15 21:01 ` Siva Chandra
1 sibling, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-15 20:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Doug Evans, Tom Tromey, Eli Zaretskii
Fixed all of Doug's nits. Patch attached.
2013-02-15 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
doc/
* gdb.texinfo (Architectures In Python): Add description about
the new method gdb.Architecture.disassemble.
testsuite/
* gdb.python/py-arch.c: New test case
* gdb.python/py-arch.exp: New tests to test
gdb.Architecture.disassemble
* gdb.python/Makefile.in: Add py-arch to the list of
EXECUTABLES.
Thanks,
Siva Chandra
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 20:36 ` Siva Chandra
@ 2013-02-15 21:01 ` Siva Chandra
2013-02-16 5:30 ` Doug Evans
2013-02-16 8:47 ` Eli Zaretskii
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-15 21:01 UTC (permalink / raw)
To: gdb-patches; +Cc: Doug Evans, Tom Tromey, Eli Zaretskii
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
sorry again! twice today...
patch attached this time.
On Fri, Feb 15, 2013 at 12:35 PM, Siva Chandra <sivachandra@google.com> wrote:
> Fixed all of Doug's nits. Patch attached.
>
> 2013-02-15 Siva Chandra Reddy <sivachandra@google.com>
>
> Add a new method 'disassemble' to gdb.Architecture class.
> * python/py-arch.c (archpy_disassmble): Implementation of the
> new method gdb.Architecture.disassemble.
> (arch_object_methods): Add entry for the new method.
>
> doc/
>
> * gdb.texinfo (Architectures In Python): Add description about
> the new method gdb.Architecture.disassemble.
>
> testsuite/
>
> * gdb.python/py-arch.c: New test case
> * gdb.python/py-arch.exp: New tests to test
> gdb.Architecture.disassemble
> * gdb.python/Makefile.in: Add py-arch to the list of
> EXECUTABLES.
>
> Thanks,
> Siva Chandra
[-- Attachment #2: arch_disassemble_patch_v7.txt --]
[-- Type: text/plain, Size: 11128 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e3f336e..ff0fc02 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26040,6 +26040,38 @@ A @code{gdb.Architecture} class has the following methods:
Return the name (string value) of the architecture.
@end defun
+@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
+Return a list of at most @var{count} disassembled instructions
+whose start address falls in the closed memory address interval from
+@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
+@var{count} is specified, then @var{count} number of instructions
+starting from the address @var{start_pc} are returned. If @var{count}
+is not specified but @var{end_pc} is specified, then all instructions
+whose start address falls in the closed memory address interval from
+@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
+nor @var{count} are specified, then a single instruction at
+@var{start_pc} is returned. For all of these cases, the elements of the
+returned list are a Python @code{dict} with the following string keys:
+
+@table @code
+
+@item addr
+The value corresponding to this key is a Python long integer capturing
+the memory address of the instruction.
+
+@item asm
+The value corresponding to this key is a string value which represents
+the instruction with assembly language mnemonics. The assembly
+language flavor used is the same as that specified by the current CLI
+variable @code{disassembly-flavor}. @xref{Machine Code}.
+
+@item length
+The value corresponding to this key is the length (integer value) of the
+instruction in bytes.
+
+@end table
+@end defun
+
@node Python Auto-loading
@subsection Python Auto-loading
@cindex Python auto-loading
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..b41de06 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,145 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of
+ gdb.Architecture.disassemble (self, start_pc [, end_pc [,count]]) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a Python dict object.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "start_pc", "end_pc", "count", NULL };
+ CORE_ADDR start, end = 0;
+ CORE_ADDR pc;
+ gdb_py_ulongest start_temp;
+ long count = 0, i;
+ PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
+ &start_temp, &end_obj, &count_obj))
+ return NULL;
+
+ start = start_temp;
+ if (end_obj)
+ {
+ if (PyObject_TypeCheck (end_obj, &PyInt_Type))
+ /* If the end_pc value is specified without a trailing 'L', end_obj will
+ be an integer and not a long integer. */
+ end = PyInt_AsLong (end_obj);
+ else if (PyObject_TypeCheck (end_obj, &PyLong_Type))
+ end = PyLong_AsUnsignedLongLong (end_obj);
+ else
+ {
+ Py_DECREF (end_obj);
+ Py_XDECREF (count_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'end_pc' should be a (long) integer."));
+
+ return NULL;
+ }
+
+ if (end < start)
+ {
+ Py_DECREF (end_obj);
+ Py_XDECREF (count_obj);
+ PyErr_SetString (PyExc_ValueError,
+ _("Argument 'end_pc' should be greater than or "
+ "equal to the argument 'start_pc'."));
+
+ return NULL;
+ }
+ }
+ if (count_obj)
+ {
+ count = PyInt_AsLong (count_obj);
+ if (PyErr_Occurred () || count < 0)
+ {
+ Py_DECREF (count_obj);
+ Py_XDECREF (end_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'count' should be an non-negative "
+ "integer."));
+
+ return NULL;
+ }
+ }
+
+ result_list = PyList_New (0);
+ if (result_list == NULL)
+ return NULL;
+
+ for (pc = start, i = 0;
+ /* All args are specified. */
+ (end_obj && count_obj && pc <= end && i < count)
+ /* end_pc is specified, but no count. */
+ || (end_obj && count_obj == NULL && pc <= end)
+ /* end_pc is not specified, but a count is. */
+ || (end_obj == NULL && count_obj && i < count)
+ /* Both end_pc and count are not specified. */
+ || (end_obj == NULL && count_obj == NULL && pc == start);)
+ {
+ int insn_len = 0;
+ char *as = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (insn_dict == NULL)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+ ui_file_delete (memfile);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ as = ui_file_xstrdup (memfile, NULL);
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (*as ? as : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len)))
+ {
+ Py_DECREF (result_list);
+
+ ui_file_delete (memfile);
+ xfree (as);
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ i++;
+ ui_file_delete (memfile);
+ xfree (as);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +245,11 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (start_pc [, end_pc [, count]]) -> List.\n\
+Return a list of at most COUNT disassembled instructions from START_PC to\n\
+END_PC." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
index 4e286b5..0b81507 100644
--- a/gdb/testsuite/gdb.python/Makefile.in
+++ b/gdb/testsuite/gdb.python/Makefile.in
@@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
py-shared python lib-types py-events py-evthreads py-frame \
py-mi py-pp-maint py-progspace py-section-script py-objfile \
py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
- py-explore-cc
+ py-explore-cc py-arch
MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
new file mode 100644
index 0000000..e2fe55c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 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
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
new file mode 100644
index 0000000..4e736b8
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -0,0 +1,54 @@
+# Copyright 2013 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
+gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
+gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
+gdb_py_test_silent_cmd "python insn_list1 = arch.disassemble(pc, pc, 1)" \
+ "disassemble" 0
+gdb_py_test_silent_cmd "python insn_list2 = arch.disassemble(pc, pc)" \
+ "disassemble no count" 0
+gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
+ "disassemble no end" 0
+gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
+ "disassemble no end no count" 0
+
+gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
+gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
+gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
+gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+
+gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
+
+gdb_test "python print \"addr\" in insn" "True" "test key addr"
+gdb_test "python print \"asm\" in insn" "True" "test key asm"
+gdb_test "python print \"length\" in insn" "True" "test key length"
+
+# Negative test
+gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
+ "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 21:01 ` Siva Chandra
@ 2013-02-16 5:30 ` Doug Evans
2013-02-16 8:47 ` Eli Zaretskii
1 sibling, 0 replies; 39+ messages in thread
From: Doug Evans @ 2013-02-16 5:30 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, Tom Tromey, Eli Zaretskii
On Fri, Feb 15, 2013 at 1:00 PM, Siva Chandra <sivachandra@google.com> wrote:
> sorry again! twice today...
> patch attached this time.
>
> On Fri, Feb 15, 2013 at 12:35 PM, Siva Chandra <sivachandra@google.com> wrote:
>> Fixed all of Doug's nits. Patch attached.
>>
>> 2013-02-15 Siva Chandra Reddy <sivachandra@google.com>
>>
>> Add a new method 'disassemble' to gdb.Architecture class.
>> * python/py-arch.c (archpy_disassmble): Implementation of the
>> new method gdb.Architecture.disassemble.
>> (arch_object_methods): Add entry for the new method.
>>
>> doc/
>>
>> * gdb.texinfo (Architectures In Python): Add description about
>> the new method gdb.Architecture.disassemble.
>>
>> testsuite/
>>
>> * gdb.python/py-arch.c: New test case
>> * gdb.python/py-arch.exp: New tests to test
>> gdb.Architecture.disassemble
>> * gdb.python/Makefile.in: Add py-arch to the list of
>> EXECUTABLES.
>>
>> Thanks,
>> Siva Chandra
Looks ok to me.
Thanks!
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-15 21:01 ` Siva Chandra
2013-02-16 5:30 ` Doug Evans
@ 2013-02-16 8:47 ` Eli Zaretskii
2013-02-19 5:36 ` Siva Chandra
1 sibling, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2013-02-16 8:47 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, dje, tromey
> Date: Fri, 15 Feb 2013 13:00:19 -0800
> From: Siva Chandra <sivachandra@google.com>
> Cc: Doug Evans <dje@google.com>, Tom Tromey <tromey@redhat.com>, Eli Zaretskii <eliz@gnu.org>
>
> +@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
> +Return a list of at most @var{count} disassembled instructions
> +whose start address falls in the closed memory address interval from
> +@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
> +@var{count} is specified, then @var{count} number of instructions
> +starting from the address @var{start_pc} are returned. If @var{count}
> +is not specified but @var{end_pc} is specified, then all instructions
> +whose start address falls in the closed memory address interval from
> +@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
> +nor @var{count} are specified, then a single instruction at
> +@var{start_pc} is returned.
The description of the optional arguments makes sense, but the @defun
line is in contradiction with the description, because it says that
one can specify all 3 arguments. IOW, there should be a '|' somewhere
to signal that either end_pc or count, but not both, could be used.
> For all of these cases, the elements of the
> +returned list are a Python @code{dict}
Does it make sense in Python to talk about a list that is a 'dict'?
IOW, is a 'dict' a special case of a list in Python? My reading of
http://docs.python.org/2/library/stdtypes.html is that it is not.
Otherwise OK. Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-16 8:47 ` Eli Zaretskii
@ 2013-02-19 5:36 ` Siva Chandra
2013-02-19 15:51 ` Paul_Koning
2013-02-19 16:38 ` Eli Zaretskii
0 siblings, 2 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-19 5:36 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches, dje, tromey
On Sat, Feb 16, 2013 at 12:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> +@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
>> +Return a list of at most @var{count} disassembled instructions
>> +whose start address falls in the closed memory address interval from
>> +@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
>> +@var{count} is specified, then @var{count} number of instructions
>> +starting from the address @var{start_pc} are returned. If @var{count}
>> +is not specified but @var{end_pc} is specified, then all instructions
>> +whose start address falls in the closed memory address interval from
>> +@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
>> +nor @var{count} are specified, then a single instruction at
>> +@var{start_pc} is returned.
>
> The description of the optional arguments makes sense, but the @defun
> line is in contradiction with the description, because it says that
> one can specify all 3 arguments. IOW, there should be a '|' somewhere
> to signal that either end_pc or count, but not both, could be used.
>
One can specify all three arguments. Does the description anywhere
indicate otherwise?
>> For all of these cases, the elements of the
>> +returned list are a Python @code{dict}
>
> Does it make sense in Python to talk about a list that is a 'dict'?
> IOW, is a 'dict' a special case of a list in Python? My reading of
> http://docs.python.org/2/library/stdtypes.html is that it is not.
I am not saying that the list is a dict, but that the elements of the
list are dicts. Should it be worded in another fashion?
Thanks,
Siva Chandra
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-19 5:36 ` Siva Chandra
@ 2013-02-19 15:51 ` Paul_Koning
2013-02-19 16:35 ` Eli Zaretskii
2013-02-19 16:38 ` Eli Zaretskii
1 sibling, 1 reply; 39+ messages in thread
From: Paul_Koning @ 2013-02-19 15:51 UTC (permalink / raw)
To: sivachandra; +Cc: eliz, gdb-patches, dje, tromey
On Feb 19, 2013, at 12:36 AM, Siva Chandra wrote:
> On Sat, Feb 16, 2013 at 12:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> ...
>
>>> For all of these cases, the elements of the
>>> +returned list are a Python @code{dict}
>>
>> Does it make sense in Python to talk about a list that is a 'dict'?
>> IOW, is a 'dict' a special case of a list in Python? My reading of
>> http://docs.python.org/2/library/stdtypes.html is that it is not.
>
> I am not saying that the list is a dict, but that the elements of the
> list are dicts. Should it be worded in another fashion?
How about "each element of the returned list is a Python dict" ?
paul
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-19 15:51 ` Paul_Koning
@ 2013-02-19 16:35 ` Eli Zaretskii
0 siblings, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2013-02-19 16:35 UTC (permalink / raw)
To: Paul_Koning; +Cc: sivachandra, gdb-patches, dje, tromey
> From: <Paul_Koning@Dell.com>
> CC: <eliz@gnu.org>, <gdb-patches@sourceware.org>, <dje@google.com>, <tromey@redhat.com>
> Date: Tue, 19 Feb 2013 15:51:24 +0000
>
>
> On Feb 19, 2013, at 12:36 AM, Siva Chandra wrote:
>
> > On Sat, Feb 16, 2013 at 12:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >>> ...
> >
> >>> For all of these cases, the elements of the
> >>> +returned list are a Python @code{dict}
> >>
> >> Does it make sense in Python to talk about a list that is a 'dict'?
> >> IOW, is a 'dict' a special case of a list in Python? My reading of
> >> http://docs.python.org/2/library/stdtypes.html is that it is not.
> >
> > I am not saying that the list is a dict, but that the elements of the
> > list are dicts. Should it be worded in another fashion?
>
> How about "each element of the returned list is a Python dict" ?
Fine with me, thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-19 5:36 ` Siva Chandra
2013-02-19 15:51 ` Paul_Koning
@ 2013-02-19 16:38 ` Eli Zaretskii
2013-02-20 12:34 ` Siva Chandra
1 sibling, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2013-02-19 16:38 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches, dje, tromey
> Date: Mon, 18 Feb 2013 21:36:00 -0800
> From: Siva Chandra <sivachandra@google.com>
> Cc: gdb-patches@sourceware.org, dje@google.com, tromey@redhat.com
>
> On Sat, Feb 16, 2013 at 12:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> +@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
> >> +Return a list of at most @var{count} disassembled instructions
> >> +whose start address falls in the closed memory address interval from
> >> +@var{start_pc} to @var{end_pc}. If @var{end_pc} is not specified, but
> >> +@var{count} is specified, then @var{count} number of instructions
> >> +starting from the address @var{start_pc} are returned. If @var{count}
> >> +is not specified but @var{end_pc} is specified, then all instructions
> >> +whose start address falls in the closed memory address interval from
> >> +@var{start_pc} to @var{end_pc} are returned. If neither @var{end_pc}
> >> +nor @var{count} are specified, then a single instruction at
> >> +@var{start_pc} is returned.
> >
> > The description of the optional arguments makes sense, but the @defun
> > line is in contradiction with the description, because it says that
> > one can specify all 3 arguments. IOW, there should be a '|' somewhere
> > to signal that either end_pc or count, but not both, could be used.
> >
>
> One can specify all three arguments. Does the description anywhere
> indicate otherwise?
The text does not describe that possibility at all, so I assumed that
it cannot happen.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-19 16:38 ` Eli Zaretskii
@ 2013-02-20 12:34 ` Siva Chandra
2013-02-20 18:44 ` Eli Zaretskii
0 siblings, 1 reply; 39+ messages in thread
From: Siva Chandra @ 2013-02-20 12:34 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 958 bytes --]
On Tue, Feb 19, 2013 at 8:38 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> One can specify all three arguments. Does the description anywhere
>> indicate otherwise?
>
> The text does not describe that possibility at all, so I assumed that
> it cannot happen.
How does the attached patch look?
2013-02-20 Siva Chandra Reddy <sivachandra@google.com>
Add a new method 'disassemble' to gdb.Architecture class.
* python/py-arch.c (archpy_disassmble): Implementation of the
new method gdb.Architecture.disassemble.
(arch_object_methods): Add entry for the new method.
doc/
* gdb.texinfo (Architectures In Python): Add description about
the new method gdb.Architecture.disassemble.
testsuite/
* gdb.python/py-arch.c: New test case
* gdb.python/py-arch.exp: New tests to test
gdb.Architecture.disassemble
* gdb.python/Makefile.in: Add py-arch to the list of
EXECUTABLES.
[-- Attachment #2: arch_disassemble_patch_v8.txt --]
[-- Type: text/plain, Size: 11421 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e3f336e..417ea66 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26040,6 +26040,42 @@ A @code{gdb.Architecture} class has the following methods:
Return the name (string value) of the architecture.
@end defun
+@defun Architecture.disassemble (@var{start_pc} @r{[}, @var{end_pc} @r{[}, @var{count}@r{]]})
+Return a list of disassembled instructions starting from the memory
+address @var{start_pc}. The optional arguments @var{end_pc} and
+@var{count} determine the number of instructions in the returned list.
+If both the optional arguments @var{end_pc} and @var{count} are
+specified, then a list of at most @var{count} disassembled instructions
+whose start address falls in the closed memory address interval from
+@var{start_pc} to @var{end_pc} are returned. If @var{end_pc} is not
+specified, but @var{count} is specified, then @var{count} number of
+instructions starting from the address @var{start_pc} are returned. If
+@var{count} is not specified but @var{end_pc} is specified, then all
+instructions whose start address falls in the closed memory address
+interval from @var{start_pc} to @var{end_pc} are returned. If neither
+@var{end_pc} nor @var{count} are specified, then a single instruction at
+@var{start_pc} is returned. For all of these cases, each element of the
+returned list is a Python @code{dict} with the following string keys:
+
+@table @code
+
+@item addr
+The value corresponding to this key is a Python long integer capturing
+the memory address of the instruction.
+
+@item asm
+The value corresponding to this key is a string value which represents
+the instruction with assembly language mnemonics. The assembly
+language flavor used is the same as that specified by the current CLI
+variable @code{disassembly-flavor}. @xref{Machine Code}.
+
+@item length
+The value corresponding to this key is the length (integer value) of the
+instruction in bytes.
+
+@end table
+@end defun
+
@node Python Auto-loading
@subsection Python Auto-loading
@cindex Python auto-loading
diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index edd508f..b41de06 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "gdbarch.h"
#include "arch-utils.h"
+#include "disasm.h"
#include "python-internal.h"
typedef struct arch_object_type_object {
@@ -86,6 +87,145 @@ archpy_name (PyObject *self, PyObject *args)
return py_name;
}
+/* Implementation of
+ gdb.Architecture.disassemble (self, start_pc [, end_pc [,count]]) -> List.
+ Returns a list of instructions in a memory address range. Each instruction
+ in the list is a Python dict object.
+*/
+
+static PyObject *
+archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
+{
+ static char *keywords[] = { "start_pc", "end_pc", "count", NULL };
+ CORE_ADDR start, end = 0;
+ CORE_ADDR pc;
+ gdb_py_ulongest start_temp;
+ long count = 0, i;
+ PyObject *result_list, *end_obj = NULL, *count_obj = NULL;
+ struct gdbarch *gdbarch = arch_object_to_gdbarch (self);
+
+ if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
+ &start_temp, &end_obj, &count_obj))
+ return NULL;
+
+ start = start_temp;
+ if (end_obj)
+ {
+ if (PyObject_TypeCheck (end_obj, &PyInt_Type))
+ /* If the end_pc value is specified without a trailing 'L', end_obj will
+ be an integer and not a long integer. */
+ end = PyInt_AsLong (end_obj);
+ else if (PyObject_TypeCheck (end_obj, &PyLong_Type))
+ end = PyLong_AsUnsignedLongLong (end_obj);
+ else
+ {
+ Py_DECREF (end_obj);
+ Py_XDECREF (count_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'end_pc' should be a (long) integer."));
+
+ return NULL;
+ }
+
+ if (end < start)
+ {
+ Py_DECREF (end_obj);
+ Py_XDECREF (count_obj);
+ PyErr_SetString (PyExc_ValueError,
+ _("Argument 'end_pc' should be greater than or "
+ "equal to the argument 'start_pc'."));
+
+ return NULL;
+ }
+ }
+ if (count_obj)
+ {
+ count = PyInt_AsLong (count_obj);
+ if (PyErr_Occurred () || count < 0)
+ {
+ Py_DECREF (count_obj);
+ Py_XDECREF (end_obj);
+ PyErr_SetString (PyExc_TypeError,
+ _("Argument 'count' should be an non-negative "
+ "integer."));
+
+ return NULL;
+ }
+ }
+
+ result_list = PyList_New (0);
+ if (result_list == NULL)
+ return NULL;
+
+ for (pc = start, i = 0;
+ /* All args are specified. */
+ (end_obj && count_obj && pc <= end && i < count)
+ /* end_pc is specified, but no count. */
+ || (end_obj && count_obj == NULL && pc <= end)
+ /* end_pc is not specified, but a count is. */
+ || (end_obj == NULL && count_obj && i < count)
+ /* Both end_pc and count are not specified. */
+ || (end_obj == NULL && count_obj == NULL && pc == start);)
+ {
+ int insn_len = 0;
+ char *as = NULL;
+ struct ui_file *memfile = mem_fileopen ();
+ PyObject *insn_dict = PyDict_New ();
+ volatile struct gdb_exception except;
+
+ if (insn_dict == NULL)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return NULL;
+ }
+ if (PyList_Append (result_list, insn_dict))
+ {
+ Py_DECREF (result_list);
+ Py_DECREF (insn_dict);
+ ui_file_delete (memfile);
+
+ return NULL; /* PyList_Append Sets the exception. */
+ }
+
+ TRY_CATCH (except, RETURN_MASK_ALL)
+ {
+ insn_len = gdb_print_insn (gdbarch, pc, memfile, NULL);
+ }
+ if (except.reason < 0)
+ {
+ Py_DECREF (result_list);
+ ui_file_delete (memfile);
+
+ return gdbpy_convert_exception (except);
+ }
+
+ as = ui_file_xstrdup (memfile, NULL);
+ if (PyDict_SetItemString (insn_dict, "addr",
+ gdb_py_long_from_ulongest (pc))
+ || PyDict_SetItemString (insn_dict, "asm",
+ PyString_FromString (*as ? as : "<unknown>"))
+ || PyDict_SetItemString (insn_dict, "length",
+ PyInt_FromLong (insn_len)))
+ {
+ Py_DECREF (result_list);
+
+ ui_file_delete (memfile);
+ xfree (as);
+
+ return NULL;
+ }
+
+ pc += insn_len;
+ i++;
+ ui_file_delete (memfile);
+ xfree (as);
+ }
+
+ return result_list;
+}
+
/* Initializes the Architecture class in the gdb module. */
void
@@ -105,6 +245,11 @@ static PyMethodDef arch_object_methods [] = {
{ "name", archpy_name, METH_NOARGS,
"name () -> String.\n\
Return the name of the architecture as a string value." },
+ { "disassemble", (PyCFunction) archpy_disassemble,
+ METH_VARARGS | METH_KEYWORDS,
+ "disassemble (start_pc [, end_pc [, count]]) -> List.\n\
+Return a list of at most COUNT disassembled instructions from START_PC to\n\
+END_PC." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/Makefile.in b/gdb/testsuite/gdb.python/Makefile.in
index 4e286b5..0b81507 100644
--- a/gdb/testsuite/gdb.python/Makefile.in
+++ b/gdb/testsuite/gdb.python/Makefile.in
@@ -6,7 +6,7 @@ EXECUTABLES = py-type py-value py-prettyprint py-template py-block \
py-shared python lib-types py-events py-evthreads py-frame \
py-mi py-pp-maint py-progspace py-section-script py-objfile \
py-finish-breakpoint py-finish-breakpoint2 py-value-cc py-explore \
- py-explore-cc
+ py-explore-cc py-arch
MISCELLANEOUS = py-shared-sl.sl py-events-shlib.so py-events-shlib-nodebug.so
diff --git a/gdb/testsuite/gdb.python/py-arch.c b/gdb/testsuite/gdb.python/py-arch.c
new file mode 100644
index 0000000..e2fe55c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 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
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
new file mode 100644
index 0000000..4e736b8
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -0,0 +1,54 @@
+# Copyright 2013 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_py_test_silent_cmd "python frame = gdb.selected_frame()" "get frame" 0
+gdb_py_test_silent_cmd "python arch = frame.architecture()" "get arch" 0
+gdb_py_test_silent_cmd "python pc = frame.pc()" "get pc" 0
+gdb_py_test_silent_cmd "python insn_list1 = arch.disassemble(pc, pc, 1)" \
+ "disassemble" 0
+gdb_py_test_silent_cmd "python insn_list2 = arch.disassemble(pc, pc)" \
+ "disassemble no count" 0
+gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
+ "disassemble no end" 0
+gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
+ "disassemble no end no count" 0
+
+gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
+gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
+gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
+gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+
+gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
+
+gdb_test "python print \"addr\" in insn" "True" "test key addr"
+gdb_test "python print \"asm\" in insn" "True" "test key asm"
+gdb_test "python print \"length\" in insn" "True" "test key length"
+
+# Negative test
+gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
+ "test exception"
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-20 12:34 ` Siva Chandra
@ 2013-02-20 18:44 ` Eli Zaretskii
2013-02-21 1:49 ` Siva Chandra
0 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2013-02-20 18:44 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
> Date: Wed, 20 Feb 2013 04:34:23 -0800
> From: Siva Chandra <sivachandra@google.com>
> Cc: gdb-patches@sourceware.org
>
> On Tue, Feb 19, 2013 at 8:38 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> One can specify all three arguments. Does the description anywhere
> >> indicate otherwise?
> >
> > The text does not describe that possibility at all, so I assumed that
> > it cannot happen.
>
> How does the attached patch look?
Looks good, thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
2013-02-20 18:44 ` Eli Zaretskii
@ 2013-02-21 1:49 ` Siva Chandra
0 siblings, 0 replies; 39+ messages in thread
From: Siva Chandra @ 2013-02-21 1:49 UTC (permalink / raw)
To: gdb-patches
On Wed, Feb 20, 2013 at 10:43 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> How does the attached patch look?
>
> Looks good, thanks.
Committed.
Thanks for the review to everyone.
Siva Chandra
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2013-02-21 1:49 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 14:09 [RFC - Python Scripting] New method gdb.Architecture.disassemble Siva Chandra
2013-02-05 23:28 ` Doug Evans
2013-02-06 1:53 ` Siva Chandra
2013-02-06 20:00 ` Tom Tromey
2013-02-08 18:05 ` Doug Evans
2013-02-09 17:55 ` Matt Rice
2013-02-12 14:56 ` Siva Chandra
2013-02-12 21:18 ` Tom Tromey
2013-02-13 14:37 ` Siva Chandra
2013-02-13 17:52 ` Eli Zaretskii
2013-02-13 18:03 ` Tom Tromey
2013-02-13 19:50 ` Siva Chandra
2013-02-13 20:42 ` Doug Evans
2013-02-14 22:46 ` Siva Chandra
2013-02-15 6:43 ` Doug Evans
2013-02-15 17:32 ` Doug Evans
2013-02-15 17:40 ` Siva Chandra
2013-02-15 17:41 ` Siva Chandra
2013-02-15 18:57 ` Doug Evans
2013-02-15 20:36 ` Siva Chandra
2013-02-15 21:01 ` Siva Chandra
2013-02-16 5:30 ` Doug Evans
2013-02-16 8:47 ` Eli Zaretskii
2013-02-19 5:36 ` Siva Chandra
2013-02-19 15:51 ` Paul_Koning
2013-02-19 16:35 ` Eli Zaretskii
2013-02-19 16:38 ` Eli Zaretskii
2013-02-20 12:34 ` Siva Chandra
2013-02-20 18:44 ` Eli Zaretskii
2013-02-21 1:49 ` Siva Chandra
2013-02-06 19:58 ` Tom Tromey
2013-02-06 20:31 ` Phil Muldoon
2013-02-06 22:31 ` Matt Rice
2013-02-06 23:19 ` Siva Chandra
2013-02-07 1:11 ` Siva Chandra
2013-02-07 23:03 ` Matt Rice
[not found] ` <20130206235707.GA2353@klara.mpi.htwm.de>
2013-02-07 1:18 ` Siva Chandra
2013-02-07 14:14 ` Siva Chandra
2013-02-07 16:42 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox