From patchwork Tue Jan 9 14:26:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 83647 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 E17113857B92 for ; Tue, 9 Jan 2024 14:30:56 +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 B64683857C74 for ; Tue, 9 Jan 2024 14:27:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B64683857C74 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 B64683857C74 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=1704810431; cv=none; b=oGl4ojHlDZiOoJzkQx/R7kPC1NEe6juLXG3nHvsPcbNG8dRaZ5MXUxXJ+vy8t3wyEb+BDJgGNy2681S4ZW7DsiSPZaPnqvM0a/+krZ9Ns4IF6Up8C5rgrQooQ3HH/3ZolgzTHJ590GGqUGXFzsXlE/BQrs00lDJ91iF+aVf8QuM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704810431; c=relaxed/simple; bh=WYvl/rYIYBX+DHvtpaSHRby7Oa3qN7lrA3Uik6xB31k=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=vSedezjk8dkX3AjP0k2o0MLgVPqEAXuKIZlygHjRA9VQodxvm2l3I+6416MWMzrrvZhjDbU3+pWFpDJI5/u7GoR6+em3XlmSza9L3se/F6/eAUg5dKEb7lxF0Dz9Leckl4tQ/7vYmZfP7V1ePwJweTxxDgcrQWcT+58iarcVvk8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704810427; 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=X9Zb1iWSZmiYm4hiz+CxKeb4FtK9fhl9UF0yLQpKelY=; b=D1RU1r5mxC6nFLHStuQLBC4xAOfN/E60lmcfsGFMCdxbQo8HuIznwX/TCk4QWn7znVMsmI ZLmERRoPH7SstUhToDxPVKPBrYPauIblk+tq4w+ckcul3MdGgUhuvzKOmNdzBtLAR1CPcp XI1t/lvEUmeSQXUR3aaJ/ql6oKColn8= 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-34-k5McOR5EM1mAmAj_YDZUDw-1; Tue, 09 Jan 2024 09:27:05 -0500 X-MC-Unique: k5McOR5EM1mAmAj_YDZUDw-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-33769c5db4aso1488623f8f.0 for ; Tue, 09 Jan 2024 06:27:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704810424; x=1705415224; 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=X9Zb1iWSZmiYm4hiz+CxKeb4FtK9fhl9UF0yLQpKelY=; b=m3iWFFncUU3mmmzyg2h71TIp+m8/MqJ4AeEqWGKXIq9ChtlZEkaIQF8Di4OwsaqXTf s8MMJh4iKziQoaI/aRC4CS9ujZse67slfII32g2rEUAYQN2Z/hm8cpP4OoK5GGgRDxin YYGiPuXDlHZuXditRZdo+R8sOCecoHB4WgXtvTyf/KodE4jAPKm91VtWT2JEdSGHesyB DIo9DaaSsIHhbct8IWLphouJOMNuLhh3W8qu/Ru4uoIygld5JNpwjYtmCaESlokIuFuz VbkhbELgoSLVymDBnkVyR8SNlpsAY40eR+/Y1cKaiD+oBgDhmHBQ1sFq0kB85LfObQb7 IAzQ== X-Gm-Message-State: AOJu0Yxzojk0f31Bg1MuzLdaFDED3fZ8qs+irT9ames2Y9EQ0tabC32X x+rj/bCRG8A/0UF3UguUkKA4cUwbhUqtkTaRAefs24MV+Sr3TSLbQc8umlRWHHYKhD0mesEt5fu yx/my+mZT2r5cwgj2Gw6RNI9V/zZ4QkD9Yw69j0seIYCVJYhL+EMz8bxMWW7PBBOpp4yqWJtylo a7B5FNaJK8pYdaaw== X-Received: by 2002:a05:6000:543:b0:336:ec62:ea8b with SMTP id b3-20020a056000054300b00336ec62ea8bmr618568wrf.63.1704810424193; Tue, 09 Jan 2024 06:27:04 -0800 (PST) X-Google-Smtp-Source: AGHT+IEU2fBrAVDqgVucxoeEP4IWltlwmftafMkFYY/ay3uzRRHEUMUR/578pCNQO3eBa8C4sRJkgA== X-Received: by 2002:a05:6000:543:b0:336:ec62:ea8b with SMTP id b3-20020a056000054300b00336ec62ea8bmr618555wrf.63.1704810423805; Tue, 09 Jan 2024 06:27:03 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id s15-20020a5d424f000000b0033743c2d17dsm2556500wrr.28.2024.01.09.06.27.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jan 2024 06:27:02 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Michael Weghorn Subject: [PATCH 14/16] gdb/gdbserver: remove some uses of free_vector_argv Date: Tue, 9 Jan 2024 14:26:37 +0000 Message-Id: <530cf2c6789513d079c3a3817c00e9625ba18c49.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, 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 commit removes some of the uses of free_vector_argv and instead makes use of gdb::unique_xmalloc_ptr to manage memory. Most of this patch is about changing various APIs to accept the gdb::unique_xmalloc_ptr vectors instead of raw 'char *'. However, there are a couple of places, one in gdbserver, and one in GDB, where retaining the old API will remove the need to malloc copies of all the incoming arguments -- this is for the initial set of arguments delivered from the OS. To support these I've added an overload of construct_inferior_arguments that retains the old API (taking a gdb::array_view), and we make use of this to avoid unnecessary malloc/free calls. There is a place in gdb/nat/fork-inferior.c where it seems unavoidable that free_vector_argv is needed. In this case we need to build an array of 'char *' to pass to an exec call, so using gdb::unique_xmalloc_ptr is out of the question I think. There should be no user visible changes after this commit. --- gdb/inferior.c | 9 ----- gdb/inferior.h | 8 +++-- gdb/python/py-inferior.c | 7 ++-- gdb/unittests/remote-arg-selftests.c | 33 +++---------------- gdbserver/server.cc | 22 ++++--------- gdbsupport/common-inferior.cc | 49 ++++++++++++++++++++++++---- gdbsupport/common-inferior.h | 7 ++++ gdbsupport/remote-args.cc | 2 +- gdbsupport/remote-args.h | 2 +- 9 files changed, 71 insertions(+), 68 deletions(-) diff --git a/gdb/inferior.c b/gdb/inferior.c index ed138888024..d2cdf31551a 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -165,15 +165,6 @@ inferior::tty () return m_terminal; } -/* See inferior.h. */ - -void -inferior::set_args (gdb::array_view args, - escape_args_func escape_func) -{ - set_args (construct_inferior_arguments (args, escape_func)); -} - void inferior::set_arch (gdbarch *arch) { diff --git a/gdb/inferior.h b/gdb/inferior.h index d3824236ef6..3dad403b244 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -526,8 +526,12 @@ class inferior : public refcounted_object, }; /* Set the argument string from some strings. */ - void set_args (gdb::array_view args, - escape_args_func escape_func); + template + void set_args (gdb::array_view args, + escape_args_func escape_func) + { + this->set_args (construct_inferior_arguments (args, escape_func)); + } /* Get the argument string to use when running this inferior. diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c index 5b7c7fb9365..c7a47e92ded 100644 --- a/gdb/python/py-inferior.c +++ b/gdb/python/py-inferior.c @@ -915,11 +915,8 @@ infpy_set_args (PyObject *self, PyObject *value, void *closure) return -1; args.push_back (std::move (str)); } - std::vector argvec; - for (const auto &arg : args) - argvec.push_back (arg.get ()); - gdb::array_view view (argvec.data (), argvec.size ()); - inf->inferior->set_args (view, escape_quotes_and_white_space); + gdb::array_view const> arg_view (args); + inf->inferior->set_args (arg_view, escape_quotes_and_white_space); } else { diff --git a/gdb/unittests/remote-arg-selftests.c b/gdb/unittests/remote-arg-selftests.c index 3240ab9aeea..b732d94792d 100644 --- a/gdb/unittests/remote-arg-selftests.c +++ b/gdb/unittests/remote-arg-selftests.c @@ -59,32 +59,6 @@ arg_test_desc desc[] = { { "1 '\n' 3", { "1", "\n", "3" }, "1 '\n' 3" }, }; -/* Convert a std::vector into std::vector. This - requires copying all of the string content. This class takes care of - freeing the memory once we are done with it. */ - -struct args_as_c_strings -{ - args_as_c_strings (std::vector args) - { - for (const auto & a : args) - m_data.push_back (xstrdup (a.c_str ())); - } - - ~args_as_c_strings () - { - free_vector_argv (m_data); - } - - std::vector &get () - { - return m_data; - } - -private: - std::vector m_data; -}; - /* Run the remote argument passing self tests. */ static void @@ -132,9 +106,10 @@ self_test () } /* Now join the arguments. */ - args_as_c_strings split_args_c_str (split_args); - std::string joined_args - = gdb::remote_args::join (split_args_c_str.get ()); + std::vector> temp_args; + for (const auto & a : split_args) + temp_args.push_back (make_unique_xstrdup (a.c_str ())); + std::string joined_args = gdb::remote_args::join (temp_args); if (run_verbose ()) debug_printf ("Joined (%s), expected (%s)\n", diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 0445fa0237f..9c5d8ee4f54 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -3385,7 +3385,7 @@ handle_v_run (char *own_buf) { client_state &cs = get_client_state (); char *p, *next_p; - std::vector new_argv; + std::vector> new_argv; gdb::unique_xmalloc_ptr new_program_name; int i; @@ -3405,7 +3405,7 @@ handle_v_run (char *own_buf) else if (p == next_p) { /* Empty argument. */ - new_argv.push_back (xstrdup ("")); + new_argv.push_back (make_unique_xstrdup ("")); } else { @@ -3416,14 +3416,13 @@ handle_v_run (char *own_buf) if (arg == nullptr) { write_enn (own_buf); - free_vector_argv (new_argv); return; } if (i == 0) new_program_name = std::move (arg); else - new_argv.push_back (arg.release ()); + new_argv.push_back (std::move (arg)); } if (*next_p == '\0') break; @@ -3436,7 +3435,6 @@ handle_v_run (char *own_buf) if (program_path.get () == nullptr) { write_enn (own_buf); - free_vector_argv (new_argv); return; } } @@ -3451,15 +3449,13 @@ handle_v_run (char *own_buf) return; } else if (new_argv.size () == 1) - program_args = std::string (new_argv[0]); + program_args = std::string (new_argv[0].get ()); else program_args.clear (); } else program_args = gdb::remote_args::join (new_argv); - free_vector_argv (new_argv); - target_create_inferior (program_path.get (), program_args); if (cs.last_status.kind () == TARGET_WAITKIND_STOPPED) @@ -4345,16 +4341,12 @@ captured_main (int argc, char *argv[]) if (pid == 0 && *next_arg != NULL) { - int i, n; - - n = argc - (next_arg - argv); program_path.set (next_arg[0]); - std::vector temp_arg_vector; - for (i = 1; i < n; i++) - temp_arg_vector.push_back (next_arg[i]); escape_args_func escape_func = (escape_args ? escape_shell_characters : escape_quotes_and_white_space); - program_args = construct_inferior_arguments (temp_arg_vector, + int arg_count = argc - (next_arg - argv) - 1; + gdb::array_view arg_view (&next_arg[1], arg_count); + program_args = construct_inferior_arguments (arg_view, escape_func); /* Wait till we are at first instruction in program. */ diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc index cf2cd9a090a..458d78295ab 100644 --- a/gdbsupport/common-inferior.cc +++ b/gdbsupport/common-inferior.cc @@ -20,30 +20,67 @@ #include "gdbsupport/common-defs.h" #include "gdbsupport/common-inferior.h" +#include "gdbsupport/function-view.h" /* See common-inferior.h. */ bool startup_with_shell = true; -/* See common-inferior.h. */ +/* Helper function for the two construct_inferior_arguments overloads + below. Accept a gdb::array_view over objects of type T. Convert each T + to a std::string by calling CB, and join all the resulting strings + together with a single space between each. */ -std::string -construct_inferior_arguments (gdb::array_view argv, - escape_args_func escape_func) +template +static std::string +construct_inferior_arguments_1 (gdb::array_view argv, + gdb::function_view cb) { std::string result; - for (const char *a : argv) + for (const T &a : argv) { if (!result.empty ()) result += " "; - result += escape_func (a); + result += cb (a); } return result; } +/* See common-inferior.h. */ + +std::string +construct_inferior_arguments + (gdb::array_view const> argv, + escape_args_func escape_func) +{ + /* Convert ARG to a std::string by applying ESCAPE_FUNC. */ + auto escape_cb = [&] (const gdb::unique_xmalloc_ptr &arg) + { + return escape_func (arg.get ()); + }; + + return construct_inferior_arguments_1> + (argv, escape_cb); +} + +/* See common-inferior.h. */ + +std::string +construct_inferior_arguments (gdb::array_view argv, + escape_args_func escape_func) +{ + /* Convert ARG to a std::string by applying ESCAPE_FUNC. */ + auto escape_cb = [&] (const char * const &arg) + { + return escape_func (arg); + }; + + return construct_inferior_arguments_1 (argv, escape_cb); +} + /* Escape characters in ARG and return an updated string. The string SPECIAL contains the set of characters that must be escaped. SPECIAL must not be nullptr, and it is assumed that SPECIAL contains the newline diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h index 7cc01fb2f28..c607ab6d98e 100644 --- a/gdbsupport/common-inferior.h +++ b/gdbsupport/common-inferior.h @@ -74,6 +74,13 @@ extern std::string escape_quotes_and_white_space (const char *arg); into a single string, separating each element with a single space character. */ +extern std::string +construct_inferior_arguments + (gdb::array_view const> args, + escape_args_func escape_func); + +/* An overload of the above that takes an array of raw pointers. */ + extern std::string construct_inferior_arguments (gdb::array_view args, escape_args_func escape_func); diff --git a/gdbsupport/remote-args.cc b/gdbsupport/remote-args.cc index 96c12ffac67..b808a64efce 100644 --- a/gdbsupport/remote-args.cc +++ b/gdbsupport/remote-args.cc @@ -37,7 +37,7 @@ gdb::remote_args::split (std::string args) /* See remote-args.h. */ std::string -gdb::remote_args::join (std::vector &args) +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 index c0acce9b7c4..1397a4d16d9 100644 --- a/gdbsupport/remote-args.h +++ b/gdbsupport/remote-args.h @@ -37,7 +37,7 @@ extern std::vector split (std::string args); 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); +extern std::string join (std::vector > &args); } /* namespace remote_args */