From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102149 invoked by alias); 9 Sep 2018 11:42:51 -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 102124 invoked by uid 89); 9 Sep 2018 11:42:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=later.=c2, work?= X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 09 Sep 2018 11:42:48 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 2A11E8139A; Sun, 9 Sep 2018 13:42:46 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cL05ikjPkfC4; Sun, 9 Sep 2018 13:42:46 +0200 (CEST) Received: from dev192-150-182-225.eduroam.manchester.ac.uk (unknown [192.150.182.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id F02A781393; Sun, 9 Sep 2018 13:42:45 +0200 (CEST) Subject: Re: [RFA 3/5 v2] Darwin: set startup-with-shell to off on Sierra and later. To: Simon Marchi Cc: gdb-patches@sourceware.org, brobecker@adacore.com, Tom Tromey References: <1536488268-8535-1-git-send-email-roirand@adacore.com> From: Xavier Roirand Message-ID: <39b1e71b-a5bc-e2f0-42f7-1551651823e2@adacore.com> Date: Sun, 09 Sep 2018 11:42:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00228.txt.bz2 Le 9/9/18 à 1:04 PM, Simon Marchi a écrit : > Thanks for the update! > > CCing Tom, because he had a similar patch that was unfortunately not > looked at yet... so maybe he has an opinion about this. > > On 2018-09-09 11:17, 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. This disabling is done temporary before forking >> inferior and restored after the fork. >> >> gdb/ChangeLog: >> >>         * darwin-nat.c (should_disable_startup_with_shell): >>         New function. >>         (darwin_nat_target::create_inferior): Add call. >> >> Change-Id: Ie4d9090f65fdf2e83ecf7a0f9d0647fb1c27cdcc >> --- >>  gdb/darwin-nat.c | 32 ++++++++++++++++++++++++++++++++ >>  1 file changed, 32 insertions(+) >> >> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c >> index be80163d22e..d23706d79fd 100644 >> --- a/gdb/darwin-nat.c >> +++ b/gdb/darwin-nat.c >> @@ -1854,15 +1854,47 @@ darwin_execvp (const char *file, char * const >> argv[], char * const env[]) >>    posix_spawnp (NULL, argv[0], NULL, &attr, argv, env); >>  } >> >> +/* Read kernel version, and return false on Sierra or later.  */ > > I think this is the other way around (return trun on Sierra or later). > >> + >> +static int >> +should_disable_startup_with_shell () >> +{ >> +  char str[16]; >> +  size_t sz = sizeof (str); >> +  int ret; >> + >> +  ret = sysctlbyname ("kern.osrelease", str, &sz, NULL, 0); >> +  if (ret == 0 && sz < sizeof (str)) >> +    { >> +      unsigned long ver = strtoul (str, NULL, 10); >> +      if (ver >= 16) >> +        return TRUE; >> +    } >> +  return FALSE; >> +} >> + >>  void >>  darwin_nat_target::create_inferior (const char *exec_file, >>                      const std::string &allargs, >>                      char **env, int from_tty) >>  { >> +  gdb::optional> restore_startup_with_shell; >> +  int startup_shell_disabled = 0; >> + >> +  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); >> +      startup_shell_disabled = 1; >> +    } >> + >>    /* Do the hard work.  */ >>    fork_inferior (exec_file, allargs, env, darwin_ptrace_me, >>           darwin_ptrace_him, darwin_pre_ptrace, NULL, >>           darwin_execvp); >> + >> +  if (startup_shell_disabled) >> +    restore_startup_with_shell.emplace (&startup_with_shell, 1); > > You shouldn't need this last bit, nor the startup_shell_disabled > variable.  When it gets destroyed, the restore_startup_with_shell object > takes care of restoring the value of the startup_with_shell variable to > the value it had when it was constructed (the first emplace call). > > In other words, this call: > >   restore_startup_with_shell.emplace (&startup_with_shell, 0); > > means "save the current value of startup_with_shell, then put 0 in it". > > When the destructor of restore_startup_with_shell is called, it restores > the saved value. > > With that fixed, it would LGTM, although I did not try it because I > don't have access to a Mac at the moment. > > Simon Thanks, I'll fix this.