[PATCH 6/6] gdb: improve gdb_rl_find_completion_word for quoted words

[Andrew Burgess <aburgess@redhat.com>]

The function gdb_rl_find_completion_word is very similar to the
readline function _rl_find_completion_word, but was either an older
version of that function, or was trimmed when copying to remove code
which was considered unnecessary.

We maintain this copy because the _rl_find_completion_word function is
not part of the public readline API, and we need to replicate the
functionality of that function as part of the 'complete' command.

Within gdb_rl_find_completion_word when looking for the completion
word, if we don't find a unclosed quoted string (which would become
the completion word) then we scan backwards looking for a word break
character.  For example, given:

  (gdb) complete file /tmp/foo

There is no unclosed quoted string so we end up scanning backwards
from the end looking for a word break character.  In this case the
space after 'file' and before '/tmp/foo' is found, so '/tmp/foo'
becomes the completion word.

However, given this:

  (gdb) complete file /tmp/foo\"

There is still no unclosed quoted string, however, when we can
backwards the '"' (double quotes) are treated as a word break
character, and so we end up using the empty string as the completion
word.

The readline function _rl_find_completion_word avoids this mistake by
using the rl_char_is_quoted_p hook.  This function will return true
for the double quote character as it is preceded by a backslash.  An
earlier commit in this series supplied a rl_char_is_quoted_p function
for the filename completion case, however, gdb_rl_find_completion_word
doesn't call rl_char_is_quoted_p so this doesn't help for the
'complete' case.

In this commit I've copied the code to call rl_char_is_quoted_p from
_rl_find_completion_word into gdb_rl_find_completion_word.

This half solves the problem.  In the case:

  (gdb) complete file /tmp/foo\"

We do now try to complete on the string '/tmp/foo\"', however, when we
reach filename_completer we call back into readline to actually
perform filename completion.  However, at this point the WORD variable
points to a string that still contains the backslash.  The backslash
isn't part of the actual filename, that's just an escape character.

Our expectation is that readline will remove the backslash when
looking for matching filenames.  However, readline contains an
optimisation to avoid unnecessary work trying to remove escape
characters.

The readline variable rl_completion_found_quote is set in the readline
function gen_completion_matches before the generation of completion
matches.  This variable is set to true (non-zero) if there is (or
might be) escape characters within the completion word.

The function rl_filename_completion_function, which generates the
filename matches, only removes escape characters when
rl_completion_found_quote is true.  When GDB generates completions
through readline (e.g. tab completion) then rl_completion_found_quote
is set correctly.

But when we use the 'complete' command we don't pass through readline,
and so gen_completion_matches is never called and
rl_completion_found_quote is not set.  In this case when we call
rl_filename_completion_function readline doesn't remove the escapes
from the completion word, and so in our case above, readline looks for
completions of the exact filename '/tmp/foo\"', that is, the filename
including the backslash.

To work around this problem I've added a new flag to our function
gdb_rl_find_completion_word which is set true when we find any quoting
or escaping.  This matches what readline does.

Then in the 'complete' function we can set rl_completion_found_quote
prior to generating completion matches.

With this done the 'complete' command now works correctly when trying
to complete filenames that contain escaped word break characters.  The
tests have been updated accordingly.
---
 gdb/completer.c                               | 60 +++++++++++++++----
 .../gdb.base/filename-completion.exp          | 12 ++--
 2 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 710c8c206cb..924fd08b469 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -49,7 +49,8 @@
 /* Forward declarations.  */
 static const char *completion_find_completion_word (completion_tracker &tracker,
 						    const char *text,
-						    int *quote_char);
+						    int *quote_char,
+						    bool *found_any_quoting);
 
 static void set_rl_completer_word_break_characters (const char *break_chars);
 
@@ -458,7 +459,9 @@ filename_completer_handle_brkchars (struct cmd_list_element *ignore,
    boundaries of the current word.  QC, if non-null, is set to the
    opening quote character if we found an unclosed quoted substring,
    '\0' otherwise.  DP, if non-null, is set to the value of the
-   delimiter character that caused a word break.  */
+   delimiter character that caused a word break.  FOUND_ANY_QUOTING, if
+   non-null, is set to true if we found any quote characters (single or
+   double quotes, or a backslash) while finding the completion word.  */
 
 struct gdb_rl_completion_word_info
 {
@@ -469,7 +472,7 @@ struct gdb_rl_completion_word_info
 
 static const char *
 gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
-			     int *qc, int *dp,
+			     int *qc, int *dp, bool *found_any_quoting,
 			     const char *line_buffer)
 {
   int scan, end, delimiter, pass_next, isbrk;
@@ -491,6 +494,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
   end = point;
   delimiter = 0;
   quote_char = '\0';
+  bool found_quote = false;
 
   brkchars = info->word_break_characters;
 
@@ -516,6 +520,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
 	  if (quote_char != '\'' && line_buffer[scan] == '\\')
 	    {
 	      pass_next = 1;
+	      found_quote = true;
 	      continue;
 	    }
 
@@ -536,6 +541,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
 	      /* Found start of a quoted substring.  */
 	      quote_char = line_buffer[scan];
 	      point = scan + 1;
+	      found_quote = true;
 	    }
 	}
     }
@@ -549,8 +555,22 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
 	{
 	  scan = line_buffer[point];
 
-	  if (strchr (brkchars, scan) != 0)
-	    break;
+	  if (strchr (brkchars, scan) == 0)
+	    continue;
+
+	  /* Call the application-specific function to tell us whether
+	     this word break character is quoted and should be skipped.
+	     The const_cast is needed here to comply with the readline
+	     API.  The only function we register for rl_char_is_quoted_p
+	     treats the input buffer as 'const', so we're OK.  */
+	  if (rl_char_is_quoted_p != nullptr && found_quote
+	      && (*rl_char_is_quoted_p) (const_cast<char *> (line_buffer),
+					 point))
+	    continue;
+
+	  /* Convoluted code, but it avoids an n^2 algorithm with calls
+	     to char_is_quoted.  */
+	  break;
 	}
     }
 
@@ -574,6 +594,8 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
 	}
     }
 
+  if (found_any_quoting != nullptr)
+    *found_any_quoting = found_quote;
   if (qc != NULL)
     *qc = quote_char;
   if (dp != NULL)
@@ -600,7 +622,7 @@ advance_to_completion_word (completion_tracker &tracker,
 
   int delimiter;
   const char *start
-    = gdb_rl_find_completion_word (&info, NULL, &delimiter, text);
+    = gdb_rl_find_completion_word (&info, nullptr, &delimiter, nullptr, text);
 
   tracker.advance_custom_word_point_by (start - text);
 
@@ -673,7 +695,8 @@ complete_nested_command_line (completion_tracker &tracker, const char *text)
 
   int quote_char = '\0';
   const char *word = completion_find_completion_word (tracker, text,
-						      &quote_char);
+						      &quote_char,
+						      nullptr);
 
   if (tracker.use_custom_word_point ())
     {
@@ -1891,8 +1914,11 @@ complete (const char *line, char const **word, int *quote_char)
 
   try
     {
+      bool found_any_quoting = false;
+
       *word = completion_find_completion_word (tracker_handle_brkchars,
-					      line, quote_char);
+					       line, quote_char,
+					       &found_any_quoting);
 
       /* Completers that provide a custom word point in the
 	 handle_brkchars phase also compute their completions then.
@@ -1902,6 +1928,12 @@ complete (const char *line, char const **word, int *quote_char)
 	tracker = &tracker_handle_brkchars;
       else
 	{
+	  /* Setting this global matches what readline does within
+	     gen_completion_matches.  We need this set correctly in case
+	     our completion function calls back into readline to perform
+	     completion (e.g. filename_completer does this).  */
+	  rl_completion_found_quote = found_any_quoting;
+
 	  complete_line (tracker_handle_completions, *word, line, strlen (line));
 	  tracker = &tracker_handle_completions;
 	}
@@ -2176,7 +2208,7 @@ gdb_completion_word_break_characters () noexcept
 
 static const char *
 completion_find_completion_word (completion_tracker &tracker, const char *text,
-				 int *quote_char)
+				 int *quote_char, bool *found_any_quoting)
 {
   size_t point = strlen (text);
 
@@ -2186,6 +2218,13 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
     {
       gdb_assert (tracker.custom_word_point () > 0);
       *quote_char = tracker.quote_char ();
+      /* This isn't really correct, we're ignoring the case where we found
+	 a backslash escaping a character.  However, this isn't an issue
+	 right now as we only rely on *FOUND_ANY_QUOTING being set when
+	 performing filename completion, which doesn't go through this
+	 path.  */
+      if (found_any_quoting != nullptr)
+	*found_any_quoting = *quote_char != '\0';
       return text + tracker.custom_word_point ();
     }
 
@@ -2195,7 +2234,8 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
   info.quote_characters = rl_completer_quote_characters;
   info.basic_quote_characters = rl_basic_quote_characters;
 
-  return gdb_rl_find_completion_word (&info, quote_char, NULL, text);
+  return gdb_rl_find_completion_word (&info, quote_char, nullptr,
+				      found_any_quoting, text);
 }
 
 /* See completer.h.  */
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index f8a48269528..fd4c407ac23 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -148,10 +148,8 @@ proc run_tests { root } {
 
 	if { $qc eq "'" } {
 	    set dq "\""
-	    set dq_re "\""
 	} else {
 	    set dq "\\\""
-	    set dq_re "\\\\\""
 	}
 
 	test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
@@ -168,8 +166,8 @@ proc run_tests { root } {
 	    } "" "${qc}" false \
 	    "expand filenames containing quotes"
 
-	test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${dq}" \
-	    "file ${qc}${root}/bb1/aa${dq_re}bb${qc}" " " \
+	test_gdb_complete_unique "file ${qc}${root}/bb1/aa${dq}" \
+	    "file ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \
 	    "expand unique filename containing double quotes"
 
 	# It is not possible to include a single quote character
@@ -180,14 +178,12 @@ proc run_tests { root } {
 	if { $qc ne "'" } {
 	    if { $qc eq "" } {
 		set sq "\\'"
-		set sq_re "\\\\'"
 	    } else {
 		set sq "'"
-		set sq_re "'"
 	    }
 
-	    test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${sq}" \
-		"file ${qc}${root}/bb1/aa${sq_re}bb${qc}" " " \
+	    test_gdb_complete_unique "file ${qc}${root}/bb1/aa${sq}" \
+		"file ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \
 		"expand unique filename containing single quote"
 	}
     }
-- 
2.25.4