From patchwork Wed Mar 12 22:49:39 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 57 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (caibbdcaabij.dreamhost.com [208.113.200.189]) by wilcox.dreamhost.com (Postfix) with ESMTP id 83AE33600B3 for ; Wed, 12 Mar 2014 15:49:51 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 384F44F19602; Wed, 12 Mar 2014 15:49:51 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 068DA4F0AF9C for ; Wed, 12 Mar 2014 15:49:50 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=lPSVQgZKkw4dnL8w +LM2uLmBnkW7mDfqh+tiuuqZLpJnb4UMTZfHTxeXHfGjXxjg2bXXo+pdA1cHOyaF huGmuUtND1h/w4WHMzXEtNKwmtm6UgBULXF9PTibao0Cu69upckfTC/Pxubab4wO N2QMDswYTM8Qtr5Gj/74pZLTMbg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; s=default; bh=McbbeU5efazyhy/xko4mn8 fSsao=; b=ohJ/NWuXlJMcb9uLCUHMBg81ztp8CCjYhOwQ0Y6XOaEnFqWz4FOO9W vrKG7+pwvFZSL0p+F6+hdrTEQyAWKMUuqHF63dKrkHfiwS24VAQofRsIXSQqVZa1 2T4/ScgoiNnh4g0PQCyMugP8Ci0MJ7k5XTz0ZcMV/nEUdOs8Bh4gM= Received: (qmail 25223 invoked by alias); 12 Mar 2014 22:49:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25211 invoked by uid 89); 12 Mar 2014 22:49:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Mar 2014 22:49:45 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2CMniMU003646 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 12 Mar 2014 18:49:44 -0400 Received: from psique (ovpn-113-62.phx2.redhat.com [10.3.113.62]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s2CMnei0026793 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 12 Mar 2014 18:49:41 -0400 From: Sergio Durigan Junior To: GDB Patches Cc: Phil Muldoon Subject: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class X-URL: http://www.redhat.com Date: Wed, 12 Mar 2014 19:49:39 -0300 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in Hi, 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. OK to apply? Thanks, diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d36ab4e..0c4d996 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -164,6 +164,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, + void (*completer_handle_brkchars) (struct cmd_list_element *self)) +{ + cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok. */ +} + /* 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 @@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int), 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 c6edc87..a043734 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). */ + + void (*completer_handle_brkchars) (struct cmd_list_element *self); + /* 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 a5040a4..058d133 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -160,6 +160,12 @@ typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *, 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 *, + void (*f) + (struct cmd_list_element *)); + /* 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 94f70a9..a9809a2 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -450,6 +450,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. @@ -481,7 +496,6 @@ typedef enum } complete_line_internal_reason; - /* Internal function used to handle completions. @@ -678,6 +692,9 @@ complete_line_internal (const char *text, p--) ; } + if (reason == handle_brkchars + && c->completer_handle_brkchars != NULL) + (*c->completer_handle_brkchars) (c); if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } @@ -751,6 +768,9 @@ complete_line_internal (const char *text, p--) ; } + if (reason == handle_brkchars + && c->completer_handle_brkchars != NULL) + (*c->completer_handle_brkchars) (c); if (reason != handle_brkchars && c->completer != NULL) list = (*c->completer) (c, p, word); } diff --git a/gdb/completer.h b/gdb/completer.h index 5b90773..cb9c389 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, 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 c24bca7..df0aeed 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -206,6 +206,79 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty) do_cleanups (cleanup); } +/* Python function called to determine the break characters of a + certain completer. We use dummy variables for the "text" and + "word" arguments for the completer, and call it. We're actually + 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) +{ + cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command); + PyObject *textobj, *wordobj, *resultobj = NULL; + const char dummy_text[] = "dummytext"; + const char dummy_word[] = "dummyword"; + 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. */ + goto done; + } + + textobj = PyUnicode_Decode (dummy_text, sizeof (dummy_text), + host_charset (), NULL); + if (!textobj) + error (_("Could not convert argument to Python string.")); + wordobj = PyUnicode_Decode (dummy_word, sizeof (dummy_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 (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: + + Py_XDECREF (resultobj); + do_cleanups (cleanup); +} + /* Called by gdb for command completion. */ static VEC (char_ptr) * @@ -546,6 +619,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/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp new file mode 100644 index 0000000..9f8a5b2 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-completion.exp @@ -0,0 +1,49 @@ +# 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" + +# This one should always pass. +send_gdb "myfirstcommand /hom\t" +gdb_test_multiple "" "myfirstcommand completion" { + -re "^myfirstcommand /home/$" { + pass "myfirstcommand completion" + } +} + +# Just discarding whatever we typed. +send_gdb "\n" +gdb_test ".*" + +# This is the problematic one. +send_gdb "mysecondcommand /hom\t" +gdb_test_multiple "" "mysecondcommand completion" { + -re "^mysecondcommand /home $" { + fail "mysecondcommand completion (did not correctly complete filename)" + } + -re "^mysecondcommand /home/$" { + pass "mysecondcommand completion" + } +} diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py new file mode 100644 index 0000000..bf7f77b --- /dev/null +++ b/gdb/testsuite/gdb.python/py-completion.py @@ -0,0 +1,38 @@ +# 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 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()