diff mbox

gdb: Remove a non-const reference parameter

Message ID 20190702152119.31612-1-andrew.burgess@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess July 2, 2019, 3:21 p.m. UTC
Non-const reference parameter should be avoided according to the GDB
coding standard:

  https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead

This commit updates the gdbarch method gdbarch_stap_adjust_register,
and the one implementation i386_stap_adjust_register to avoid using a
non-const reference parameter.

I've also removed the kfail from the testsuite for bug 24541, as this
issue is now resolved.

gdb/ChangeLog:

	PR breakpoints/24541
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh: Adjust return type and parameter types for
	'stap_adjust_register'.
	(i386_stap_adjust_register): Adjust signature and return new
	register name.
	* stap-probe.c (stap_parse_register_operand): Adjust use of
	'gdbarch_stap_adjust_register'.

gdb/testsuite/ChangeLog:

	PR breakpoints/24541
	* gdb.mi/mi-catch-cpp-exceptions.exp: Remove kfail due to 24541.
---
 gdb/ChangeLog                                    | 12 ++++++++++++
 gdb/gdbarch.c                                    |  6 +++---
 gdb/gdbarch.h                                    |  4 ++--
 gdb/gdbarch.sh                                   |  2 +-
 gdb/i386-tdep.c                                  | 12 ++++++++----
 gdb/stap-probe.c                                 | 15 ++++++++-------
 gdb/testsuite/ChangeLog                          |  5 +++++
 gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp | 22 +---------------------
 8 files changed, 40 insertions(+), 38 deletions(-)

Comments

Simon Marchi July 2, 2019, 3:35 p.m. UTC | #1
On 2019-07-02 11:21 a.m., Andrew Burgess wrote:
> Non-const reference parameter should be avoided according to the GDB
> coding standard:
> 
>   https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
> 
> This commit updates the gdbarch method gdbarch_stap_adjust_register,
> and the one implementation i386_stap_adjust_register to avoid using a
> non-const reference parameter.
> 
> I've also removed the kfail from the testsuite for bug 24541, as this
> issue is now resolved.

Hi Andrew,

So I was the one who added this rule to the guide.  When I asked for opinions and
didn't get any, I thought it was because it wasn't very controversial and chose to add it.

Since then, Pedro mentioned a few times he didn't agree with it.

I'd like to know what others think, should we keep it (and enforce it) or remove it?

Simon
Sergio Durigan Junior July 2, 2019, 4:40 p.m. UTC | #2
On Tuesday, July 02 2019, Andrew Burgess wrote:

> Non-const reference parameter should be avoided according to the GDB
> coding standard:
>
>   https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
>
> This commit updates the gdbarch method gdbarch_stap_adjust_register,
> and the one implementation i386_stap_adjust_register to avoid using a
> non-const reference parameter.
>
> I've also removed the kfail from the testsuite for bug 24541, as this
> issue is now resolved.

Thanks, Andrew (and sorry for the delay; yesterday was a holiday here).

This is OK.

> gdb/ChangeLog:
>
> 	PR breakpoints/24541
> 	* gdbarch.c: Regenerate.
> 	* gdbarch.h: Regenerate.
> 	* gdbarch.sh: Adjust return type and parameter types for
> 	'stap_adjust_register'.
> 	(i386_stap_adjust_register): Adjust signature and return new
> 	register name.
> 	* stap-probe.c (stap_parse_register_operand): Adjust use of
> 	'gdbarch_stap_adjust_register'.
>
> gdb/testsuite/ChangeLog:
>
> 	PR breakpoints/24541
> 	* gdb.mi/mi-catch-cpp-exceptions.exp: Remove kfail due to 24541.
> ---
>  gdb/ChangeLog                                    | 12 ++++++++++++
>  gdb/gdbarch.c                                    |  6 +++---
>  gdb/gdbarch.h                                    |  4 ++--
>  gdb/gdbarch.sh                                   |  2 +-
>  gdb/i386-tdep.c                                  | 12 ++++++++----
>  gdb/stap-probe.c                                 | 15 ++++++++-------
>  gdb/testsuite/ChangeLog                          |  5 +++++
>  gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp | 22 +---------------------
>  8 files changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index cc7d0ace66c..725b98fcd9f 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -4530,14 +4530,14 @@ gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch)
>    return gdbarch->stap_adjust_register != NULL;
>  }
>  
> -void
> -gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum)
> +std::string
> +gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum)
>  {
>    gdb_assert (gdbarch != NULL);
>    gdb_assert (gdbarch->stap_adjust_register != NULL);
>    if (gdbarch_debug >= 2)
>      fprintf_unfiltered (gdb_stdlog, "gdbarch_stap_adjust_register called\n");
> -  gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
> +  return gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
>  }
>  
>  void
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 0857d2f3027..c3556d38419 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1376,8 +1376,8 @@ extern void set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, gdbar
>  
>  extern int gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch);
>  
> -typedef void (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
> -extern void gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
> +typedef std::string (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
> +extern std::string gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
>  extern void set_gdbarch_stap_adjust_register (struct gdbarch *gdbarch, gdbarch_stap_adjust_register_ftype *stap_adjust_register);
>  
>  /* DTrace related functions.
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index f3d1bf489ac..2f9fbbc56cd 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1053,7 +1053,7 @@ M;int;stap_parse_special_token;struct stap_parse_info *p;p
>  # replace the register name from %ax to %eax.
>  #
>  # The rationale for this can be found at PR breakpoints/24541.
> -M;void;stap_adjust_register;struct stap_parse_info *p, std::string \&regname, int regnum;p, regname, regnum
> +M;std::string;stap_adjust_register;struct stap_parse_info *p, const std::string \&regname, int regnum;p, regname, regnum
>  
>  # DTrace related functions.
>  
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 00c1f8d7499..b956604178f 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -4389,9 +4389,9 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
>  /* Implementation of 'gdbarch_stap_adjust_register', as defined in
>     gdbarch.h.  */
>  
> -static void
> +static std::string
>  i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
> -			   std::string &regname, int regnum)
> +			   const std::string &regname, int regnum)
>  {
>    static const std::unordered_set<std::string> reg_assoc
>      = { "ax", "bx", "cx", "dx",
> @@ -4402,14 +4402,18 @@ i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
>        /* If we're dealing with a register whose size is greater or
>  	 equal than the size specified by the "[-]N@" prefix, then we
>  	 don't need to do anything.  */
> -      return;
> +      return regname;
>      }
>  
>    if (reg_assoc.find (regname) != reg_assoc.end ())
>      {
>        /* Use the extended version of the register.  */
> -      regname = "e" + regname;
> +      std::string tmp = "e" + regname;
> +      return tmp;
>      }
> +
> +  /* Use the requested register.  */
> +  return regname;
>  }
>  
>  
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index aa1c8144d8a..747bbcfb8c1 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -774,23 +774,24 @@ stap_parse_register_operand (struct stap_parse_info *p)
>       code would like to perform on the register name.  */
>    if (gdbarch_stap_adjust_register_p (gdbarch))
>      {
> -      std::string oldregname = regname;
> +      std::string newregname
> +	= gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
>  
> -      gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
> -
> -      if (regname != oldregname)
> +      if (regname != newregname)
>  	{
>  	  /* This is just a check we perform to make sure that the
>  	     arch-dependent code has provided us with a valid
>  	     register name.  */
> -	  regnum = user_reg_map_name_to_regnum (gdbarch, regname.c_str (),
> -						regname.size ());
> +	  regnum = user_reg_map_name_to_regnum (gdbarch, newregname.c_str (),
> +						newregname.size ());
>  
>  	  if (regnum == -1)
>  	    internal_error (__FILE__, __LINE__,
>  			    _("Invalid register name '%s' after replacing it"
>  			      " (previous name was '%s')"),
> -			    regname.c_str (), oldregname.c_str ());
> +			    newregname.c_str (), regname.c_str ());
> +
> +	  regname = newregname;
>  	}
>      }
>  
> diff --git a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> index fa5b11e3e58..4b87e4b62ba 100644
> --- a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> +++ b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> @@ -130,27 +130,7 @@ with_test_prefix "all with invalid regexp" {
>      setup_catchpoint "throw" "-r blahblah"
>      setup_catchpoint "rethrow" "-r woofwoof"
>      setup_catchpoint "catch" "-r miowmiow"
> -
> -    # Would like to use 'continue_to_breakpoint_in_main' here, if
> -    # there wasn't a bug that requires a use of kfail.
> -
> -    mi_send_resuming_command "exec-continue" \
> -	"exec-continue"
> -    set testname "run until breakpoint in main"
> -    gdb_expect {
> -	-re "could not find minimal symbol for typeinfo address.*$mi_gdb_prompt$" {
> -	    kfail "gdb/24541" "${testname}"
> -	}
> -	-re "\\*stopped,bkptno=\"$decimal\",reason=\"breakpoint-hit\",disp=\"keep\".*func=\"__cxa_throw\".*$mi_gdb_prompt$" {
> -	    kfail "gdb/24541" "${testname}"
> -	}
> -	-re "\\*stopped,reason=\"breakpoint-hit\".*func=\"main\".*line=\"${main_lineno}\".*$mi_gdb_prompt$" {
> -	    pass "${testname}"
> -	}
> -	timeout {
> -	    fail "${testname} (timeout)"
> -	}
> -    }
> +    continue_to_breakpoint_in_main
>  }
>  
>  # Now check that all of the commands with a regexp that does match,
> -- 
> 2.14.5
Tom Tromey July 2, 2019, 5:55 p.m. UTC | #3
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Non-const reference parameter should be avoided according to the GDB
Andrew> coding standard:

Andrew>   https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead

Andrew> This commit updates the gdbarch method gdbarch_stap_adjust_register,
Andrew> and the one implementation i386_stap_adjust_register to avoid using a
Andrew> non-const reference parameter.

Thanks for doing this.  I probably should have pushed back on the
original patch a little for this reason.

Andrew> +      std::string tmp = "e" + regname;
Andrew> +      return tmp;

This can just be `return "e" + regname' I think.

Looks ok otherwise.

Tom
Tom Tromey July 2, 2019, 5:55 p.m. UTC | #4
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> So I was the one who added this rule to the guide.  When I asked for opinions and
Simon> didn't get any, I thought it was because it wasn't very controversial and chose to add it.

Simon> Since then, Pedro mentioned a few times he didn't agree with it.

Simon> I'd like to know what others think, should we keep it (and enforce it) or remove it?

Pedro raised good points on this topic, but in this particular case I
think the code is clearer without modifying the parameter.

Tom
Andrew Burgess July 17, 2019, 3:32 p.m. UTC | #5
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-07-02 16:21:19 +0100]:

> Non-const reference parameter should be avoided according to the GDB
> coding standard:
> 
>   https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
> 
> This commit updates the gdbarch method gdbarch_stap_adjust_register,
> and the one implementation i386_stap_adjust_register to avoid using a
> non-const reference parameter.
> 
> I've also removed the kfail from the testsuite for bug 24541, as this
> issue is now resolved.
> 
> gdb/ChangeLog:
> 
> 	PR breakpoints/24541
> 	* gdbarch.c: Regenerate.
> 	* gdbarch.h: Regenerate.
> 	* gdbarch.sh: Adjust return type and parameter types for
> 	'stap_adjust_register'.
> 	(i386_stap_adjust_register): Adjust signature and return new
> 	register name.
> 	* stap-probe.c (stap_parse_register_operand): Adjust use of
> 	'gdbarch_stap_adjust_register'.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR breakpoints/24541
> 	* gdb.mi/mi-catch-cpp-exceptions.exp: Remove kfail due to
> 	24541.

I went ahead and pushed this patch, after making the improvement Tom
suggested.

There didn't seem to be a rush of voices suggesting the non-const rule
be dropped.  We can always revert this if the rule is later removed.

Thanks,
Andrew



> ---
>  gdb/ChangeLog                                    | 12 ++++++++++++
>  gdb/gdbarch.c                                    |  6 +++---
>  gdb/gdbarch.h                                    |  4 ++--
>  gdb/gdbarch.sh                                   |  2 +-
>  gdb/i386-tdep.c                                  | 12 ++++++++----
>  gdb/stap-probe.c                                 | 15 ++++++++-------
>  gdb/testsuite/ChangeLog                          |  5 +++++
>  gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp | 22 +---------------------
>  8 files changed, 40 insertions(+), 38 deletions(-)
> 
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index cc7d0ace66c..725b98fcd9f 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -4530,14 +4530,14 @@ gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch)
>    return gdbarch->stap_adjust_register != NULL;
>  }
>  
> -void
> -gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum)
> +std::string
> +gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum)
>  {
>    gdb_assert (gdbarch != NULL);
>    gdb_assert (gdbarch->stap_adjust_register != NULL);
>    if (gdbarch_debug >= 2)
>      fprintf_unfiltered (gdb_stdlog, "gdbarch_stap_adjust_register called\n");
> -  gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
> +  return gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
>  }
>  
>  void
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 0857d2f3027..c3556d38419 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1376,8 +1376,8 @@ extern void set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, gdbar
>  
>  extern int gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch);
>  
> -typedef void (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
> -extern void gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
> +typedef std::string (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
> +extern std::string gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
>  extern void set_gdbarch_stap_adjust_register (struct gdbarch *gdbarch, gdbarch_stap_adjust_register_ftype *stap_adjust_register);
>  
>  /* DTrace related functions.
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index f3d1bf489ac..2f9fbbc56cd 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1053,7 +1053,7 @@ M;int;stap_parse_special_token;struct stap_parse_info *p;p
>  # replace the register name from %ax to %eax.
>  #
>  # The rationale for this can be found at PR breakpoints/24541.
> -M;void;stap_adjust_register;struct stap_parse_info *p, std::string \&regname, int regnum;p, regname, regnum
> +M;std::string;stap_adjust_register;struct stap_parse_info *p, const std::string \&regname, int regnum;p, regname, regnum
>  
>  # DTrace related functions.
>  
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 00c1f8d7499..b956604178f 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -4389,9 +4389,9 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
>  /* Implementation of 'gdbarch_stap_adjust_register', as defined in
>     gdbarch.h.  */
>  
> -static void
> +static std::string
>  i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
> -			   std::string &regname, int regnum)
> +			   const std::string &regname, int regnum)
>  {
>    static const std::unordered_set<std::string> reg_assoc
>      = { "ax", "bx", "cx", "dx",
> @@ -4402,14 +4402,18 @@ i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
>        /* If we're dealing with a register whose size is greater or
>  	 equal than the size specified by the "[-]N@" prefix, then we
>  	 don't need to do anything.  */
> -      return;
> +      return regname;
>      }
>  
>    if (reg_assoc.find (regname) != reg_assoc.end ())
>      {
>        /* Use the extended version of the register.  */
> -      regname = "e" + regname;
> +      std::string tmp = "e" + regname;
> +      return tmp;
>      }
> +
> +  /* Use the requested register.  */
> +  return regname;
>  }
>  
>  
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index aa1c8144d8a..747bbcfb8c1 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -774,23 +774,24 @@ stap_parse_register_operand (struct stap_parse_info *p)
>       code would like to perform on the register name.  */
>    if (gdbarch_stap_adjust_register_p (gdbarch))
>      {
> -      std::string oldregname = regname;
> +      std::string newregname
> +	= gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
>  
> -      gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
> -
> -      if (regname != oldregname)
> +      if (regname != newregname)
>  	{
>  	  /* This is just a check we perform to make sure that the
>  	     arch-dependent code has provided us with a valid
>  	     register name.  */
> -	  regnum = user_reg_map_name_to_regnum (gdbarch, regname.c_str (),
> -						regname.size ());
> +	  regnum = user_reg_map_name_to_regnum (gdbarch, newregname.c_str (),
> +						newregname.size ());
>  
>  	  if (regnum == -1)
>  	    internal_error (__FILE__, __LINE__,
>  			    _("Invalid register name '%s' after replacing it"
>  			      " (previous name was '%s')"),
> -			    regname.c_str (), oldregname.c_str ());
> +			    newregname.c_str (), regname.c_str ());
> +
> +	  regname = newregname;
>  	}
>      }
>  
> diff --git a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> index fa5b11e3e58..4b87e4b62ba 100644
> --- a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> +++ b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> @@ -130,27 +130,7 @@ with_test_prefix "all with invalid regexp" {
>      setup_catchpoint "throw" "-r blahblah"
>      setup_catchpoint "rethrow" "-r woofwoof"
>      setup_catchpoint "catch" "-r miowmiow"
> -
> -    # Would like to use 'continue_to_breakpoint_in_main' here, if
> -    # there wasn't a bug that requires a use of kfail.
> -
> -    mi_send_resuming_command "exec-continue" \
> -	"exec-continue"
> -    set testname "run until breakpoint in main"
> -    gdb_expect {
> -	-re "could not find minimal symbol for typeinfo address.*$mi_gdb_prompt$" {
> -	    kfail "gdb/24541" "${testname}"
> -	}
> -	-re "\\*stopped,bkptno=\"$decimal\",reason=\"breakpoint-hit\",disp=\"keep\".*func=\"__cxa_throw\".*$mi_gdb_prompt$" {
> -	    kfail "gdb/24541" "${testname}"
> -	}
> -	-re "\\*stopped,reason=\"breakpoint-hit\".*func=\"main\".*line=\"${main_lineno}\".*$mi_gdb_prompt$" {
> -	    pass "${testname}"
> -	}
> -	timeout {
> -	    fail "${testname} (timeout)"
> -	}
> -    }
> +    continue_to_breakpoint_in_main
>  }
>  
>  # Now check that all of the commands with a regexp that does match,
> -- 
> 2.14.5
>
diff mbox

Patch

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index cc7d0ace66c..725b98fcd9f 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -4530,14 +4530,14 @@  gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch)
   return gdbarch->stap_adjust_register != NULL;
 }
 
-void
-gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum)
+std::string
+gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->stap_adjust_register != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_stap_adjust_register called\n");
-  gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
+  return gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 0857d2f3027..c3556d38419 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1376,8 +1376,8 @@  extern void set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, gdbar
 
 extern int gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch);
 
-typedef void (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
-extern void gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string &regname, int regnum);
+typedef std::string (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
+extern std::string gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string &regname, int regnum);
 extern void set_gdbarch_stap_adjust_register (struct gdbarch *gdbarch, gdbarch_stap_adjust_register_ftype *stap_adjust_register);
 
 /* DTrace related functions.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index f3d1bf489ac..2f9fbbc56cd 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1053,7 +1053,7 @@  M;int;stap_parse_special_token;struct stap_parse_info *p;p
 # replace the register name from %ax to %eax.
 #
 # The rationale for this can be found at PR breakpoints/24541.
-M;void;stap_adjust_register;struct stap_parse_info *p, std::string \&regname, int regnum;p, regname, regnum
+M;std::string;stap_adjust_register;struct stap_parse_info *p, const std::string \&regname, int regnum;p, regname, regnum
 
 # DTrace related functions.
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 00c1f8d7499..b956604178f 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4389,9 +4389,9 @@  i386_stap_parse_special_token (struct gdbarch *gdbarch,
 /* Implementation of 'gdbarch_stap_adjust_register', as defined in
    gdbarch.h.  */
 
-static void
+static std::string
 i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
-			   std::string &regname, int regnum)
+			   const std::string &regname, int regnum)
 {
   static const std::unordered_set<std::string> reg_assoc
     = { "ax", "bx", "cx", "dx",
@@ -4402,14 +4402,18 @@  i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
       /* If we're dealing with a register whose size is greater or
 	 equal than the size specified by the "[-]N@" prefix, then we
 	 don't need to do anything.  */
-      return;
+      return regname;
     }
 
   if (reg_assoc.find (regname) != reg_assoc.end ())
     {
       /* Use the extended version of the register.  */
-      regname = "e" + regname;
+      std::string tmp = "e" + regname;
+      return tmp;
     }
+
+  /* Use the requested register.  */
+  return regname;
 }
 
 
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index aa1c8144d8a..747bbcfb8c1 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -774,23 +774,24 @@  stap_parse_register_operand (struct stap_parse_info *p)
      code would like to perform on the register name.  */
   if (gdbarch_stap_adjust_register_p (gdbarch))
     {
-      std::string oldregname = regname;
+      std::string newregname
+	= gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
 
-      gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
-
-      if (regname != oldregname)
+      if (regname != newregname)
 	{
 	  /* This is just a check we perform to make sure that the
 	     arch-dependent code has provided us with a valid
 	     register name.  */
-	  regnum = user_reg_map_name_to_regnum (gdbarch, regname.c_str (),
-						regname.size ());
+	  regnum = user_reg_map_name_to_regnum (gdbarch, newregname.c_str (),
+						newregname.size ());
 
 	  if (regnum == -1)
 	    internal_error (__FILE__, __LINE__,
 			    _("Invalid register name '%s' after replacing it"
 			      " (previous name was '%s')"),
-			    regname.c_str (), oldregname.c_str ());
+			    newregname.c_str (), regname.c_str ());
+
+	  regname = newregname;
 	}
     }
 
diff --git a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
index fa5b11e3e58..4b87e4b62ba 100644
--- a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
+++ b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
@@ -130,27 +130,7 @@  with_test_prefix "all with invalid regexp" {
     setup_catchpoint "throw" "-r blahblah"
     setup_catchpoint "rethrow" "-r woofwoof"
     setup_catchpoint "catch" "-r miowmiow"
-
-    # Would like to use 'continue_to_breakpoint_in_main' here, if
-    # there wasn't a bug that requires a use of kfail.
-
-    mi_send_resuming_command "exec-continue" \
-	"exec-continue"
-    set testname "run until breakpoint in main"
-    gdb_expect {
-	-re "could not find minimal symbol for typeinfo address.*$mi_gdb_prompt$" {
-	    kfail "gdb/24541" "${testname}"
-	}
-	-re "\\*stopped,bkptno=\"$decimal\",reason=\"breakpoint-hit\",disp=\"keep\".*func=\"__cxa_throw\".*$mi_gdb_prompt$" {
-	    kfail "gdb/24541" "${testname}"
-	}
-	-re "\\*stopped,reason=\"breakpoint-hit\".*func=\"main\".*line=\"${main_lineno}\".*$mi_gdb_prompt$" {
-	    pass "${testname}"
-	}
-	timeout {
-	    fail "${testname} (timeout)"
-	}
-    }
+    continue_to_breakpoint_in_main
 }
 
 # Now check that all of the commands with a regexp that does match,