gdb/python: handle completion returning a non-sequence

Message ID 87fa2a8161452cf2eb17ca7ba561831eebe4f254.1700146245.git.aburgess@redhat.com
State New
Headers
Series gdb/python: handle completion returning a non-sequence |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Andrew Burgess Nov. 16, 2023, 2:55 p.m. UTC
  GDB's Python API documentation for gdb.Command.complete() says:

  The 'complete' method can return several values:
     * If the return value is a sequence, the contents of the
       sequence are used as the completions.  It is up to 'complete'
       to ensure that the contents actually do complete the word.  A
       zero-length sequence is allowed, it means that there were no
       completions available.  Only string elements of the sequence
       are used; other elements in the sequence are ignored.

     * If the return value is one of the 'COMPLETE_' constants
       defined below, then the corresponding GDB-internal completion
       function is invoked, and its result is used.

     * All other results are treated as though there were no
       available completions.

So, returning a non-sequence, and non-integer from a complete method
should be fine; it should just be treated as though there are no
completions.

However, if I write a complete method that returns None, I see this
behaviour:

  (gdb) complete completefilenone x
  Python Exception <class 'TypeError'>: 'NoneType' object is not iterable
  warning: internal error: Unhandled Python exception
  (gdb)

Which is caused because we are currently missing a call to
PyErr_Clear().

Fixed in this commit, and added a test.

I've also added a test in which the complete() function raises an
exception.  This case was already handled (again, we treat this as if
there are no completions), but I don't believe this case was being
tested ... it is now.
---
 gdb/python/py-cmd.c                        |  6 +++++-
 gdb/testsuite/gdb.python/py-completion.exp |  7 +++++++
 gdb/testsuite/gdb.python/py-completion.py  | 24 ++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)


base-commit: 8d081332318b07f1a2b4af0fdfbdc256e37dca93
  

Comments

Paul Koning Nov. 16, 2023, 3:02 p.m. UTC | #1
> On Nov 16, 2023, at 9:55 AM, Andrew Burgess <aburgess@redhat.com> wrote:
> 
> GDB's Python API documentation for gdb.Command.complete() says:
> 
>  The 'complete' method can return several values:
>     * If the return value is a sequence, the contents of the
>       sequence are used as the completions.  It is up to 'complete'
>       to ensure that the contents actually do complete the word.  A
>       zero-length sequence is allowed, it means that there were no
>       completions available.  Only string elements of the sequence
>       are used; other elements in the sequence are ignored.
> 
>     * If the return value is one of the 'COMPLETE_' constants
>       defined below, then the corresponding GDB-internal completion
>       function is invoked, and its result is used.
> 
>     * All other results are treated as though there were no
>       available completions.
> 
> So, returning a non-sequence, and non-integer from a complete method
> should be fine; it should just be treated as though there are no
> completions. ...
> 
> I've also added a test in which the complete() function raises an
> exception.  This case was already handled (again, we treat this as if
> there are no completions), but I don't believe this case was being
> tested ... it is now.

"All other results" doesn't clearly include an exception being raised, so a rewording of the third bullet would help.  "All other results, including an exception being raised, are treated..."

	paul
  
Tom Tromey Nov. 16, 2023, 3:28 p.m. UTC | #2
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew>        gdbpy_ref<> iter (PyObject_GetIter (resultobj.get ()));
 
Andrew>        if (iter == NULL)
Andrew> -	return;
Andrew> +	{
Andrew> +	  /* Ignore any errors, don't offer any completions.  */
Andrew> +	  PyErr_Clear ();
Andrew> +	  return;
 
Just dropping exceptions makes it hard to debug Python extensions.
I think it's probably better to call gdbpy_print_stack.

Tom
  

Patch

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 20a384d6907..a906766c381 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -295,7 +295,11 @@  cmdpy_completer (struct cmd_list_element *command,
       gdbpy_ref<> iter (PyObject_GetIter (resultobj.get ()));
 
       if (iter == NULL)
-	return;
+	{
+	  /* Ignore any errors, don't offer any completions.  */
+	  PyErr_Clear ();
+	  return;
+	}
 
       bool got_matches = false;
       while (true)
diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp
index 23f981e944a..6264df0f291 100644
--- a/gdb/testsuite/gdb.python/py-completion.exp
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -46,6 +46,13 @@  if { [readline_is_used] && ![is_remote host] } {
     # Just discarding whatever we typed.
     gdb_test " " ".*" "discard #[incr discard]"
 
+    # These should offer no suggestions - the complete() methods
+    # either return a non-sequence or raise an exception.
+    foreach_with_prefix cmd { completefilenone completefileexception} {
+	gdb_test_no_output "complete $cmd ${testdir_complete}" \
+	    "no suggestions given"
+    }
+
     # This is the problematic one.
     send_gdb "completefilemethod ${testdir_complete}\t"
     gdb_test_multiple "" "completefilemethod completion" {
diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py
index abec06921c0..19628cbcceb 100644
--- a/gdb/testsuite/gdb.python/py-completion.py
+++ b/gdb/testsuite/gdb.python/py-completion.py
@@ -28,6 +28,28 @@  class CompleteFileInit(gdb.Command):
         raise gdb.GdbError("not implemented")
 
 
+class CompleteFileNone(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, "completefilenone", gdb.COMMAND_USER)
+
+    def invoke(self, argument, from_tty):
+        raise gdb.GdbError("not implemented")
+
+    def complete(self, text, word):
+        return None
+
+
+class CompleteFileException(gdb.Command):
+    def __init__(self):
+        gdb.Command.__init__(self, "completefileexception", gdb.COMMAND_USER)
+
+    def invoke(self, argument, from_tty):
+        raise gdb.GdbError("not implemented")
+
+    def complete(self, text, word):
+        raise gdb.GdbError("failed completion")
+
+
 class CompleteFileMethod(gdb.Command):
     def __init__(self):
         gdb.Command.__init__(self, "completefilemethod", gdb.COMMAND_USER)
@@ -203,6 +225,8 @@  class CompleteLimit7(gdb.Command):
 
 
 CompleteFileInit()
+CompleteFileNone()
+CompleteFileException()
 CompleteFileMethod()
 CompleteFileCommandCond()
 CompleteLimit1()