From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 6268A385842D for ; Sun, 21 Jan 2024 03:57:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6268A385842D 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 6268A385842D 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=1705809449; cv=none; b=jpEhbQO+z3bwlOxd0kC866IuPByL9LGF/4KY8tii3LMDlWVgml2KLFKemfIpCXezelmaPx9p10FfJmlbd5PMdZ2O+dt0TjQgCjIxu2khh6nNDzA9IuBj16EYfQS8KUq0n14DCmE6mp+gXgzHVF47fAO7jkMSVnEyvIG/Nj5SL/c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705809449; c=relaxed/simple; bh=u1PPtOLHoe3YkAl97NZy68mTysYKr51u8Ii/+Z5SQbo=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=FUKCbVFouA3v9oaCY+5zgUh6S0Ox8MA8TCieF5t8C2P/iNIrJp/u1nLN+LlPRGuQvZAy/M9YPxSpuTaFdEli6bu/aSnCCOVqxbzurwOzHT1WiF+NNfKQnCggxZ3Sqbf0/u4G6iNBulWztS1lQA5GyC1RrCm7GDhp3b+AElLGe24= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705809447; 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=jsFSfeo4uKal8oRM11+CMcxLBYdcs2TSY3oB2ZLxh6s=; b=Ac3A9H+mopBWaMVfmqESCtY9ms/dMQqERbsUsXqs3GkW74LekbvBfKsMeqRuTT9Zn5J4wX MmsWbZhmycxDrshlXjuZkVolQC7k/ykHXAREMKKkY4rlnYDN0BSD0qCuPg+93d+F4+LHHl dj1/0jNOlJhvi3/ZoUv4ZInkQSgPjX8= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-644-WBub0-VgMjql88birhh2lQ-1; Sat, 20 Jan 2024 22:57:25 -0500 X-MC-Unique: WBub0-VgMjql88birhh2lQ-1 Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-29045a73796so1842305a91.1 for ; Sat, 20 Jan 2024 19:57:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705809444; x=1706414244; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jsFSfeo4uKal8oRM11+CMcxLBYdcs2TSY3oB2ZLxh6s=; b=ImVbARk5zVG0tjx6WYGu7x0+jQm5f8hTntmBBWKlGm1uPzHcSmlWmALDhvkXfN6iAF ZQHXLgXOZpeZno5mFqxQFgq2dP/xR+HnX78IUFdBp8yPWSw4voxBLkPo0PQmqSZYO1Xv kWZJg7mMkojUnshEOlbIelGox1q4ML7yryNVO+gWNgms+7yfL/N3Lq/lxoj8bAeR5S+9 o9GMmEDce0P3xzvWuIESUQ9o3e4/fo9BmsEQvuX8TKjNMAhbi/fY9lSpyHez8MGnGdDF X4eAUBc33XGiNPZqQh2URwMRFrE+qJ9SCVW78WteGY+EB8vjbpSDLA+qgq9dkUfD/Td9 yjBg== X-Gm-Message-State: AOJu0Yz61LYL9bYslSKni+2g9D1ZKczsDz+liDc/ObzzgJwIPBa6g6u9 yLvOwBxFAHx8lE73+kSCWzQ7r9aNutIOOc0DcbSnntzPswYUM5vi5cNDM/gd149uLT++MFqbKMe mMoXwaREH7Rr9qnIo3fJfoEXlvfh+WxeXVB/O19hj1djMNhKf8izdU3DbNxhOiCojtf8= X-Received: by 2002:a17:90a:8ca:b0:290:4b14:3638 with SMTP id 10-20020a17090a08ca00b002904b143638mr2690857pjn.12.1705809444293; Sat, 20 Jan 2024 19:57:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IFtnBNn6rsPMPeYrqQGms36/qKMotOVL7VKsrsc/Nj2ZKI8xBs2dbnu1wfsWeLuOVW0LRY+Sg== X-Received: by 2002:a17:90a:8ca:b0:290:4b14:3638 with SMTP id 10-20020a17090a08ca00b002904b143638mr2690851pjn.12.1705809443900; Sat, 20 Jan 2024 19:57:23 -0800 (PST) Received: from [150.1.200.12] (174-21-92-140.tukw.qwest.net. [174.21.92.140]) by smtp.gmail.com with ESMTPSA id db11-20020a17090ad64b00b002900eeada1asm6834134pjb.43.2024.01.20.19.57.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 20 Jan 2024 19:57:23 -0800 (PST) Message-ID: <34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com> Date: Sat, 20 Jan 2024 19:57:23 -0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 13/16] gdb: allow 'set args' and run commands to contain newlines To: Andrew Burgess , gdb-patches@sourceware.org Cc: Michael Weghorn References: <4b226e07edabf10a704aef70fd9ef4a3f2a5bd34.1704809585.git.aburgess@redhat.com> From: Keith Seitz In-Reply-To: <4b226e07edabf10a704aef70fd9ef4a3f2a5bd34.1704809585.git.aburgess@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 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 List-Id: On 1/9/24 06:26, Andrew Burgess wrote: > After this commit 'set args' as well as 'run', 'start', and 'starti', > will now accept multi-line inferior arguments, e.g.: > > (gdb) set args "abc > > def" > (gdb) show args > Argument list to give program being debugged when it is started is ""abc > def"". > While playing with this, I noticed some odd behavior. I don't know if this was introduced by your patch or (more likely) I just noticed it with your patch. [I don't think there's any reason to block this patch pending a solution in either case.] I'm noticing some extra newlines output with this new command if you use ^d to interrupt entering input: (gdb) set args " > quit (gdb) show args Argument list to give program being debugged when it is started is "". (gdb) Notice the extra newline output after the last command. > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index a3c32792491..bad8736aad2 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -120,11 +120,112 @@ show_inferior_tty_command (struct ui_file *file, int from_tty, > "is \"%s\".\n"), inferior_tty.c_str ()); > } > > +/* Return true if the inferior argument string ARGS represents a "complete" > + set of arguments. Arguments are considered complete so long as they > + don't contain unbalanced single or double quoted strings. Unbalanced > + means that a single or double quoted argument is started, but not > + finished. */ > + > +static bool > +args_complete_p (const std::string &args) > +{ > + const char *input = args.c_str (); > + bool squote = false, dquote = false; > + > + while (*input != '\0') > + { > + input = skip_spaces (input); > + > + if (squote) > + { > + /* Inside a single quoted argument, look for the closing single > + quote. */ > + if (*input == '\'') > + squote = false; > + } > + else if (dquote) > + { > + /* If we see either '\"' or '\\' within a double quoted argument > + then skip both characters (one is skipped here, and the other > + at the end of the loop). We need to skip the '\"' so that we > + don't consider the '"' as closing the double quoted argument, > + and we don't skip the entire '\\' then we'll only skip the > + first '\', in which case we might see the second '\' as a '\"' > + sequence, which would be wrong. */ > + if (*input == '\\' && strchr ("\"\\", *(input + 1)) != nullptr) > + ++input; > + /* Otherwise, just look for the closing double quote. */ > + else if (*input == '"') > + dquote = false; > + } > + else > + { > + /* Outside of either a single or double quoted argument, we need > + to check for '\"', '\'', and '\\'. The escaped quotes we > + obviously need to skip so we don't think that we have started > + a quoted argument. The '\\' we need to skip so we don't just > + skip the first '\' and then incorrectly consider the second > + '\' are part of a '\"' or '\'' sequence. */ > + if (*input == '\\' && strchr ("\"\\'", *(input + 1)) != nullptr) > + ++input; > + /* Otherwise, check for the start of a single or double quoted > + argument. */ > + else if (*input == '\'') > + squote = true; > + else if (*input == '"') > + dquote = true; > + } > + > + ++input; > + } > + > + return (!dquote && !squote); > +} > + > +/* ... */ ^^^ Did you forget to finish your comment here? Keith > + > +static std::string > +get_complete_args (std::string args) > +{ > + /* If the user wants an argument containing a newline then they need to > + do so within quotes. Use args_complete_p to check if the ARGS string > + contains balanced double and single quotes. If not then prompt the > + user for additional arguments and append this to ARGS. */ > + const char *prompt = nullptr; > + while (!args_complete_p (args)) > + { > + if (prompt == nullptr) > + { > + prompt = getenv ("PS2"); > + if (prompt == nullptr) > + prompt = "> "; > + } > + > + std::string buffer; > + const char *content = command_line_input (buffer, prompt, "set_args"); > + if (content == nullptr) > + return {}; > + > + args += "\n" + buffer; > + } > + > + return args; > +} > + > /* Store the new value passed to 'set args'. */ > > static void > -set_args_value (const std::string &args) > +set_args_value (const std::string &args_in) > { > + std::string args; > + > + if (!args_in.empty ()) > + { > + args = get_complete_args (args_in); > + if (args.empty ()) > + return; > + } > + > current_inferior ()->set_args (args); > } > > @@ -376,6 +477,21 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) > > dont_repeat (); > > + gdb::unique_xmalloc_ptr stripped = strip_bg_char (args, &async_exec); > + args = stripped.get (); > + > + std::string inf_args; > + /* If there were other args, beside '&', process them. */ > + if (args != nullptr) > + { > + /* If ARGS is only a partial argument string then this call will > + interactively read more arguments from the user. If the user > + quits then we shouldn't start the inferior. */ > + inf_args = get_complete_args (args); > + if (inf_args.empty ()) > + return; > + } > + > scoped_disable_commit_resumed disable_commit_resumed ("running"); > > kill_if_already_running (from_tty); > @@ -397,9 +513,6 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) > reopen_exec_file (); > reread_symbols (from_tty); > > - gdb::unique_xmalloc_ptr stripped = strip_bg_char (args, &async_exec); > - args = stripped.get (); > - > /* Do validation and preparation before possibly changing anything > in the inferior. */ > > @@ -412,6 +525,9 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) > > /* Done. Can now set breakpoints, change inferior args, etc. */ > > + if (!inf_args.empty ()) > + current_inferior ()->set_args (inf_args); > + > /* Insert temporary breakpoint in main function if requested. */ > if (run_how == RUN_STOP_AT_MAIN) > { > @@ -433,10 +549,6 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) > the user has to manually nuke all symbols between runs if they > want them to go away (PR 2207). This is probably reasonable. */ > > - /* If there were other args, beside '&', process them. */ > - if (args != nullptr) > - current_inferior ()->set_args (args); > - > if (from_tty) > { > uiout->field_string (nullptr, "Starting program"); > diff --git a/gdb/testsuite/gdb.base/inferior-args.exp b/gdb/testsuite/gdb.base/inferior-args.exp > index 6c22ecb3c54..b259e4a4217 100644 > --- a/gdb/testsuite/gdb.base/inferior-args.exp > +++ b/gdb/testsuite/gdb.base/inferior-args.exp > @@ -211,12 +211,15 @@ lappend test_desc_list [list "test four" \ > [list "$hex \"'\"" \ > "$hex \"\\\\\"\""]] > > -# Run all tests in the global TEST_DESC_LIST. > +# Run all tests in the global TEST_DESC_LIST, as well as some tests of > +# inferior arguments containing newlines. > proc run_all_tests {} { > + set all_methods { "start" "starti" "run" "set args" } > + > foreach desc $::test_desc_list { > lassign $desc name stub_suitable args re_list > with_test_prefix $name { > - foreach_with_prefix set_method { "start" "starti" "run" "set args" } { > + foreach_with_prefix set_method $all_methods { > foreach_with_prefix startup_with_shell { on off } { > do_test $set_method $startup_with_shell $args $re_list \ > $stub_suitable > @@ -224,6 +227,70 @@ proc run_all_tests {} { > } > } > } > + > + # Check the multi-line argument entry. This isn't going to work when > + # using the gdbstub, as the only way to set arguments in this case is > + # via the gdbserver command line, which isn't what we're testing here. > + if { ![use_gdb_stub] } { > + foreach_with_prefix set_method $all_methods { > + clean_restart $::binfile > + > + # First check that we can abort entering multi-line arguments. > + set saw_prompt false > + gdb_test_multiple "$set_method \"ab" "abort argument entry" { > + -re "^$set_method \"ab\r\n" { > + exp_continue > + } > + -re "^> $" { > + set saw_prompt true > + send_gdb "\004" > + exp_continue > + } > + -re "quit\r\n$::gdb_prompt $" { > + gdb_assert {$saw_prompt} \ > + $gdb_test_name > + } > + } > + > + # Now place a breakpoint on main. > + if { ![gdb_breakpoint "main" message] } { > + fail "could not set breakpoint on main" > + continue > + } > + > + # And actually enter some multi-line arguments. > + set saw_prompt false > + gdb_test_multiple "$set_method \"xy" "complete argument entry" { > + -re "^$set_method \"xy\r\n" { > + exp_continue > + } > + -re "^> $" { > + set saw_prompt true > + send_gdb "12\"\n" > + exp_continue > + } > + > + -re "$::gdb_prompt $" { > + gdb_assert { $saw_prompt } \ > + $gdb_test_name > + } > + } > + > + # For the two methods that don't automatically run to main, > + # poke the inferior along to main. > + if { $set_method == "set args" } { > + if { ![runto_main] } { > + continue > + } > + } elseif { $set_method == "starti" } { > + gdb_continue_to_breakpoint "b/p in main" > + } > + > + # And check we correctly see the argument containing a newline. > + gdb_test "print argc" " = 2" > + gdb_test "print argv\[1\]" " = $::hex \"xy\\\\n12\"" > + } > + } > } > > run_all_tests