[1/2] gdb: fix for completing a second filename for a command

Message ID 4e11f96cab0a6915bc82894d0f68b34f97b26f03.1726314378.git.aburgess@redhat.com
State New
Headers
Series Filename completion fixes |

Checks

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

Commit Message

Andrew Burgess Sept. 14, 2024, 11:51 a.m. UTC
  After the recent filename completion changes I noticed that the
following didn't work as expected:

  (gdb) file "/path/to/some/file" /path/to/so<TAB>

Now, I know that the 'file' command doesn't actually take multiple
filenames, but currently (and this was true before the recent filename
completion changes too) the completion function doesn't know that the
command only expects a single filename, and should complete any number
of filenames.  And indeed, this works:

  (gdb) file "/path/to/some/file" "/path/to/so<TAB>

In this case I quoted the second path, and now GDB is happy to offer
completions.

It turns out that the problem in the first case is an off-by-one bug
in gdb_completer_file_name_char_is_quoted.  This function tells GDB if
a character within the line being completed is escaped or not.  An
escaped character cannot be a word separator.

The algorithm in gdb_completer_file_name_char_is_quoted is to scan
forward through the line keeping track of whether we are inside double
or single quotes, or if a character follows a backslash.  When we find
an opening quote we skip forward to the closing quote and then check
to see if we skipped over the character we are looking for, if we did
then the character is within the quoted string.

The problem is that this "is character inside quoted string" check
used '>=' instead if '>'.  As a consequence a character immediately
after a quoted string would be thought of as inside the quoted string.

In our first example this means that the single white space character
after the quoted string was thought to be quoted, and was not
considered a word breaking character.  As such, GDB would not try to
complete the second path.  And indeed, if we tried this:

  (gdb) file "/path/to/some/file"    /path/to/so<TAB>

That is, place multiple spaces after the first path, then GDB would
consider the first space as quoted, but the second space is NOT
quoted, and would be a word break.  Now GDB does complete the second
path.

By changing '>=' to '>' in gdb_completer_file_name_char_is_quoted this
bug is resolved, now the original example works and GDB will correctly
complete the second path.

For testing I've factored out the core of one testing proc, and I now
run those tests multiple times, once with no initial path, once with
an initial path in double quotes, once with an initial path in
single quotes, and finally, with an unquoted initial path.
---
 gdb/completer.c                               |   2 +-
 .../gdb.base/filename-completion.exp          | 222 ++++++++++--------
 2 files changed, 121 insertions(+), 103 deletions(-)
  

Patch

diff --git a/gdb/completer.c b/gdb/completer.c
index e8f5e7bb577..64c6611f636 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -258,7 +258,7 @@  gdb_completer_file_name_char_is_quoted (char *string, int eindex)
 	     as quoted, though this might not be completely correct; the
 	     opening and closing quotes are not themselves quoted.  But so
 	     far this doesn't seem to have caused any issues.  */
-	  if (i >= eindex)
+	  if (i > eindex)
 	    return 1;
 	}
       else
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 95a9fbaa857..cdd597f689d 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -113,6 +113,119 @@  proc test_gdb_complete_filename_multiple {
 	$testname
 }
 
+# Run filename completetion tests for those command that accept quoting and
+# escaping of the filename argument.  CMD is the inital part of the command
+# line, paths to complete will be added after CMD.
+#
+# ROOT is the base directory as returned from setup_directory_tree, though,
+# if ROOT is a sub-directory of the user's home directory ROOT might have
+# been modified to replace the $HOME prefix with a single "~" character.
+proc run_quoting_and_escaping_tests_1 { root cmd } {
+    gdb_start
+
+    # Completing 'thread apply all ...' commands uses a custom word
+    # point.  At one point we had a bug where doing this would break
+    # completion of quoted filenames that contained white space.
+    test_gdb_complete_unique "thread apply all hel" \
+	"thread apply all help" " " false \
+	"complete a 'thread apply all' command"
+
+    foreach_with_prefix qc [list "" "'" "\""] {
+	test_gdb_complete_none "$cmd ${qc}${root}/xx" \
+	    "expand a non-existent filename"
+
+	test_gdb_complete_unique "$cmd ${qc}${root}/a" \
+	    "$cmd ${qc}${root}/aaa/" "" false \
+	    "expand a unique directory name"
+
+	test_gdb_complete_unique "$cmd ${qc}${root}/cc2" \
+	    "$cmd ${qc}${root}/cc2${qc}" " " false \
+	    "expand a unique filename"
+
+	test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
+	    "b" "b" {
+		"bb1/"
+		"bb2/"
+	    } "" "${qc}" false \
+	    "expand multiple directory names"
+
+	test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
+	    "c" "c" {
+		"cc1/"
+		"cc2"
+	    } "" "${qc}" false \
+	    "expand mixed directory and file names"
+
+	if { $qc ne "" } {
+	    set sp " "
+	} else {
+	    set sp "\\ "
+	}
+
+	if { $qc eq "'" } {
+	    set dq "\""
+	} else {
+	    set dq "\\\""
+	}
+
+	test_gdb_complete_unique "${cmd} ${qc}${root}/bb2/dir${sp}1/" \
+	    "${cmd} ${qc}${root}/bb2/dir${sp}1/unique${sp}file${qc}" " " \
+	    false \
+	    "expand a unique file name in a directory containing a space"
+
+	test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb2/" \
+	    "d" "ir${sp}" {
+		"dir 1/"
+		"dir 2/"
+	    } "" "${qc}" false \
+	    "expand multiple directory names containing spaces"
+
+	test_gdb_complete_filename_multiple "${cmd} ${qc}${root}/bb2/dir${sp}2/" \
+	    "f" "ile${sp}" {
+		"file 1"
+		"file 2"
+	    } "" "${qc}" false \
+	    "expand contents of a directory containing a space"
+
+	test_gdb_complete_filename_multiple "$cmd ${qc}${root}/aaa/" \
+	    "a" "a${sp}" {
+		"aa bb"
+		"aa cc"
+	    } "" "${qc}" false \
+	    "expand filenames containing spaces"
+
+	test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb1/" \
+	    "a" "a" {
+		"aa\"bb"
+		"aa'bb"
+	    } "" "${qc}" false \
+	    "expand filenames containing quotes"
+
+	test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${dq}" \
+	    "$cmd ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \
+	    "expand unique filename containing double quotes"
+
+	# It is not possible to include a single quote character
+	# within a single quoted string.  However, GDB does not do
+	# anything smart if a user tries to do this.  Avoid testing
+	# this case.  Maybe in the future we'll figure a way to avoid
+	# this situation.
+	if { $qc ne "'" } {
+	    if { $qc eq "" } {
+		set sq "\\'"
+	    } else {
+		set sq "'"
+	    }
+
+	    test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${sq}" \
+		"$cmd ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \
+		"expand unique filename containing single quote"
+	}
+    }
+
+    gdb_exit
+}
+
 # Run filename completetion tests for those command that accept quoting and
 # escaping of the filename argument.
 #
@@ -127,109 +240,14 @@  proc run_quoting_and_escaping_tests { root } {
 				  "target core" "target exec" "target tfile" \
 				  "maint print c-tdesc" "compile file" \
 				  "save gdb-index" "save gdb-index -dwarf-5" } {
-	gdb_start
-
-	# Completing 'thread apply all ...' commands uses a custom word
-	# point.  At one point we had a bug where doing this would break
-	# completion of quoted filenames that contained white space.
-	test_gdb_complete_unique "thread apply all hel" \
-	    "thread apply all help" " " false \
-	    "complete a 'thread apply all' command"
-
-	foreach_with_prefix qc [list "" "'" "\""] {
-	    test_gdb_complete_none "$cmd ${qc}${root}/xx" \
-		"expand a non-existent filename"
-
-	    test_gdb_complete_unique "$cmd ${qc}${root}/a" \
-		"$cmd ${qc}${root}/aaa/" "" false \
-		"expand a unique directory name"
-
-	    test_gdb_complete_unique "$cmd ${qc}${root}/cc2" \
-		"$cmd ${qc}${root}/cc2${qc}" " " false \
-		"expand a unique filename"
-
-	    test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
-		"b" "b" {
-		    "bb1/"
-		    "bb2/"
-		} "" "${qc}" false \
-		"expand multiple directory names"
-
-	    test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
-		"c" "c" {
-		    "cc1/"
-		    "cc2"
-		} "" "${qc}" false \
-		"expand mixed directory and file names"
-
-	    if { $qc ne "" } {
-		set sp " "
-	    } else {
-		set sp "\\ "
-	    }
-
-	    if { $qc eq "'" } {
-		set dq "\""
-	    } else {
-		set dq "\\\""
-	    }
-
-	    test_gdb_complete_unique "${cmd} ${qc}${root}/bb2/dir${sp}1/" \
-		"${cmd} ${qc}${root}/bb2/dir${sp}1/unique${sp}file${qc}" " " \
-		false \
-		"expand a unique file name in a directory containing a space"
-
-	    test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb2/" \
-		"d" "ir${sp}" {
-		    "dir 1/"
-		    "dir 2/"
-		} "" "${qc}" false \
-		"expand multiple directory names containing spaces"
-
-	    test_gdb_complete_filename_multiple "${cmd} ${qc}${root}/bb2/dir${sp}2/" \
-		"f" "ile${sp}" {
-		    "file 1"
-		    "file 2"
-		} "" "${qc}" false \
-		"expand contents of a directory containing a space"
-
-	    test_gdb_complete_filename_multiple "$cmd ${qc}${root}/aaa/" \
-		"a" "a${sp}" {
-		    "aa bb"
-		    "aa cc"
-		} "" "${qc}" false \
-		"expand filenames containing spaces"
-
-	    test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb1/" \
-		"a" "a" {
-		    "aa\"bb"
-		    "aa'bb"
-		} "" "${qc}" false \
-		"expand filenames containing quotes"
-
-	    test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${dq}" \
-		"$cmd ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \
-		"expand unique filename containing double quotes"
-
-	    # It is not possible to include a single quote character
-	    # within a single quoted string.  However, GDB does not do
-	    # anything smart if a user tries to do this.  Avoid testing
-	    # this case.  Maybe in the future we'll figure a way to avoid
-	    # this situation.
-	    if { $qc ne "'" } {
-		if { $qc eq "" } {
-		    set sq "\\'"
-		} else {
-		    set sq "'"
-		}
-
-		test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${sq}" \
-		    "$cmd ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \
-		    "expand unique filename containing single quote"
-	    }
+	# Try each test placing the filename as the first argument
+	# then again with a quoted string immediately after the
+	# command.  This works because the filename completer will
+	# complete any number of filenames, even if the command only
+	# takes a single filename.
+	foreach_with_prefix filler { "" " \"xxx\"" " 'zzz'" " yyy"} {
+	    run_quoting_and_escaping_tests_1 $root "$cmd$filler"
 	}
-
-	gdb_exit
     }
 }