public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/python: handle completion returning a non-sequence
@ 2023-11-16 14:55 Andrew Burgess
  2023-11-16 15:02 ` Paul Koning
  2023-11-16 15:28 ` Tom Tromey
  0 siblings, 2 replies; 3+ messages in thread
From: Andrew Burgess @ 2023-11-16 14:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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(-)

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()

base-commit: 8d081332318b07f1a2b4af0fdfbdc256e37dca93
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb/python: handle completion returning a non-sequence
  2023-11-16 14:55 [PATCH] gdb/python: handle completion returning a non-sequence Andrew Burgess
@ 2023-11-16 15:02 ` Paul Koning
  2023-11-16 15:28 ` Tom Tromey
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Koning @ 2023-11-16 15:02 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches



> 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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb/python: handle completion returning a non-sequence
  2023-11-16 14:55 [PATCH] gdb/python: handle completion returning a non-sequence Andrew Burgess
  2023-11-16 15:02 ` Paul Koning
@ 2023-11-16 15:28 ` Tom Tromey
  1 sibling, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2023-11-16 15:28 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-11-16 15:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-16 14:55 [PATCH] gdb/python: handle completion returning a non-sequence Andrew Burgess
2023-11-16 15:02 ` Paul Koning
2023-11-16 15:28 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).