From patchwork Thu Oct 27 17:55:37 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 16865 Received: (qmail 33275 invoked by alias); 27 Oct 2016 17:55:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 33264 invoked by uid 89); 27 Oct 2016 17:55:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=array_size, ARRAY_SIZE, 0x10, gdbarch_list 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; Thu, 27 Oct 2016 17:55:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F1136F8050; Thu, 27 Oct 2016 17:55:38 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9RHtbvf003201; Thu, 27 Oct 2016 13:55:37 -0400 Subject: Re: [PATCH 00/13] Split brekapoint_from_pc to breakpoint_kind_from_pc and sw_breakpoint_from_kind To: Yao Qi References: <1472655965-12212-1-git-send-email-yao.qi@linaro.org> Cc: "gdb-patches@sourceware.org" From: Pedro Alves Message-ID: Date: Thu, 27 Oct 2016 18:55:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: On 10/27/2016 04:41 PM, Yao Qi wrote: > Hi Pedro, > Thanks for the review, > > On Thu, Oct 27, 2016 at 3:58 PM, Pedro Alves wrote: >> >> My only question is what happens to the GDBARCH_BREAKPOINT_MANIPULATION >> / SET_GDBARCH_BREAKPOINT_MANIPULATION macros? I was hoping they'd >> disappear in the end, but looks like not? (I find the "manipulation" >> name to be very opaque here, btw.) > > Both are the intermediate structure during the the process of conversion. > I didn't remove them as I think they two can "simplify" the code, because > a lot of gdbarch have one breakpoint instruction. Their > breakpoint_kind_from_pc and sw_breakpoint_from_kind look quite similar. > I see. Guess we can proceed with what you have, and then maybe replace the macros with templates? Here's how it might look like. I confirmed that i386-tdep.o and rs6000-tdep.o build with this, and confirmed that the x86 version works at least with a version of the patch that kept the template inside i386-tdep.c only. Without C++11, we have to remove the "constexpr", and we can't make the arrays static nor const -- only extern arrays can be used as non-type parameters. So with C++03 that'd be a little regression compared to the macros. C++11 wins here too. :-) Another option would be to replace the two gdbarch methods with a single one that returns an object that itself has the two conversion methods. Those methods in the new object would have to be virtual, so it'd end up being more hops at run time. From 51cc27113394b07ad1b3cec429973412d88baa7f Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 27 Oct 2016 18:21:44 +0100 Subject: [PATCH] BREAKPOINT_MANIPULATION: macro -> template --- gdb/arch-utils.h | 80 ++++++++++++++++++++++++++++--------------------------- gdb/i386-tdep.c | 9 ++++--- gdb/rs6000-tdep.c | 12 +++++---- 3 files changed, 54 insertions(+), 47 deletions(-) diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index 9592580..4b0848f 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -26,48 +26,50 @@ struct minimal_symbol; struct type; struct gdbarch_info; -#define GDBARCH_BREAKPOINT_MANIPULATION(ARCH,BREAK_INSN) \ - static int \ - ARCH##_breakpoint_kind_from_pc (struct gdbarch *gdbarch, \ - CORE_ADDR *pcptr) \ - { \ - return sizeof (BREAK_INSN); \ - } \ - static const gdb_byte * \ - ARCH##_sw_breakpoint_from_kind (struct gdbarch *gdbarch, \ - int kind, int *size) \ - { \ - *size = kind; \ - return BREAK_INSN; \ +template +struct bp_manipulation +{ + static int + kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr) + { + return bp_size; } -#define SET_GDBARCH_BREAKPOINT_MANIPULATION(ARCH) \ - set_gdbarch_breakpoint_kind_from_pc (gdbarch, \ - ARCH##_breakpoint_kind_from_pc); \ - set_gdbarch_sw_breakpoint_from_kind (gdbarch, \ - ARCH##_sw_breakpoint_from_kind) - -#define GDBARCH_BREAKPOINT_MANIPULATION_ENDIAN(ARCH, \ - LITTLE_BREAK_INSN, \ - BIG_BREAK_INSN) \ - static int \ - ARCH##_breakpoint_kind_from_pc (struct gdbarch *gdbarch, \ - CORE_ADDR *pcptr) \ - { \ - gdb_static_assert (ARRAY_SIZE (LITTLE_BREAK_INSN) \ - == ARRAY_SIZE (BIG_BREAK_INSN)); \ - return sizeof (BIG_BREAK_INSN); \ - } \ - static const gdb_byte * \ - ARCH##_sw_breakpoint_from_kind (struct gdbarch *gdbarch, \ - int kind, int *size) \ - { \ - *size = kind; \ - if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG) \ - return BIG_BREAK_INSN; \ - else \ - return LITTLE_BREAK_INSN; \ + static const gdb_byte * + bp_from_kind (struct gdbarch *gdbarch, int kind, int *size) + { + *size = kind; + return break_insn; } +}; + +template +struct bp_manipulation_endian +{ + static int + kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr) + { + return bp_size; + } + + static const gdb_byte * + bp_from_kind (struct gdbarch *gdbarch, int kind, int *size) + { + *size = kind; + if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG) + return break_insn_big; + else + return break_insn_little; + } +}; + +#define BP_MANIPULATION(BREAK_INSN) \ + bp_manipulation + +#define BP_MANIPULATION_ENDIAN(BREAK_INSN_LITTLE, BREAK_INSN_BIG) \ + bp_manipulation_endian /* An implementation of gdbarch_displaced_step_copy_insn for processors that don't need to modify the instruction before diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 697edc6..a1e078b 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -588,9 +588,10 @@ static const char *disassembly_flavor = att_flavor; This function is 64-bit safe. */ -static const gdb_byte break_insn[] = { 0xcc }; /* int 3 */ +constexpr gdb_byte i386_break_insn[] = { 0xcc }; /* int 3 */ + +typedef BP_MANIPULATION (i386_break_insn) i386_breakpoint; -GDBARCH_BREAKPOINT_MANIPULATION (i386, break_insn) /* Displaced instruction handling. */ @@ -8453,7 +8454,9 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* Stack grows downward. */ set_gdbarch_inner_than (gdbarch, core_addr_lessthan); - SET_GDBARCH_BREAKPOINT_MANIPULATION (i386); + set_gdbarch_breakpoint_kind_from_pc (gdbarch, i386_breakpoint::kind_from_pc); + set_gdbarch_sw_breakpoint_from_kind (gdbarch, i386_breakpoint::bp_from_kind); + set_gdbarch_decr_pc_after_break (gdbarch, 1); set_gdbarch_max_insn_length (gdbarch, I386_MAX_INSN_LEN); diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 7cb7a9e..5578b33 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -967,11 +967,11 @@ rs6000_fetch_pointer_argument (struct frame_info *frame, int argi, /* Sequence of bytes for breakpoint instruction. */ -static unsigned char big_breakpoint[] = { 0x7d, 0x82, 0x10, 0x08 }; -static unsigned char little_breakpoint[] = { 0x08, 0x10, 0x82, 0x7d }; +constexpr gdb_byte big_breakpoint[] = { 0x7d, 0x82, 0x10, 0x08 }; +constexpr gdb_byte little_breakpoint[] = { 0x08, 0x10, 0x82, 0x7d }; -GDBARCH_BREAKPOINT_MANIPULATION_ENDIAN (rs6000, little_breakpoint, - big_breakpoint) +typedef BP_MANIPULATION_ENDIAN (little_breakpoint, big_breakpoint) + rs6000_breakpoint; /* Instruction masks for displaced stepping. */ #define BRANCH_MASK 0xfc000000 @@ -6479,7 +6479,9 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_skip_main_prologue (gdbarch, rs6000_skip_main_prologue); set_gdbarch_inner_than (gdbarch, core_addr_lessthan); - SET_GDBARCH_BREAKPOINT_MANIPULATION (rs6000); + + set_gdbarch_breakpoint_kind_from_pc (gdbarch, rs6000_breakpoint::kind_from_pc); + set_gdbarch_sw_breakpoint_from_kind (gdbarch, rs6000_breakpoint::bp_from_kind); /* The value of symbols of type N_SO and N_FUN maybe null when it shouldn't be. */