Re: [patch] [python] find_line_pc_range

[Matt Rice <ratmice@gmail.com>]

[-- 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"