From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id sdj6OklQrWp82hsAWB0awg (envelope-from ) for ; Fri, 18 Sep 2026 10:52:57 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YtiZcjr7; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id EB8981E01F; Fri, 18 Sep 2026 10:52:57 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 9FD5F1E01F for ; Fri, 18 Sep 2026 10:52:55 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C79C44BB1C25 for ; Fri, 18 Sep 2026 14:52:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C79C44BB1C25 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YtiZcjr7 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 022224BA900C for ; Fri, 18 Sep 2026 14:52:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 022224BA900C Authentication-Results: sourceware.org; dmarc=pass (p=quarantine 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 022224BA900C Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789743143; cv=none; b=Irh9y1RqMFjmrxMGkI1XzoJepnYENs76T/3reUYtWz9OIBsfbwD3qTIkfnHBaUF3vqIaVfZ6fUTGivpUNAW8l88jLx0zk4VNvsbStUvFGz4/sDxbp09nMZ6DgBUqR+KbJmbX7DrANcoUZn9fWJv0tDzbFVHkT6FmUcgeNlIvyhw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1789743143; c=relaxed/simple; bh=zvN0txktxw32uj+L4+mB3P7DNpoCgF8QNCsExDRy4Wk=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=t3cw/5DGCTk8G2l436E/FUhnPOWYYZl/+d54euk727Hlzo6djZQ9WNjyxc1F+WNQMrHESW9h6BdUQfII/3aYqsgujuyYKu7LpzwnEA8zr57KCOSRE6PqZrX3BywAWAsXtu9szzrh7vOpumpUIxgmV9/zbFR8iQySVMbEDdq88CM= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YtiZcjr7 DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 022224BA900C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1789743142; 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=zj3ppTCUCMLL7RtNP2ec8GiSKjwmfCnhen5M4giH1u4=; b=YtiZcjr7sAyRBv3qYvJcxF9OqDf5/WqxbAOstsc4is5yoRas7LMpXPBI0CwGL6pm6orlJw Y126rp+RT9hvIjtTcH0h7FcWLl8czP9KNAUktP9Th2c5IFXP/sQDabuIbUt/OlpXz3I1Jn nawC7ozK0+kV/9VrURJh99cutIQgicE= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-536-IXTvHUXJNRmFbiuw63tkzw-1; Fri, 18 Sep 2026 10:52:21 -0400 X-MC-Unique: IXTvHUXJNRmFbiuw63tkzw-1 X-Mimecast-MFC-AGG-ID: IXTvHUXJNRmFbiuw63tkzw_1789743140 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-c210c66672eso108190066b.2 for ; Fri, 18 Sep 2026 07:52:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1789743140; x=1790347940; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=zj3ppTCUCMLL7RtNP2ec8GiSKjwmfCnhen5M4giH1u4=; b=dDN/gxGVI6lfGKKdEAa0MsJzhKay3M3FrjOTQTEWZTqt2JaZelNlbDvD+cpTqyhUwh tKWc74d68b/JB5sXNGLqI+fHqdDVrvNDn58FuFlu7ZKRLyPtry6vV54Jgbsd8TWE/OX1 d90v2gIveesLJ2IheCJO9UknIh0EkEZ7Hlyb96wlxLi1Ca40k+dI+TX10i2PWetOUrQ+ 4Q/h/5VmOq71zvYH7Nnts/paGayi2GFzNbaHVcWotV21eAKyVZGN+YNZEwf7uuD6lAUu Ojwx0fDzE/BiS3JKySKVxMNKGRoaQLijQJSut55cEHFFxdPVFt2X6HqbGAqDWn7oSLCq FChg== X-Gm-Message-State: AFuF++l1nA0YAudWG7E9yk7HuSvfZ7BR5aUwiVUuCzMV/Lrehu8z2zz6 Dt4xmKGzYrIi8P3vcYaE+Z+mQ6KFSFU/B04MRMPGYz36pTqElQA6w0xMj2RRuMurqsJOVtZzPvF O9FrLtSFD0gHwKvge98sxZy2+E1X2liflw6hqhgOEtZasQQ9BHOQeVjHXCugmELmW2Z5oCRroDZ +/J42+vZ5/mWldgqgpf5XcJqVt7tpyvCkyxNuKAQI0dggWKzQ= X-Gm-Gg: AYBFou1NI2hUDNnpkH/trtf6ex16ph1U4lORC/JLA1wkxv7QFQvFUl9q/pQl8z/iuPX trBenQ0JDA+HQntBD3/KhoZP2ly+SrJPC2aS8EURB0m5epeEV29Q51epUhTA46TKIhWjuO+7GnR KKqSrb2UPbO/uTpcFsCC/ULCSIpHMtMF3RnIP9RnUbhoJT47asqX21ga403J9Ll9sz6aLUzjwTq ExMk/c31HdGh5tO2Tlk8KfFCBRPQKYhIhxkPe2ZmjQRUNPLN62pyfE2MpFtpYyydijrZ28nd3fL TxQHceCZJFvqg6x+orFm8HaWpibs0fEfDCxgFmZaHBTsrj/ynLzc05K/IgOvSFyOlwC6KIP1 X-Received: by 2002:a17:907:c16:b0:c29:f5d8:9c7f with SMTP id a640c23a62f3a-c2a15ab2b5amr257500066b.46.1789743139011; Fri, 18 Sep 2026 07:52:19 -0700 (PDT) X-Received: by 2002:a17:907:c16:b0:c29:f5d8:9c7f with SMTP id a640c23a62f3a-c2a15ab2b5amr257496766b.46.1789743138153; Fri, 18 Sep 2026 07:52:18 -0700 (PDT) Received: from localhost ([83.216.138.214]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c2a1bb06bcbsm67999166b.12.2026.09.18.07.52.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Sep 2026 07:52:17 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Eli Zaretskii , Guinevere Larsen Subject: [PUSHED] gdb/gdbserver: rework argument splitting and joining Date: Fri, 18 Sep 2026 15:52:15 +0100 Message-Id: <98211c87c9483a0c4b3dca29444d55bbe4ccc702.1789742711.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <87a7403694d088d34a5f8f8da7e08fc4c04e2f84.1780429677.git.aburgess@redhat.com> References: <87a7403694d088d34a5f8f8da7e08fc4c04e2f84.1780429677.git.aburgess@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: E8bUvWXitYqIZ46-18Ti-Npgd5_owJ4Pu_F-N31QypE_1789743140 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit content-type: text/plain; charset="US-ASCII"; x-default=true 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~public-inbox=simark.ca@sourceware.org I've gone ahead and pushed this patch. This only effects old gdbservers now, as by default inferior arguments are sent as a single string, but it felt a shame to discard this work, which I think does make the legacy approach to argument passing a little better. The version I pushed is below, changes are: * Rebase and retest. * Spelling and grammar fixes throughout. * Refactored some common constants into a new header file in gdbsuport/. * No changes to the underlying code though since my original post. I wanted to land this early in the GDB 19 cycle in case this does cause problems for anyone. If anyone does run into any problems, then please let me know. Thanks, Andrew ~~~~~ This commit attempts to improve how GDB splits and joins inferior arguments when passing them to a remote target. The splitting and joining is now the legacy approach for passing arguments to a remote target; the preference is to pass arguments as a single string, however, if GDB is connecting to a target that doesn't support passing inferior arguments as a single string then the joining and splitting approach is still used, and this commit tries to improve that approach a little. Currently when GDB passes arguments to a remote target this is done by using the gdb_argv class. This is the same technique used to split the argument vector in order to start an inferior without a shell. This approach has a problem. Consider a native target, and the user sets the inferior arguments like this: (gdb) set args \$VAR In this case the user has escaped the '$' character, removing its special meaning. When started under a shell the '\$VAR' will not result in variable expansion, and the inferior will see a literal '$VAR'. When startup-with-shell is off then gdb_argv is used to split the inferior arguments, and gdb_argv also removes the backslash escape, so the inferior again sees a literal '$VAR' string. I think this consistency makes sense. Now consider if the user sets the inferior arguments like this: (gdb) set args $VAR In the startup-with-shell case the shell will expand '$VAR' and the inferior will see whatever the value of '$VAR' was. But if startup-with-shell is off then there is no shell, and the inferior will just be passed a literal '$VAR' string. Given the above, when we consider using gdb_argv for splitting inferior arguments in order to pass them to a remote target, the problem we have is this: there is a many-to-one relationship between input strings and output strings. Both '\$VAR' and '$VAR' result in an output string of '$VAR'. The remote target can't know what the actual input was. If the remote adds the escaping then (when startup-with-shell is on) the shell will not substitute variables when it should, and if the remote doesn't add escaping then the shell will try to substitute variables when it shouldn't. So I think we have to admit that the current approach is not ideal, but we cannot change things too much; in simple cases (i.e. without too many special shell characters) the existing approach does work, so if we do something too different then existing targets are going to break even for simple cases. The current approach then is for the remote target to escape any special shell characters within the arguments received from GDB. This is too aggressive. An ideal solution might be to have GDB remove quoting, but to keep, or apply all required escaping, and for the remote target to do nothing but join the incoming arguments with a single whitespace character. This would certainly be easiest all around, but would break a lot of backward compatibility, for example given this: (gdb) set args 'ab cd' GDB could strip the single quotes, and escape the whitespace, sending the argument to the remote target as (ab\ cd) -- without the parentheses -- then on the remote end the target could use this argument directly. However, if we adopt this approach existing targets, those that escape special shell characters, are likely to do the wrong thing, it's likely that both the backslash and space would be escaped. So, what we are searching for is a solution that sits somewhere halfway between GDB performs all the escaping, and the current situation, where the target is expected to apply the escaping. I propose that the rules be something like: + GDB removes the quotes, and escapes all special shell characters except for the following: + Whitespace is not escaped, the remote is expected to escape any whitespace within an argument, + Quote characters are not escaped, the remote is expected to escape any single or double quote characters within an argument. + Any other special shell characters will have been escaped, if needed, by GDB, and the escaping included in the transmitted argument. The solution presented here takes into account unquoted arguments, as well as single and double quoted arguments. WARNING: This is a potentially breaking change in the remote protocol. A user, using an existing remote target, that handles inferior arguments in a particular way may find that this change now causes some cases to behave differently. My hope is that by handling whitespace and quotes differently we can preserve enough behaviour that the problem cases are not too common. As part of this commit I updated gdb/unittests/remote-arg-selftests.c, and it's possible to see examples of when the arguments passed from GDB to the remote will change, and when they remain the same. And there are some tests in gdb.base/startup-with-shell.exp that now pass even when using a remote target board and when the single-inferior-arg feature is turned off. All of the other argument passing tests, e.g. gdb.base/args.exp, and gdb.base/inferior-args.exp continue to pass, and also already run with the single-inferior-arg feature both enabled and disabled, so the changes in this commit should be well tested already. What isn't addressed at all in this commit is parameter expansion, command substitution, or arithmetic expansion. These are similar to quotes, but instead of single or double quotes the argument is surrounded with ${...}, $(...), or $((....)). However, unlike single or double quotes I don't think that we can remove the quoting in these cases, if we did, how would the remote target know what to add back? However, I'm not proposing to even try to address these cases. The single-inferior-arg feature has already been merged into GDB, and is the default when using the latest version of GDB and gdbserver. My feeling is that the old split/join approach will never be able to handle every corner case, and we should just accept that. This work was inspired by this series: https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/ I originally posted this patch as part of this series: https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com Since then I've been merging the individual patches from that series separately, this is the last remaining patch that I think is worth merging. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392 Reviewed-By: Eli Zaretskii Tested-By: Guinevere Larsen --- gdb/NEWS | 8 + gdb/doc/gdb.texinfo | 13 + gdb/testsuite/gdb.base/args.exp | 45 ---- gdb/testsuite/gdb.base/startup-with-shell.exp | 27 +-- gdb/unittests/remote-arg-selftests.c | 29 ++- gdbsupport/common-inferior.cc | 14 +- gdbsupport/remote-args.cc | 224 +++++++++++++++++- gdbsupport/remote-args.h | 2 +- gdbsupport/special-shell-chars.h | 35 +++ 9 files changed, 309 insertions(+), 88 deletions(-) create mode 100644 gdbsupport/special-shell-chars.h diff --git a/gdb/NEWS b/gdb/NEWS index bce3b358c3f..77ad2d3fc22 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -25,6 +25,14 @@ support. The AMX registers themselves are not yet readable, but other xsave state can be read. +* There have been changes to how inferior arguments are passed to + remote targets. When using the latest GDB and gdbserver the new + single-inf-arg feature will be used for argument passing. When + using remote targets that don't support single-inf-arg, GDB will now + escape special shell characters (except for whitespace and quotes) + within arguments before passing them to the remote target. Remote + targets will need updating to take this into account. + * New targets i386 Windows/MSVC i[34567]86-*-windows-msvc diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 0bb0a46738f..a24f67cb8de 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -44169,6 +44169,19 @@ Packets @var{argument} string, which includes all inferior arguments, separated with whitespace. +If @value{GDBN} didn't send @samp{single-inf-arg}, or the remote +didn't choose to use this feature, then @value{GDBN} will split the +inferior arguments before passing them to the remote target. Single +and double quotes surrounding any argument will have been removed by +@value{GDBN}. Any special shell characters within each argument will +have been escaped with a backslash character except for quote and +whitespace characters. These will not be escaped within the arguments; +the remote target is expected to escape these characters itself. + +These slightly complex rules exist for backward compatibility reasons. +When implementing a new remote target, it is suggested that the +@samp{single-inf-arg} feature be supported. + @c FIXME: What about non-stop mode? This packet is only available in extended mode (@pxref{extended mode}). diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp index a0db77664c4..f7297d57281 100644 --- a/gdb/testsuite/gdb.base/args.exp +++ b/gdb/testsuite/gdb.base/args.exp @@ -30,37 +30,6 @@ if {[build_executable $testfile.exp $testfile $srcfile] == -1} { return } -# Assuming a running GDB. Check the status of the single inferior -# argument feature. When this feature is on GDB passes inferior -# arguments as a single combined string. When this feature is off GDB -# will split the inferior arguments into multiple separate strings. -# -# Return true when arguments are being split, or false when a single -# combined string is being sent. - -proc is_argument_splitting_on {} { - set arg_splitting true - - gdb_test_multiple "show remote single-inferior-argument-feature-packet" "" { - -re -wrap "Support for the 'single-inferior-argument-feature' packet on the current remote target is \"(on|off)\"\\.(.*)" { - set value $expect_out(1,string) - pass $gdb_test_name - if { $value eq "on" } { - set arg_splitting false - } - } - -re -wrap "Support for the 'single-inferior-argument-feature' packet on the current remote target is \"auto\", currently (enabled|disabled)\\.(.*)" { - set value $expect_out(1,string) - pass $gdb_test_name - if { $value eq "enabled" } { - set arg_splitting false - } - } - } - - return $arg_splitting -} - # NAME is the name to use for the tests and ARGLIST is the list of # arguments that are passed to GDB when it is started. # @@ -127,20 +96,6 @@ proc args_test { name arglist {re_esc_list {}} {re_no_esc_list {}} } { set arg {\\\"} } - # If we are starting with a shell, we're not escaping - # special shell characters in arguments passed to GDB, - # we're using a remote target, and the arguments contain - # a shell variable (indicated with a '$'), then this - # test will currently fail if we are splitting the - # arguments. - if { $startup_with_shell eq "on" - && $arg_flag eq "no-escape-args" - && [gdb_protocol_is_remote] - && [string first "\$" $arglist] != -1 - && [is_argument_splitting_on] } { - setup_xfail "*-*-*" gdb/28392 - } - gdb_test "print argv\[$i\]" \ "${::valnum_re} = $::hex \"$arg\"" \ "argv\[$i\] for $name" diff --git a/gdb/testsuite/gdb.base/startup-with-shell.exp b/gdb/testsuite/gdb.base/startup-with-shell.exp index 6f30841bdb8..ac5f35247e5 100644 --- a/gdb/testsuite/gdb.base/startup-with-shell.exp +++ b/gdb/testsuite/gdb.base/startup-with-shell.exp @@ -58,11 +58,8 @@ proc initial_setup_simple { startup_with_shell run_args } { # it to ON_RE if startup-with-shell is on, otherwise compare to # OFF_RE. # -# If PROBLEMATIC_ON is true then when startup-with-shell is on we -# expect the comparison to fail, so setup an xfail. -# # TESTNAME is a string used in the test names. -proc run_test { args on_re off_re testname { problematic_on false } } { +proc run_test { args on_re off_re testname } { foreach startup_with_shell { "on" "off" } { with_test_prefix "$testname, startup_with_shell: ${startup_with_shell}" { if {![initial_setup_simple $startup_with_shell $args]} { @@ -71,14 +68,8 @@ proc run_test { args on_re off_re testname { problematic_on false } } { if { $startup_with_shell } { set re $on_re - set problematic $problematic_on } else { set re $off_re - set problematic false - } - - if { $problematic } { - setup_xfail "*-*-*" gdb/28392 } gdb_test "print argv\[1\]" "${::valnum_re} = $::hex $re" $testname @@ -94,15 +85,14 @@ proc run_test_same { args re testname } { } # Run the actual tests -proc run_all_tests { { is_remote_with_split_args false } } { +proc run_all_tests { } { # The regexp to match a single '\' character. set bs "\\\\" run_test "$::unique_file_dir/*.unique-extension" \ "\"$::unique_file\"" \ "\"$::unique_file_dir/\\\*\.unique-extension\"" \ - "arg is glob" \ - $is_remote_with_split_args + "arg is glob" run_test_same "$::unique_file_dir/\\*.unique-extension" \ "\"$::unique_file_dir/\\\*\.unique-extension\"" \ @@ -113,8 +103,7 @@ proc run_all_tests { { is_remote_with_split_args false } } { run_test "\$TEST" \ "\"1234\"" \ "\"\\\$TEST\"" \ - "arg is shell variable" \ - $is_remote_with_split_args + "arg is shell variable" run_test_same "\\\$TEST" \ "\"\\\$TEST\"" \ @@ -124,14 +113,12 @@ proc run_all_tests { { is_remote_with_split_args false } } { run_test "\$(echo foo)" \ "\"foo\"" \ "\"\\\$\\(echo\"" \ - "arg is parameter expansion, command execution" \ - $is_remote_with_split_args + "arg is parameter expansion, command execution" run_test "\$((2 + 3))" \ "\"5\"" \ "\"\\\$\\(\\(2\"" \ - "arg is parameter expansion, expression evaluation" \ - $is_remote_with_split_args + "arg is parameter expansion, expression evaluation" run_test_same "\"\\a\"" \ "\"${bs}${bs}a\"" \ @@ -183,7 +170,7 @@ if {[target_info gdb_protocol] == "extended-remote"} { with_test_prefix "single-inferior-arg disabled" { save_vars { GDBFLAGS } { append GDBFLAGS " -ex \"set remote single-inferior-argument-feature-packet off\"" - run_all_tests true + run_all_tests } } } diff --git a/gdb/unittests/remote-arg-selftests.c b/gdb/unittests/remote-arg-selftests.c index 7d4e63deefd..e55a7185c61 100644 --- a/gdb/unittests/remote-arg-selftests.c +++ b/gdb/unittests/remote-arg-selftests.c @@ -40,22 +40,41 @@ struct arg_test_desc /* The list of tests. */ arg_test_desc desc[] = { + { "", {}, "" }, { "abc", { "abc" }, "abc" }, { "a b c", { "a", "b", "c" }, "a b c" }, { "\"a b\" 'c d'", { "a b", "c d" }, "a\\ b c\\ d" }, { "\\' \\\"", { "'", "\"" }, "\\' \\\"" }, - { "'\\'", { "\\" }, "\\\\" }, - { "\"\\\\\" \"\\\\\\\"\"", { "\\", "\\\"" }, "\\\\ \\\\\\\"" }, + { "'\\'", { "\\\\" }, "\\\\" }, + { "\"\\\\\" \"\\\\\\\"\"", { "\\\\", "\\\\\"" }, "\\\\ \\\\\\\"" }, { "\\ \" \" ' '", { " ", " ", " "}, "\\ \\ \\ " }, { "\"'\"", { "'" }, "\\'" }, - { "'\"' '\\\"'", { "\"", "\\\"" } , "\\\" \\\\\\\""}, + { "'\"' '\\\"'", { "\"", "\\\\\"" } , "\\\" \\\\\\\""}, { "\"first arg\" \"\" \"third-arg\" \"'\" \"\\\"\" \"\\\\\\\"\" \" \" \"\"", - { "first arg", "", "third-arg", "'", "\"", "\\\""," ", "" }, + { "first arg", "", "third-arg", "'", "\"", "\\\\\""," ", "" }, "first\\ arg '' third-arg \\' \\\" \\\\\\\" \\ ''"}, { "\"\\a\" \"\\&\" \"\\#\" \"\\<\" \"\\^\"", - { "\\a", "\\&", "\\#" , "\\<" , "\\^"}, + { "\\\\a", "\\\\\\&", "\\\\\\#" , "\\\\\\<" , "\\\\\\^"}, + "\\\\a \\\\\\& \\\\\\# \\\\\\< \\\\\\^" }, + /* The following entry produces the same split output as the previous + one: inside double quotes, \x where x is not a special character, is + considered a literal backslash followed by a literal 'x', equivalent + to the explicitly escaped form \\x. */ + { "\"\\\\a\" \"\\\\&\" \"\\\\#\" \"\\\\<\" \"\\\\^\"", + { "\\\\a", "\\\\\\&", "\\\\\\#" , "\\\\\\<" , "\\\\\\^"}, "\\\\a \\\\\\& \\\\\\# \\\\\\< \\\\\\^" }, { "1 '\n' 3", { "1", "\n", "3" }, "1 '\n' 3" }, + { "abc* abc\\* abc\\\\*", { "abc*", "abc\\*", "abc\\\\*" }, + "abc* abc\\* abc\\\\*" }, + { "\"ab\\ cd\" \"ef\\'gh\"", { "ab\\\\ cd", "ef\\\\'gh" }, + "ab\\\\\\ cd ef\\\\\\'gh" }, + { "\"$VAR\" \"ab`foo`cd\"", { "$VAR", "ab`foo`cd" }, "$VAR ab`foo`cd" }, + { "'$VAR'", { "\\$VAR" }, "\\$VAR" }, + /* Unterminated quotes are treated as extending to end-of-string. */ + { "\"abc", { "abc" }, "abc" }, + { "'abc", { "abc" }, "abc" }, + { "a \"b c", { "a", "b c" }, "a b\\ c" }, + { "a 'b c", { "a", "b c" }, "a b\\ c" }, }; /* Run the remote argument passing self tests. */ diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc index 8134761ef73..e8783e55bc8 100644 --- a/gdbsupport/common-inferior.cc +++ b/gdbsupport/common-inferior.cc @@ -19,6 +19,7 @@ along with this program. If not, see . */ #include "gdbsupport/common-inferior.h" +#include "gdbsupport/special-shell-chars.h" /* See common-inferior.h. */ @@ -100,18 +101,7 @@ escape_characters (const char *arg, const char *special) static std::string escape_shell_characters (const char *arg) { -#ifdef __MINGW32__ - /* This holds all the characters considered special to the - Windows shells. */ - static const char special[] = "\"!&*|[]{}<>?`~^=;, \t\n"; -#else - /* This holds all the characters considered special to the - typical Unix shells. We include `^' because the SunOS - /bin/sh treats it as a synonym for `|'. */ - static const char special[] = "\"!#$&*()\\|[]{}<>?'`~^; \t\n"; -#endif - - return escape_characters (arg, special); + return escape_characters (arg, special_shell_characters); } /* Return a version of ARG that has quote characters and white space diff --git a/gdbsupport/remote-args.cc b/gdbsupport/remote-args.cc index 2abf61aa86b..66724c75130 100644 --- a/gdbsupport/remote-args.cc +++ b/gdbsupport/remote-args.cc @@ -18,17 +18,231 @@ #include "gdbsupport/remote-args.h" #include "gdbsupport/common-inferior.h" #include "gdbsupport/buildargv.h" +#include "gdbsupport/special-shell-chars.h" + +/* This file contains the function used for splitting an argument string + into separate arguments in preparation for sending the arguments over + the remote protocol, as well as the function to merge the separate + arguments back together into a single argument string. + + The logic within these functions is slightly more complex than it should + be. This is in order to maintain some level of backward compatibility. + + In the following text, examples of command line arguments will be given. + To avoid confusion, arguments, and argument strings, will be delimited + with '(' and ')', the parentheses are not part of the argument itself. + This is clearer than using quotes, as some of the examples will include + quotes within the arguments. + + Historically, the algorithm used to split the argument string into + separate arguments removed a level of quoting from the arguments. For + example consider the following argument string: (abc* abc\*). The + historic algorithm would split this into (abc*) and (abc*). Notice that + the two arguments are identical. On the remote end we are now destined + for failure; either we apply an escape to both '*' characters, or we + apply an escape to neither. In either case, we get one of the arguments + wrong. The historic approach was just broken. + + However, the historic approach has been in place for many years. + Clearly not all arguments were corrupted in the manner described above, + so lots of things did work. For example, the string: ("ab cd" "ef") + will be split into (ab cd) and (ef). And the string ('"') will become + just ("). + + What we can observe from all of these examples is that the historic + approach at the remote end was to simply apply an escape to every + special shell character, quotes, white space, as well as every other + special character (e.g. (*)). The problem with this approach is that + sometimes special shell characters shouldn't be escaped. + + If we could start from scratch, then the simple approach would be to + retain all escaping while splitting the argument string, and, where + quotes are used, convert this into backslash escaping as needed. Thus + the argument string ("ab cd" "ef") would become (ab\ cd) and (ef). And + the argument string (abc* abc\*) would become (abc*) (abc\*). On the + remote end, joining these arguments is as simple as concatenation with a + single space character between. + + However, if we change to this approach now, then consider ("ab cd"). + Previously this was sent as (ab cd), but now it would be (ab\ cd). This + breaks backward compatibility. Remember, we need to think about newer + GDB being used with older gdbserver, or newer GDB being used with + targets that are not gdbserver at all. + + And so, this is where the complexity comes in. + + The strategy here is to split the arguments, removing all double and + single quotes. While removing quotes, special shell characters are + escaped as needed. But white space characters and quote characters + are not escaped. These characters must always be escaped, and so we can + safely drop the escape in these cases. This provides some degree of + backward compatibility, but is still a breaking change from the old + scheme. */ + +/* Return true if C is a double or single quote character. */ + +static bool +isquote (const char c) +{ + return c == '"' || c == '\''; +} /* See remote-args.h. */ std::vector gdb::remote_args::split (const std::string &args) { - std::vector results; + std::vector remote_args; - gdb_argv argv (args.c_str ()); - for (int i = 0; argv[i] != nullptr; i++) - results.emplace_back (argv[i]); + const char *input = args.c_str (); + bool squote = false, dquote = false; - return results; + /* Create a shorter alias for SPECIAL_SHELL_CHARACTERS. */ + const char *special = special_shell_characters; + + /* Characters that are special within double quotes. */ + static const char dquote_special[] = "$`\\"; + + input = skip_spaces (input); + + while (*input != '\0') + { + std::string arg; + + while (*input != '\0') + { + if (isspace (*input) && !squote && !dquote) + break; + else if (*input == '\\' && !squote) + { + if (input[1] == '\0') + arg += input[0]; + else if (input[1] == '\n') + ++input; + else if (dquote && input[1] == '"') + { + arg += input[1]; + ++input; + } + else if (dquote && strchr (dquote_special, input[1]) != nullptr) + { + /* Within double quotes, these characters would usually + have special meaning, but they are escaped with a + backslash. Preserve the escaping by adding both a + backslash and the special character to ARG. */ + arg += '\\'; + arg += input[1]; + ++input; + } + else if (dquote) + { + /* Within double quotes, none of the remaining characters + have any special meaning, the backslash before the + character is a literal backslash. + + To retain a backslash with the quotes removed, we need + to escape the backslash with a second backslash. */ + arg += '\\'; + arg += '\\'; + + /* If the character after the backslash has special + meaning outside of the double quotes, then we need to + escape it now, otherwise it will gain special meaning + as we remove the surrounding quotes. However, as per + the comments at the head of this file; we don't + escape white space or quotes. */ + if (!isspace (input[1]) + && !isquote (input[1]) + && strchr (special, input[1]) != nullptr) + arg += '\\'; + + /* Now just add the character. */ + arg += input[1]; + ++input; + } + else if (isspace (input[1]) || isquote (input[1])) + { + /* We remove the escaping from white space and quote + characters, just insert the character. */ + arg += input[1]; + ++input; + } + else + { + /* For everything else, retain the escaping by adding back + a backslash and then the character. */ + arg += '\\'; + arg += input[1]; + ++input; + } + } + else if (squote) + { + /* Inside a single quote argument there are no special + characters. A single quote finishes the argument. */ + if (*input == '\'') + squote = false; + /* Don't add escaping to white space or quotes. We already + handled single quotes above, so the isquote call here will + only find double quotes. */ + else if (isspace (*input) || isquote (*input)) + arg += *input; + /* Any other special shell character needs a backslash added + to avoid the character gaining special meaning outside of + the single quotes. */ + else if (strchr (special, *input) != nullptr) + { + arg += '\\'; + arg += *input; + } + /* Any other character just gets added to the output. */ + else + arg += *input; + } + else if (dquote) + { + /* Inside a double quoted argument. A double quote closes + the argument. An escaped double quote will have been + handled in the backslash handling block above. */ + if (*input == '"') + dquote = false; + /* Don't add escaping for white space or quotes. We already + handled double quotes above, so the isquote call here will + only find single quotes. */ + else if (isspace (*input) || isquote (*input)) + arg += *input; + /* Any character that is not one of the few characters that + retain its special meaning without double quotes, but is + otherwise a special character, needs an escape character + added to avoid the character gaining special meaning + outside of the quotes. */ + else if (strchr (dquote_special, *input) == nullptr + && strchr (special, *input) != nullptr) + { + arg += '\\'; + arg += *input; + } + /* Anything else just gets passed through to the output. */ + else + arg += *input; + } + else + { + /* Found a character outside of a single or double quoted + argument, and not preceded by a backslash. */ + if (*input == '\'') + squote = true; + else if (*input == '"') + dquote = true; + else + arg += *input; + } + ++input; + } + + remote_args.push_back (std::move (arg)); + input = skip_spaces (input); + } + + return remote_args; } diff --git a/gdbsupport/remote-args.h b/gdbsupport/remote-args.h index d714702a9b8..b39eb20eeb7 100644 --- a/gdbsupport/remote-args.h +++ b/gdbsupport/remote-args.h @@ -62,7 +62,7 @@ template inline std::string join (const std::vector &args) { - return construct_inferior_arguments (gdb::array_view (args), true); + return construct_inferior_arguments (gdb::array_view (args), false); } } /* namespace remote_args */ diff --git a/gdbsupport/special-shell-chars.h b/gdbsupport/special-shell-chars.h new file mode 100644 index 00000000000..9d018aa7eab --- /dev/null +++ b/gdbsupport/special-shell-chars.h @@ -0,0 +1,35 @@ +/* Characters that are treated as special by OS shells. + + Copyright (C) 2026 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +#ifndef GDBSUPPORT_SPECIAL_SHELL_CHARS_H +#define GDBSUPPORT_SPECIAL_SHELL_CHARS_H + +constexpr inline char special_shell_characters[] +#ifdef __MINGW32__ + /* This holds all the characters considered special to the + Windows shells. */ + = "\"!&*|[]{}<>?`~^=;, \t\n"; +#else + /* This holds all the characters considered special to the + typical Unix shells. We include `^' because the SunOS + /bin/sh treats it as a synonym for `|'. */ + = "\"!#$&*()\\|[]{}<>?'`~^; \t\n"; +#endif + +#endif /* GDBSUPPORT_SPECIAL_SHELL_CHARS_H */ base-commit: 5d1d89c9ebd515e9312728c914256c83572d1ef3 -- 2.25.4