From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25181 invoked by alias); 23 Nov 2004 17:50:18 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 24574 invoked from network); 23 Nov 2004 17:49:38 -0000 Received: from unknown (HELO pippin.tausq.org) (64.81.244.94) by sourceware.org with SMTP; 23 Nov 2004 17:49:38 -0000 Received: by pippin.tausq.org (Postfix, from userid 1000) id 30258CD77D; Tue, 23 Nov 2004 09:49:37 -0800 (PST) Date: Tue, 23 Nov 2004 17:50:00 -0000 From: Randolph Chung To: Mark Kettenis Cc: cagney@gnu.org, gdb-patches@sources.redhat.com Subject: Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED Message-ID: <20041123174937.GL9148@tausq.org> Reply-To: Randolph Chung References: <20041118000159.GG15714@tausq.org> <419CB118.7080401@gnu.org> <20041118162108.GK15714@tausq.org> <200411181655.iAIGthDa026050@juw15.nfra.nl> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200411181655.iAIGthDa026050@juw15.nfra.nl> X-GPG: for GPG key, see http://www.tausq.org/gpg.txt User-Agent: Mutt/1.5.5.1+cvs20040105i X-SW-Source: 2004-11/txt/msg00451.txt.bz2 > i was trying to figure out that piece of commented out code too and why > it was changed. looked through cvs history but didn't find it. I'm not > particularly fond of introducing new almost-arch-specific gdbarch > methods either, but this does seem to be doing slightly different things > than the existing ones. OTOH this is almost a "cosmetic" feature, so one > alternative is to drop the INSTRUCTION_NULLIFIED logic completely.... > > Perhaps that isn't such a bad idea if it doesn't confuse GDB too much. i did some more investigations.... turns out this is not a cosmetic piece of code at all :) suppose we have a function that ended with a branch-with-nullify-next instruction back to the caller. if you did a "step" on the branch, and we don't skip the nullified instruction, we would end up on the nullified instruction which actually belongs to the next function. for example: Dump of assembler code for function call_with_trampolines: 0x0001217c : copy r3,r1 0x00012180 : copy sp,r3 0x00012184 : stw,ma r1,40(,sp) 0x00012188 : ldi -28,r19 0x0001218c : fstd fr5,r19(,r3) 0x00012190 : ldi -28,r19 0x00012194 : fldd r19(,r3),fr22 0x00012198 : fcpy,dbl fr22,fr4 0x0001219c : ldo 40(r3),sp 0x000121a0 : ldw,mb -40(,sp),r3 0x000121a4 : bv,n r0(rp) End of assembler dump. (gdb) disassemble 0x121a8 Dump of assembler code for function marker_indirect_call: 0x000121a8 : copy r3,r1 0x000121ac : copy sp,r3 0x000121b0 : stw,ma r1,40(,sp) 0x000121b4 : ldo 40(r3),sp 0x000121b8 : ldw,mb -40(,sp),r3 0x000121bc : bv,n r0(rp) if we are at 0x121a4, and we do a step, it will stop at "marker_indirect_call" instead of back at the caller of "call_with_trampolines". since the insn at 0x121a8 is not actually executed in this call sequence, the correct thing to do is to blindly step past the nullified instruction before we make any decision on what to do. here's an updated patch to multiarch INSTRUCTION_NULLIFIED, with updated comments and a somewhat cleaner interface. comments? ok to check in? 2004-11-23 Randolph Chung * arch-utils.c (generic_instruction_nullified): New. * arch-utils.h (generic_instruction_nullified): New. * gdbarch.sh (instruction_nullified): New method. * gdbarch.c: Regenerate. * gdbarch.h: Regenerate. * infrun.c (INSTRUCTION_NULLIFIED): Delete. (handle_inferior_event): Replace INSTRUCTION_NULLIFIED with calls to new gdbarch method. * config/pa/tm-hppa.h (INSTRUCTION_NULLIFIED): Delete definition. * hppa-tdep.c (hppa_instruction_nullified): Remove prototype and make static. Rewrite to work directly off the passed regcache. (hppa_gdbarch_init): Set instruction_nullified method. Index: arch-utils.c =================================================================== RCS file: /cvs/src/src/gdb/arch-utils.c,v retrieving revision 1.123 diff -u -p -r1.123 arch-utils.c --- arch-utils.c 31 Oct 2004 21:42:32 -0000 1.123 +++ arch-utils.c 23 Nov 2004 17:36:09 -0000 @@ -325,6 +325,13 @@ default_stabs_argument_has_addr (struct return 0; } +int +generic_instruction_nullified (struct gdbarch *gdbarch, + struct regcache *regcache) +{ + return 0; +} + /* Functions to manipulate the endianness of the target. */ Index: arch-utils.h =================================================================== RCS file: /cvs/src/src/gdb/arch-utils.h,v retrieving revision 1.76 diff -u -p -r1.76 arch-utils.h --- arch-utils.h 31 Oct 2004 21:42:32 -0000 1.76 +++ arch-utils.h 23 Nov 2004 17:36:09 -0000 @@ -117,6 +117,9 @@ extern int generic_convert_register_p (i extern int default_stabs_argument_has_addr (struct gdbarch *gdbarch, struct type *type); +extern int generic_instruction_nullified (struct gdbarch *gdbarch, + struct regcache *regcache); + /* For compatibility with older architectures, returns (LEGACY_SIM_REGNO_IGNORE) when the register doesn't have a valid name. */ Index: gdbarch.c =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.c,v retrieving revision 1.314 diff -u -p -r1.314 gdbarch.c --- gdbarch.c 31 Oct 2004 21:21:41 -0000 1.314 +++ gdbarch.c 23 Nov 2004 17:36:09 -0000 @@ -212,6 +212,7 @@ struct gdbarch gdbarch_smash_text_address_ftype *smash_text_address; gdbarch_software_single_step_ftype *software_single_step; gdbarch_single_step_through_delay_ftype *single_step_through_delay; + gdbarch_instruction_nullified_ftype *instruction_nullified; gdbarch_print_insn_ftype *print_insn; gdbarch_skip_trampoline_code_ftype *skip_trampoline_code; gdbarch_skip_solib_resolver_ftype *skip_solib_resolver; @@ -338,6 +339,7 @@ struct gdbarch startup_gdbarch = 0, /* smash_text_address */ 0, /* software_single_step */ 0, /* single_step_through_delay */ + generic_instruction_nullified, /* instruction_nullified */ 0, /* print_insn */ 0, /* skip_trampoline_code */ generic_skip_solib_resolver, /* skip_solib_resolver */ @@ -435,6 +437,7 @@ gdbarch_alloc (const struct gdbarch_info current_gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity; current_gdbarch->addr_bits_remove = core_addr_identity; current_gdbarch->smash_text_address = core_addr_identity; + current_gdbarch->instruction_nullified = generic_instruction_nullified; current_gdbarch->skip_trampoline_code = generic_skip_trampoline_code; current_gdbarch->skip_solib_resolver = generic_skip_solib_resolver; current_gdbarch->in_solib_return_trampoline = generic_in_solib_return_trampoline; @@ -591,6 +594,7 @@ verify_gdbarch (struct gdbarch *current_ /* Skip verify of smash_text_address, invalid_p == 0 */ /* Skip verify of software_single_step, has predicate */ /* Skip verify of single_step_through_delay, has predicate */ + /* Skip verify of instruction_nullified, invalid_p == 0 */ if (current_gdbarch->print_insn == 0) fprintf_unfiltered (log, "\n\tprint_insn"); /* Skip verify of skip_trampoline_code, invalid_p == 0 */ @@ -1195,6 +1199,9 @@ gdbarch_dump (struct gdbarch *current_gd fprintf_unfiltered (file, "gdbarch_dump: inner_than = <0x%lx>\n", (long) current_gdbarch->inner_than); + fprintf_unfiltered (file, + "gdbarch_dump: instruction_nullified = <0x%lx>\n", + (long) current_gdbarch->instruction_nullified); #ifdef TARGET_INT_BIT fprintf_unfiltered (file, "gdbarch_dump: TARGET_INT_BIT # %s\n", @@ -3366,6 +3373,23 @@ set_gdbarch_single_step_through_delay (s } int +gdbarch_instruction_nullified (struct gdbarch *gdbarch, struct regcache *regcache) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->instruction_nullified != NULL); + if (gdbarch_debug >= 2) + fprintf_unfiltered (gdb_stdlog, "gdbarch_instruction_nullified called\n"); + return gdbarch->instruction_nullified (gdbarch, regcache); +} + +void +set_gdbarch_instruction_nullified (struct gdbarch *gdbarch, + gdbarch_instruction_nullified_ftype instruction_nullified) +{ + gdbarch->instruction_nullified = instruction_nullified; +} + +int gdbarch_print_insn (struct gdbarch *gdbarch, bfd_vma vma, struct disassemble_info *info) { gdb_assert (gdbarch != NULL); Index: gdbarch.h =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.h,v retrieving revision 1.275 diff -u -p -r1.275 gdbarch.h --- gdbarch.h 31 Oct 2004 21:21:41 -0000 1.275 +++ gdbarch.h 23 Nov 2004 17:36:09 -0000 @@ -1227,6 +1227,16 @@ typedef int (gdbarch_single_step_through extern int gdbarch_single_step_through_delay (struct gdbarch *gdbarch, struct frame_info *frame); extern void set_gdbarch_single_step_through_delay (struct gdbarch *gdbarch, gdbarch_single_step_through_delay_ftype *single_step_through_delay); +/* On some systems, the PC may be left pointing at an instruction that won't + actually be executed. This is usually indicated by a bit in the PSW. If + we find ourselves in such a state, then we step the target beyond the + nullified instruction before returning control to gdb. + Return non-zero if the processor is about to execute a nullified instruction. */ + +typedef int (gdbarch_instruction_nullified_ftype) (struct gdbarch *gdbarch, struct regcache *regcache); +extern int gdbarch_instruction_nullified (struct gdbarch *gdbarch, struct regcache *regcache); +extern void set_gdbarch_instruction_nullified (struct gdbarch *gdbarch, gdbarch_instruction_nullified_ftype *instruction_nullified); + /* FIXME: cagney/2003-08-28: Need to find a better way of selecting the disassembler. Perhaps objdump can handle it? */ Index: gdbarch.sh =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.sh,v retrieving revision 1.350 diff -u -p -r1.350 gdbarch.sh --- gdbarch.sh 31 Oct 2004 21:21:41 -0000 1.350 +++ gdbarch.sh 23 Nov 2004 17:36:09 -0000 @@ -614,6 +614,12 @@ F:=:void:software_single_step:enum targe # Return non-zero if the processor is executing a delay slot and a # further single-step is needed before the instruction finishes. M::int:single_step_through_delay:struct frame_info *frame:frame +# On some systems, the PC may be left pointing at an instruction that won't +# actually be executed. This is usually indicated by a bit in the PSW. If +# we find ourselves in such a state, then we step the target beyond the +# nullified instruction before returning control to gdb. +# Return non-zero if the processor is about to execute a nullified instruction. +m::int:instruction_nullified:struct regcache *regcache:regcache::generic_instruction_nullified::0 # FIXME: cagney/2003-08-28: Need to find a better way of selecting the # disassembler. Perhaps objdump can handle it? f:TARGET_PRINT_INSN:int:print_insn:bfd_vma vma, struct disassemble_info *info:vma, info::0: Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.182 diff -u -p -r1.182 infrun.c --- infrun.c 8 Nov 2004 17:25:23 -0000 1.182 +++ infrun.c 23 Nov 2004 17:36:10 -0000 @@ -163,16 +163,6 @@ static int debug_infrun = 0; #define SOLIB_IN_DYNAMIC_LINKER(pid,pc) 0 #endif -/* On some systems, the PC may be left pointing at an instruction that won't - actually be executed. This is usually indicated by a bit in the PSW. If - we find ourselves in such a state, then we step the target beyond the - nullified instruction before returning control to the user so as to avoid - confusion. */ - -#ifndef INSTRUCTION_NULLIFIED -#define INSTRUCTION_NULLIFIED 0 -#endif - /* We can't step off a permanent breakpoint in the ordinary way, because we can't remove it. Instead, we have to advance the PC to the next instruction. This macro should expand to a pointer to a function that @@ -1741,14 +1731,15 @@ handle_inferior_event (struct execution_ } /* If PC is pointing at a nullified instruction, then step beyond - it so that the user won't be confused when GDB appears to be ready - to execute it. */ + it before deciding what to do. This is required when we are stepping + through a function where the last instruction is a branch with a + nullified instruction in the delay slot that belongs to the next + line (which may be in a different function altogether). */ - /* if (INSTRUCTION_NULLIFIED && currently_stepping (ecs)) */ - if (INSTRUCTION_NULLIFIED) + if (gdbarch_instruction_nullified (current_gdbarch, current_regcache)) { if (debug_infrun) - printf_unfiltered ("infrun: INSTRUCTION_NULLIFIED\n"); + printf_unfiltered ("infrun: instruction nullified\n"); registers_changed (); target_resume (ecs->ptid, 1, TARGET_SIGNAL_0); Index: config/pa/tm-hppa.h =================================================================== RCS file: /cvs/src/src/gdb/config/pa/tm-hppa.h,v retrieving revision 1.77 diff -u -p -r1.77 tm-hppa.h --- config/pa/tm-hppa.h 13 Nov 2004 02:27:04 -0000 1.77 +++ config/pa/tm-hppa.h 23 Nov 2004 17:36:10 -0000 @@ -28,9 +28,3 @@ extern int hppa_pc_requires_run_before_use (CORE_ADDR pc); #define DEPRECATED_PC_REQUIRES_RUN_BEFORE_USE(pc) hppa_pc_requires_run_before_use (pc) - -/* PA specific macro to see if the current instruction is nullified. */ -#ifndef INSTRUCTION_NULLIFIED -extern int hppa_instruction_nullified (void); -#define INSTRUCTION_NULLIFIED hppa_instruction_nullified () -#endif Index: hppa-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/hppa-tdep.c,v retrieving revision 1.181 diff -u -p -r1.181 hppa-tdep.c --- hppa-tdep.c 13 Nov 2004 02:15:32 -0000 1.181 +++ hppa-tdep.c 23 Nov 2004 17:43:02 -0000 @@ -71,7 +71,6 @@ const struct objfile_data *hppa_objfile_ /* FIXME: brobecker 2002-11-07: We will likely be able to make the following functions static, once we hppa is partially multiarched. */ int hppa_pc_requires_run_before_use (CORE_ADDR pc); -int hppa_instruction_nullified (void); /* Handle 32/64-bit struct return conventions. */ @@ -2292,14 +2291,18 @@ hppa_pc_requires_run_before_use (CORE_AD return (!target_has_stack && (pc & 0xFF000000)); } -int -hppa_instruction_nullified (void) +static int +hppa_instruction_nullified (struct gdbarch *gdbarch, struct regcache *regcache) { - /* brobecker 2002/11/07: Couldn't we use a ULONGEST here? It would - avoid the type cast. I'm leaving it as is for now as I'm doing - semi-mechanical multiarching-related changes. */ - const int ipsw = (int) read_register (HPPA_IPSW_REGNUM); - const int flags = (int) read_register (HPPA_FLAGS_REGNUM); + ULONGEST tmp, ipsw, flags; + + regcache_cooked_read (regcache, HPPA_IPSW_REGNUM, &tmp); + ipsw = extract_unsigned_integer (&tmp, + register_size (gdbarch, HPPA_IPSW_REGNUM)); + + regcache_cooked_read (regcache, HPPA_FLAGS_REGNUM, &tmp); + flags = extract_unsigned_integer (&tmp, + register_size (gdbarch, HPPA_FLAGS_REGNUM)); return ((ipsw & 0x00200000) && !(flags & 0x2)); } @@ -2570,6 +2573,7 @@ hppa_gdbarch_init (struct gdbarch_info i set_gdbarch_breakpoint_from_pc (gdbarch, hppa_breakpoint_from_pc); set_gdbarch_pseudo_register_read (gdbarch, hppa_pseudo_register_read); + set_gdbarch_instruction_nullified (gdbarch, hppa_instruction_nullified); /* Frame unwind methods. */ set_gdbarch_unwind_dummy_id (gdbarch, hppa_unwind_dummy_id); -- Randolph Chung Debian GNU/Linux Developer, hppa/ia64 ports http://www.tausq.org/