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.129.124]) by sourceware.org (Postfix) with ESMTP id 55FA43858D21 for ; Wed, 23 Oct 2024 13:01:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 55FA43858D21 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 55FA43858D21 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=1729688493; cv=none; b=dXGZ+y5VRtcOMJsSGn/05Zf8BXiz00607kmYCaD3VPPSHDGCUAXZcWbDZbP5js2fKvLBTO8bbCOMJ9p9cqhPsPCEYbHb55mb1i6iYVuZWB9KjVgr9PpEe0mspsLhZYvY4+D1v0TZZDv+AmSD2qAEVwa4nQFcKgi2RQZ41Ax2pz8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729688493; c=relaxed/simple; bh=fuzTLR/WTwV/pdDuhUUMIw4Yug/U+9CJnmoos4ajG2w=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=RCE64hwVpXqVHVqM4lN18hJ/mt5jwwWOBW4OlzuIUev7xNtzXLMNVXbLChxMSUkBgFr+PkKGIKf1pB51NtkXbKg6QpuoEPqPM5uZIR4HkCxt67N75VBAoJoUkvuTku2RKd3zAwovIX3FNhWJdGcpuIhffmxQDXldCFUAIwFWUiI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729688491; 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: in-reply-to:in-reply-to:references:references; bh=nZshedwMq8baYqxH2l08XbLFOLRC93fqKCBjFAXqthc=; b=NHtNUjh23XP+/rSC0JvTdBOIoaGQFOAGi5Eqz6vZ6YM6/Dg4HV8IBif+a6T7zxlfhMCihv dgSF/V9eolh/6Irhboyd7IVnljLg+omQFQkPTo6IVUKwdAQb8XVKUsjocTqiM4HWks5WVO l8+qZjRBkb77Gm8Hzw7C+ROOUp6/RdE= 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-85-aVHyehp0OaGlPchpAj-maw-1; Wed, 23 Oct 2024 09:01:29 -0400 X-MC-Unique: aVHyehp0OaGlPchpAj-maw-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-37d4a211177so3397907f8f.0 for ; Wed, 23 Oct 2024 06:01:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729688488; x=1730293288; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nZshedwMq8baYqxH2l08XbLFOLRC93fqKCBjFAXqthc=; b=JPNpaIbs70lVWvIkSpswEk0oLRO4jhN6bWwWBGxp6Sn9NkcNmUACkTCzdC2jayRSsR 2yuEiyeD6RZ2NI/IE4tjRJnvRO2OZVfdqozEiAM6uy3BE9eIxZ6kf5lrCpqMdYXl7Drc 9jMLFoMqfP//HGE8XC9RDz9G3ggwnzC76nFG6Pr2uOlYvVSQzN2+SzvxaqqLU3H0STp1 yBrs977ZgGmLo9iqmcDJoZfwG2qu4djfyOAc2m1f4UrbreQmRIF0pAqy8wJnVqlLmVL/ RpmN1ySOrYCWeeMy2/bJhqRy526YuP9boDrUkktdMvDyncc4GZtXaBV0jO6c3ea5Aq+Q vkug== X-Forwarded-Encrypted: i=1; AJvYcCU8i4SHaZqfhEisnaK1PT4PjXQ5jMhwVonyl2OLp/hSLiwdpksAndZ8Wie19qOTRo7q53PhaSOmXI2FVA==@sourceware.org X-Gm-Message-State: AOJu0YyX0ZHwbjrdtsxJkgGwzYmtv12UHEnAmt7IZ4WOsrbjo3c9hlf4 5dgSttvRHo9pYSRw36JDTVpsZS+SHaUoxiQMvhHN/zmB6vz29o6AYXG9GeFmUB0llAddhJZh1OD ee8wVGMdHzG6JgKnC4DH+WtlzqlWgJ0+4TKi/Xf3qG9AMiiX8Orpsvf574+g= X-Received: by 2002:a05:6000:cd0:b0:37d:51f8:46fd with SMTP id ffacd0b85a97d-37efcf149ccmr1677539f8f.22.1729688488037; Wed, 23 Oct 2024 06:01:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGyrzupPDm4tBgRYmb1e/vKuW8CSfEZN3K3oHnYTdLr5h6BZayk+XB9GwsFsZUH38xYWO1vVw== X-Received: by 2002:a05:6000:cd0:b0:37d:51f8:46fd with SMTP id ffacd0b85a97d-37efcf149ccmr1677167f8f.22.1729688484581; Wed, 23 Oct 2024 06:01:24 -0700 (PDT) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-37ee0a48903sm8851591f8f.37.2024.10.23.06.01.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Oct 2024 06:01:23 -0700 (PDT) From: Andrew Burgess To: Keith Seitz , gdb-patches@sourceware.org Cc: Michael Weghorn Subject: Re: [PATCH 13/16] gdb: allow 'set args' and run commands to contain newlines In-Reply-To: <34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com> References: <4b226e07edabf10a704aef70fd9ef4a3f2a5bd34.1704809585.git.aburgess@redhat.com> <34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com> Date: Wed, 23 Oct 2024 14:01:21 +0100 Message-ID: <87ttd3vuqm.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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_H2,SPF_HELO_NONE,SPF_NONE,TXREP 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: Keith Seitz writes: > 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. I know it's been a while, but can you give any more details about the environment in which you saw this extra newline bug? I rebased this series onto a recent master and it doesn't reproduce for me. Then I went back and tried this patch as it was based when I originally posted it, and I can't reproduce it then either. Any hints to try and reproduce this would be awesome. Thanks, Andrew > >> 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