From patchwork Thu May 15 20:43:42 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 952 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id ABACA36006F for ; Thu, 15 May 2014 13:43:51 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 6FC0D58E76BC; Thu, 15 May 2014 13:43:51 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 3DE9258EED5D for ; Thu, 15 May 2014 13:43:51 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=jzLnA1pCN+L1dQtHOVi0PIfnLiyrj pzFd71aBNlwaKIWP6lWWt+3V83JHwXTWtytZrccmUqHWKasy78sw6Qav+oLYzxZN 4JiJPaxkFQOAelZ0rG961qe+AWoFT22D3zBoOgdH9wIDmQxDYtnR7MMmhu2FGpSk gsfOdq9T+6sB4A= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; s=default; bh=gkst+ggWDwf+LWI5LOsDOOymlio=; b=ReY Xa/bh/PKN0jiWm2t4FlN0Yyh8AxLtCx2yvjs/ugx9F31AzoH6VCMqbLWV54Zmcrx wnt198vghhwJpuBaANS8u8tx8PWBsKIWF7O5M0987wa7mGeEXBa6mx60+j/BGmwO D40X0UJRNj3OPJZLbISF5cJ7VnDaNdmIqvWVDCtw= Received: (qmail 19061 invoked by alias); 15 May 2014 20:43:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 19044 invoked by uid 89); 15 May 2014 20:43:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL, BAYES_00, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 May 2014 20:43:47 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1Wl2VD-00024Q-MM from donb@codesourcery.com for gdb-patches@sourceware.org; Thu, 15 May 2014 13:43:43 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 15 May 2014 13:43:43 -0700 Received: from build2-lucid-cs (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.2.247.3; Thu, 15 May 2014 13:43:42 -0700 Received: by build2-lucid-cs (Postfix, from userid 1905) id CB4813A093D; Thu, 15 May 2014 13:43:42 -0700 (PDT) From: Don Breazeal To: , Subject: Re: [PATCH] Fix for gdb/PR 14808, vfork/exec inferior problem Date: Thu, 15 May 2014 13:43:42 -0700 Message-ID: <1400186622-3321-1-git-send-email-donb@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in On 5/15/2014 12:06 AM, Yao Qi wrote: > 05/13/2014 07:34 AM, donb@codesourcery.com wrote: >> @@ -649,6 +649,7 @@ handle_vfork_child_exec_or_exit (int exec) >> struct cleanup *old_chain; >> struct program_space *pspace; >> struct address_space *aspace; >> + struct inferior *parent_inf; >> > > Local parent_inf is only used in the "if (exec)" block below, so better > to declare it there. OK, done. --snip-- >> + where the parent called vfork. Now that the child has >> + called exec and we are detaching from the parent, the >> + parent inferior needs to have its own pspace and aspace > > parent inferior has its own pspace, but may not have its own aspace, > depending on gdbarch_has_shared_address_space. Fixed. > >> + so that changes in the child don't affect it. We have >> + to give the new spaces to the parent since we saved the >> + child's spaces as the current spaces above. Even though >> + we are detaching the parent, we want to keep the >> + corresponding entry in the inferiors list intact. */ >> + parent_inf = current_inferior (); >> + parent_inf->aspace = new_address_space (); > > Rather than creating a new address space, use maybe_new_address_space, like >> + parent_inf->pspace = add_program_space (parent_inf->aspace); > > parent_inf->pspace > = add_program_space (maybe_new_address_space ()); > parent_inf->aspace = parent_inf->pspace->aspace; Done. > >> + parent_inf->removable = inf->removable; > > Field removable of parent inferior should be unchanged, IMO. It turns out this assignment was a NOP. Removed. > >> + set_current_program_space (parent_inf->pspace); >> + clone_program_space (parent_inf->pspace, pspace); > > Do we need to unlink parent and child? I am not very sure. > > /* Break the bonds. */ > inf->vfork_parent->vfork_child = NULL; > We don't need to do it here, since this happens in target_detach. Since parent_inf is the parent, it's fork_child pointer is set. The unlinking happens in inferior.c:exit_inferior_1, which is ultimately called via target_detach. The child's vfork_parent pointer is cleared by existing code near the end of handle_vfork_child_exec_or_exit. Here is the updated patch. --Don gdb/ 2014-05-15 Don Breazeal * infrun.c (handle_vfork_child_exec_or_exit): For the case of a vfork where we follow the child and detach the parent, and the child execs, create a new pspace for the parent inferior. gdb/testsuite 2014-05-15 Don Breazeal * gdb.base/foll-vfork.exp (vfork_relations_in_info_inferiors): Test that after a vfork and child exec, the parent's exec file name has not been changed. --- gdb/infrun.c | 44 ++++++++++++++++++++++---------- gdb/testsuite/gdb.base/foll-vfork.exp | 11 ++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index ab39b6e..20ab003 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -665,27 +665,42 @@ handle_vfork_child_exec_or_exit (int exec) else old_chain = save_current_space_and_thread (); - /* We're letting loose of the parent. */ + /* Make the parent the current inferior for target_detach. */ tp = any_live_thread_of_process (inf->vfork_parent->pid); switch_to_thread (tp->ptid); - /* We're about to detach from the parent, which implicitly - removes breakpoints from its address space. There's a - catch here: we want to reuse the spaces for the child, - but, parent/child are still sharing the pspace at this - point, although the exec in reality makes the kernel give - the child a fresh set of new pages. The problem here is - that the breakpoints module being unaware of this, would - likely chose the child process to write to the parent - address space. Swapping the child temporarily away from - the spaces has the desired effect. Yes, this is "sort - of" a hack. */ - + /* The child inferior may be dead, so avoid giving the + breakpoints module the option to write through to it + by swapping the child temporarily away from the spaces + (cloning a program space resets breakpoints). */ pspace = inf->pspace; aspace = inf->aspace; inf->aspace = NULL; inf->pspace = NULL; + if (exec) + { + struct inferior *parent_inf; + + /* The parent and child inferiors have been sharing + program and address space structures from the point + where the parent called vfork. Now that the child has + called exec and we are detaching from the parent, the + parent inferior needs to have its own pspace so that + changes in the child don't affect it. We have to give + the new pspace to the parent (instead of the child) + since we saved the child's spaces as the current spaces + above. Even though we are detaching the parent, we + want to keep the corresponding entry in the inferiors + list intact. */ + parent_inf = current_inferior (); + parent_inf->pspace + = add_program_space (maybe_new_address_space ()); + parent_inf->aspace = parent_inf->pspace->aspace; + set_current_program_space (parent_inf->pspace); + clone_program_space (parent_inf->pspace, pspace); + } + if (debug_infrun || info_verbose) { target_terminal_ours (); @@ -702,9 +717,10 @@ handle_vfork_child_exec_or_exit (int exec) inf->vfork_parent->pid); } + /* Detach the parent. */ target_detach (NULL, 0); - /* Put it back. */ + /* Put the child spaces back. */ inf->pspace = pspace; inf->aspace = aspace; diff --git a/gdb/testsuite/gdb.base/foll-vfork.exp b/gdb/testsuite/gdb.base/foll-vfork.exp index fe3663c..e9b0110 100644 --- a/gdb/testsuite/gdb.base/foll-vfork.exp +++ b/gdb/testsuite/gdb.base/foll-vfork.exp @@ -442,6 +442,17 @@ proc vfork_relations_in_info_inferiors { variant } { pass $test } } + + # Make sure the exec file name of the vfork parent is not + # changed when the child's is changed. + if { $variant == "exec" } { + set test "exec file name change" + gdb_test_multiple "info inferiors" $test { + -re " 2 .*vforked-prog.* 1 .*foll-vfork.*$gdb_prompt " { + pass $test + } + } + } }} proc do_vfork_and_follow_parent_tests {} {