From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26698 invoked by alias); 26 Nov 2017 14:41:44 -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 26685 invoked by uid 89); 26 Nov 2017 14:41:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 26 Nov 2017 14:41:42 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id vAQEfZqO006310 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sun, 26 Nov 2017 09:41:40 -0500 Received: by simark.ca (Postfix, from userid 112) id 5A1EA1E586; Sun, 26 Nov 2017 09:41:35 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 0BBB21E02D; Sun, 26 Nov 2017 09:41:24 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sun, 26 Nov 2017 14:41:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFA] Change maybe_disable_address_space_randomization to a class In-Reply-To: <20171125211113.31419-1-tom@tromey.com> References: <20171125211113.31419-1-tom@tromey.com> Message-ID: <4baf657b03f2ddf99d5d969f3936dd2a@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.2 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sun, 26 Nov 2017 14:41:35 +0000 X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00662.txt.bz2 On 2017-11-25 16:11, Tom Tromey wrote: > This changes maybe_disable_address_space_randomization to be an RAII > class, rather than having it return a cleanup. > > Regression tested by the buildbot. > > ChangeLog > 2017-11-25 Tom Tromey > > * nat/linux-personality.h (class > maybe_disable_address_space_randomization): New class. > (maybe_disable_address_space_randomization): Don't declare > function. > * nat/linux-personality.c (restore_personality) > (make_disable_asr_cleanup): Remove. > (maybe_disable_address_space_randomization): Now a constructor. > (~maybe_disable_address_space_randomization): New destructor. > * linux-nat.c (linux_nat_create_inferior): Update. > > gdbserver/ChangeLog > 2017-11-25 Tom Tromey > > * linux-low.c (linux_create_inferior): Update. > --- > gdb/ChangeLog | 12 ++++++++ > gdb/gdbserver/ChangeLog | 4 +++ > gdb/gdbserver/linux-low.c | 19 ++++++------ > gdb/linux-nat.c | 6 ++-- > gdb/nat/linux-personality.c | 71 > +++++++++++++++------------------------------ > gdb/nat/linux-personality.h | 27 +++++++++++++---- > 6 files changed, 72 insertions(+), 67 deletions(-) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 274263a947..c2420fdf57 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -998,16 +998,17 @@ linux_create_inferior (const char *program, > struct lwp_info *new_lwp; > int pid; > ptid_t ptid; > - struct cleanup *restore_personality > - = maybe_disable_address_space_randomization > (disable_randomization); > - std::string str_program_args = stringify_argv (program_args); > > - pid = fork_inferior (program, > - str_program_args.c_str (), > - get_environ ()->envp (), linux_ptrace_fun, > - NULL, NULL, NULL, NULL); > - > - do_cleanups (restore_personality); > + { > + maybe_disable_address_space_randomization restore_personality > + (disable_randomization); > + std::string str_program_args = stringify_argv (program_args); > + > + pid = fork_inferior (program, > + str_program_args.c_str (), > + get_environ ()->envp (), linux_ptrace_fun, > + NULL, NULL, NULL, NULL); > + } > > linux_add_process (pid, 0); > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index a6395f4b5e..96cb21a2cf 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -1118,8 +1118,8 @@ linux_nat_create_inferior (struct target_ops > *ops, > const char *exec_file, const std::string &allargs, > char **env, int from_tty) > { > - struct cleanup *restore_personality > - = maybe_disable_address_space_randomization > (disable_randomization); > + maybe_disable_address_space_randomization restore_personality > + (disable_randomization); > > /* The fork_child mechanism is synchronous and calls target_wait, so > we have to mask the async mode. */ > @@ -1128,8 +1128,6 @@ linux_nat_create_inferior (struct target_ops > *ops, > linux_nat_pass_signals (ops, 0, NULL); > > linux_ops->to_create_inferior (ops, exec_file, allargs, env, > from_tty); > - > - do_cleanups (restore_personality); > } > > /* Callback for linux_proc_attach_tgid_threads. Attach to PTID if not > diff --git a/gdb/nat/linux-personality.c b/gdb/nat/linux-personality.c > index 21a9ca9d83..bcec2ce5f3 100644 > --- a/gdb/nat/linux-personality.c > +++ b/gdb/nat/linux-personality.c > @@ -27,68 +27,43 @@ > # endif /* ! HAVE_DECL_ADDR_NO_RANDOMIZE */ > #endif /* HAVE_PERSONALITY */ > > -#ifdef HAVE_PERSONALITY > - > -/* Restore address space randomization of the inferior. ARG is the > - original inferior's personality value before the address space > - randomization was disabled. */ > - > -static void > -restore_personality (void *arg) > -{ > - int personality_orig = (int) (uintptr_t) arg; > - > - errno = 0; > - personality (personality_orig); > - if (errno != 0) > - warning (_("Error restoring address space randomization: %s"), > - safe_strerror (errno)); > -} > -#endif /* HAVE_PERSONALITY */ > - > -/* Return a cleanup responsible for restoring the inferior's > - personality (and restoring the inferior's address space > - randomization) if HAVE_PERSONALITY and if PERSONALITY_SET is not > - zero; return a null cleanup if not HAVE_PERSONALITY or if > - PERSONALITY_SET is zero. */ > - > -static struct cleanup * > -make_disable_asr_cleanup (int personality_set, int personality_orig) > -{ > -#ifdef HAVE_PERSONALITY > - if (personality_set != 0) > - return make_cleanup (restore_personality, > - (void *) (uintptr_t) personality_orig); > -#endif /* HAVE_PERSONALITY */ > - > - return make_cleanup (null_cleanup, NULL); > -} > - > /* See comment on nat/linux-personality.h. */ > > -struct cleanup * > +maybe_disable_address_space_randomization:: > maybe_disable_address_space_randomization (int disable_randomization) > + : m_personality_set (false), > + m_personality_orig (0) > { > - int personality_orig = 0; > - int personality_set = 0; > - > #ifdef HAVE_PERSONALITY > if (disable_randomization) > { > errno = 0; > - personality_orig = personality (0xffffffff); > - if (errno == 0 && !(personality_orig & ADDR_NO_RANDOMIZE)) > + m_personality_orig = personality (0xffffffff); > + if (errno == 0 && !(m_personality_orig & ADDR_NO_RANDOMIZE)) > { > - personality_set = 1; > - personality (personality_orig | ADDR_NO_RANDOMIZE); > + m_personality_set = true; > + personality (m_personality_orig | ADDR_NO_RANDOMIZE); > } > - if (errno != 0 || (personality_set > + if (errno != 0 || (m_personality_set > && !(personality (0xffffffff) & ADDR_NO_RANDOMIZE))) > warning (_("Error disabling address space randomization: %s"), > safe_strerror (errno)); > } > #endif /* HAVE_PERSONALITY */ > +} > + > +maybe_disable_address_space_randomization:: > +~maybe_disable_address_space_randomization () > +{ > +#ifdef HAVE_PERSONALITY > > - return make_disable_asr_cleanup (personality_set, > - personality_orig); > + if (m_personality_set) > + { > + errno = 0; > + personality (m_personality_orig); > + if (errno != 0) > + warning (_("Error restoring address space randomization: %s"), > + safe_strerror (errno)); > + } > +#endif /* HAVE_PERSONALITY */ > } > diff --git a/gdb/nat/linux-personality.h b/gdb/nat/linux-personality.h > index 687086e68b..fcea02a67d 100644 > --- a/gdb/nat/linux-personality.h > +++ b/gdb/nat/linux-personality.h > @@ -20,12 +20,27 @@ > #ifndef NAT_LINUX_PERSONALITY_H > #define NAT_LINUX_PERSONALITY_H > > -/* Disable the inferior's address space randomization if > - DISABLE_RANDOMIZATION is not zero and if we have > - . Return a cleanup which, when called, will > - re-enable the inferior's address space randomization. */ > +class maybe_disable_address_space_randomization > +{ > +public: > > -extern struct cleanup *maybe_disable_address_space_randomization > - (int disable_randomization); > + /* Disable the inferior's address space randomization if > + DISABLE_RANDOMIZATION is not zero and if we have > + . */ > + maybe_disable_address_space_randomization (int > disable_randomization); > + > + /* On destruction, re-enable address space randomization. */ > + ~maybe_disable_address_space_randomization (); > + > + DISABLE_COPY_AND_ASSIGN (maybe_disable_address_space_randomization); > + > +private: > + > + /* True if the personality was set in the constructor. */ > + bool m_personality_set; > + > + /* If m_personality_set is true, the original personality value. */ > + int m_personality_orig; > +}; > > #endif /* ! NAT_LINUX_PERSONALITY_H */ That looks good to me. Simon