From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115621 invoked by alias); 28 Jan 2018 16:50:33 -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 115611 invoked by uid 89); 28 Jan 2018 16:50:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*r:112, palves, hey X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 28 Jan 2018 16:50:30 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w0SGoN94001538 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 28 Jan 2018 11:50:28 -0500 Received: by simark.ca (Postfix, from userid 112) id D068B1E5B7; Sun, 28 Jan 2018 11:50:23 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 43C491E093; Sun, 28 Jan 2018 11:50:22 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 28 Jan 2018 16:50:00 -0000 From: Simon Marchi To: Sergio Durigan Junior Cc: gdb-patches@sourceware.org, Simon Marchi , Pedro Alves Subject: Re: [Regression] Segfault on native-extended-gdbserver + fork In-Reply-To: <87efmaebo3.fsf_-_@redhat.com> References: <20180119161628.21611-1-simon.marchi@polymtl.ca> <20180119161628.21611-3-simon.marchi@polymtl.ca> <87efmaebo3.fsf_-_@redhat.com> Message-ID: <931f8b594f7405649778f66ab2960a40@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.2 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 28 Jan 2018 16:50:23 +0000 X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00574.txt.bz2 On 2018-01-28 01:32, Sergio Durigan Junior wrote: > Hey Simon, > > While working on something else, I noticed a regression introduced by > this patch. Hi Sergio, Thanks for reporting and fixing this! > Consider the following example program: > > #include > > int > main (int argc, char *argv[]) > { > fork (); > > return 0; > } > > When running it under gdbserver: > > # ./gdb/gdbserver/gdbserver --multi --once :2345 > > And debugging it under GDB: > > # ./gdb/gdb -q -batch -ex 'set remote exec-file ./a.out' -ex 'tar > extended-remote :2345' -ex r ./a.out > Starting program: > ... > [Detaching after fork from child process 16102.] > Segmentation fault (core dumped) > > The problem happens on inferior.c:detach_inferior: > > void > detach_inferior (inferior *inf) > { > /* Save the pid, since exit_inferior_1 will reset it. */ > int pid = inf->pid; > ^^^^^^^^^ > > exit_inferior_1 (inf, 0); > > if (print_inferior_events) > printf_unfiltered (_("[Inferior %d detached]\n"), pid); > } > > When this code is called from remote.c:remote_follow_fork, the PID is > valid but there is not 'inferior' associated with it, which means that > 'inf == NULL'. Ah yeah, I hadn't thought about that. > I've been thinking about the proper fix to this, and arrived at the > patch attached (without a ChangeLog entry; will add that if the patch > seems OK for you). Since we will still want to print inferior events > even if 'inf == NULL', I've duplicated the print on the > "detach_inferior > (int pid)" version. Other than that, the patch is basically restoring > the old behaviour of just skipping the detach procedure if there's no > inferior object. I'm fine with this, but I was curious about what happens in Pedro's multi-target branch. I remember he said that the detach_inferior(int) version disappears in that branch, though I can't find where he said that. But looking at the branch I can see it's indeed the case: https://github.com/palves/gdb/blob/palves/multi-target/gdb/inferior.c#L250 So I was wondering what remote_follow_fork calls in that case, since it can't call the detach_inferior(inferior *) version without an inferior. Apparently it calls a new remote_detach_pid function: https://github.com/palves/gdb/blob/palves/multi-target/gdb/remote.c#L5859 This means (I just tried it) that it won't show the "[Inferior %d detached]\n" message in that case. So what I would suggest is putting if (print_inferior_events) printf_unfiltered (_("[Inferior %d detached]\n"), pid); in its own function, called by both versions of detach_inferior for now (bonus, it de-duplicates the printing of the message). In the multi-target branch, remote_target::follow_fork (renamed from remote_follow_fork) can call this function in the case where we don't have an inferior object. > I'm running a regression test on BuildBot to make sure no regressions > are introduced. I was going to write a testcase to exercise this > scenario, but we already have one, gdb.base/foll-vfork.exp. The > failures were marked as ERROR's by dejagnu, which may explain why they > were missed...? Not sure. Oh, and this regression is not present in > the 8.1 branch. > > WDYT? That's fine with me with the printing of the message in its own function, as explained above. > -- > Sergio > GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 > Please send encrypted e-mail if possible > http://sergiodj.net/ > > diff --git a/gdb/inferior.c b/gdb/inferior.c > index 38b7369275..94432a92b1 100644 > --- a/gdb/inferior.c > +++ b/gdb/inferior.c > @@ -272,7 +272,15 @@ detach_inferior (inferior *inf) > void > detach_inferior (int pid) > { > - detach_inferior (find_inferior_pid (pid)); > + inferior *inf = find_inferior_pid (pid); > + > + if (inf != NULL) > + detach_inferior (inf); > + else > + { > + if (print_inferior_events) > + printf_unfiltered (_("[Inferior %d detached]\n"), pid); > + } > } > > void Thanks, Simon