From 7d793aa9f0986828d5dde8f3811a7adafc38b6b4 Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Wed, 3 Sep 2014 16:30:28 -0400 Subject: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class This PR came from a Red Hat bug that was filed recently. I checked and it still exists on HEAD, so here's a proposed fix. Although this is marked as a Python backend bug, this is really about the completion mechanism used by GDB. Since this code reminds me of my first attempt to make a good noodle, it took me quite some time to fix it in a non-intrusive way. The problem is triggered when one registers a completion method inside a class in a Python script, rather than registering the command using a completer class directly. For example, consider the following script: class MyFirstCommand(gdb.Command): def __init__(self): gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME) def invoke(self,argument,from_tty): raise gdb.GdbError('not implemented') class MySecondCommand(gdb.Command): def __init__(self): gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER) def invoke(self,argument,from_tty): raise gdb.GdbError('not implemented') def complete(self,text,word): return gdb.COMPLETE_FILENAME MyFirstCommand () MySecondCommand () When one loads this into GDB and tries to complete filenames for both myfirstcommand and mysecondcommand, she gets: (gdb) myfirstcommand /hom (gdb) myfirstcommand /home/ ^ ... (gdb) mysecondcommand /hom (gdb) mysecondcommand /home ^ (The "^" marks the final position of the cursor after the TAB). So we see that myfirstcommand honors the COMPLETE_FILENAME class (as specified in the command creation), but mysecondcommand does not. After some investigation, I found that the problem lies with the set of word break characters that is used for each case. The set should be the same for both commands, but it is not. During the process of deciding which type of completion should be used, the code in gdb/completer.c:complete_line_internal analyses the command that requested the completion and tries to determine the type of completion wanted by checking which completion function will be called (e.g., filename_completer for filenames, location_completer for locations, etc.). This all works fine for myfirstcommand, because immediately after the command registration the Python backend already sets its completion function to filename_completer (which then causes the complete_line_internal function to choose the right set of word break chars). However, for mysecondcommand, this decision is postponed to when the completer function is evaluated, and the Python backend uses an internal completer (called cmdpy_completer). complete_line_internal doesn't know about this internal completer, and can't choose the right set of word break chars in time, which then leads to a bad decision when completing the "/hom" word. So, after a few attempts, I decided to create another callback in "struct cmd_list_element" that will be responsible for handling the case when there is an unknown completer function for complete_line_internal to work with. So far, only the Python backend uses this callback, and only when the user provides a completer method instead of registering the command directly with a completer class. I think this is the best option because it not very intrusive (all the other commands will still work normally), but especially because the whole completion code is so messy that it would be hard to fix this without having to redesign things. I have regtested this on Fedora 18 x86_64, without regressions. I also included a testcase. gdb/ChangeLog: 2014-09-03 Sergio Durigan Junior PR python/16699 * cli/cli-decode.c (set_cmd_completer_handle_brkchars): New function. (add_cmd): Set "completer_handle_brkchars" to NULL. * cli/cli-decode.h (struct cmd_list_element) : New field. * command.h (completer_ftype_void): New typedef. (set_cmd_completer_handle_brkchars): New prototype. * completer.c (set_gdb_completion_word_break_characters): New function. (complete_line_internal): Call "completer_handle_brkchars" callback from command. * completer.h: Include "command.h". (set_gdb_completion_word_break_characters): New prototype. * python/py-cmd.c (cmdpy_completer_helper): New function. (cmdpy_completer_handle_brkchars): New function. (cmdpy_completer): Adjust to use cmdpy_completer_helper. (cmdpy_init): Set completer_handle_brkchars to cmdpy_completer_handle_brkchars. gdb/testsuite/ChangeLog: 2014-09-03 Sergio Durigan Junior PR python/16699 * gdb.python/py-completion.exp: New file. * gdb.python/py-completion.py: Likewise. --- gdb/ChangeLog | 22 +++ gdb/cli/cli-decode.c | 10 ++ gdb/cli/cli-decode.h | 9 ++ gdb/command.h | 8 + gdb/completer.c | 21 +++ gdb/completer.h | 8 + gdb/python/py-cmd.c | 176 +++++++++++++++++---- gdb/testsuite/ChangeLog | 6 + gdb/testsuite/gdb.python/py-completion.exp | 69 ++++++++ gdb/testsuite/gdb.python/py-completion.py | 58 +++++++ 10 files changed, 359 insertions(+), 28 deletions(-) create mode 100644 gdb/testsuite/gdb.python/py-completion.exp create mode 100644 gdb/testsuite/gdb.python/py-completion.py diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 80961d6fc24..d4166233495 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,25 @@ +2014-09-03 Sergio Durigan Junior + + PR python/16699 + * cli/cli-decode.c (set_cmd_completer_handle_brkchars): New + function. + (add_cmd): Set "completer_handle_brkchars" to NULL. + * cli/cli-decode.h (struct cmd_list_element) + : New field. + * command.h (completer_ftype_void): New typedef. + (set_cmd_completer_handle_brkchars): New prototype. + * completer.c (set_gdb_completion_word_break_characters): New + function. + (complete_line_internal): Call "completer_handle_brkchars" + callback from command. + * completer.h: Include "command.h". + (set_gdb_completion_word_break_characters): New prototype. + * python/py-cmd.c (cmdpy_completer_helper): New function. + (cmdpy_completer_handle_brkchars): New function. + (cmdpy_completer): Adjust to use cmdpy_completer_helper. + (cmdpy_init): Set completer_handle_brkchars to + cmdpy_completer_handle_brkchars. + 2014-09-03 Gary Benson * nat/x86-dregs.h (ALL_DEBUG_REGISTERS): Renamed as... diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index c409d9c3d1c..7bc51a1b4d1 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -161,6 +161,15 @@ set_cmd_completer (struct cmd_list_element *cmd, completer_ftype *completer) cmd->completer = completer; /* Ok. */ } +/* See definition in commands.h. */ + +void +set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd, + completer_ftype_void *completer_handle_brkchars) +{ + cmd->completer_handle_brkchars = completer_handle_brkchars; +} + /* Add element named NAME. Space for NAME and DOC must be allocated by the caller. CLASS is the top level category into which commands are broken down @@ -236,6 +245,7 @@ add_cmd (const char *name, enum command_class class, cmd_cfunc_ftype *fun, c->prefix = NULL; c->abbrev_flag = 0; set_cmd_completer (c, make_symbol_completion_list_fn); + c->completer_handle_brkchars = NULL; c->destroyer = NULL; c->type = not_set_cmd; c->var = NULL; diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 865d4a07f09..59205596ef9 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -176,6 +176,15 @@ struct cmd_list_element "baz/foo", return "baz/foobar". */ completer_ftype *completer; + /* Handle the word break characters for this completer. Usually + this function need not be defined, but for some types of + completers (e.g., Python completers declared as methods inside + a class) the word break chars may need to be redefined + depending on the completer type (e.g., for filename + completers). */ + + completer_ftype_void *completer_handle_brkchars; + /* Destruction routine for this command. If non-NULL, this is called when this command instance is destroyed. This may be used to finalize the CONTEXT field, if needed. */ diff --git a/gdb/command.h b/gdb/command.h index 4ac3bfb8558..3b962124c7c 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -159,8 +159,16 @@ extern void set_cmd_sfunc (struct cmd_list_element *cmd, typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *, const char *, const char *); +typedef void completer_ftype_void (struct cmd_list_element *, + const char *, const char *); + extern void set_cmd_completer (struct cmd_list_element *, completer_ftype *); +/* Set the completer_handle_brkchars callback. */ + +extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *, + completer_ftype_void *); + /* HACK: cagney/2002-02-23: Code, mostly in tracepoints.c, grubs around in cmd objects to test the value of the commands sfunc(). */ extern int cmd_cfunc_eq (struct cmd_list_element *cmd, diff --git a/gdb/completer.c b/gdb/completer.c index 44920dd453d..67d7f458bc6 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -449,6 +449,21 @@ expression_completer (struct cmd_list_element *ignore, return location_completer (ignore, p, word); } +/* See definition in completer.h. */ + +void +set_gdb_completion_word_break_characters (completer_ftype *fn) +{ + /* So far we are only interested in differentiating filename + completers from everything else. */ + if (fn == filename_completer) + rl_completer_word_break_characters + = gdb_completer_file_name_break_characters; + else + rl_completer_word_break_characters + = gdb_completer_command_word_break_characters; +} + /* Here are some useful test cases for completion. FIXME: These should be put in the test suite. They should be tested with both M-? and TAB. @@ -677,6 +692,9 @@ complete_line_internal (const char *text, p--) ; } + if (reason == handle_brkchars + && c->completer_handle_brkchars != NULL) + (*c->completer_handle_brkchars) (c, p, word); if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } @@ -750,6 +768,9 @@ complete_line_internal (const char *text, p--) ; } + if (reason == handle_brkchars + && c->completer_handle_brkchars != NULL) + (*c->completer_handle_brkchars) (c, p, word); if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } diff --git a/gdb/completer.h b/gdb/completer.h index 7aa0f3bea9e..bc7ed963022 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -18,6 +18,7 @@ #define COMPLETER_H 1 #include "gdb_vecs.h" +#include "command.h" extern VEC (char_ptr) *complete_line (const char *text, const char *line_buffer, @@ -48,6 +49,13 @@ extern char *get_gdb_completer_quote_characters (void); extern char *gdb_completion_word_break_characters (void); +/* Set the word break characters array to the corresponding set of + chars, based on FN. This function is useful for cases when the + completer doesn't know the type of the completion until some + calculation is done (e.g., for Python functions). */ + +extern void set_gdb_completion_word_break_characters (completer_ftype *fn); + /* Exported to linespec.c */ extern const char *skip_quoted_chars (const char *, const char *, diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index 21f2a20572d..8bc4bf796e5 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -208,45 +208,163 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty) do_cleanups (cleanup); } +/* Helper function for the Python command completers (both "pure" + completer and brkchar handler). This function takes COMMAND, TEXT + and WORD and tries to call the Python method for completion with + these arguments. It also takes HANDLE_BRKCHARS_P, an argument to + identify whether it is being called from the brkchar handler or + from the "pure" completer. In the first case, it effectively calls + the Python method for completion, and records the PyObject in a + static variable (used as a "cache"). In the second case, it just + returns that variable, without actually calling the Python method + again. This saves us one Python method call. + + The reason for this two step dance is that we need to know the set + of "brkchars" to use early on, before we actually try to perform + the completion. But if a Python command supplies a "complete" + method then we have to call that method first: it may return as its + result the kind of completion to perform and that will in turn + specify which brkchars to use. IOW, we need the result of the + "complete" method before we actually perform the completion. + + It is important to mention that this function is built on the + assumption that the calls to cmdpy_completer_handle_brkchars and + cmdpy_completer will be subsequent with nothing intervening. This + is true for our completer mechanism. + + This function returns the PyObject representing the Python method + call. */ + +static PyObject * +cmdpy_completer_helper (struct cmd_list_element *command, + const char *text, const char *word, + int handle_brkchars_p) +{ + cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command); + PyObject *textobj, *wordobj; + /* This static variable will server as a "cache" for us, in order to + store the PyObject that results from calling the Python + function. */ + static PyObject *resultobj = NULL; + + if (handle_brkchars_p) + { + /* If we were called to handle brkchars, it means this is the + first function call of two that will happen in a row. + Therefore, we need to call the completer ourselves, and cache + the return value in the static variable RESULTOBJ. Then, in + the second call, we can just use the value of RESULTOBJ to do + our job. */ + if (resultobj != NULL) + Py_DECREF (resultobj); + + resultobj = NULL; + if (obj == NULL) + error (_("Invalid invocation of Python command object.")); + if (!PyObject_HasAttr ((PyObject *) obj, complete_cst)) + { + /* If there is no complete method, don't error. */ + return NULL; + } + + textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL); + if (textobj == NULL) + error (_("Could not convert argument to Python string.")); + wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL); + if (wordobj == NULL) + { + Py_DECREF (textobj); + error (_("Could not convert argument to Python string.")); + } + + resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst, + textobj, wordobj, NULL); + Py_DECREF (textobj); + Py_DECREF (wordobj); + if (!resultobj) + { + /* Just swallow errors here. */ + PyErr_Clear (); + } + + Py_XINCREF (resultobj); + } + + return resultobj; +} + +/* Python function called to determine the break characters of a + certain completer. We are only interested in knowing if the + completer registered by the user will return one of the integer + codes (see COMPLETER_* symbols). */ + +static void +cmdpy_completer_handle_brkchars (struct cmd_list_element *command, + const char *text, const char *word) +{ + PyObject *resultobj = NULL; + struct cleanup *cleanup; + + cleanup = ensure_python_env (get_current_arch (), current_language); + + /* Calling our helper to obtain the PyObject of the Python + function. */ + resultobj = cmdpy_completer_helper (command, text, word, 1); + + /* Check if there was an error. */ + if (resultobj == NULL) + goto done; + + if (PyInt_Check (resultobj)) + { + /* User code may also return one of the completion constants, + thus requesting that sort of completion. We are only + interested in this kind of return. */ + long value; + + if (!gdb_py_int_as_long (resultobj, &value)) + { + /* Ignore. */ + PyErr_Clear (); + } + else if (value >= 0 && value < (long) N_COMPLETERS) + { + /* This is the core of this function. Depending on which + completer type the Python function returns, we have to + adjust the break characters accordingly. */ + set_gdb_completion_word_break_characters + (completers[value].completer); + } + } + + done: + + /* We do not call Py_XDECREF here because RESULTOBJ will be used in + the subsequent call to cmdpy_completer function. */ + do_cleanups (cleanup); +} + /* Called by gdb for command completion. */ static VEC (char_ptr) * cmdpy_completer (struct cmd_list_element *command, const char *text, const char *word) { - cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command); - PyObject *textobj, *wordobj, *resultobj = NULL; + PyObject *resultobj = NULL; VEC (char_ptr) *result = NULL; struct cleanup *cleanup; cleanup = ensure_python_env (get_current_arch (), current_language); - if (! obj) - error (_("Invalid invocation of Python command object.")); - if (! PyObject_HasAttr ((PyObject *) obj, complete_cst)) - { - /* If there is no complete method, don't error -- instead, just - say that there are no completions. */ - goto done; - } + /* Calling our helper to obtain the PyObject of the Python + function. */ + resultobj = cmdpy_completer_helper (command, text, word, 0); - textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL); - if (! textobj) - error (_("Could not convert argument to Python string.")); - wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL); - if (! wordobj) - error (_("Could not convert argument to Python string.")); - - resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst, - textobj, wordobj, NULL); - Py_DECREF (textobj); - Py_DECREF (wordobj); - if (! resultobj) - { - /* Just swallow errors here. */ - PyErr_Clear (); - goto done; - } + /* If the result object of calling the Python function is NULL, it + means that there was an error. In this case, just give up and + return NULL. */ + if (resultobj == NULL) + goto done; result = NULL; if (PyInt_Check (resultobj)) @@ -302,7 +420,6 @@ cmdpy_completer (struct cmd_list_element *command, done: - Py_XDECREF (resultobj); do_cleanups (cleanup); return result; @@ -548,6 +665,9 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw) set_cmd_context (cmd, self); set_cmd_completer (cmd, ((completetype == -1) ? cmdpy_completer : completers[completetype].completer)); + if (completetype == -1) + set_cmd_completer_handle_brkchars (cmd, + cmdpy_completer_handle_brkchars); } if (except.reason < 0) { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index d996b7c2897..d6db24be57b 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2014-09-03 Sergio Durigan Junior + + PR python/16699 + * gdb.python/py-completion.exp: New file. + * gdb.python/py-completion.py: Likewise. + 2014-08-28 Doug Evans * gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp new file mode 100644 index 00000000000..a3bac8b6f95 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-completion.exp @@ -0,0 +1,69 @@ +# Copyright (C) 2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set testfile "py-completion" + +load_lib gdb-python.exp + +gdb_exit +gdb_start + +# Skip all tests if Python scripting is not enabled. +if { [skip_python_tests] } { continue } + +gdb_test_no_output "source ${srcdir}/${subdir}/${testfile}.py" + +# Create a temporary directory +set testdir "[standard_output_file "py-completion-testdir"]/" +set testdir_regex [string_to_regexp $testdir] +set testdir_complete [standard_output_file "py-completion-test"] +file mkdir $testdir + +# This one should always pass. +send_gdb "completefileinit ${testdir_complete}\t" +gdb_test_multiple "" "completefileinit completion" { + -re "^completefileinit ${testdir_regex}$" { + pass "completefileinit completion" + } +} + +# Just discarding whatever we typed. +gdb_test " " ".*" "discard #1" + +# This is the problematic one. +send_gdb "completefilemethod ${testdir_complete}\t" +gdb_test_multiple "" "completefilemethod completion" { + -re "^completefilemethod ${testdir_regex} $" { + fail "completefilemethod completion (completed filename as wrong command arg)" + } + -re "^completefilemethod ${testdir_regex}$" { + pass "completefilemethod completion" + } +} + +# Discarding again +gdb_test " " ".*" "discard #2" + +# Another problematic +set completion_regex "[string_to_regexp [standard_output_file "py-completion-t"]]" +send_gdb "completefilecommandcond [standard_output_file "py-completion-t\t"]" +gdb_test_multiple "" "completefilecommandcond completion" { + -re "^completefilecommandcond ${testdir}$" { + fail "completefilecommandcond completion (completed filename instead of command)" + } + -re "^completefilecommandcond ${completion_regex}\007$" { + pass "completefilecommandcond completion" + } +} diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py new file mode 100644 index 00000000000..23592d0de6e --- /dev/null +++ b/gdb/testsuite/gdb.python/py-completion.py @@ -0,0 +1,58 @@ +# Copyright (C) 2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This testcase tests PR python/16699 + +import gdb + +class CompleteFileInit(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completefileinit',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + +class CompleteFileMethod(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completefilemethod',gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + return gdb.COMPLETE_FILENAME + +class CompleteFileCommandCond(gdb.Command): + def __init__(self): + gdb.Command.__init__(self,'completefilecommandcond',gdb.COMMAND_USER) + + def invoke(self,argument,from_tty): + raise gdb.GdbError('not implemented') + + def complete(self,text,word): + # This is a test made to know if the command + # completion still works fine. When the user asks to + # complete something like "completefilecommandcond + # /path/to/py-completion-t", it should not complete to + # "/path/to/py-completion-test/", but instead just + # wait for input. + if "py-completion-t" in text: + return gdb.COMPLETE_COMMAND + else: + return gdb.COMPLETE_FILENAME + +CompleteFileInit() +CompleteFileMethod() +CompleteFileCommandCond() -- 2.30.2