From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19313 invoked by alias); 28 Jan 2018 06:32:17 -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 19290 invoked by uid 89); 28 Jan 2018 06:32:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=chose, friday, hey X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 28 Jan 2018 06:32:14 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 94A7B80462; Sun, 28 Jan 2018 06:32:13 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6B14860170; Sun, 28 Jan 2018 06:32:13 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: gdb-patches@sourceware.org, Simon Marchi Subject: [Regression] Segfault on native-extended-gdbserver + fork (was: Re: [PATCH v2 3/3] Make linux_nat_detach/thread_db_detach use the inferior parameter) References: <20180119161628.21611-1-simon.marchi@polymtl.ca> <20180119161628.21611-3-simon.marchi@polymtl.ca> Date: Sun, 28 Jan 2018 06:32:00 -0000 In-Reply-To: <20180119161628.21611-3-simon.marchi@polymtl.ca> (Simon Marchi's message of "Fri, 19 Jan 2018 11:16:28 -0500") Message-ID: <87efmaebo3.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-01/txt/msg00573.txt.bz2 On Friday, January 19 2018, Simon Marchi wrote: > From: Simon Marchi > > No changes in v2. > > This patch makes these two functions actually use the inferior parameter > added by the previous patch, instead of reading inferior_ptid. I chose > these two, because they are the one actually used when I detach on my > GNU/Linux system, so they were easy to test. > > I took the opportunity to pass the inferior being detached to > inf_ptrace_detach_success, so it could use it too. From there, it made > sense to add an overload of detach_inferior that takes the inferior > directly rather than the pid, to avoid having to pass inf->pid only for > the callee to look up the inferior structure by pid. Hey Simon, While working on something else, I noticed a regression introduced by this patch. 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'. 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 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? -- 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