From patchwork Fri Mar 29 11:42:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 87816 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 767223858D39 for ; Fri, 29 Mar 2024 11:44:20 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 801773858401 for ; Fri, 29 Mar 2024 11:42:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 801773858401 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 801773858401 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711712572; cv=none; b=kUuGDD4wOwreW3zkIsQtTyzLj6dvUaHqVFlb+ZbmRnA1NYfl8m1yHycnKuM0Q8E7Z/ZLQKPrxu11LpPRBjrZrweu4okPMK1nsic3WL6B7wx4W/cgbMVYxFcR4k+zknyrsmEMsGJbAZ8lqjQxWTntXcwWRVzvaVLCw0u/BC4sAxs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711712572; c=relaxed/simple; bh=2GpZJcO8LknM81ozDck7VdAwIq3FW6526Di7CD6vnEM=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=iW+BGx6y7pKlC3mCKfSX3K/mTbJlwKIb1AE3VycEvXnCP6vslLeAdeX9xO8P+2ks1iZnbMv3GcgtYyquXwDG/GQFxLVo66Kwmk7a94A2CQYnNKXpiaWzRf6B9W/FaX1T3ewqQG7bjGbFE6Z3UZBBMj2DZElCzKQ4NSLZZAQJa+Y= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711712567; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XittYiVyRI1Yh/6iznxWAbB0B1avYc8owUO5vG6ZcSg=; b=hf8946JtAWu0IzkJtFZJEXTOSL7ucRJftWna3h8+3qILDszxVb35Xp619HdqajsAikECxI xW4zOaJnFa60KkXKsFyePqVUBwa0G9hay3IpSLxJgAfFkTR6uHo2x8WtLkV6SZdrUN/2Wv vYn/rVRuAOm33SxGfZRJQ6ZYVqSbSY4= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-605-lxAlcwxvO8GZQblGJcNeew-1; Fri, 29 Mar 2024 07:42:45 -0400 X-MC-Unique: lxAlcwxvO8GZQblGJcNeew-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-a46d59152c9so145798466b.0 for ; Fri, 29 Mar 2024 04:42:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711712563; x=1712317363; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XittYiVyRI1Yh/6iznxWAbB0B1avYc8owUO5vG6ZcSg=; b=e2lBF1oOjoLfChdqGzAczf4hKls1k7xTHpA4iRMoU8vapAUH0TTAhjKtl2pWa4KYoJ 8N76XKCD7LPnxLINhhCI8/fd/59/rBbvBCguozyPnbhnOrHMQrKLAIEbD0S506xrYNJX Tm6Ue5tVPLiYadsHgKIR+aNcEQW1147vf2v/cCVkTCdPjizojSEmJiWfDNSDY2NQMWV2 TjC8AEAn1e/TLnY4CG3rv26qPGEbkG8LWRftUJDP7djeoujwBlpm6KjXDjJGwItdOcTO RA9/QyS0tkfmqavokc1nVHYa57EgT/KPPzRlfrGe4x9q17/dzEGEgHNVXFM9f7PdwZ0q ++RQ== X-Gm-Message-State: AOJu0YwDLGVKpJPVCU2mBB7DOFSlYt1ENS0LTnVRPibeh1wGwt8fa69h d8bVDMhqS3YecFvZDeO2h6eZvsuygf2hPkRfbj8rcUc6J5g5xbNiePlpUdpm/KHrov/QsCEmXhv wMdkyH8L+qB96sCAmtwySGy+5lzEVTkFZqAL8gSKn5KUS4wf/HIGMa06J/eToWjYOWW5P0JXEEp wS5fzCCNcAMyIVHtfVGRs4SmPdEvbkbtX7tJCVDOZq76Q= X-Received: by 2002:a17:906:2b55:b0:a47:348d:cc6f with SMTP id b21-20020a1709062b5500b00a47348dcc6fmr1308622ejg.3.1711712562674; Fri, 29 Mar 2024 04:42:42 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG93KFrLL9iCvrem87yweUZO/OdX87KIfvLjj8CwZQkC7hSQTs2vnw8uVwNe5+vIzb2rHOmDQ== X-Received: by 2002:a17:906:2b55:b0:a47:348d:cc6f with SMTP id b21-20020a1709062b5500b00a47348dcc6fmr1308605ejg.3.1711712561968; Fri, 29 Mar 2024 04:42:41 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id ho19-20020a1709070e9300b00a473631e261sm1834434ejc.28.2024.03.29.04.42.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Mar 2024 04:42:41 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 6/6] gdb: improve gdb_rl_find_completion_word for quoted words Date: Fri, 29 Mar 2024 11:42:32 +0000 Message-Id: <3735f2a98f16b040fe5aac440cde075f3f7b1997.1711712401.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org 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 (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, - "e_char); + "e_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" } }