* [patch] [python] find_line_pc_range
@ 2011-07-02 4:03 Matt Rice
2011-07-03 16:35 ` Phil Muldoon
0 siblings, 1 reply; 10+ messages in thread
From: Matt Rice @ 2011-07-02 4:03 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
don't know a whole much about python but,
would it be better to return None on error, instead of a tuple containing Nones?
using it to have a linespec enabled disas command, similar to:
python
x = gdb.decode_line("test.c:14")[1][0].find_line_pc_range();
gdb.execute("disassemble " + str(x[0]) + "," + str(x[1]))
end
2011-07-01 Matt Rice <ratmice@gmail.com>
* python/py-symtab.c: Populate sal_object_methods.
(salpy_find_line_pc_range): New function.
2011-07-01 Matt Rice <ratmice@gmail.com>
* gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range method.
[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 2110 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dbaf30e..c0fd555 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23293,6 +23293,13 @@ exist in @value{GDBN} any longer. All other
@code{gdb.Symtab_and_line} methods will throw an exception if it is
invalid at the time the method is called.
@end defmethod
+
+@defmethod Symtab_and_line find_line_pc_range
+Returns a @code{Tuple} with the start and end program counter addresses
+for the line attribute of the @code{gdb.Symtab_and_line} object.
+The start and end addresses will be @code{None} if it could not find the
+specified line.
+@end defmethod
@end table
A @code{gdb.Symtab} object has the following attributes:
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 107cdec..678dea3 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -215,6 +215,36 @@ salpy_get_line (PyObject *self, void *closure)
}
static PyObject *
+salpy_find_line_pc_range (PyObject *self, PyObject *args)
+{
+ struct symtab_and_line *sal = NULL;
+ CORE_ADDR start_pc, end_pc;
+ PyObject *start;
+ PyObject *end;
+ PyObject *ret_tuple;
+
+ SALPY_REQUIRE_VALID (self, sal);
+
+ ret_tuple = PyTuple_New (2);
+
+ if (find_line_pc_range (*sal, &start_pc, &end_pc))
+ {
+
+ start = gdb_py_long_from_ulongest (start_pc);
+ end = gdb_py_long_from_ulongest (end_pc);
+ PyTuple_SET_ITEM (ret_tuple, 0, start);
+ PyTuple_SET_ITEM (ret_tuple, 1, end);
+ }
+ else
+ {
+ PyTuple_SET_ITEM (ret_tuple, 0, Py_None);
+ PyTuple_SET_ITEM (ret_tuple, 1, Py_None);
+ }
+
+ return ret_tuple;
+}
+
+static PyObject *
salpy_get_symtab (PyObject *self, void *closure)
{
struct symtab_and_line *sal;
@@ -526,6 +556,8 @@ static PyMethodDef sal_object_methods[] = {
{ "is_valid", salpy_is_valid, METH_NOARGS,
"is_valid () -> Boolean.\n\
Return true if this symbol table and line is valid, false if not." },
+ { "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS,
+ "Return a tuple of start and end ranges for the symbol table." },
{NULL} /* Sentinel */
};
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-02 4:03 [patch] [python] find_line_pc_range Matt Rice
@ 2011-07-03 16:35 ` Phil Muldoon
2011-07-04 5:38 ` Phil Muldoon
2011-07-04 6:36 ` Matt Rice
0 siblings, 2 replies; 10+ messages in thread
From: Phil Muldoon @ 2011-07-03 16:35 UTC (permalink / raw)
To: Matt Rice; +Cc: gdb-patches
Matt Rice <ratmice@gmail.com> writes:
> don't know a whole much about python but,
> would it be better to return None on error, instead of a tuple containing Nones?
Thanks. I would return either an exception or None depending on the
situational context of the failure. If a None type scenario is expected,
and normal then it is okay to return that, given that the Python
scripter would understand the reason for that. So documentation is key
here. I think find_line_pc_range is just a utility function,
which can very well not find the line range. So I would replicate that
functionality in your API, except I would return a single Py_None over a
tuple.
> +salpy_find_line_pc_range (PyObject *self, PyObject *args)
> +{
> + struct symtab_and_line *sal = NULL;
> + CORE_ADDR start_pc, end_pc;
> + PyObject *start;
> + PyObject *end;
> + PyObject *ret_tuple;
> +
> + SALPY_REQUIRE_VALID (self, sal);
> +
> + ret_tuple = PyTuple_New (2);
This call can fail and return NULL, so in this case you need to return
NULL to propagate the failure.
> + if (find_line_pc_range (*sal, &start_pc, &end_pc))
> + {
> +
> + start = gdb_py_long_from_longest (start_pc);
> + end = gdb_py_long_from_longest (end_pc);
> + PyTuple_SET_ITEM (ret_tuple, 0, start);
> + PyTuple_SET_ITEM (ret_tuple, 1, end);
> + }
start and end could be local to this block I think.
Also gdb_py_long_from_longest is macro that points to Py
Long_FromUnsignedLongLong. This can also return NULL which indicates a
failure, so similar to above, you need to check the return and return
NULL if one of them fails. If one of them does return NULL, you also
need to appropriately clean-up previous references that were created in
Python. We normally do this by creating a local error: goto.
A minor nit, SET_ITEM does not do any error checking, while SetItem
does. As this is a new tuple, then it is ok to use it. But normally
(and personally, in new tuples) I always use the error checking
equivalent.
+ else
+ {
+ PyTuple_SET_ITEM (ret_tuple, 0, Py_None);
+ PyTuple_SET_ITEM (ret_tuple, 1, Py_None);
+ }
+
In this case I would just return Py_None (see first paragraph). It
seems a little redundant to return a tuple with Py_None for both
elements, and this will always be the case on a lookup failure with
find_line_pc_range. Make sure you incref Py_None before returning it,
also.
Also, this patch needs tests.
Thanks for the patch and your work!
Cheers,
Phil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-03 16:35 ` Phil Muldoon
@ 2011-07-04 5:38 ` Phil Muldoon
2011-07-04 6:36 ` Matt Rice
1 sibling, 0 replies; 10+ messages in thread
From: Phil Muldoon @ 2011-07-04 5:38 UTC (permalink / raw)
To: Matt Rice; +Cc: gdb-patches
Phil Muldoon <pmuldoon@redhat.com> writes:
>
>> + start = gdb_py_long_from_longest (start_pc);
>> + end = gdb_py_long_from_longest (end_pc);
>> + PyTuple_SET_ITEM (ret_tuple, 0, start);
>> + PyTuple_SET_ITEM (ret_tuple, 1, end);
>> + }
>
>>
> Also gdb_py_long_from_longest is macro that points to Py
> Long_FromUnsignedLongLong. This can also return NULL which indicates a
> failure, so similar to above, you need to check the return and return
> NULL if one of them fails. If one of them does return NULL, you also
> need to appropriately clean-up previous references that were created in
> Python. We normally do this by creating a local error: goto.
I read this back, and I thought, I need to explain this a little more
because of the weird reference stealing behaviour of Python
Tuples/Lists. In a "normal" scenario, if you create a Python object,
and later on some bit of code fails, you need to decrement that object
in your local error block (or later, if the function succeeds). But
because Py_Tuple "steals" a reference, it takes ownership of that
object, even if the Tuple itself somehow fails. So in the above case,
if "start" was added ok, but "end" did not, you do not need to decrement
"start", just decrement the tuple. I think this is one of two Python
APIs that does this, so you don't come across it often.
Cheers,
Phil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-03 16:35 ` Phil Muldoon
2011-07-04 5:38 ` Phil Muldoon
@ 2011-07-04 6:36 ` Matt Rice
2011-07-04 10:09 ` Phil Muldoon
2011-07-04 10:50 ` Eli Zaretskii
1 sibling, 2 replies; 10+ messages in thread
From: Matt Rice @ 2011-07-04 6:36 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 3155 bytes --]
On Sun, Jul 3, 2011 at 9:18 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Matt Rice <ratmice@gmail.com> writes:
>
>> don't know a whole much about python but,
>> would it be better to return None on error, instead of a tuple containing Nones?
>
> Thanks. I would return either an exception or None depending on the
> situational context of the failure. If a None type scenario is expected,
> and normal then it is okay to return that, given that the Python
> scripter would understand the reason for that. So documentation is key
> here. I think find_line_pc_range is just a utility function,
> which can very well not find the line range. So I would replicate that
> functionality in your API, except I would return a single Py_None over a
> tuple.
>
>
>> +salpy_find_line_pc_range (PyObject *self, PyObject *args)
>> +{
>> + struct symtab_and_line *sal = NULL;
>> + CORE_ADDR start_pc, end_pc;
>> + PyObject *start;
>> + PyObject *end;
>> + PyObject *ret_tuple;
>> +
>> + SALPY_REQUIRE_VALID (self, sal);
>> +
>> + ret_tuple = PyTuple_New (2);
>
> This call can fail and return NULL, so in this case you need to return
> NULL to propagate the failure.
>
>> + if (find_line_pc_range (*sal, &start_pc, &end_pc))
>> + {
>> +
>> + start = gdb_py_long_from_longest (start_pc);
>> + end = gdb_py_long_from_longest (end_pc);
>> + PyTuple_SET_ITEM (ret_tuple, 0, start);
>> + PyTuple_SET_ITEM (ret_tuple, 1, end);
>> + }
>
> start and end could be local to this block I think.
>
> Also gdb_py_long_from_longest is macro that points to Py
> Long_FromUnsignedLongLong. This can also return NULL which indicates a
> failure, so similar to above, you need to check the return and return
> NULL if one of them fails. If one of them does return NULL, you also
> need to appropriately clean-up previous references that were created in
> Python. We normally do this by creating a local error: goto.
>
> A minor nit, SET_ITEM does not do any error checking, while SetItem
> does. As this is a new tuple, then it is ok to use it. But normally
> (and personally, in new tuples) I always use the error checking
> equivalent.
>
>
> + else
> + {
> + PyTuple_SET_ITEM (ret_tuple, 0, Py_None);
> + PyTuple_SET_ITEM (ret_tuple, 1, Py_None);
> + }
> +
>
> In this case I would just return Py_None (see first paragraph). It
> seems a little redundant to return a tuple with Py_None for both
> elements, and this will always be the case on a lookup failure with
> find_line_pc_range. Make sure you incref Py_None before returning it,
> also.
>
> Also, this patch needs tests.
Thanks, attached is an updated patch that also includes tests.
2011-07-03 Matt Rice <ratmice@gmail.com>
* python/py-symtab.c: Populate sal_object_methods.
(salpy_find_line_pc_range): New function.
2011-07-03 Matt Rice <ratmice@gmail.com>
* gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range method.
2011-07-03 Matt Rice <ratmice@gmail.com>
* gdb.python/py-symtab.exp: New Tests for find_line_pc_range.
[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 3354 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dbaf30e..c637ba2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23293,6 +23293,11 @@ exist in @value{GDBN} any longer. All other
@code{gdb.Symtab_and_line} methods will throw an exception if it is
invalid at the time the method is called.
@end defmethod
+
+@defmethod Symtab_and_line find_line_pc_range
+If found returns a @code{Tuple} containing the start and end program counter
+addresses for the line attribute. Otherwise returns @code{None}.
+@end defmethod
@end table
A @code{gdb.Symtab} object has the following attributes:
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 107cdec..21811d1 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -215,6 +215,53 @@ salpy_get_line (PyObject *self, void *closure)
}
static PyObject *
+salpy_find_line_pc_range (PyObject *self, PyObject *args)
+{
+ struct symtab_and_line *sal = NULL;
+ CORE_ADDR start_pc, end_pc;
+
+ SALPY_REQUIRE_VALID (self, sal);
+
+ if (find_line_pc_range (*sal, &start_pc, &end_pc))
+ {
+ PyObject *ret_tuple;
+ PyObject *start;
+ PyObject *end;
+
+ ret_tuple = PyTuple_New (2);
+ if (!ret_tuple)
+ return Py_None;
+
+ start = gdb_py_long_from_ulongest (start_pc);
+ if (!start)
+ goto fail_ret;
+ if (PyTuple_SetItem (ret_tuple, 0, start) != 0)
+ {
+ Py_XDECREF (start);
+ goto fail_ret;
+ }
+
+ end = gdb_py_long_from_ulongest (end_pc);
+ if (!end)
+ goto fail_ret;
+ if (PyTuple_SetItem (ret_tuple, 1, end) != 0)
+ {
+ Py_XDECREF (end);
+ goto fail_ret;
+ }
+
+ return ret_tuple;
+
+ fail_ret:
+ Py_XDECREF (ret_tuple);
+ return Py_None;
+ }
+
+ return Py_None;
+
+}
+
+static PyObject *
salpy_get_symtab (PyObject *self, void *closure)
{
struct symtab_and_line *sal;
@@ -526,6 +573,8 @@ static PyMethodDef sal_object_methods[] = {
{ "is_valid", salpy_is_valid, METH_NOARGS,
"is_valid () -> Boolean.\n\
Return true if this symbol table and line is valid, false if not." },
+ { "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS,
+ "Return a tuple of start and end ranges for the symbol table." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/py-symtab.exp b/gdb/testsuite/gdb.python/py-symtab.exp
index c52f5ef..b6f3423 100644
--- a/gdb/testsuite/gdb.python/py-symtab.exp
+++ b/gdb/testsuite/gdb.python/py-symtab.exp
@@ -58,6 +58,14 @@ gdb_test "python print sal.symtab" "gdb/testsuite/gdb.python/py-symbol.c.*" "Tes
gdb_test "python print sal.pc" "${decimal}" "Test sal.pc"
gdb_test "python print sal.line" "42" "Test sal.line"
gdb_test "python print sal.is_valid()" "True" "Test sal.is_valid"
+gdb_test "python print sal.pc == sal.find_line_pc_range()\[0\]" "True" "Test sal.find_line_pc_range() 1"
+gdb_test "python print sal.find_line_pc_range()\[1\]" "${decimal}" "Test sal.find_line_pc_range() 2"
+
+# Test sal find_line_pc_range errors.
+gdb_test "python bad_line_sal = gdb.decode_line(\"py-symbol.c:404\")\[1\]\[0\]" "" \
+"test decode_line python.c:404"
+gdb_test "python print bad_line_sal.find_line_pc_range() == None" "True" \
+"find_line_pc_range for non-existent line"
# Test symbol table.
gdb_test "python print symtab.filename" "testsuite/gdb.python/py-symbol.c.*" "Test symtab.filename"
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-04 6:36 ` Matt Rice
@ 2011-07-04 10:09 ` Phil Muldoon
2011-07-05 2:47 ` Matt Rice
2011-07-04 10:50 ` Eli Zaretskii
1 sibling, 1 reply; 10+ messages in thread
From: Phil Muldoon @ 2011-07-04 10:09 UTC (permalink / raw)
To: Matt Rice; +Cc: gdb-patches
Matt Rice <ratmice@gmail.com> writes:
> Thanks, attached is an updated patch that also includes tests.
Thanks.
> + ret_tuple = PyTuple_New (2);
> + if (!ret_tuple)
> + return Py_None;
Return NULL here. This signifies an error, and that we have abandoned
the function. When a Python function returns NULL, the exception is
eventually printed and cleared either by the callers, or by the GDB
Python top-level exception handling code.
> + start = gdb_py_long_from_ulongest (start_pc);
> + if (!start)
> + goto fail_ret;
> + if (PyTuple_SetItem (ret_tuple, 0, start) != 0)
> + {
> + Py_XDECREF (start);
> + goto fail_ret;
> + }
You do not need to decrement "start" here. Even if the tuple SetItem
call failed, you have already passed the responsibility to the tuple,
and you no longer own it (stolen reference). SetItem would decrement
this on failure. So just goto to your local error block.
> + end = gdb_py_long_from_ulongest (end_pc);
> + if (!end)
> + goto fail_ret;
> + if (PyTuple_SetItem (ret_tuple, 1, end) != 0)
> + {
> + Py_XDECREF (end);
> + goto fail_ret;
> + }
Same as above.
> + fail_ret:
> + Py_XDECREF (ret_tuple);
Small nit, and this is generally ok, but you already know that if this
error block is called, ret_tuple will be instantiated. So in this case
the XDECREF is a tiny bit redundant. You could just use DECREF here and
skip the NULL check that XDECREF does. Like I said, a tiny nit, but I
just thought I would point it out.
> + return Py_None;
If there is a Python exception that you cannot handle internally to your
function, you must always return NULL. This informs callers, and
ultimately the exception managing code in GDB that some Python call
failed, to print the exception, and clear it.
> @@ -526,6 +573,8 @@ static PyMethodDef sal_object_methods[] = {
> { "is_valid", salpy_is_valid, METH_NOARGS,
> "is_valid () -> Boolean.\n\
> Return true if this symbol table and line is valid, false if not." },
> + { "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS,
> + "Return a tuple of start and end ranges for the symbol table." },
> {NULL} /* Sentinel */
> };
We try to put the return type in the help now as a formal statement.
Look at the two-line help example above: "is_valid()" for guidance.
Nearly there, thanks for the patch!
Cheers,
Phil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-04 6:36 ` Matt Rice
2011-07-04 10:09 ` Phil Muldoon
@ 2011-07-04 10:50 ` Eli Zaretskii
2011-07-04 10:56 ` Matt Rice
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2011-07-04 10:50 UTC (permalink / raw)
To: Matt Rice; +Cc: pmuldoon, gdb-patches
> Date: Sun, 3 Jul 2011 23:03:51 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: gdb-patches@sourceware.org
>
> Thanks, attached is an updated patch that also includes tests.
>
> 2011-07-03 Matt Rice <ratmice@gmail.com>
>
> * python/py-symtab.c: Populate sal_object_methods.
> (salpy_find_line_pc_range): New function.
>
> 2011-07-03 Matt Rice <ratmice@gmail.com>
>
> * gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range method.
>
> 2011-07-03 Matt Rice <ratmice@gmail.com>
>
> * gdb.python/py-symtab.exp: New Tests for find_line_pc_range.
Thanks.
Comments about the documentation part:
> +@defmethod Symtab_and_line find_line_pc_range
> +If found returns a @code{Tuple} containing the start and end program counter
> +addresses for the line attribute. Otherwise returns @code{None}.
Sorry, I don't understand what you are trying to say here. "If found"
what? And what "line attribute" do you refer to?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-04 10:50 ` Eli Zaretskii
@ 2011-07-04 10:56 ` Matt Rice
2011-07-04 13:14 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Matt Rice @ 2011-07-04 10:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: pmuldoon, gdb-patches
On Mon, Jul 4, 2011 at 3:37 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sun, 3 Jul 2011 23:03:51 -0700
>> From: Matt Rice <ratmice@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> Thanks, attached is an updated patch that also includes tests.
>>
>> 2011-07-03 Matt Rice <ratmice@gmail.com>
>>
>> * python/py-symtab.c: Populate sal_object_methods.
>> (salpy_find_line_pc_range): New function.
>>
>> 2011-07-03 Matt Rice <ratmice@gmail.com>
>>
>> * gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range method.
>>
>> 2011-07-03 Matt Rice <ratmice@gmail.com>
>>
>> * gdb.python/py-symtab.exp: New Tests for find_line_pc_range.
>
> Thanks.
>
> Comments about the documentation part:
>
>> +@defmethod Symtab_and_line find_line_pc_range
>> +If found returns a @code{Tuple} containing the start and end program counter
>> +addresses for the line attribute. Otherwise returns @code{None}.
>
> Sorry, I don't understand what you are trying to say here.
> "If found"
> what?
Sorry, I abhor writing.
find_line_pc_range will not find a range in cases where
the code is not associated with a line (no debug symbols), or the line
is not associated with code (ifdef'd out or something).
> And what "line attribute" do you refer to?
this thing:
— Instance Variable of Symtab_and_line: line
Indicates the current line number for this object. This attribute is
not writable.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-04 10:56 ` Matt Rice
@ 2011-07-04 13:14 ` Eli Zaretskii
0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2011-07-04 13:14 UTC (permalink / raw)
To: Matt Rice; +Cc: pmuldoon, gdb-patches
> Date: Mon, 4 Jul 2011 03:50:11 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: pmuldoon@redhat.com, gdb-patches@sourceware.org
>
> >> +@defmethod Symtab_and_line find_line_pc_range
> >> +If found returns a @code{Tuple} containing the start and end program counter
> >> +addresses for the line attribute. Â Otherwise returns @code{None}.
> >
> > Sorry, I don't understand what you are trying to say here.
> > Â "If found"
> > what?
>
> Sorry, I abhor writing.
> find_line_pc_range will not find a range in cases where
> the code is not associated with a line (no debug symbols), or the line
> is not associated with code (ifdef'd out or something).
>
> > Â And what "line attribute" do you refer to?
>
> this thing:
> — Instance Variable of Symtab_and_line: line
>
> Indicates the current line number for this object. This attribute is
> not writable.
Thanks for explaining. I would rephrase as follows:
Attempt to find the range of program counter addresses for the
@code{line} attribute of the @code{Symtab_and_line} object. If
found, return a @code{Tuple} containing the start and end addresses
for the line. Otherwise (e.g., a line with no corresponding code or
not present in the debug info), return @code{None}.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-04 10:09 ` Phil Muldoon
@ 2011-07-05 2:47 ` Matt Rice
2011-07-05 20:41 ` Phil Muldoon
0 siblings, 1 reply; 10+ messages in thread
From: Matt Rice @ 2011-07-05 2:47 UTC (permalink / raw)
To: pmuldoon; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 3382 bytes --]
On Mon, Jul 4, 2011 at 2:13 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Matt Rice <ratmice@gmail.com> writes:
>
>> Thanks, attached is an updated patch that also includes tests.
>
> Thanks.
>
>> + ret_tuple = PyTuple_New (2);
>> + if (!ret_tuple)
>> + return Py_None;
>
> Return NULL here. This signifies an error, and that we have abandoned
> the function. When a Python function returns NULL, the exception is
> eventually printed and cleared either by the callers, or by the GDB
> Python top-level exception handling code.
Thanks for explaining, that is what I wanted. You said so in your first email,
but for some reason it didn't latch on, sorry.
>> + start = gdb_py_long_from_ulongest (start_pc);
>> + if (!start)
>> + goto fail_ret;
>> + if (PyTuple_SetItem (ret_tuple, 0, start) != 0)
>> + {
>> + Py_XDECREF (start);
>> + goto fail_ret;
>> + }
>
> You do not need to decrement "start" here. Even if the tuple SetItem
> call failed, you have already passed the responsibility to the tuple,
> and you no longer own it (stolen reference). SetItem would decrement
> this on failure. So just goto to your local error block.
OK, the docs were silent on this regard, and I guessed on error it
wouldn't steal it.
>> + end = gdb_py_long_from_ulongest (end_pc);
>> + if (!end)
>> + goto fail_ret;
>> + if (PyTuple_SetItem (ret_tuple, 1, end) != 0)
>> + {
>> + Py_XDECREF (end);
>> + goto fail_ret;
>> + }
>
> Same as above.
>
>> + fail_ret:
>> + Py_XDECREF (ret_tuple);
>
> Small nit, and this is generally ok, but you already know that if this
> error block is called, ret_tuple will be instantiated. So in this case
> the XDECREF is a tiny bit redundant. You could just use DECREF here and
> skip the NULL check that XDECREF does. Like I said, a tiny nit, but I
> just thought I would point it out.
k.
>> + return Py_None;
>
> If there is a Python exception that you cannot handle internally to your
> function, you must always return NULL. This informs callers, and
> ultimately the exception managing code in GDB that some Python call
> failed, to print the exception, and clear it.
>
>> @@ -526,6 +573,8 @@ static PyMethodDef sal_object_methods[] = {
>> { "is_valid", salpy_is_valid, METH_NOARGS,
>> "is_valid () -> Boolean.\n\
>> Return true if this symbol table and line is valid, false if not." },
>> + { "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS,
>> + "Return a tuple of start and end ranges for the symbol table." },
>> {NULL} /* Sentinel */
>> };
>
> We try to put the return type in the help now as a formal statement.
> Look at the two-line help example above: "is_valid()" for guidance.
Updated that text a little bit also, along with using Eli's text for the manual.
2011-07-04 Matt Rice <ratmice@gmail.com>
* python/py-symtab.c: Populate sal_object_methods.
(salpy_find_line_pc_range): New function.
2011-07-04 Matt Rice <ratmice@gmail.com>
Eli Zaretskii <eliz@gnu.org>
* gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range method.
2011-07-04 Matt Rice <ratmice@gmail.com>
* gdb.python/py-symtab.exp: New Tests for find_line_pc_range.
[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 3528 bytes --]
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dbaf30e..e183693 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23293,6 +23293,14 @@ exist in @value{GDBN} any longer. All other
@code{gdb.Symtab_and_line} methods will throw an exception if it is
invalid at the time the method is called.
@end defmethod
+
+@defmethod Symtab_and_line find_line_pc_range
+Attempt to find the range of program counter addresses for the
+@code{line} attribute of the @code{Symtab_and_line} object. If
+found, return a @code{Tuple} containing the start and end addresses
+for the line. Otherwise (e.g., a line with no corresponding code or
+not present in the debug info), return @code{None}.
+@end defmethod
@end table
A @code{gdb.Symtab} object has the following attributes:
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 107cdec..5ae1975 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -215,6 +215,46 @@ salpy_get_line (PyObject *self, void *closure)
}
static PyObject *
+salpy_find_line_pc_range (PyObject *self, PyObject *args)
+{
+ struct symtab_and_line *sal = NULL;
+ CORE_ADDR start_pc, end_pc;
+
+ SALPY_REQUIRE_VALID (self, sal);
+
+ if (find_line_pc_range (*sal, &start_pc, &end_pc))
+ {
+ PyObject *ret_tuple;
+ PyObject *start;
+ PyObject *end;
+
+ ret_tuple = PyTuple_New (2);
+ if (!ret_tuple)
+ return NULL;
+
+ start = gdb_py_long_from_ulongest (start_pc);
+ if (!start)
+ goto fail_ret;
+ if (PyTuple_SetItem (ret_tuple, 0, start) != 0)
+ goto fail_ret;
+
+ end = gdb_py_long_from_ulongest (end_pc);
+ if (!end)
+ goto fail_ret;
+ if (PyTuple_SetItem (ret_tuple, 1, end) != 0)
+ goto fail_ret;
+
+ return ret_tuple;
+
+ fail_ret:
+ Py_DECREF (ret_tuple);
+ return NULL;
+ }
+
+ return Py_None;
+}
+
+static PyObject *
salpy_get_symtab (PyObject *self, void *closure)
{
struct symtab_and_line *sal;
@@ -526,6 +566,10 @@ static PyMethodDef sal_object_methods[] = {
{ "is_valid", salpy_is_valid, METH_NOARGS,
"is_valid () -> Boolean.\n\
Return true if this symbol table and line is valid, false if not." },
+ { "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS,
+ "find_line_pc_range () -> Tuple.\n\
+Return a tuple of start and end ranges for the symbol table's line,\n\
+or None if not found." },
{NULL} /* Sentinel */
};
diff --git a/gdb/testsuite/gdb.python/py-symtab.exp b/gdb/testsuite/gdb.python/py-symtab.exp
index c52f5ef..e27c066 100644
--- a/gdb/testsuite/gdb.python/py-symtab.exp
+++ b/gdb/testsuite/gdb.python/py-symtab.exp
@@ -58,6 +58,14 @@ gdb_test "python print sal.symtab" "gdb/testsuite/gdb.python/py-symbol.c.*" "Tes
gdb_test "python print sal.pc" "${decimal}" "Test sal.pc"
gdb_test "python print sal.line" "42" "Test sal.line"
gdb_test "python print sal.is_valid()" "True" "Test sal.is_valid"
+gdb_test "python print sal.pc == sal.find_line_pc_range()\[0\]" "True" "Test sal.find_line_pc_range() 1"
+gdb_test "python print sal.find_line_pc_range()\[1\]" "${decimal}" "Test sal.find_line_pc_range() 2"
+
+# Test sal find_line_pc_range errors.
+gdb_test "python bad_line_sal = gdb.decode_line(\"py-symbol.c:404\")\[1\]\[0\]" "" \
+"test decode_line py-symbol.c:404"
+gdb_test "python print bad_line_sal.find_line_pc_range() == None" "True" \
+"find_line_pc_range for non-existent line"
# Test symbol table.
gdb_test "python print symtab.filename" "testsuite/gdb.python/py-symbol.c.*" "Test symtab.filename"
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] [python] find_line_pc_range
2011-07-05 2:47 ` Matt Rice
@ 2011-07-05 20:41 ` Phil Muldoon
0 siblings, 0 replies; 10+ messages in thread
From: Phil Muldoon @ 2011-07-05 20:41 UTC (permalink / raw)
To: Matt Rice; +Cc: gdb-patches
Matt Rice <ratmice@gmail.com> writes:
>>> Thanks, attached is an updated patch that also includes tests.
Matt
This looks ok by me. A maintainer will still have to look over it and
ACK it.
Cheers,
Phil
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-07-05 20:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-02 4:03 [patch] [python] find_line_pc_range Matt Rice
2011-07-03 16:35 ` Phil Muldoon
2011-07-04 5:38 ` Phil Muldoon
2011-07-04 6:36 ` Matt Rice
2011-07-04 10:09 ` Phil Muldoon
2011-07-05 2:47 ` Matt Rice
2011-07-05 20:41 ` Phil Muldoon
2011-07-04 10:50 ` Eli Zaretskii
2011-07-04 10:56 ` Matt Rice
2011-07-04 13:14 ` Eli Zaretskii
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox