From patchwork Tue Jan 9 14:26:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 83650 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 BDC263857B92 for ; Tue, 9 Jan 2024 14:31:32 +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 D627F385843E for ; Tue, 9 Jan 2024 14:26:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D627F385843E 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 D627F385843E 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=1704810421; cv=none; b=ry3M93sTX14QkLAleKsqc5jpk79zklbvjgKBzs45ztx+A8O4trgcFHlVbVgUhzmCfkfyFAapdESxbc7XVGd4z3ECTbt8IfTcbgdCFyAfcNmUBFXmpyIVkqntZRPv9VIJqGL5P5//uWAbd2qwF5c51NSNGa3yOeitAojl7XcsFCU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704810421; c=relaxed/simple; bh=cZX1I+OEGikbzD1fTAV6PsOPF7O3yiHurx1mYuRWDvI=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=sCLp0Bh8so/reY73Jsa6+3ydVxfzJukWDIZzkeDXuLwWAu0y8GeWlV6PRLzma+Zok769r9+1ApZrgtH8JIxhJeTm29iDRDzF0JZRN9VJI+L8BSiKVM94x9FEI/Pe/pRGoFPz/Uigz6VADIOEPJqTp8Gj+JBvhf3scYqdV/8Jry4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704810417; 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=ZmhUqlnwj7sOwcBEJcy9Nz4mNrNDX8wDgNmcIIQo85I=; b=G4A8EiFhHBfyg56iTbRSiCHXdMydXPNWJHV5s/EEc69SdepaRQbhmC4rw4i/wwha1q97aS artArxF3Mrk68FlcxG/ckkWKt9lov/QlZYdCtEI+DDadM0H4dYFn15vOeMPTdUrw/57gBu dUPBTKY1HqmQHxdjdIP2nHxRLkdhBTk= 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-90-CHtmejjVNhilo__aAQXX0g-1; Tue, 09 Jan 2024 09:26:56 -0500 X-MC-Unique: CHtmejjVNhilo__aAQXX0g-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-40e4576b7e6so17451735e9.0 for ; Tue, 09 Jan 2024 06:26:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704810414; x=1705415214; 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=ZmhUqlnwj7sOwcBEJcy9Nz4mNrNDX8wDgNmcIIQo85I=; b=sxs65GJaOWOLvDqPJeUmKHtAxiwa9KpwEIBDVXb98ZD9P198Rv8qjijbv2MUBVmwf1 6D2DykvCUao47U8fFA4vENHnsizP0Zq9AyIZaa6tqLuldYrK7J82KlGoJjyasF/MTmwS V8+2MNGyIZux5IR7cv+G2F5O1qQf/MYxpmkBqxfwHHBh+cWmxzp1sfdd5KB+bYvkVOdw jwg66Bf+hfeey16p6QSiG2VDyBfaRDjMGT4qtgGX5IH82f/4QjmT7nKySPnd7/utzmuX AMJqOQEdtF56cEYGpwKk66tsS+n9a3iQ44JT3POhzz7jIsv2gAEIsFhYPnlbz1K2ryss VVDQ== X-Gm-Message-State: AOJu0YzJ0ccHbqPjSX0EDjTkYaWgwdPLcbRIiSUcCvTxfwi1OzMaG8gk 2XfEMvOGhNwoVINBBFlpNQum8swtbADYqLSjGSxOmkS0LCarZC9kF3D9S1DcBElxwnXO8W3Yiwx bjtSWgSJzYxPp2zYIzHiR8H5gUl/DGMR5FoWfDhQRC1PG/CqjLIEfTsm6B1F86UdHrH/EJzKUG/ kuXCmN91I4pTvZfA== X-Received: by 2002:a05:600c:3590:b0:40d:8dee:26fb with SMTP id p16-20020a05600c359000b0040d8dee26fbmr1868238wmq.251.1704810414408; Tue, 09 Jan 2024 06:26:54 -0800 (PST) X-Google-Smtp-Source: AGHT+IGBrrfMmj9VuTHYTITt2rklw6zcsO2MfBlYDVj1mY9bRpiO2eXKmeWpmc/p8Ann1EBJweyZcQ== X-Received: by 2002:a05:600c:3590:b0:40d:8dee:26fb with SMTP id p16-20020a05600c359000b0040d8dee26fbmr1868230wmq.251.1704810414053; Tue, 09 Jan 2024 06:26:54 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id v11-20020a05600c444b00b0040e34835a58sm3796808wmn.22.2024.01.09.06.26.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jan 2024 06:26:53 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Michael Weghorn Subject: [PATCH 08/16] gdb: move remote arg splitting and joining into gdbsupport/ Date: Tue, 9 Jan 2024 14:26:31 +0000 Message-Id: <0b19b3f83eb1df36d1d4bfa2ccdb5ca9754f095f.1704809585.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=-13.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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 This is a refactoring commit. When passing inferior arguments to gdbserver we have two actions that need to be performed, splitting and joining. On the GDB side, we take the inferior arguments, a single string, and split the string into a list of individual arguments. These are then sent to gdbserver over the remote protocol. On the gdbserver side we receive the list of individual arguments and join these back together into a single inferior argument string. In a later commit I would like to add some unit testing for this remote argument passing process. Ideally, for unit testing, we want to use the exact same code for splitting and joining the arguments as is actually used -- we could duplicate the code within the unit test, and this would validate the algorithm as it is today, but there is always the risk that a future change would not be mirrored within the tests, which makes the tests useless. So in this commit I propose to move the splitting and joining logic out into a separate file, we can then use this within GDB and gdbserver when passing arguments between GDB and gdbserver, but we can also use the exact same code for some unit tests. In this commit I'm not adding the unit tests, they will be added later in this series, so for now there should be no user visible changes after this commit. --- gdb/remote.c | 12 +++++------ gdbserver/server.cc | 4 ++-- gdbsupport/Makefile.am | 1 + gdbsupport/Makefile.in | 13 +++++++----- gdbsupport/remote-args.cc | 43 ++++++++++++++++++++++++++++++++++++++ gdbsupport/remote-args.h | 44 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 gdbsupport/remote-args.cc create mode 100644 gdbsupport/remote-args.h diff --git a/gdb/remote.c b/gdb/remote.c index dcc1a0d0639..ebef409ffed 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -79,6 +79,7 @@ #include #include "async-event.h" #include "gdbsupport/selftest.h" +#include "gdbsupport/remote-args.h" /* The remote target. */ @@ -10625,16 +10626,15 @@ remote_target::extended_remote_run (const std::string &args) if (!args.empty ()) { - int i; + std::vector split_args = gdb::remote_args::split (args); - gdb_argv argv (args.c_str ()); - for (i = 0; argv[i] != NULL; i++) + for (const auto &a : split_args) { - if (strlen (argv[i]) * 2 + 1 + len >= get_remote_packet_size ()) + if (a.size () * 2 + 1 + len >= get_remote_packet_size ()) error (_("Argument list too long for run packet")); rs->buf[len++] = ';'; - len += 2 * bin2hex ((gdb_byte *) argv[i], rs->buf.data () + len, - strlen (argv[i])); + len += 2 * bin2hex ((gdb_byte *) a.c_str (), rs->buf.data () + len, + a.size ()); } } diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 52ce9240ca3..13abb0b7636 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -51,6 +51,7 @@ #include "gdbsupport/gdb_select.h" #include "gdbsupport/scoped_restore.h" #include "gdbsupport/search.h" +#include "gdbsupport/remote-args.h" /* PBUFSIZ must also be at least as big as IPA_CMD_BUF_SIZE, because the client state data is passed directly to some agent @@ -3437,8 +3438,7 @@ handle_v_run (char *own_buf) else program_path.set (new_program_name.get ()); - program_args = construct_inferior_arguments (new_argv, - escape_shell_characters); + program_args = gdb::remote_args::join (new_argv); free_vector_argv (new_argv); target_create_inferior (program_path.get (), program_args); diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am index f1a641308fe..6f5a5ef36c6 100644 --- a/gdbsupport/Makefile.am +++ b/gdbsupport/Makefile.am @@ -72,6 +72,7 @@ libgdbsupport_a_SOURCES = \ pathstuff.cc \ print-utils.cc \ ptid.cc \ + remote-args.cc \ rsp-low.cc \ run-time-clock.cc \ safe-strerror.cc \ diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in index 9fdc23c39a9..5d860302baa 100644 --- a/gdbsupport/Makefile.in +++ b/gdbsupport/Makefile.in @@ -163,11 +163,12 @@ am_libgdbsupport_a_OBJECTS = agent.$(OBJEXT) btrace-common.$(OBJEXT) \ gdb_tilde_expand.$(OBJEXT) gdb_wait.$(OBJEXT) \ gdb_vecs.$(OBJEXT) job-control.$(OBJEXT) netstuff.$(OBJEXT) \ new-op.$(OBJEXT) pathstuff.$(OBJEXT) print-utils.$(OBJEXT) \ - ptid.$(OBJEXT) rsp-low.$(OBJEXT) run-time-clock.$(OBJEXT) \ - safe-strerror.$(OBJEXT) scoped_mmap.$(OBJEXT) search.$(OBJEXT) \ - signals.$(OBJEXT) signals-state-save-restore.$(OBJEXT) \ - tdesc.$(OBJEXT) thread-pool.$(OBJEXT) xml-utils.$(OBJEXT) \ - $(am__objects_1) $(am__objects_2) + ptid.$(OBJEXT) remote-args.$(OBJEXT) rsp-low.$(OBJEXT) \ + run-time-clock.$(OBJEXT) safe-strerror.$(OBJEXT) \ + scoped_mmap.$(OBJEXT) search.$(OBJEXT) signals.$(OBJEXT) \ + signals-state-save-restore.$(OBJEXT) tdesc.$(OBJEXT) \ + thread-pool.$(OBJEXT) xml-utils.$(OBJEXT) $(am__objects_1) \ + $(am__objects_2) libgdbsupport_a_OBJECTS = $(am_libgdbsupport_a_OBJECTS) AM_V_P = $(am__v_P_@AM_V@) am__v_P_ = $(am__v_P_@AM_DEFAULT_V@) @@ -429,6 +430,7 @@ libgdbsupport_a_SOURCES = \ pathstuff.cc \ print-utils.cc \ ptid.cc \ + remote-args.cc \ rsp-low.cc \ run-time-clock.cc \ safe-strerror.cc \ @@ -538,6 +540,7 @@ distclean-compile: @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pathstuff.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/print-utils.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ptid.Po@am__quote@ +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/remote-args.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rsp-low.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/run-time-clock.Po@am__quote@ @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/safe-strerror.Po@am__quote@ diff --git a/gdbsupport/remote-args.cc b/gdbsupport/remote-args.cc new file mode 100644 index 00000000000..96c12ffac67 --- /dev/null +++ b/gdbsupport/remote-args.cc @@ -0,0 +1,43 @@ +/* Copyright (C) 2023 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 . */ + +#include "gdbsupport/common-defs.h" +#include "gdbsupport/remote-args.h" +#include "gdbsupport/common-inferior.h" +#include "gdbsupport/buildargv.h" + +/* See remote-args.h. */ + +std::vector +gdb::remote_args::split (std::string args) +{ + std::vector results; + + gdb_argv argv (args.c_str ()); + for (int i = 0; argv[i] != nullptr; i++) + results.emplace_back (argv[i]); + + return results; +} + +/* See remote-args.h. */ + +std::string +gdb::remote_args::join (std::vector &args) +{ + return construct_inferior_arguments (args, escape_shell_characters); +} diff --git a/gdbsupport/remote-args.h b/gdbsupport/remote-args.h new file mode 100644 index 00000000000..c0acce9b7c4 --- /dev/null +++ b/gdbsupport/remote-args.h @@ -0,0 +1,44 @@ +/* Functions to help when passing arguments between GDB and gdbserver. + + Copyright (C) 2023 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 . */ + +namespace gdb +{ + +namespace remote_args +{ + +/* ARGS is an inferior argument string. This function splits ARGS into + individual arguments and returns a vector containing each argument. */ + +extern std::vector split (std::string args); + +/* Join together the separate arguments in ARGS and build a single + inferior argument string. The string returned by this function will be + equivalent, but not necessarily identical to the string passed to + ::split, for example passing the string '"a b"' (without the single + quotes, but including the double quotes) to ::split, will return an + argument of 'a b' (without the single quotes). When this argument is + passed through ::join we will get back the string 'a\ b' (without the + single quotes), that is, we choose to escape the white space, rather + than wrap the argument in quotes. */ +extern std::string join (std::vector &args); + +} /* namespace remote_args */ + +} /* namespac gdb */