From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50595 invoked by alias); 7 Jun 2017 10:16:01 -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 50467 invoked by uid 89); 7 Jun 2017 10:15:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-15.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=Share, inprocess, blame, agent X-HELO: mail-wm0-f52.google.com Received: from mail-wm0-f52.google.com (HELO mail-wm0-f52.google.com) (74.125.82.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Jun 2017 10:15:44 +0000 Received: by mail-wm0-f52.google.com with SMTP id d64so8740551wmf.1 for ; Wed, 07 Jun 2017 03:15:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=wikeh0sy1cyiud/QO3+32jJL/0iuD9m43I0hAdSBcoY=; b=eATaoytHpUsuxLUjmBqCdJmgF6xTW1GuHOzZiNYUA+baAOq5lAkamHBp9IdTEBp/W5 U4t3jEnPw/enrOf8vxufa2+0xXWFwdDNcYMreNuU2hy1rQyxXY5tumdMFhNyAIvZc1Qj 1UczfLJMjSYdchsMJJR7DEMuCqzT/qMT34vfT5zUXpNjhuXa4hGfev5F4qHOPk0x/eG4 kNu/uEppE/lRtn/fG1gkO5G0Kpj84YmoMWYthHSLrMO+A1n4E4/KaAv6/0IzcxF9kpS2 llfWH62NHtr7dXhgtDg+/ZywxQjVFbZSKkb1FD1uqLVy9xoZVLjPW25k6iTNX3gFUv8x QgFA== X-Gm-Message-State: AKS2vOxYW9dTmPXElzYND5JnL3+NJ/j0nRgFsgys7fKA/ledTep7twi4 Qb/Dfgn8w4I3nUsag20/ag== X-Received: by 10.28.220.86 with SMTP id t83mr1554947wmg.66.1496830546504; Wed, 07 Jun 2017 03:15:46 -0700 (PDT) Received: from [192.168.0.102] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id 47sm1540932wrb.55.2017.06.07.03.15.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Jun 2017 03:15:45 -0700 (PDT) Subject: Re: [PATCH v6 3/4] Share fork_inferior et al with gdbserver To: Sergio Durigan Junior References: <1482464361-4068-1-git-send-email-sergiodj@redhat.com> <20170504052954.16936-1-sergiodj@redhat.com> <20170504052954.16936-4-sergiodj@redhat.com> <0e95d746-9293-3301-15ed-84d6c4280116@redhat.com> <87poep8zdg.fsf@redhat.com> Cc: GDB Patches From: Pedro Alves Message-ID: Date: Wed, 07 Jun 2017 10:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <87poep8zdg.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00168.txt.bz2 On 05/31/2017 04:43 AM, Sergio Durigan Junior wrote: > >>> >> index 4ea7913..8aa85db 100644 >>> >> --- a/gdb/gdbserver/configure.ac >>> >> +++ b/gdb/gdbserver/configure.ac >>> >> @@ -462,7 +462,9 @@ esac], >>> >> >>> >> if $want_ipa ; then >>> >> if $have_ipa ; then >>> >> - IPA_DEPFILES="$ipa_obj" >>> >> + # Needed because safe_strerror's definition is host-dependent >> > >> > Why do we end up needing safe_strerror in the IPA in the first place? > This is needed because I moved the definition of > trace_start_error_with_name from the old gdb/fork-child.c to > common/common-utils.c. This function which uses safe_strerror, and > common/common-utils.c is compiled by IPA. > > An option would be to keep these trace_start_error.* functions in > nat/fork-inferior.c, but I think it is more logical to keep them on > common-utils.c. I'd rather not add them to the IPA. The least unnecessary code is included in that library the better, because it is injected into the target process. So keeping them in fork-inferior.c sounds better. > > Now, I can obviously expand the comment if that's what you meant. > >>> >> + strerror_obj="`echo $srv_host_obs | sed 's/\(.*-strerror\)\.o/\1-ipa.o/'`" >>> >> + IPA_DEPFILES="$ipa_obj $strerror_obj" >>> >> extra_libraries="$extra_libraries libinproctrace.so" >>> >> else >>> >> AC_MSG_ERROR([inprocess agent not supported for this target]) >> > >> > >> > >>> >> -#if defined(__UCLIBC__) && defined(HAS_NOMMU) >>> >> - pid = vfork (); >> > >> > Does fork_inferior end up always using vfork on no-MMU >> > ports somehow? > Sorry, I am not sure. How would I go about finding that? I'm not sure either. configure.ac for anything fork related? Look at git blame history around those lines, see what other bits were touched at the same time? gdbserver clearly cares about being built for no-MMU ports, so the new code must too. We can't just delete that support without an alternative. >>> >> + >>> >> +/* This function is NOT reentrant. Some of the variables have been >>> >> + made static to ensure that they survive the vfork call. */ >>> >> +extern pid_t fork_inferior (const char *exec_file_arg, >>> >> + const std::string &allargs, >> > >> > Should include . > Should it? It's compiling fine without it. I included just for the > sake of clarity anyway. Yes, headers should include what they depend on directly. > Now, something that came up while I was testing things with mingw here. > gdb/gdbserver/server.c now calls startup_inferior (define in > nat/fork-inferior.c) directly, instead of doing things on its own. This > is one of the goals of this series, but for targets that don't compile > fork-inferior.c (like Windows) this is an obvious problem. So here's > what I'm doing for the new version of the series: I'll move > startup_inferior to common/ (probably common/common-fork-inferior.c or > some such), so that all targets can have access to it (it's > target-agnostic anyway). If you have any comments about this, please > let me know. This sounds quite contorted, but I'll take a better look at it in v7. Thanks, Pedro Alves