[RFA] Change maybe_disable_address_space_randomization to a class

Simon Marchi simon.marchi@polymtl.ca
Sun Nov 26 14:41:00 GMT 2017


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  <tom@tromey.com>
> 
> 	* 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  <tom@tromey.com>
> 
> 	* 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
> -   <sys/personality.h>.  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
> +     <sys/personality.h>. */
> +  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



More information about the Gdb-patches mailing list