[PATCH] python: accept address and explicit locations in gdb.decode_line

[PATCH] python: accept address and explicit locations in gdb.decode_line

[Markus Metzger]

The gdb.decode_line python function is documented to support the same location
expressions as the "break" command.  It currently expects a linespec location.

Instead of creating a linespec location directly, create the location via
string_to_event_location_basic.

It's not clear to me whether I should use python_language or current_language,
though.  Is there some comment that explains it?

2016-06-20  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* python/python.c (gdbpy_decode_line): Call
	string_to_event_location_basic.

testsuite/
	* gdb.python/python.exp: Test decode_line("*0").
---
 gdb/python/python.c                 | 2 +-
 gdb/testsuite/gdb.python/python.exp | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1f1fece..7bee890 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -738,7 +738,7 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
 
   if (arg != NULL)
     {
-      location = new_linespec_location (&arg);
+      location = string_to_event_location_basic (&arg, python_language);
       make_cleanup_delete_event_location (location);
     }
 
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 3f8c46f..4072fa3 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -251,6 +251,13 @@ gdb_py_test_silent_cmd {python symtab = gdb.decode_line ("func1,func2")} \
     "test decode_line func1,func2" 1
 gdb_test {python print (symtab[0])} ",func2" "stop at comma in linespec"
 
+gdb_py_test_silent_cmd "python symtab = gdb.decode_line(\"*0\")" "Test decode_line *0" 1
+gdb_test "python print (len(symtab))" "2" "Test decode_line *0 result length"
+gdb_test "python print (symtab\[0\])" "None" "Test decode_line *0 unparsed"
+gdb_test "python print (len(symtab\[1\]))" "1" "Test decode_line *0 locations length"
+gdb_test "python print (symtab\[1\]\[0\].symtab)" "None" "Test decode_line *0 filename"
+gdb_test "python print (symtab\[1\]\[0\].pc)" "0" "Test decode_line *0 pc"
+
 # gdb.write
 gdb_test "python print (sys.stderr)" ".*gdb.GdbOutputErrorFile (instance|object) at.*" "Test stderr location"
 gdb_test "python print (sys.stdout)" ".*gdb.GdbOutputFile (instance|object) at.*" "Test stdout location"
-- 
1.8.3.1

Re: [PATCH] python: accept address and explicit locations in gdb.decode_line

[Keith Seitz]

On 06/20/2016 07:33 AM, Markus Metzger wrote:
> The gdb.decode_line python function is documented to support the same location
> expressions as the "break" command.  It currently expects a linespec location.
> 
> Instead of creating a linespec location directly, create the location via
> string_to_event_location_basic.
> 

I must have missed python when Joel asked me to take a look at this.
Your patch is correct (with the language correction below).

> It's not clear to me whether I should use python_language or current_language,
> though.  Is there some comment that explains it?

Although string_to_event_location_basic does not use the language
parameter, I kept it for parallelism with string_to_event_location. What
can I say? I really dislike using globals!

The correct language to use is the language in which the linespec is to
be evaluated. Most typically, that is current_language.

Keith

RE: [PATCH] python: accept address and explicit locations in gdb.decode_line

[Metzger, Markus T]

Hi Keith,

Thanks for your quick review.


> > It's not clear to me whether I should use python_language or
> current_language,
> > though.  Is there some comment that explains it?
> 
> Although string_to_event_location_basic does not use the language
> parameter, I kept it for parallelism with string_to_event_location. What
> can I say? I really dislike using globals!
> 
> The correct language to use is the language in which the linespec is to
> be evaluated. Most typically, that is current_language.

I found this comment in gdb/python/python.c:

/* Architecture and language to be used in callbacks from
   the Python interpreter.  */
struct gdbarch *python_gdbarch;
const struct language_defn *python_language;


The function this patch is modifying is:

/* A Python function which is a wrapper for decode_line_1.  */

static PyObject *
gdbpy_decode_line (PyObject *self, PyObject *args)


I'd say this qualifies as a callback from the Python interpreter.


I don't see how this is intended to work when those callbacks call
GDB functions.  As long as language and gdbarch are passed as
arguments, we're fine.  But making sure to not add an implicit use
of current_language or target_gdbarch () that ends up being used
by the Python layer sounds a bit tricky to me.

Maybe I got the whole idea behind it wrong.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] python: accept address and explicit locations in gdb.decode_line

[Keith Seitz]

On 06/23/2016 02:15 AM, Metzger, Markus T wrote:

> /* Architecture and language to be used in callbacks from
>    the Python interpreter.  */
> struct gdbarch *python_gdbarch;
> const struct language_defn *python_language;
> 
> 
> The function this patch is modifying is:
> 
> /* A Python function which is a wrapper for decode_line_1.  */
> 
> static PyObject *
> gdbpy_decode_line (PyObject *self, PyObject *args)
> 
> 
> I'd say this qualifies as a callback from the Python interpreter.

I see what's happening. The code is littered with calls to
ensure_python_env (..., current_language), which saves current_language
into a structure. This gets saved and restored every time the
interpreter is entered/left. "python_language" is really the
"current_language_used_by_python." :-)

So I'd say I was incorrect and your original patch is correct. Good job
keeping me "honest!"

Keith

RE: [PATCH] python: accept address and explicit locations in gdb.decode_line

[Metzger, Markus T]

Hi Keith,

Do you know who would approve patches in this area?

https://sourceware.org/ml/gdb-patches/2016-06/msg00328.html.

Thanks,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH] python: accept address and explicit locations in gdb.decode_line

[Pedro Alves]

On 06/29/2016 03:13 PM, Metzger, Markus T wrote:
> Hi Keith,
> 
> Do you know who would approve patches in this area?
> 
> https://sourceware.org/ml/gdb-patches/2016-06/msg00328.html.

This is OK.  Sorry for the long delay.

Thanks,
Pedro Alves