From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117537 invoked by alias); 17 Apr 2018 20:07:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 117513 invoked by uid 89); 17 Apr 2018 20:07:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Apr 2018 20:07:53 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C3DB94023BAD for ; Tue, 17 Apr 2018 20:07:51 +0000 (UTC) Received: from localhost (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 664087C27; Tue, 17 Apr 2018 20:07:49 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Jan Kratochvil Subject: Re: [PATCH v6] Enable 'set print inferior-events' and improve detach/fork/kill/exit messages References: <20180124194714.26222-1-sergiodj@redhat.com> <20180416200444.30090-1-sergiodj@redhat.com> Date: Tue, 17 Apr 2018 20:07:00 -0000 In-Reply-To: (Pedro Alves's message of "Tue, 17 Apr 2018 16:56:52 +0100") Message-ID: <87fu3tfvfu.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-04/txt/msg00358.txt.bz2 On Tuesday, April 17 2018, Pedro Alves wrote: > On 04/16/2018 09:04 PM, Sergio Durigan Junior wrote: >> Changes from v5: >> >> - Save result from 'target_pid_to_str' using std::string instead of >> 'const char *', avoiding a possible race condition. > > Sorry Sergio, we're getting there, but this will need more work. I'm the one who apologizes. >> Built and regtested on BuildBot, without regressions. >> > >> * gdb.threads/process-dies-while-detaching.c >> (parent_function): Use 'usleep' in order to avoid >> race-conditions. > > This is stale. There's no 'usleep' any more. Removed. >> @@ -121,9 +120,11 @@ struct inferior * >> add_inferior (int pid) >> { >> struct inferior *inf = add_inferior_silent (pid); >> + std::string infpid = target_pid_to_str (pid_to_ptid (pid)); >> >> if (print_inferior_events) >> - printf_unfiltered (_("[New inferior %d]\n"), pid); >> + printf_unfiltered (_("[New inferior %d (%s)]\n"), >> + inf->num, infpid.c_str ()); > > Let's call target_pid_to_str directly here without the > deep copy. > > You needed to call target_pid_to_str early in kill_command > because there first kill the target, and print the > string _afterwards_. "target_kill" may pop the target from the > target stack, so we can't use target_pid_to_str after the > target is gone already. > > But here, we're not messing with the target stack. > > (Nor are we calling target_pid_to_str twice in a row in > the same expression.) I confess I knew this, but decided to do a deep copy anyway because I was aiming for consistency. But no problem at all, I'll simplify this part. > >> void >> @@ -261,12 +259,13 @@ void >> detach_inferior (inferior *inf) >> { >> /* Save the pid, since exit_inferior_1 will reset it. */ >> - int pid = inf->pid; >> + std::string infpid = target_pid_to_str (pid_to_ptid (inf->pid)); >> >> exit_inferior_1 (inf, 0); >> >> if (print_inferior_events) >> - printf_unfiltered (_("[Inferior %d detached]\n"), pid); >> + printf_unfiltered (_("[Inferior %d (%s) detached]\n"), >> + inf->num, infpid.c_str ()); > > Same here. Fixed. >> diff --git a/gdb/remote.c b/gdb/remote.c >> index f54a38833b..be118c5d97 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -5137,7 +5137,12 @@ remote_detach_1 (int from_tty, inferior *inf) >> /* If doing detach-on-fork, we don't mourn, because that will delete >> breakpoints that should be available for the followed inferior. */ >> if (!is_fork_parent) >> - target_mourn_inferior (inferior_ptid); >> + { >> + target_mourn_inferior (inferior_ptid); >> + if (print_inferior_events) >> + printf_unfiltered (_("[Inferior %d (process %d) detached]\n"), >> + inf->num, pid); > > This is still using "(process %d)", which is incorrect > for target remote. You're right, sorry about this. > Also, here, if debugging in "target remote" mode, and this is > the last live process, that target_mourn_inferior call will > unpush the remote target from the target stack. So this is another > case where you need to call target_pid_to_str _before_ calling > target_mourn_inferior. Good point. This is fixed. > As mentioned before, not all targets print "process $decimal" in > their target_pid_to_str implementation. So your testsuite changes > will need to adapt too. > > Please try this: hack away gdbserver/server.c's support for > the RSP multi-process extensions (look for "multiprocess+"), and > test with "target remote". That hack will emulate older gdbservers and > other stubs that don't support telling gdb the process's pid. > Manually confirm that you get "Remote target" instead of > "process $pid" -- see remote.c's implementation of > target_pid_to_str. Then run the testsuite against that gdbserver. [ After a quick chat to clarify things... ] I will make sure to adjust all regexps and take into account the many outputs that can happen here. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/