Bug 16699 - GDB Python command completion with overriden complete vs. completer class
Summary: GDB Python command completion with overriden complete vs. completer class
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: python (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: ---
Assignee: Sergio Durigan Junior
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-12 19:52 UTC by Sergio Durigan Junior
Modified: 2015-04-08 22:29 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Durigan Junior 2014-03-12 19:52:54 UTC
This bug is a clone of a Red Hat bug.

create a file completion.py and add the below lines:

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

(gdb) source completion.py 
(gdb) mysecondcommand /home      <- that is the issue  
(gdb) myfirstcommand /home/
fotis/      lost+found/ 


Actual results:

(gdb) myfirstcommand /h<TAB>
it completes to /home/ and offers files within that directory

However for:
(gdb) mysecondcommand /h<TAB>
it completes to /home<SPACE> 

My command doesn't take directories, so I really want completion to work as for the first command.

Is this expected GDB behavior? How do I make it work?

Expected results:

(gdb) mysecondcommand /home/
fotis       lost+found
Comment 1 Sergio Durigan Junior 2014-03-12 19:53:19 UTC
Testing a patch.
Comment 2 Sergio Durigan Junior 2014-03-13 02:10:51 UTC
Proposed patch: https://sourceware.org/ml/gdb-patches/2014-03/msg00301.html
Comment 3 Sourceware Commits 2014-09-03 20:35:32 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  7d793aa9f0986828d5dde8f3811a7adafc38b6b4 (commit)
      from  62661c935d1f299e8f9daeebd5559cd1c2d84712 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7d793aa9f0986828d5dde8f3811a7adafc38b6b4

commit 7d793aa9f0986828d5dde8f3811a7adafc38b6b4
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Wed Sep 3 16:30:28 2014 -0400

    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<TAB>
        (gdb) myfirstcommand /home/
                                   ^
        ...
        (gdb) mysecondcommand /hom<TAB>
        (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  <sergiodj@redhat.com>
    
    	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)
    	<completer_handle_brkchars>: 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  <sergiodj@redhat.com>
    
    	PR python/16699
    	* gdb.python/py-completion.exp: New file.
    	* gdb.python/py-completion.py: Likewise.

-----------------------------------------------------------------------

Summary of changes:
 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
Comment 4 Sergio Durigan Junior 2014-09-03 20:37:09 UTC
Fix pushed.
Comment 5 Sourceware Commits 2015-04-08 22:29:01 UTC
The master branch has been updated by Sergio Durigan Junior <sergiodj@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6d62641c832525382336c1b04731d85cb6c398e7

commit 6d62641c832525382336c1b04731d85cb6c398e7
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Wed Apr 8 18:27:10 2015 -0400

    Fix Python completion when using the "complete" command
    
    This patch is related to PR python/16699, and is an improvement over the
    patch posted here:
    
      <https://sourceware.org/ml/gdb-patches/2014-03/msg00301.html>
    
    Keith noticed that, when using the "complete" command on GDB to complete
    a Python command, some strange things could happen.  In order to
    understand what can go wrong, I need to explain how the Python
    completion mechanism works.
    
    When the user requests a completion of a Python command by using TAB,
    GDB will first try to determine the right set of "brkchars" that will be
    used when doing the completion.  This is done by actually calling the
    "complete" method of the Python class.  Then, when we already know the
    "brkchars" that will be used, we call the "complete" method again, for
    the same values.
    
    If you read the thread mentioned above, you will see that one of the
    design decisions was to make the "cmdpy_completer_helper" (which is the
    function the does the actual calling of the "complete" method) cache the
    first result of the completion, since this result will be used in the
    second call, to do the actual completion.
    
    The problem is that the "complete" command does not process the
    brkchars, and the current Python completion mechanism (improved by the
    patch mentioned above) relies on GDB trying to determine the brkchars,
    and then doing the completion itself.  Therefore, when we use the
    "complete" command instead of doing a TAB-completion on GDB, there is a
    scenario where we can use the invalid cache of a previous Python command
    that was completed before.  For example:
    
      (gdb) A <TAB>
      (gdb) complete B
      B value1
      B value10
      B value2
      B value3
      B value4
      B value5
      B value6
      B value7
      B value8
      B value9
      (gdb) B <TAB>
      comp1   comp2   comp4   comp6   comp8
      comp10  comp3   comp5   comp7   comp9
    
    Here, we see that "complete B " gave a different result than "B <TAB>".
    The reason for that is because "A <TAB>" was called before, and its
    completion results were "value*", so when GDB tried to "complete B " it
    wrongly answered with the results for A.  The problem here is using a
    wrong cache (A's cache) for completing B.
    
    We tried to come up with a solution that would preserve the caching
    mechanism, but it wasn't really possible.  So I decided to completely
    remove the cache, and doing the method calling twice for every
    completion.  This is not optimal, but I do not think it will impact
    users noticeably.
    
    It is worth mentioning another small issue that I found.  The code was
    doing:
    
      wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL);
    
    which is totally wrong, because using "sizeof" here will lead to always
    the same result.  So I changed this to use "strlen".  The testcase also
    catches this problem.
    
    Keith kindly expanded the existing testcase to cover the problem
    described above, and everything is passing.
    
    gdb/ChangeLog:
    2015-04-08  Sergio Durigan Junior  <sergiodj@redhat.com>
    
    	PR python/16699
    	* python/py-cmd.c (cmdpy_completer_helper): Adjust function to not
    	use a caching mechanism.  Adjust comments and code to reflect
    	that.  Replace 'sizeof' by 'strlen' when fetching 'wordobj'.
    	(cmdpy_completer_handle_brkchars): Adjust call to
    	cmdpy_completer_helper.  Call Py_XDECREF for 'resultobj'.
    	(cmdpy_completer): Likewise.
    
    gdb/testsuite/ChangeLog:
    2015-04-08  Keith Seitz  <keiths@redhat.com>
    
    	PR python/16699
    	* gdb.python/py-completion.exp: New tests for completion.
    	* gdb.python/py-completion.py (CompleteLimit1): New class.
    	(CompleteLimit2): Likewise.
    	(CompleteLimit3): Likewise.
    	(CompleteLimit4): Likewise.
    	(CompleteLimit5): Likewise.
    	(CompleteLimit6): Likewise.
    	(CompleteLimit7): Likewise.