From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22129 invoked by alias); 22 Aug 2018 14:37:13 -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 21603 invoked by uid 89); 22 Aug 2018 14:37:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS 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; Wed, 22 Aug 2018 14:37:11 +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 310B287A6D; Wed, 22 Aug 2018 14:37:09 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1CC6AA9E84; Wed, 22 Aug 2018 14:37:07 +0000 (UTC) Subject: Re: [RFA 3/5] Darwin: set startup-with-shell to off on Sierra and later. To: Simon Marchi , Xavier Roirand References: <1534932677-9496-1-git-send-email-roirand@adacore.com> <1534932677-9496-4-git-send-email-roirand@adacore.com> <9a64f5298b404c3be126333b4287e390@polymtl.ca> Cc: gdb-patches@sourceware.org, brobecker@adacore.com From: Pedro Alves Message-ID: <271518a7-d06a-219b-b4a0-6958e8f1dc51@redhat.com> Date: Wed, 22 Aug 2018 14:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <9a64f5298b404c3be126333b4287e390@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-08/txt/msg00533.txt.bz2 On 08/22/2018 03:20 PM, Simon Marchi wrote: > On 2018-08-22 06:11, Xavier Roirand wrote: >> On Mac OS X Sierra and later, the shell is not allowed to be >> debug so add a check and disable startup with shell in that >> case. > > Ah, that's a really good idea to do it automatically, instead of asking the user to do it. See also: https://sourceware.org/ml/gdb-patches/2018-06/msg00734.html BTW, on IRC, Tromey and I discussed how to make startup-with-shell work, which led to: https://sourceware.org/bugzilla/show_bug.cgi?id=23364 which points at what seems like the simplest solution (copy the shell to /tmp). An alternative would be to use a similar approach to lldb's -- run a helper tool (lldb-argdumper) via the shell whose only purpose is to return back its shell-expanded argv[0..N]. That would work because gdb wouldn't debug that program, just run it normally, so it wouldn't run into the SIP restrictions. Instead of a separate helper tool, I would imagine we could also build the helper functionality inside gdb itself -- we'd make gdb behave in that "expand args" mode via an environment variable, for example. > >> gdb/ChangeLog: >>     * darwin-nat.c (disable_startup_with_shell): New function. >>     (_initialize_darwin_inferior): Add call. >> >> Change-Id: Ia3cbeaa89b2b44a173b93ee22cce0d3884a16924 >> --- >>  gdb/darwin-nat.c | 22 ++++++++++++++++++++++ >>  1 file changed, 22 insertions(+) >> >> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c >> index be80163..96f70cf 100644 >> --- a/gdb/darwin-nat.c >> +++ b/gdb/darwin-nat.c >> @@ -2362,6 +2362,26 @@ darwin_nat_target::supports_multi_process () >>    return true; >>  } >> >> +/* Read kernel version, and set startup-with-shell to false on Sierra or >> +   later.  */ >> + >> +void >> +disable_startup_with_shell () > > This function should be static. > >> +{ >> +  char str[16]; >> +    size_t sz = sizeof (str); >> +    int ret; >> +    unsigned long ver; >> + >> +    ret = sysctlbyname ("kern.osrelease", str, &sz, NULL, 0); >> +    if (ret == 0 && sz < sizeof (str)) >> +      { >> +       ver = strtoul (str, NULL, 10); >> +       if (ver >= 16) >> +         startup_with_shell = 0; >> +      } >> +} > > The indentation is not quite right.  You can also declare variables when they are used/initialized. > >> + >>  void >>  _initialize_darwin_nat () >>  { >> @@ -2396,4 +2416,6 @@ When this mode is on, all low level exceptions >> are reported before being\n\ >>  reported by the kernel."), >>                 &set_enable_mach_exceptions, NULL, >>                 &setlist, &showlist); >> + >> +  disable_startup_with_shell (); >>  } > > I don't think we should do that in _initialize_darwin_nat.  Since startup-with-shell is supported with remote debugging, you could still use it when starting a Linux remote program from a macOS host. > > Instead, we should probably only disable it at the moment we create a new inferior in the darwin_nat target, so in darwin_nat_target::create_inferior.  We also don't want to permanently change the setting, so we should restore the value.  Presumably, putting something like this in darwin_nat_target::create_inferior should work (I haven't tested it, and sorry for the potential formatting mess my email client will do): > >   gdb::optional> restore_startup_with_shell; >   if (startup_with_shell && should_disable_startup_with_shell ()) >     { >       warning (_("startup-with-shell not supported on this macOS version, disabling it.")); >       restore_startup_with_shell.emplace (&startup_with_shell, 0); >     } > > Instead of setting/restoring startup_with_shell, it might be better if its value was passed as a parameter all the way down instead of having functions read the global variable, but that's a bigger job. Thanks, Pedro Alves