From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81143 invoked by alias); 2 Jul 2019 16:40:21 -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 81135 invoked by uid 89); 2 Jul 2019 16:40:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.4 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.1 spammy=encrypted, holiday, yesterday, 237A X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 02 Jul 2019 16:40:18 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 49B9B37EEF; Tue, 2 Jul 2019 16:40:14 +0000 (UTC) Received: from localhost (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1A19617AA7; Tue, 2 Jul 2019 16:40:14 +0000 (UTC) From: Sergio Durigan Junior To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: Remove a non-const reference parameter References: <20190629145820.GR23204@embecosm.com> <20190702152119.31612-1-andrew.burgess@embecosm.com> Date: Tue, 02 Jul 2019 16:40:00 -0000 In-Reply-To: <20190702152119.31612-1-andrew.burgess@embecosm.com> (Andrew Burgess's message of "Tue, 2 Jul 2019 16:21:19 +0100") Message-ID: <87pnmsfl02.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg00060.txt.bz2 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 ®name, int regnum) > +std::string > +gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, 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 ®name, int regnum); > -extern void gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string ®name, int regnum); > +typedef std::string (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, int regnum); > +extern std::string gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, 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 \®name, int regnum;p, regname, regnum > +M;std::string;stap_adjust_register;struct stap_parse_info *p, const std::string \®name, 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 ®name, int regnum) > + const std::string ®name, int regnum) > { > static const std::unordered_set 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 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/