From patchwork Tue Jan 9 14:26: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: 83645 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 25F22385829F for ; Tue, 9 Jan 2024 14:30:22 +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.133.124]) by sourceware.org (Postfix) with ESMTPS id 504BC3858422 for ; Tue, 9 Jan 2024 14:27:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 504BC3858422 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 504BC3858422 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704810425; cv=none; b=REsMmS8PwnvkOn6/zuL7JRJg4iTCdrrHbZp7a2nNnw0Uz8PQX5PjOC1e1qYOPWUQffdJCLbuqv96b8SzOeeaDHKPucQUsXkAt7GCnES4pRyKdgdqxSjaEx7tmIXb3qnCI8h1TIlKgp0911aD6MByw2VRwW6rMVZIbtRCrqCCAE0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704810425; c=relaxed/simple; bh=jtJWle8OHynSnjbt8CuExylvba3X5MUhpQYqQlNlWWw=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=ChPLfb5JpQaNKd7bdWVe5QHw1zErDJbgpCyItXQbB/xhJF8D14mFTps9oBWJYzmlPBXsXBC5GyOwPsDdl/ocozuyQdRS4OGIGwjvtJV0/0PygXRCKaIcCgRhPYwcyYfQf2WbPOCz3HwgQmmntmn3KMNB7332cFnK58UeV6FosXo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704810420; 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=s4A9G0oL4n5HgXpfnhGHTZbgokBsVuUb3vgL2fzsAx0=; b=KqGQYLAnjsox+YGJzwuiBTY3RjOFgd351BDCPp/MnsUhRgqaCUNBRk9R2AW1bIJf5uNiWZ vBGM0kdyMBfLQEnBIyHWeW4p4YoS5bePDqUgWuZD0dhFUHa2nWDru46+EXUungNdgL2KFU IrRnBVUxj9FtPg98js0zJmKoNjnsJEk= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-222-_-tnzgCkMI6fJKSo6H-Jyg-1; Tue, 09 Jan 2024 09:26:58 -0500 X-MC-Unique: _-tnzgCkMI6fJKSo6H-Jyg-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-40e417aa684so21927025e9.3 for ; Tue, 09 Jan 2024 06:26:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704810416; x=1705415216; 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=s4A9G0oL4n5HgXpfnhGHTZbgokBsVuUb3vgL2fzsAx0=; b=C/AlLZhFoO8eL/FdHPdoY2NGpjdzNJpiALKmpKNwz5AMdlbSPz8ejaEVxNVzNQvfWP jqvW7RfaZsjQL4ugIl/+VrNrmsNIx04+dWXml+4TXjYhjs+5lz4WQujGn7Gv3q/s5/Ft htZmeILq6WI2Ak1agMaL25agmof4Q7kYnoTdd9YypMkC/WkFeZ4vsUQFhkS2TB38REzc QXtLGZEcsHQAUvXfGHXrpdn5OUoOK82ZS/Ni4eZbp4Z1E+8Ww4MdouRXOv1XHnoMExxW inBVq9OkoIh6lslug8czLRiMnOrsdr/TE5U0t/gbWh/8oOKBkXZrAbS5LwnazhBmmfh4 OFMw== X-Gm-Message-State: AOJu0YyUh9+rJWFkxeHtwinkLGhcjSLPGKbK28I7+GQcnyxsfmLzWGwr lAToK3SzEwos9RWgX9Nypr9i97U31MKJRCudVZqfLY25uw5HhMBZRzxepjCgyLfj1Udh7C3fwZk AV0QmSbKMFdLBmwp8qb0+2vwjAqYOiFUdq6UpFWB58SCzdWkwLjt8yxPGtPpmnW7Gp3GO9FAMYG pF1dPKVwuu39WJHg== X-Received: by 2002:a05:600c:2909:b0:40e:4e0c:ecdd with SMTP id i9-20020a05600c290900b0040e4e0cecddmr673912wmd.50.1704810416002; Tue, 09 Jan 2024 06:26:56 -0800 (PST) X-Google-Smtp-Source: AGHT+IFM85/vQBEE1uTNLtMYFYUvhm4GjnAABMzFvsn5D2WW8MWS7GQdjY7xR3kh6kOwYCfJGMPyvw== X-Received: by 2002:a05:600c:2909:b0:40e:4e0c:ecdd with SMTP id i9-20020a05600c290900b0040e4e0cecddmr673900wmd.50.1704810415582; Tue, 09 Jan 2024 06:26:55 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id h4-20020a05600c314400b0040d7c3d5454sm14911429wmo.3.2024.01.09.06.26.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jan 2024 06:26:54 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Michael Weghorn Subject: [PATCH 09/16] gdb/python: change escaping rules when setting arguments Date: Tue, 9 Jan 2024 14:26:32 +0000 Message-Id: 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=-13.4 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_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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 It is possible to set an inferior's arguments through the Python API by assigning to the gdb.Inferior.arguments attribute. This attribute can be assigned a string, in which case the string is taken verbatim as the inferior's argument string. Or this attribute can be assigned a sequence, in which case the members of the sequence are combined (with some escaping applied) to create the inferior's argument string. Currently, the when the arguments from a Python list are escaped, we use escape_shell_characters. I suspect the reasons for this are mostly accidental. When the gdb.Inferior.arguments attribute was introduced in commit: commit 3153113252f3b949a159439a17e88af8ff0dce30 Date: Mon May 1 13:53:59 2023 -0600 Add attributes and methods to gdb.Inferior GDB's inferior::set_args method called construct_inferior_arguments, and construct_inferior_arguments didn't take an escaping function as a parameter, the only option was escape_shell_characters as that was the escaping hard-coded into construct_inferior_arguments. The commit message makes no comments for or against escaping of special shell characters, and no tests were added that checked this behaviour. All of this leads me to think that the handling of special shell characters wasn't really considered (an understandable oversight). But I'd like to consider it, and I think the current behaviour is not ideal. Consider this case: (gdb) python gdb.selected_inferior().arguments = ['$VAR'] (gdb) show args Argument list to give program being debugged when it is started is "\$VAR". This means that when the inferior is run it will see literal '$VAR' as its argument. If instead, the user wants to pass the shell expanded value of $VAR to the inferior, there's no way to achieve this result using the list assignment method. In this commit I propose that we change this behaviour so that we instead see this: (gdb) python gdb.selected_inferior().arguments = ['$VAR'] (gdb) show args Argument list to give program being debugged when it is started is "$VAR". Now the '$' character is not escaped. If the inferior is started under a shell then the user will see the shell expanded value of '$VAR'. Of course, if the user wants to pass a literal '$VAR' (with no expansion) then they can do: (gdb) python gdb.selected_inferior().arguments = ['\$VAR'] This actually feels more natural to me, the user writes the argument as they would present it to a shell. So, after this commit, GDB only escapes quote characters and white space characters. This keeps some level of backward compatibility with the existing behaviour (for things other than shell special characters), but also seems natural, from the user's point of view, the arguments they are providing are already quoted (by Python's string quotes) so there's no need to quote white space. It's only when GDB converts the Python sequence into a single string that the white space actually needs quoting. There are tests for the updated functionality, and I've updated the docs and added a NEWS entry. Reviewed-By: Eli Zaretskii --- gdb/NEWS | 4 +++ gdb/doc/python.texi | 7 +++-- gdb/python/py-inferior.c | 2 +- gdb/testsuite/gdb.python/py-inferior.exp | 36 ++++++++++++++++++++---- gdbsupport/common-inferior.cc | 14 +++++++++ gdbsupport/common-inferior.h | 5 ++++ 6 files changed, 60 insertions(+), 8 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 11cd6c0663e..b72ba3d87e8 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -87,6 +87,10 @@ show remote thread-options-packet ** New function gdb.interrupt(), that interrupts GDB as if the user typed control-c. + ** When assigning a sequence to gdb.Inferior.arguments, only quote + and whitespace characters will be escaped. Everything else will + be left unmodified. + * Debugger Adapter Protocol changes ** GDB now emits the "process" event. diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index d74defeec0c..e04e79cafa5 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -3580,8 +3580,11 @@ Either a string or a sequence of strings can be assigned to this attribute. When a string is assigned, it is assumed to have any -necessary quoting for the shell; when a sequence is assigned, the -quoting is applied by @value{GDBN}. +necessary quoting for the shell; when a sequence is assigned, quoting +is applied by @value{GDBN} so that the individual strings can be +concatenated into a single string, with a single space between each +argument. This means that shell quote characters and whitespace +characters will be escaped. @end defvar A @code{gdb.Inferior} object has the following methods: diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c index 8641d8a068b..5b7c7fb9365 100644 --- a/gdb/python/py-inferior.c +++ b/gdb/python/py-inferior.c @@ -919,7 +919,7 @@ infpy_set_args (PyObject *self, PyObject *value, void *closure) for (const auto &arg : args) argvec.push_back (arg.get ()); gdb::array_view view (argvec.data (), argvec.size ()); - inf->inferior->set_args (view, escape_shell_characters); + inf->inferior->set_args (view, escape_quotes_and_white_space); } else { diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp index 6fbcdd6822f..108c85c0165 100644 --- a/gdb/testsuite/gdb.python/py-inferior.exp +++ b/gdb/testsuite/gdb.python/py-inferior.exp @@ -410,11 +410,37 @@ gdb_test "show args" \ [string_to_regexp "Argument list to give program being debugged when it is started is \"a b c\"."] \ "show args from string" -gdb_test_no_output "python gdb.selected_inferior().arguments = \['a', 'b c'\]" \ - "set arguments from list" -gdb_test "show args" \ - [string_to_regexp "Argument list to give program being debugged when it is started is \"a b\\ c\"."] \ - "show args from list" +# Test setting inferior arguments from a Python list. INPUT is a +# single string that contains the Python list, this is inserted into +# the argument setting command, and should include the surrouning +# square brackets. +# +# The OUTPUT is the string that describes the arguments as GDB will +# have stored them within the inferior, as seen in the 'show args' +# command output. OUTPUT should include the surrounding quotes. +# OUTPUT will be passed through string_to_regexp, so should be a plain +# string, not a regexp. +proc test_setting_arguments_from_list { input output } { + with_test_prefix "input: ${input}" { + gdb_test_no_output "python gdb.selected_inferior().arguments = ${input}" \ + "set arguments from list" + gdb_test "show args" \ + [string_to_regexp \ + "Argument list to give program being debugged when it is started is ${output}."] \ + "show args from list" + } +} + +# Test setting inferior arguments from a list. Try to hit all the +# potentially problematic cases. Notice that shell characters are not +# automatically quoted, if a user wants a shell character quoted then +# they must do that themselves. +test_setting_arguments_from_list "\['a', 'b c'\]" "\"a b\\ c\"" +test_setting_arguments_from_list "\[' ', '\\t', '\\n']" "\"\\ \\\t '\r\n'\"" +test_setting_arguments_from_list "\['', '']" "\"'' ''\"" +test_setting_arguments_from_list "\['\"']" "\"\\\"\"" +test_setting_arguments_from_list "\[\"'\"]" "\"\\\'\"" +test_setting_arguments_from_list "\[\"\$VAR\", \";\"]" "\"\$VAR ;\"" gdb_test_no_output "python gdb.selected_inferior().clear_env()" \ "clear environment" diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc index 6717f7d5c08..cf2cd9a090a 100644 --- a/gdbsupport/common-inferior.cc +++ b/gdbsupport/common-inferior.cc @@ -133,3 +133,17 @@ escape_shell_characters (const char *arg) return escape_characters (arg, special); } + +/* See common-inferior.h. */ + +std::string +escape_quotes_and_white_space (const char * arg) +{ +#ifdef __MINGW32__ + static const char special[] = "\" \t\n"; +#else + static const char special[] = "\"' \t\n"; +#endif + + return escape_characters (arg, special); +} diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h index 92d1954c3fe..7cc01fb2f28 100644 --- a/gdbsupport/common-inferior.h +++ b/gdbsupport/common-inferior.h @@ -65,6 +65,11 @@ using escape_args_func = std::string (*) (const char *arg); /* Return a version of ARG that has special shell characters escaped. */ extern std::string escape_shell_characters (const char *arg); +/* Return a version of ARG that has quote characters and white space + characters escaped. No other special shell characters will have been + escaped though. */ +extern std::string escape_quotes_and_white_space (const char *arg); + /* Pass each element of ARGS through ESCAPE_FUNC and combine the results into a single string, separating each element with a single space character. */