Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

Randolph Chung
2004-11-23T17:50:18Z

> 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 <call_with_trampolines+0>: copy r3,r1
0x00012180 <call_with_trampolines+4>: copy sp,r3
0x00012184 <call_with_trampolines+8>: stw,ma r1,40(,sp)
0x00012188 <call_with_trampolines+12>: ldi -28,r19
0x0001218c <call_with_trampolines+16>: fstd fr5,r19(,r3)
0x00012190 <call_with_trampolines+20>: ldi -28,r19
0x00012194 <call_with_trampolines+24>: fldd r19(,r3),fr22
0x00012198 <call_with_trampolines+28>: fcpy,dbl fr22,fr4
0x0001219c <call_with_trampolines+32>: ldo 40(r3),sp
0x000121a0 <call_with_trampolines+36>: ldw,mb -40(,sp),r3
0x000121a4 <call_with_trampolines+40>: bv,n r0(rp)
End of assembler dump.
(gdb) disassemble 0x121a8
Dump of assembler code for function marker_indirect_call:
0x000121a8 <marker_indirect_call+0>: copy r3,r1
0x000121ac <marker_indirect_call+4>: copy sp,r3
0x000121b0 <marker_indirect_call+8>: stw,ma r1,40(,sp)
0x000121b4 <marker_indirect_call+12>: ldo 40(r3),sp
0x000121b8 <marker_indirect_call+16>: ldw,mb -40(,sp),r3
0x000121bc <marker_indirect_call+20>: 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 <tausq@debian.org>

* 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;
+}
+
\f
/* 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