From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id v860JWlphmdQxw4AWB0awg (envelope-from ) for ; Tue, 14 Jan 2025 08:40:57 -0500 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=GFsMgxlK; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8CADC1E100; Tue, 14 Jan 2025 08:40:57 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.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 autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (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 9C99C1E08E for ; Tue, 14 Jan 2025 08:40:56 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 85BEE3856DF9 for ; Tue, 14 Jan 2025 13:40:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 85BEE3856DF9 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=GFsMgxlK Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 20AA2385AC3E for ; Tue, 14 Jan 2025 13:39:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 20AA2385AC3E 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 20AA2385AC3E 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=1736861949; cv=none; b=tU0sOu0oNKVzG/jzF4VIBPiGLkZoyk/UK/IlWvaOEPCFJAC1UbtgmeNKd3cqX3Kz/GMPaGNVUxlney0O6Qiq/9HaQZHQff3BC6d3D3AWF4AI6trTgkZYAKov57kXVKFHd1nIfviN1mSqPICMHyKN2/Cc2iUmVfbYZAiwIOqTiqU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1736861949; c=relaxed/simple; bh=+H51jgNgZolW7PN6Pd4a4kXbsoi+I0UTeDhZA+l6/3Q=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=BEZB6KXKv5EPe4KYpFGx7ijgy5mwNgt3237tkZIq8JmYl3OoxTqyU6pve86wIp/IM1V2tntZZQlkDs39vQ+8pRUTx3y5vZSHG/WhNsR0KLpOZVIrckdSsBuCpyMdJXakC9K64WDdH7sSyRvX449fsU/6qkh9MhI/MwbgP3L7pAA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 20AA2385AC3E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736861948; 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; bh=Ka3/sfdP4pFuYyhdDIWN8NArBvpKz3gFyn6CRqqVtrU=; b=GFsMgxlK/+efgBZKxbo7RJyfmLN3mWs9wx10N22NC4Dhpk3vEQuVeI/ilmLuKzjSIvHA4e nTN2ro1lQvw81l8cBfRaQozhsFkpz2WSkb5UZvwp6IPhmo3ze9ygtWcyyO+GhPi0k/ZXid hLlqRnY3LpdmPuQPHPlAVCmyZGrcrDI= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-679-VwjpvMYGNO2nG4RLMC3Blw-1; Tue, 14 Jan 2025 08:39:07 -0500 X-MC-Unique: VwjpvMYGNO2nG4RLMC3Blw-1 X-Mimecast-MFC-AGG-ID: VwjpvMYGNO2nG4RLMC3Blw Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-3862a49fbdaso2173588f8f.1 for ; Tue, 14 Jan 2025 05:39:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736861946; x=1737466746; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ka3/sfdP4pFuYyhdDIWN8NArBvpKz3gFyn6CRqqVtrU=; b=EH9Dh1jkngBiZotgAeKKFP9RArma3WMe2H6GfogAlLMTIHXKWYlthQ2z1UmPsHrEVU sY+ER9H5gsobOMfl251gPSF70/kPm6fc6qzZ+QYU+riUSbR8qLEgNeW2yiBAANCha0PJ venYj3wFfy1SrGdxc0d1akAexxSp23aGU2Y42vDO1dmMyaHx1FiyRnLoJ8oFAzqxDqBL mCv35/x41KOAZeRag/sDpt2t1W8Ye0xEwkCHVvLxu0kX0CmzKU3hK+YcWyfHwiaP93/G 9M/Rz5UO58Rl/D3LEmyt9q4wpJmLXyH+m4AVpJusGuEZ9HP7pn522ns1HG/rKelyF54k bKFg== X-Gm-Message-State: AOJu0YwW8cxwcpxRvUg4r6EVGNpL167PipJXa88opae9WJSFNAO6TxQt b9+LFw4gw2EIzUM5jVjtfKadUZXQsJEoanmFssHWKhb5cBU4cQFoaetYk1Y6UBM/rJjcl/Ywb4p 8GDrHWBCq3kaWDaQ1XF5E/Fexpez9BLYzdBN5ClZp+LV0aCm0kKsOgkUnIubv6z9kOc4LDGoXpj 62lAM3rpilf/Ze0Eg+BCLtxFqYRhjsac3cG68Empg4+jU= X-Gm-Gg: ASbGncssdteJCrvP0I7eRNjnBon5C8gmEZAsuvgXZMn7e6s8InMHUSmWlNy/hcEQ4N7 YMWh/wckJTFe+skG0E8JuKSSo/wtEDSVbH4wpBhCbdS8fpaTEJEIdYvueXOvXLttpTDw8yhnv0g EBZWLgOhKZVRmOC6KoC9jAkStNEgPZdXI0SjMUCGpaULUh4VNKkUdhfBK2fHRsyrmwZH/C3UHeK zyVfpe4IBxFCVxLtxQUKD+f0agXELwBDYTr/UwHfnY8QBbqyRl0iYGCMaYptS611kwn0FYTUUCZ j2uZyQ== X-Received: by 2002:a05:6000:712:b0:385:f3fb:46aa with SMTP id ffacd0b85a97d-38a87308c15mr22599535f8f.43.1736861945733; Tue, 14 Jan 2025 05:39:05 -0800 (PST) X-Google-Smtp-Source: AGHT+IHCbA2P+FNNMlXdWFBi+ToYg95nsXfAwzuOK2Px94/pDJIvz7FGUBG312WQhAtZjIjVtWLQAQ== X-Received: by 2002:a05:6000:712:b0:385:f3fb:46aa with SMTP id ffacd0b85a97d-38a87308c15mr22599487f8f.43.1736861944967; Tue, 14 Jan 2025 05:39:04 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-436e9e37d46sm174638915e9.25.2025.01.14.05.39.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jan 2025 05:39:04 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Guinevere Larsen Subject: [PATCH] gdbserver: convert program_args to a single string Date: Tue, 14 Jan 2025 13:39:03 +0000 Message-ID: X-Mailer: git-send-email 2.47.1 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: K3XCWJA_MhSCz3EfudaRsVkc3fbD58R9OOEf2ipPMEQ_1736861947 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 This commit changes how gdbserver stores the inferior arguments from being a vector of separate arguments into a single string with all of the arguments combined together. Making this change might feel a little strange; intuitively it feels like we would be better off storing the arguments as a vector, but this change is part of a larger series of work that aims to improve GDB's inferior argument handling. The full series was posted here: https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com But asking people to review a 14 patch series in unreasonable, so I'm instead posting the patches in smaller batches. This patch can stand alone, and I do think this change makes sense on its own: First, GDB already stores the inferior arguments as a single string, so doing this moves gdbserver into line with GDB. The common code into which gdbserver calls requires the arguments to be a single string, so currently each target's create_inferior implementation merged the arguments anyway, so all this commit really does is move the merging up the call stack, and store the merged result rather than storing the separate parts. However, the biggest reason for why this commit is needed, is an issue with passing arguments from GDB to gdbserver when starting a new inferior. Consider: (gdb) set args $VAR (gdb) run ... When using a native target the inferior will see the value of $VAR expanded by the shell GDB uses to start the inferior. However, if using an extended-remote target the inferior will see literally $VAR, the unexpanded name of the variable, the reason for this is that, although GDB sends '$VAR' to gdbserver, when gdbserver receives this, it converts this to '\$VAR', which prevents the variable from being expanded by the shell. The reason for this is that construct_inferior_arguments escapes all special shell characters within its arguments, and it is construct_inferior_arguments that is used to combine the separate arguments into a single string. In the future I will change construct_inferior_arguments so that it can apply different escaping strategies. When this happens we will want to escape arguments coming from the gdbserver command line differently than arguments coming from GDB (via a vRun packet), which means we need to call construct_inferior_arguments earlier, at the point where we know if the arguments came from the gdbserver command line, or from the vRun packet. This argument escaping issue is discussed in PR gdb/28392. This commit doesn't fix any issues, nor does it change construct_inferior_arguments to actually do different escaping, that will all come later. This is purely a restructuring. There should be no user visible changes after this commit. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392 Tested-By: Guinevere Larsen --- gdbserver/linux-low.cc | 5 ++--- gdbserver/linux-low.h | 2 +- gdbserver/netbsd-low.cc | 6 ++---- gdbserver/netbsd-low.h | 2 +- gdbserver/server.cc | 24 +++++++++++++++++++----- gdbserver/target.h | 6 +++--- gdbserver/win32-low.cc | 7 +++---- gdbserver/win32-low.h | 2 +- 8 files changed, 32 insertions(+), 22 deletions(-) diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 50ce2b44927..65268a6ee6c 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -974,7 +974,7 @@ linux_ptrace_fun () int linux_process_target::create_inferior (const char *program, - const std::vector &program_args) + const std::string &program_args) { client_state &cs = get_client_state (); struct lwp_info *new_lwp; @@ -984,10 +984,9 @@ linux_process_target::create_inferior (const char *program, { maybe_disable_address_space_randomization restore_personality (cs.disable_randomization); - std::string str_program_args = construct_inferior_arguments (program_args); pid = fork_inferior (program, - str_program_args.c_str (), + program_args.c_str (), get_environ ()->envp (), linux_ptrace_fun, NULL, NULL, NULL, NULL); } diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h index 5be00b8c98c..75af38d898f 100644 --- a/gdbserver/linux-low.h +++ b/gdbserver/linux-low.h @@ -141,7 +141,7 @@ class linux_process_target : public process_stratum_target public: int create_inferior (const char *program, - const std::vector &program_args) override; + const std::string &program_args) override; void post_create_inferior () override; diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc index 04e103563e7..9e7314b02a9 100644 --- a/gdbserver/netbsd-low.cc +++ b/gdbserver/netbsd-low.cc @@ -78,11 +78,9 @@ netbsd_ptrace_fun () int netbsd_process_target::create_inferior (const char *program, - const std::vector &program_args) + const std::string &program_args) { - std::string str_program_args = construct_inferior_arguments (program_args); - - pid_t pid = fork_inferior (program, str_program_args.c_str (), + pid_t pid = fork_inferior (program, program_args.c_str (), get_environ ()->envp (), netbsd_ptrace_fun, nullptr, nullptr, nullptr, nullptr); diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h index 53200ddffc4..aef1ce40cb9 100644 --- a/gdbserver/netbsd-low.h +++ b/gdbserver/netbsd-low.h @@ -42,7 +42,7 @@ class netbsd_process_target : public process_stratum_target public: int create_inferior (const char *program, - const std::vector &program_args) override; + const std::string &program_args) override; void post_create_inferior () override; diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 55898f59556..efe63ae7515 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -121,7 +121,20 @@ private: /* The program name, adjusted if needed. */ std::string m_path; } program_path; -static std::vector program_args; + +/* All program arguments are merged into a single string. This is similar + to how GDB manages the inferior arguments, and actually makes our lives + easier; the rules for how arguments are merged into a single string + differ depending on where the arguments come from. Arguments arriving + form the gdbserver command line are quoted, while arguments arriving + from GDB (via a vRun packet) are already quoted. + + NOTE: The comment above is ahead of its time. The differences between + how the PROGRAM_ARGS string is built up have not yet been implemented. + A later patch in this series will make this change, and remove this + note. */ +static std::string program_args; + static std::string wrapper_argv; /* The PID of the originally created or attached inferior. Used to @@ -3484,9 +3497,8 @@ handle_v_run (char *own_buf) else program_path.set (new_program_name.get ()); - /* Free the old argv and install the new one. */ - free_vector_argv (program_args); - program_args = new_argv; + program_args = construct_inferior_arguments (new_argv); + free_vector_argv (new_argv); try { @@ -4376,8 +4388,10 @@ captured_main (int argc, char *argv[]) n = argc - (next_arg - argv); program_path.set (next_arg[0]); + std::vector temp_arg_vector; for (i = 1; i < n; i++) - program_args.push_back (xstrdup (next_arg[i])); + temp_arg_vector.push_back (next_arg[i]); + program_args = construct_inferior_arguments (temp_arg_vector); /* Wait till we are at first instruction in program. */ target_create_inferior (program_path.get (), program_args); diff --git a/gdbserver/target.h b/gdbserver/target.h index 3643b9110da..1a013bb83db 100644 --- a/gdbserver/target.h +++ b/gdbserver/target.h @@ -77,13 +77,13 @@ class process_stratum_target /* Start a new process. PROGRAM is a path to the program to execute. - PROGRAM_ARGS is a standard NULL-terminated array of arguments, - to be passed to the inferior as ``argv'' (along with PROGRAM). + PROGRAM_ARGS is a string containing all of the arguments that will be + used to start the inferior. Returns the new PID on success, -1 on failure. Registers the new process with the process list. */ virtual int create_inferior (const char *program, - const std::vector &program_args) = 0; + const std::string &program_args) = 0; /* Do additional setup after a new process is created, including exec-wrapper completion. */ diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index da858b65e6f..139c945a2ba 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -492,12 +492,12 @@ create_process (const char *program, char *args, /* Start a new process. PROGRAM is the program name. - PROGRAM_ARGS is the vector containing the inferior's args. + PROGRAM_ARGS is a string containing all the inferior's arguments. Returns the new PID on success, -1 on failure. Registers the new process with the process list. */ int win32_process_target::create_inferior (const char *program, - const std::vector &program_args) + const std::string &program_args) { client_state &cs = get_client_state (); #ifndef USE_WIN32API @@ -508,8 +508,7 @@ win32_process_target::create_inferior (const char *program, DWORD flags; PROCESS_INFORMATION pi; DWORD err; - std::string str_program_args = construct_inferior_arguments (program_args); - char *args = (char *) str_program_args.c_str (); + char *args = (char *) program_args.c_str (); /* win32_wait needs to know we're not attaching. */ windows_process.attaching = 0; diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h index daed16a6ae6..e9dd6adc5a8 100644 --- a/gdbserver/win32-low.h +++ b/gdbserver/win32-low.h @@ -101,7 +101,7 @@ class win32_process_target : public process_stratum_target public: int create_inferior (const char *program, - const std::vector &program_args) override; + const std::string &program_args) override; int attach (unsigned long pid) override; base-commit: 127f733f88717bdb52f03c12c0c2d240bbc892e3 -- 2.47.1