[patch/RFA] multiarch INSTRUCTION_NULLIFIED

[patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

Currently only hppa uses this define, but possibly this will be useful
for other architectures which has conditionally nullified instructions
(ia64?) as well. This gets us one step closer to full multiarch 
support.. 

Tested on hppa-linux with no regressions.

ok?
randolph

2004-11-17  Randolph Chung  <tausq@debian.org>

	* gdbarch.sh (instruction_nullified): New method.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* 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.
	* 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.

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	17 Nov 2004 23:37:57 -0000
@@ -614,6 +614,13 @@ 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 the user so as to avoid
+# confusion.  Return non-zero if the processor is about to execute a 
+# nullified instruction.
+M::int:instruction_nullified:struct regcache *regcache:regcache
 # 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: 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	17 Nov 2004 23:37:57 -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 */
+  0,  /* instruction_nullified */
   0,  /* print_insn */
   0,  /* skip_trampoline_code */
   generic_skip_solib_resolver,  /* skip_solib_resolver */
@@ -591,6 +593,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, has predicate */
   if (current_gdbarch->print_insn == 0)
     fprintf_unfiltered (log, "\n\tprint_insn");
   /* Skip verify of skip_trampoline_code, invalid_p == 0 */
@@ -1195,6 +1198,12 @@ 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: gdbarch_instruction_nullified_p() = %d\n",
+                      gdbarch_instruction_nullified_p (current_gdbarch));
+  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 +3375,30 @@ set_gdbarch_single_step_through_delay (s
 }
 
 int
+gdbarch_instruction_nullified_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->instruction_nullified != NULL;
+}
+
+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	17 Nov 2004 23:37:57 -0000
@@ -1227,6 +1227,19 @@ 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 the user so as to avoid
+   confusion.  Return non-zero if the processor is about to execute a
+   nullified instruction. */
+
+extern int gdbarch_instruction_nullified_p (struct gdbarch *gdbarch);
+
+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: 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	17 Nov 2004 23:37:57 -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,19 @@ 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 (current_gdbarch, 
+			  				HPPA_IPSW_REGNUM));
+
+  regcache_cooked_read (regcache, HPPA_FLAGS_REGNUM, &tmp);
+  flags = extract_unsigned_integer (&tmp, register_size (current_gdbarch, 
+			  				 HPPA_FLAGS_REGNUM));
 
   return ((ipsw & 0x00200000) && !(flags & 0x2));
 }
@@ -2570,6 +2574,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);
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	17 Nov 2004 23:37:57 -0000
@@ -163,15 +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
@@ -1744,11 +1735,11 @@ handle_inferior_event (struct execution_
      it so that the user won't be confused when GDB appears to be ready
      to execute it. */
 
-  /*      if (INSTRUCTION_NULLIFIED && currently_stepping (ecs)) */
-  if (INSTRUCTION_NULLIFIED)
+  if (gdbarch_instruction_nullified_p (current_gdbarch)
+      && gdbarch_instruction_nullified (current_gdbarch, current_regcache))
     {
       if (debug_infrun)
-	printf_unfiltered ("infrun: INSTRUCTION_NULLIFIED\n");
+	printf_unfiltered ("infrun: nullified instruction\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	17 Nov 2004 23:37:57 -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
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
> Currently only hppa uses this define, but possibly this will be useful
> for other architectures which has conditionally nullified instructions
> (ia64?) as well. This gets us one step closer to full multiarch 
> support.. 
> 
> Tested on hppa-linux with no regressions.

I'm wondering about:

# 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

and the commented out code:

   /* 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. */

   /*      if (INSTRUCTION_NULLIFIED && currently_stepping (ecs)) */

I suspect that at one stage it was behaving like 
single_step_through_delay, but was then changed to perform the same 
operation unconditionally (i.e., when stopping from a cntrl-c say).  GDB 
trying to helpfully do a few extra steps after a cntrl-c bugs me :-)

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> I'm wondering about:
> 
> # 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

I was too, but actually this seems to do something slightly different.

single_step_through_delay seems to be asking: "does the current
instruction have a delay slot"? This is used to tell gdb that if we
wanted to do a step on a branch-insn-with-a-delay-slot, then after we
step through the branch insn, we will step one more insn.

Is that correct?

The comment is confusing because it talks about "executing a delay slot"
but at least mips-tdep.c, the only implementation of this method,
doesn't test that. It tests "executing a branch insn with a delay slot".

The new method i'm adding is a bit different though. The PA has a
concept of "nullified" instructions. Based on the result of a previous
instruction (which may or may not be a branch insn), the next
instruction may be nullified. This is in many ways the precursor to the
highly predicated insn model of IA64. By examining the insn at pc (or
previous pc) you cannot tell whether the current insn is nullified. You
can only tell that by examining processor flags. (Possibly on ia64 you
would extract the predicate register from the current insn and examine
the contents of that predicate register. I haven't looked at the
details.)

> I suspect that at one stage it was behaving like 
> single_step_through_delay, but was then changed to perform the same 
> operation unconditionally (i.e., when stopping from a cntrl-c say).  GDB 
> trying to helpfully do a few extra steps after a cntrl-c bugs me :-)

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....

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Mark Kettenis]

   Date: Thu, 18 Nov 2004 08:21:08 -0800
   From: Randolph Chung <randolph@tausq.org>

   > I'm wondering about:
   > 
   > # 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

   I was too, but actually this seems to do something slightly different.

   single_step_through_delay seems to be asking: "does the current
   instruction have a delay slot"? This is used to tell gdb that if we
   wanted to do a step on a branch-insn-with-a-delay-slot, then after we
   step through the branch insn, we will step one more insn.

   Is that correct?

I think the description is misleading.

   The comment is confusing because it talks about "executing a delay slot"
   but at least mips-tdep.c, the only implementation of this method,
   doesn't test that. It tests "executing a branch insn with a delay slot".

The problem on MIPS is that if you trap on a delay slot, the processor
backs up the PC to the preceding branch instruction.  As a result,
naively single-stepping will re-execute the branch instruction again
and again.  Therefore, operating systems and GDB's software
single-stepping machinery will step over the delay slot, effectively
executing two instructions in a row instead of one.  Now if there
would be a breakpoint-instruction in the delay slot, this won't work.
That's why we had the STEP_SKIPS_DELAY macro that makes sure the
single-step is done without any breakpoints inserted.  This was
replaced by single_step_through_delay which presumably will also be
used on a new CRISv32 port.

Another problem is when there is a breakpoint in the delay slot.
Hitting it would thoroughly confuse GDB since after hitting the
breakpoint instruction the PC will not point at the breakpoint, but at
the branch instruction.  Continuing from that point will hit the
breakpoint again and again.  I'm not sure the current code handles
this correctly.

   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.

Mark

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Orjan Friberg]

Mark Kettenis wrote:
> That's why we had the STEP_SKIPS_DELAY macro that makes sure the
> single-step is done without any breakpoints inserted.  This was
> replaced by single_step_through_delay which presumably will also be
> used on a new CRISv32 port.

Yes, CRISv32 basically has the MIPS situation in reverse: if you single-step 
into a delay slot (belonging to a branch instruction) and single-step again, the 
branch instruction will be re-executed.  As a consequence, if there is a 
breakpoint at the branch instruction itself, we need to step again before 
re-inserting it (otherwise we'd just hit it again).  (I like to think of 
single_step_through_delay as "did we just step into a situation where we need to 
step again".)

-- 
Orjan Friberg
Axis Communications

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

>    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
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Mark Kettenis]

   Date: Tue, 23 Nov 2004 09:49:37 -0800
   From: Randolph Chung <randolph@tausq.org>

   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.

   comments? ok to check in?

OK, if you make sure you wrap the lines that are too long.

Mark

   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.

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>   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?

I'm still not convinced (it's me and not mark you need to convince here ;-).

Is it possible to create a core file with the PC sitting on the 
nullified instruction, and if such a beast is created, how can GDB 
correctly handle it - GDB clearly can't single step the core file.  If 
that problem is solved, this method becomes redundant.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Is it possible to create a core file with the PC sitting on the 
> nullified instruction, and if such a beast is created, how can GDB 
> correctly handle it - GDB clearly can't single step the core file.  If 
> that problem is solved, this method becomes redundant.

what do you mean? this fix is in the handling of inferior events, so
it doesn't apply to core files anyway, right? what case with core files
do you think needs to be correctly handled?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>Is it possible to create a core file with the PC sitting on the 
>>nullified instruction, and if such a beast is created, how can GDB 
>>correctly handle it - GDB clearly can't single step the core file.  If 
>>that problem is solved, this method becomes redundant.
> 
> 
> what do you mean? this fix is in the handling of inferior events, so
> it doesn't apply to core files anyway, right?

Right.

> what case with core files
> do you think needs to be correctly handled?

The underlying problem that you described:

> 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.

If, when resuming the inferior, a double step is required, 
single_step_through_delay will do the job.

However, that doesn't solve the case of GDB encountering a frame 
(inferior) that, be it through attach, cntrl-c, a signal, or a core 
file, is already sitting on the above nullified instruction.  The 
corefile case being expecially nasty - trying to get a corefile to step 
off a nullified instruction won't get you very far :-).  I suspect that 
the code will need to modify ``pc'' so that it either appears to be one 
instruction behind (the "bv,n") or one instruction ahead (branch 
destination) of what the registers indicate.

It might also be useful to check the SPARC.  It has PC/NPC, delay slots, 
and instruction nullification, so I'd expect similar problems.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> If, when resuming the inferior, a double step is required, 
> single_step_through_delay will do the job.

this is not possible to do in the general case though, because, sitting
on the current insn at pc, you cannot necessarily determine if the 
next insn will be nullified or not. (in the current example, the 
nullification is always applied, but it can be conditional on some 
computation being done)

> However, that doesn't solve the case of GDB encountering a frame 
> (inferior) that, be it through attach, cntrl-c, a signal, or a core 
> file, is already sitting on the above nullified instruction.  The 
> corefile case being expecially nasty - trying to get a corefile to step 
> off a nullified instruction won't get you very far :-).  I suspect that 
> the code will need to modify ``pc'' so that it either appears to be one 
> instruction behind (the "bv,n") or one instruction ahead (branch 
> destination) of what the registers indicate.

oh, are you saying that:
if we are looking at a corefile, and the current pc is sitting on a
nullified insn that belonged to the next function, that we may not be
able to do a backtrace correctly?

> It might also be useful to check the SPARC.  It has PC/NPC, delay slots, 
> and instruction nullification, so I'd expect similar problems.

ok, i'll see if i can find a sparc machine to try to reproduce this
problem there.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>If, when resuming the inferior, a double step is required, 
>>single_step_through_delay will do the job.
> 
> 
> this is not possible to do in the general case though, because, sitting
> on the current insn at pc, you cannot necessarily determine if the 
> next insn will be nullified or not. (in the current example, the 
> nullification is always applied, but it can be conditional on some 
> computation being done)

I'm not sure what you mean.  What you describe sounds like the old 
STEP_SKIPS_DELAY logic - a test on the _next_ instruction.  The new 
logic instead:

# 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

checks to see if the _last_ instruction put us into a delay slot.

(The MIPS code, which is technically implementing STEP_SKIPS_DELAY, 
works because when in a delay slot the PC still points at the branch 
instruction.  A fixme would be to change the mips code to instead test 
the delay-slot bit.).

>>However, that doesn't solve the case of GDB encountering a frame 
>>(inferior) that, be it through attach, cntrl-c, a signal, or a core 
>>file, is already sitting on the above nullified instruction.  The 
>>corefile case being expecially nasty - trying to get a corefile to step 
>>off a nullified instruction won't get you very far :-).  I suspect that 
>>the code will need to modify ``pc'' so that it either appears to be one 
>>instruction behind (the "bv,n") or one instruction ahead (branch 
>>destination) of what the registers indicate.
> 
> 
> oh, are you saying that:
> if we are looking at a corefile, and the current pc is sitting on a
> nullified insn that belonged to the next function, that we may not be
> able to do a backtrace correctly?

Yes, and more to the point INSTRUCTION_NULLIFIED can't help here.  On 
the other hand, if this case is made to work INSTRUCTION_NULLIFIED is 
redundant.

However, the first question is: can such a corefile be created?  Given 
that GDB can single step an inferior into such a state, I think it can.

>>It might also be useful to check the SPARC.  It has PC/NPC, delay slots, 
>>and instruction nullification, so I'd expect similar problems.
> 
> 
> ok, i'll see if i can find a sparc machine to try to reproduce this
> problem there.

One thing to check is for isif the SPARC requires padding between functions?

--

If you want, add a DEPRECATED_INSTRUCTION_NULLIFIED to the architecture 
vector and have the PA-RISC use that.  While a sideways step, it lets 
you advance what I'm assuming is the more immediate objective of 
cleaning out PA's tm*.h files and making it pure multi-arch.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> If you want, add a DEPRECATED_INSTRUCTION_NULLIFIED to the architecture 
> vector and have the PA-RISC use that.  While a sideways step, it lets 
> you advance what I'm assuming is the more immediate objective of 
> cleaning out PA's tm*.h files and making it pure multi-arch.

well, based on Mark's earlier ok i had checked in the
instruction_nullified stuff already. since this doesn't seem to be ok,
let me try to fix this properly and see what happens.

we are pretty close to getting rid of tm-hppa.h..... but the last one(s)
are always the trickiest :)

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>If you want, add a DEPRECATED_INSTRUCTION_NULLIFIED to the architecture 
>>vector and have the PA-RISC use that.  While a sideways step, it lets 
>>you advance what I'm assuming is the more immediate objective of 
>>cleaning out PA's tm*.h files and making it pure multi-arch.
> 
> 
> well, based on Mark's earlier ok i had checked in the
> instruction_nullified stuff already. since this doesn't seem to be ok,
> let me try to fix this properly and see what happens.

I don't know why Mark did that, this one is certainly ``tricky'' :-) 
Just rename it.

> we are pretty close to getting rid of tm-hppa.h..... but the last one(s)
> are always the trickiest :)

I know, there's a tradeoff.   Hence my suggestion - add it but 
deprecated so that we know it has to be revisited later (and more 
importantly other architectures don't try to adopt it :-).

Looking over whats remaining, the tricky one looks to be REALTIME.{LO,HI}.

Thanks,
Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

In reference to a message from Andrew Cagney, dated Nov 30:
> Randolph Chung wrote:
> >>If you want, add a DEPRECATED_INSTRUCTION_NULLIFIED to the architecture 
> >>vector and have the PA-RISC use that.  While a sideways step, it lets 
> >>you advance what I'm assuming is the more immediate objective of 
> >>cleaning out PA's tm*.h files and making it pure multi-arch.
> >
> >
> >well, based on Mark's earlier ok i had checked in the
> >instruction_nullified stuff already. since this doesn't seem to be ok,
> >let me try to fix this properly and see what happens.
> 
> I don't know why Mark did that, this one is certainly ``tricky'' :-) 
> Just rename it.

well, first i want to understand the problem. because i'm still not 
yet 100% convinced that step_through_delay will work. simply using the
"instruction_nullified" method in hppa-tdep as the "step_through_delay"
method certainly is not working...

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:

> well, first i want to understand the problem. because i'm still not 
> yet 100% convinced that step_through_delay will work. simply using the
> "instruction_nullified" method in hppa-tdep as the "step_through_delay"
> method certainly is not working...

When doing a stepi?  step_through_delay certainly won't help when it 
comes to doing a backtrace from the nullified instruction.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> >well, first i want to understand the problem. because i'm still not 
> >yet 100% convinced that step_through_delay will work. simply using the
> >"instruction_nullified" method in hppa-tdep as the "step_through_delay"
> >method certainly is not working...
> 
> When doing a stepi?  step_through_delay certainly won't help when it 
> comes to doing a backtrace from the nullified instruction.

when doing a step.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> >this is not possible to do in the general case though, because, sitting
> >on the current insn at pc, you cannot necessarily determine if the 
> >next insn will be nullified or not. (in the current example, the 
> >nullification is always applied, but it can be conditional on some 
> >computation being done)
> 
> I'm not sure what you mean.  What you describe sounds like the old 
> STEP_SKIPS_DELAY logic - a test on the _next_ instruction.  The new 
> logic instead:
> 
> # 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
> 
> checks to see if the _last_ instruction put us into a delay slot.

mmm... maybe i'm not reading the code correctly, but i still don't see 
how this will work for the hppa case.

this predicate is used in two places:

in proceed(), this only applies for the first insn at a "continue" or a
"step"/"next", right? so in any case it doesn't do anything if you are 
stepping through a series of instructions and the first one is not a 
branch/delay slot insn.

in handle_inferior_event(), the condition being checked is:

  if (stop_signal == TARGET_SIGNAL_TRAP
      && trap_expected
      && gdbarch_single_step_through_delay_p (current_gdbarch)
      && currently_stepping (ecs))

trap_expected will not be set when you are doing a "step" with single
stepping. possibly this works on mips because it uses software single
step? but on hppa we have hardware single step, and as far as i can tell
trap_expected is not set as we are stepping through instructions to get
to the next line when doing a "step". this condition again seems to be
more for the case when you hit a breakpoint at a branch insn with a
delay slot and you want to determine how to single step off that branch.
in the hppa case we have no breakpoints in this case.

i experimented with another proposal which is to adjust the pc when we
are at a nullified instruction. i modified target_read_pc () to return
the previous (or next) pc when we are at a nullified instruction. this 
fixes some of the failures but causes new failures with the 
"recurse.exp" test. i need to investigate that some more. but teaching 
target_read_pc() to lie about the current pc seems to be suboptimal.

lastly a comment about sparc -- i think the sparc case is simpler
because it doesn't have conditional nullification. so looking at a
particular insn you can always determine if the next insn will be
nullified or not. this is not always the case for hppa.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Tue, Nov 30, 2004 at 10:19:24PM -0800, Randolph Chung wrote:
> i experimented with another proposal which is to adjust the pc when we
> are at a nullified instruction. i modified target_read_pc () to return
> the previous (or next) pc when we are at a nullified instruction. this 
> fixes some of the failures but causes new failures with the 
> "recurse.exp" test. i need to investigate that some more. but teaching 
> target_read_pc() to lie about the current pc seems to be suboptimal.
> 
> lastly a comment about sparc -- i think the sparc case is simpler
> because it doesn't have conditional nullification. so looking at a
> particular insn you can always determine if the next insn will be
> nullified or not. this is not always the case for hppa.

Randolph,

Here's an off-the-cuff idea for you.  Could you actually skip the
nullified instruction, if you had a hook in the right place?  That is,
when a thread stops, if it is stopped at a nullified instruction,
forcibly move it to the next instruction before returning control to
GDB.

This is probably not feasible if you have to use single-stepping to end
up in the right place.  If you can compute the right place and adjust
registers, though, it shouldn't be hard.

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Here's an off-the-cuff idea for you.  Could you actually skip the
> nullified instruction, if you had a hook in the right place?  That is,
> when a thread stops, if it is stopped at a nullified instruction,
> forcibly move it to the next instruction before returning control to
> GDB.

this is exactly what the instruction_nullified method that i added do :)

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Wed, Dec 01, 2004 at 09:17:12AM -0800, Randolph Chung wrote:
> > Here's an off-the-cuff idea for you.  Could you actually skip the
> > nullified instruction, if you had a hook in the right place?  That is,
> > when a thread stops, if it is stopped at a nullified instruction,
> > forcibly move it to the next instruction before returning control to
> > GDB.
> 
> this is exactly what the instruction_nullified method that i added do :)

Could you do this at the end of the to_wait hook, if I provided a way
for the native target to override to_wait?  Oh, I guess it's an
architecture property, not a target property, so you'd need to do it
for remote stubs too.

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> > this is exactly what the instruction_nullified method that i added do :)
> 
> Could you do this at the end of the to_wait hook, if I provided a way
> for the native target to override to_wait?  Oh, I guess it's an
> architecture property, not a target property, so you'd need to do it
> for remote stubs too.

probably yes; but why is this better?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Wed, Dec 01, 2004 at 09:25:30AM -0800, Randolph Chung wrote:
> > > this is exactly what the instruction_nullified method that i added do :)
> > 
> > Could you do this at the end of the to_wait hook, if I provided a way
> > for the native target to override to_wait?  Oh, I guess it's an
> > architecture property, not a target property, so you'd need to do it
> > for remote stubs too.
> 
> probably yes; but why is this better?

Only better if it allowed to remove all reference to this
target-specific problem from core GDB.  Which it doesn't sound like it
would.

How do you detect that an instruction is nullified, or where the next
instruction executed will be?  Is there a bit in some status register?

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> How do you detect that an instruction is nullified, or where the next
> instruction executed will be?  Is there a bit in some status register?

yes, there's a bit in the processor status word to tell if the current
instruction is nullified.

the pa has a 2-level instruction pointer. (in gdb we call them pcoqh and
pcoqt). pcoqt is the next instruction to be executed.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Only better if it allowed to remove all reference to this
> target-specific problem from core GDB.  Which it doesn't sound like it
> would.

does anybody know how ia64 works in this area? if you have an
if-converted series of instruction and we single step through the
region, what happens?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> does anybody know how ia64 works in this area? if you have an
> if-converted series of instruction and we single step through the
> region, what happens?

it's a bit hard to tell what is happening, because the ia64-gdb seems to
be rather broken.... but it seems to have problems stepping through code
like this:

int foo(int a)
{
        if (a > 10) {
                x = 1;
                y = 2;
                z = 3;
        } else {
                x = 11;
                y = 12;
                z = 13;
        }

        return x + y + z;
}

with a bit of optimization, gcc turns this into:

        .loc 1 5 0
        cmp4.ge p6, p7 = 10, r32
        .loc 1 4 0
        addl r8 = @gprel(x#), gp
        addl r16 = @gprel(y#), gp
        .mmi
        addl r15 = @gprel(z#), gp
        ;;
        .loc 1 6 0
        (p7) addl r8 = @gprel(x#), gp
        (p7) addl r14 = 1, r0
        .mii
        .loc 1 10 0
        (p6) addl r14 = 11, r0
        .loc 1 7 0
        (p7) addl r16 = @gprel(y#), gp
        ;;
        .loc 1 8 0
        (p7) addl r15 = @gprel(z#), gp
        .mii
        .loc 1 10 0
        (p6) st4 [r8] = r14
        [...]

gdb gets very confused about this :)

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Daniel Jacobowitz wrote:
> On Tue, Nov 30, 2004 at 10:19:24PM -0800, Randolph Chung wrote:
> 
>>i experimented with another proposal which is to adjust the pc when we
>>are at a nullified instruction. i modified target_read_pc () to return
>>the previous (or next) pc when we are at a nullified instruction. this 
>>fixes some of the failures but causes new failures with the 
>>"recurse.exp" test. i need to investigate that some more. but teaching 
>>target_read_pc() to lie about the current pc seems to be suboptimal.
>>
>>lastly a comment about sparc -- i think the sparc case is simpler
>>because it doesn't have conditional nullification. so looking at a
>>particular insn you can always determine if the next insn will be
>>nullified or not. this is not always the case for hppa.
> 
> 
> Randolph,
> 
> Here's an off-the-cuff idea for you.  Could you actually skip the
> nullified instruction, if you had a hook in the right place?  That is,
> when a thread stops, if it is stopped at a nullified instruction,
> forcibly move it to the next instruction before returning control to
> GDB.

Read the discussion between Randolf and myself.  This doesn't work for 
core files.

> This is probably not feasible if you have to use single-stepping to end
> up in the right place.  If you can compute the right place and adjust
> registers, though, it shouldn't be hard.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>>well, first i want to understand the problem. because i'm still not 
>>>yet 100% convinced that step_through_delay will work. simply using the
>>>"instruction_nullified" method in hppa-tdep as the "step_through_delay"
>>>method certainly is not working...
>>
>>When doing a stepi?  step_through_delay certainly won't help when it 
>>comes to doing a backtrace from the nullified instruction.
> 
> 
> when doing a step.

I was thinking that STEPI might be easier to debug :-)

Anyway, trying modifying gdbarch_read_pc and unwind_pc (I suspect you 
need to modify both - which is a bug) to read something like:

	if (instruction nullified)
		return next-pc
	else
		return this-pc

it should fool GDB into thinking that it's one instruction ahead of itself.

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Anyway, trying modifying gdbarch_read_pc and unwind_pc (I suspect you 
> need to modify both - which is a bug) to read something like:
> 
> 	if (instruction nullified)
> 		return next-pc
> 	else
> 		return this-pc

i did s/next-pc/prev-pc/ instead... still seems a bit hacky to me, but
it does seem to work.

so is this ok?

randolph


2004-12-01  Randolph Chung  <tausq@debian.org>

	* gdbarch.sh (instruction_nullified): Delete.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* hppa-tdep.c (hppa_target_read_pc): Adjust pc if instruction is
	nullified.
	(hppa_unwind_pc): Likewise. 
	(hppa_instruction_nullified): Delete.
	(hppa_gdbarch_init): Don't set instruction_nullified method.
	* infrun.c (infwait_states): Remove infwait_nullified_state.
	(handle_inferior_event): Remove handling of infwait_nullified_state.
	Remove handling of nullified instructions.

Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.315
diff -u -p -r1.315 gdbarch.c
--- gdbarch.c	23 Nov 2004 21:05:18 -0000	1.315
+++ gdbarch.c	1 Dec 2004 22:13:12 -0000
@@ -212,7 +212,6 @@ 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;
@@ -339,7 +338,6 @@ 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 */
@@ -437,7 +435,6 @@ 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;
@@ -594,7 +591,6 @@ 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 */
@@ -1199,9 +1195,6 @@ 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",
@@ -3373,23 +3366,6 @@ 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.276
diff -u -p -r1.276 gdbarch.h
--- gdbarch.h	23 Nov 2004 21:05:19 -0000	1.276
+++ gdbarch.h	1 Dec 2004 22:13:12 -0000
@@ -1227,16 +1227,6 @@ 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.351
diff -u -p -r1.351 gdbarch.sh
--- gdbarch.sh	23 Nov 2004 21:05:20 -0000	1.351
+++ gdbarch.sh	1 Dec 2004 22:13:12 -0000
@@ -614,12 +614,6 @@ 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: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.183
diff -u -p -r1.183 hppa-tdep.c
--- hppa-tdep.c	1 Dec 2004 06:54:56 -0000	1.183
+++ hppa-tdep.c	1 Dec 2004 22:13:12 -0000
@@ -1042,6 +1080,8 @@ static CORE_ADDR
 hppa_target_read_pc (ptid_t ptid)
 {
   int flags = read_register_pid (HPPA_FLAGS_REGNUM, ptid);
+  ULONGEST ipsw = read_register_pid (HPPA_IPSW_REGNUM, ptid);
+  CORE_ADDR pc;
 
   /* The following test does not belong here.  It is OS-specific, and belongs
      in native code.  */
@@ -1049,7 +1089,17 @@ hppa_target_read_pc (ptid_t ptid)
   if (flags & 2)
     return read_register_pid (31, ptid) & ~0x3;
 
-  return read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
+  pc = read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
+
+  /* If the current instruction is nullified, then we are effectively
+     still executing the previous instruction.  Pretend we are still
+     there.  This is needed when single stepping; if the nullified instruction
+     is on a different line, we don't want gdb to think we've stepped onto
+     that line.  */
+  if (ipsw & 0x00200000)
+    pc -= 4;
+
+  return pc;
 }
 
 /* Write out the PC.  If currently in a syscall, then also write the new
@@ -2185,7 +2235,21 @@ hppa_unwind_dummy_id (struct gdbarch *gd
 static CORE_ADDR
 hppa_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
-  return frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;
+  ULONGEST ipsw;
+  CORE_ADDR pc;
+
+  ipsw = frame_unwind_register_signed (next_frame, HPPA_IPSW_REGNUM);
+  pc = frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;
+
+  /* If the current instruction is nullified, then we are effectively
+     still executing the previous instruction.  Pretend we are still
+     there.  This is needed when single stepping; if the nullified instruction
+     is on a different line, we don't want gdb to think we've stepped onto
+     that line.  */
+  if (ipsw & 0x00200000)
+    pc -= 4;
+
+  return pc;
 }
 
 /* Instead of this nasty cast, add a method pvoid() that prints out a
@@ -2291,22 +2355,6 @@ hppa_pc_requires_run_before_use (CORE_AD
   return (!target_has_stack && (pc & 0xFF000000));
 }
 
-static int
-hppa_instruction_nullified (struct gdbarch *gdbarch, struct regcache *regcache)
-{
-  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));
-}
-
 /* Return the GDB type object for the "standard" data type of data
    in register N.  */
 
@@ -2573,7 +2623,6 @@ 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);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.183
diff -u -p -r1.183 infrun.c
--- infrun.c	23 Nov 2004 21:05:23 -0000	1.183
+++ infrun.c	1 Dec 2004 22:13:12 -0000
@@ -870,7 +870,6 @@ enum infwait_states
 {
   infwait_normal_state,
   infwait_thread_hop_state,
-  infwait_nullified_state,
   infwait_nonstep_watch_state
 };
 
@@ -1263,12 +1262,6 @@ handle_inferior_event (struct execution_
       stepped_after_stopped_by_watchpoint = 0;
       break;
 
-    case infwait_nullified_state:
-      if (debug_infrun)
-        printf_unfiltered ("infrun: infwait_nullified_state\n");
-      stepped_after_stopped_by_watchpoint = 0;
-      break;
-
     case infwait_nonstep_watch_state:
       if (debug_infrun)
         printf_unfiltered ("infrun: infwait_nonstep_watch_state\n");
@@ -1730,30 +1723,6 @@ handle_inferior_event (struct execution_
       singlestep_breakpoints_inserted_p = 0;
     }
 
-  /* If PC is pointing at a nullified instruction, then step beyond
-     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 (gdbarch_instruction_nullified (current_gdbarch, current_regcache))
-    {
-      if (debug_infrun)
-	printf_unfiltered ("infrun: instruction nullified\n");
-      registers_changed ();
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
-
-      /* We may have received a signal that we want to pass to
-         the inferior; therefore, we must not clobber the waitstatus
-         in WS. */
-
-      ecs->infwait_state = infwait_nullified_state;
-      ecs->waiton_ptid = ecs->ptid;
-      ecs->wp = &(ecs->tmpstatus);
-      prepare_to_wait (ecs);
-      return;
-    }
-
   /* It may not be necessary to disable the watchpoint to stop over
      it.  For example, the PA can (with some kernel cooperation)
      single step over a watchpoint without disabling the watchpoint.  */

-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>Anyway, trying modifying gdbarch_read_pc and unwind_pc (I suspect you 
>>need to modify both - which is a bug) to read something like:
>>
>>	if (instruction nullified)
>>		return next-pc
>>	else
>>		return this-pc
> 
> 
> i did s/next-pc/prev-pc/ instead... still seems a bit hacky to me, but
> it does seem to work.

> @@ -1049,7 +1089,17 @@ hppa_target_read_pc (ptid_t ptid)
>    if (flags & 2)
>      return read_register_pid (31, ptid) & ~0x3;
>  
> -  return read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
> +  pc = read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
> +
> +  /* If the current instruction is nullified, then we are effectively
> +     still executing the previous instruction.  Pretend we are still
> +     there.  This is needed when single stepping; if the nullified instruction
> +     is on a different line, we don't want gdb to think we've stepped onto
> +     that line.  */
> +  if (ipsw & 0x00200000)
> +    pc -= 4;
> +
> +  return pc;
>  }

On the SPARC architecture you can do things like:

	branch foo
	branch bar

which leads to more warped combinations such as (assuming I've got my 
diagram right):

f+0:	branch x+c
f+4:	branch x+8

x+0:	branch e+4
x+4:	branch e+0
x+8:    branch x+0
x+c:    branch,annulled x+4

and I'm fairly sure that results in:

f+0  f+4 [x+c]
      f+4  x+c [x+8]
           x+c  --- [x+4]
	       ---  x+4 [x+8] <-------- {npc+4}
                     x+4  x+8 [e+0]
                          x+8  e+0 [x+0]
                               e+0  x+0 ....

and hence the ``---'' anulled instruction at x+8 has a prev-pc of x+c 
(pc+4) and not x+4 (pc-4).

This is why I was thinking that next-pc is better (but the above could 
be wrong - my sparc is very very rusty :-().

Either way, yes ok (and thanks!)

Andrew

PS: No it's not a hack, XXX_pc projects the hardware onto an idealized 
machine, there are always perverse edge cases.

PPS: A gdb.arch/ addition to tickle the basic edge case would be a 
helpful way of capturing this knowledge.

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> PPS: A gdb.arch/ addition to tickle the basic edge case would be a 
> helpful way of capturing this knowledge.

how about this... my first gdb testcase, so hopefully i didn't do
anything too wrong :)

74 PASSes, 3 FAILs with the old code (assuming the instruction_nullified
logic is removed). 77 PASSes with my previous patch.

randolph


2004-12-01  Randolph Chung  <tausq@debian.org>

	* gdb.arch/pa-nullify.exp: New file.
	* gdb.arch/pa-nullify.c: New file.

--- /dev/null	2004-08-25 14:59:25.000000000 -0700
+++ pa-nullify.exp	2004-12-01 21:21:13.451861856 -0800
@@ -0,0 +1,217 @@
+# Copyright 2004 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+#
+# This file is part of the gdb testsuite.
+
+# NOTE: this test is specific to how gcc generates code.  In particular, we 
+# are testing for the case where the last insn of a function is a "bv,n r0(%rp)"
+# and the insn in the delay slot of that branch belongs to the next function.
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+# Test handling of nullified instructions for the pa target.
+
+if ![istarget "hppa*-*-*"] then {
+    verbose "Skipping hppa nullification tests."
+    return
+}
+
+set testfile "pa-nullify"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+set gcorefile ${objdir}/${subdir}/${testfile}.gcore
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    unsupported "Testcase compile failed."
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# In the first test, we do a "step" on a function whose last instruction 
+# contains a branch-with-nullify.  The instruction in the delay slot belongs
+# to the next function.  We verify that when we step off the first function
+# that we end up back at the caller and not at the second instruction.
+
+gdb_breakpoint foo
+gdb_test "run" ".*Breakpoint 1, foo \\(x=1\\).*" "Breakpoint at foo"
+gdb_test "step" ".*main \\(.*\\) at .*pa-nullify.c:30.*" "step back to main"
+
+# In the second test, we do a single step through each instruction of a function
+# and verify that we get a proper backtrace, even when we are in a nullified
+# instruction that belongs to the next function.  We also verify that when
+# stepping over a branch-with-nullify insn that we stay on the same insn for
+# two steps.
+
+if { ! [ runto main ] } then { gdb_suppress_tests; }
+
+send_gdb "print foo\n"
+gdb_expect {
+  -re ".* = {int \\(int\\)} ($hex) <foo>.*$gdb_prompt $" {
+    set baddr $expect_out(1,string)
+    verbose "Breakpoint address at $baddr"
+    pass "print foo = $baddr"
+  }
+  default {
+    fail "print foo"
+    gdb_suppress_tests
+  } timeout {
+    fail "cannot get address of foo (timed out)."
+    gdb_suppress_tests
+  }
+}
+
+gdb_breakpoint "*$baddr"
+
+gdb_test "display /i \$pc" "1: x/i +\\\$pc.*" "display /i \$pc"
+gdb_test "continue" ".*Breakpoint \[0-9\], foo \\(x=1\\) at .*pa-nullify.c:23.*" "break at first insn of foo"
+
+set done 0
+set iter 0
+while { $done == 0 && $iter < 20 } {
+  send_gdb "backtrace\n"
+  gdb_expect {
+    -re ".*foo \\(x=.*\\).* main \\(.*\\).*$gdb_prompt $" {
+      pass "backtrace from foo"
+    }
+    -re "$gdb_prompt $" {
+      fail "backtrace from foo"
+      gdb_suppress_tests
+    } 
+    timeout {
+      fail "(timeout) backtrace from foo"
+      gdb_suppress_tests
+    }
+  }
+
+  send_gdb "stepi\n"
+  gdb_expect {
+     -re ".*: x/i +\\\$pc.*:\[ \t\]+(.*)\r.*$gdb_prompt $" {
+       set insn $expect_out(1,string)
+       pass "x/i \$pc = $insn"
+     } timeout {
+       fail "(timeout) x/i \$pc"
+       gdb_suppress_tests
+       break
+     }
+  }
+
+  incr iter
+
+  if { $insn == "bv,n r0(rp)" } {
+    pass "found bv,n insn"
+    set done 1
+  }
+}
+
+send_gdb "stepi\n"
+gdb_expect {
+  -re ".*bv,n r0\\(rp\\)\r.*$gdb_prompt $" {
+    pass "restep on nullified instruction"
+  } 
+  -re ".*bar.*\r.*x/i.*bar.*\r.*$gdb_prompt $" {
+    fail "step on nullification instruction ended in next function"
+  }
+  timeout {
+    fail "(timeout) restep on nullified instruction"
+  } 
+}
+incr iter
+
+# Lastly, we do a single step through each instruction and generate a core file.
+# Then, load the core file and verify that we can get a proper backtrace.
+
+if ![isnative] then {
+  verbose "skipping core file tests on non-native configuration"
+  return
+}
+
+send_gdb "undisplay 1\n"
+
+proc do_core_test { count } {
+  global gcorefile
+  global gdb_prompt
+  global hex
+
+  if { ! [ runto_main ] } then { gdb_suppress_tests; }
+
+  set baddr "0"
+  send_gdb "print foo\n"
+  gdb_expect {
+    -re ".* = {int \\(int\\)} ($hex) <foo>.*$gdb_prompt $" {
+      set baddr $expect_out(1,string)
+      verbose "Breakpoint address at $baddr"
+      pass "print foo = $baddr #$count"
+    }
+    default {
+      fail "print foo #$count"
+      gdb_suppress_tests
+    } 
+    timeout {
+      fail "cannot get address of foo (timed out). #$count"
+      gdb_suppress_tests
+    }
+  }
+  gdb_breakpoint "*$baddr"
+
+  gdb_test "continue" ".*Breakpoint \[0-9\]*, foo \\(x=1\\) at .*pa-nullify.c:23.*" "break at first insn of foo #$count"
+
+  gdb_test "si $count" 
+
+  set gcore_works 0
+  set escapedfilename [string_to_regexp $gcorefile]
+  gdb_test_multiple "gcore $gcorefile" "gcore" {
+    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
+	pass "gcore #$count"
+	set gcore_works 1
+    }
+    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
+	unsupported "gcore"
+    }
+  }
+
+  if { $gcore_works } {
+    gdb_test "core $gcorefile" "Core was generated by.*" \
+      "load core file #$count" "A program is being debugged already.*" "y"
+
+    send_gdb "backtrace\n"
+    gdb_expect {
+      -re ".*foo \\(x=.*\\).* main \\(.*\\).*$gdb_prompt $" {
+        pass "backtrace from foo #$count"
+      }
+      -re "$gdb_prompt $" {
+        fail "backtrace from foo #$count"
+        gdb_suppress_tests
+      } 
+      timeout {
+        fail "(timeout) backtrace from foo in gcore #$count"
+        gdb_suppress_tests
+      }
+    }
+  }
+}
+
+for { set i 1 } { $i <= $iter } {incr i} {
+  do_core_test $i
+}
--- /dev/null	2004-08-25 14:59:25.000000000 -0700
+++ pa-nullify.c	2004-12-01 21:19:51.324347136 -0800
@@ -0,0 +1,30 @@
+/* PA nullification test program.
+
+   Copyright 2004 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+   
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+int foo(int x)
+{ return x; }
+
+void bar(void) { }
+
+int main(int argc, char **argv)
+{
+	return foo(argc);
+}

-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Wed, Dec 01, 2004 at 09:24:18PM -0800, Randolph Chung wrote:
> > PPS: A gdb.arch/ addition to tickle the basic edge case would be a 
> > helpful way of capturing this knowledge.
> 
> how about this... my first gdb testcase, so hopefully i didn't do
> anything too wrong :)

Can you do this without source-level debugging?  Since the test is in
gdb.arch, the most robust thing to do would be to write the nullified
branch in assembly.

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:
>>PPS: A gdb.arch/ addition to tickle the basic edge case would be a 
>>helpful way of capturing this knowledge.
> 
> 
> how about this... my first gdb testcase, so hopefully i didn't do
> anything too wrong :)
> 
> 74 PASSes, 3 FAILs with the old code (assuming the instruction_nullified
> logic is removed). 77 PASSes with my previous patch.
> 
> randolph
> 
> 
> 2004-12-01  Randolph Chung  <tausq@debian.org>
> 
> 	* gdb.arch/pa-nullify.exp: New file.
> 	* gdb.arch/pa-nullify.c: New file.

> +send_gdb "print foo\n"
> +gdb_expect {
> +  -re ".* = {int \\(int\\)} ($hex) <foo>.*$gdb_prompt $" {
> +    set baddr $expect_out(1,string)
> +    verbose "Breakpoint address at $baddr"
> +    pass "print foo = $baddr"
> +  }
> +  default {
> +    fail "print foo"
> +    gdb_suppress_tests
> +  } timeout {
> +    fail "cannot get address of foo (timed out)."
> +    gdb_suppress_tests
> +  }
> +}

I'd use gdb_test_multiple: it's more robust and handles more cases when 
things go wrong; and we're trying to eliminate the old style (yes, there 
is no marker to warn you of this, sigh).  It should be a drop-in 
replacement and easy to fix.

Anyway, more relevant, and as daniel asked, can it be done in assember? 
  A starting point for that might be the gdb.asm test case which uses 
assembly source code.

Andrew

> +gdb_breakpoint "*$baddr"
> +
> +gdb_test "display /i \$pc" "1: x/i +\\\$pc.*" "display /i \$pc"
> +gdb_test "continue" ".*Breakpoint \[0-9\], foo \\(x=1\\) at .*pa-nullify.c:23.*" "break at first insn of foo"
> +
> +set done 0
> +set iter 0
> +while { $done == 0 && $iter < 20 } {
> +  send_gdb "backtrace\n"
> +  gdb_expect {
> +    -re ".*foo \\(x=.*\\).* main \\(.*\\).*$gdb_prompt $" {
> +      pass "backtrace from foo"
> +    }
> +    -re "$gdb_prompt $" {
> +      fail "backtrace from foo"
> +      gdb_suppress_tests
> +    } 
> +    timeout {
> +      fail "(timeout) backtrace from foo"
> +      gdb_suppress_tests
> +    }
> +  }
> +
> +  send_gdb "stepi\n"
> +  gdb_expect {
> +     -re ".*: x/i +\\\$pc.*:\[ \t\]+(.*)\r.*$gdb_prompt $" {
> +       set insn $expect_out(1,string)
> +       pass "x/i \$pc = $insn"
> +     } timeout {
> +       fail "(timeout) x/i \$pc"
> +       gdb_suppress_tests
> +       break
> +     }
> +  }
> +
> +  incr iter
> +
> +  if { $insn == "bv,n r0(rp)" } {
> +    pass "found bv,n insn"
> +    set done 1
> +  }
> +}
> +
> +send_gdb "stepi\n"
> +gdb_expect {
> +  -re ".*bv,n r0\\(rp\\)\r.*$gdb_prompt $" {
> +    pass "restep on nullified instruction"
> +  } 
> +  -re ".*bar.*\r.*x/i.*bar.*\r.*$gdb_prompt $" {
> +    fail "step on nullification instruction ended in next function"
> +  }
> +  timeout {
> +    fail "(timeout) restep on nullified instruction"
> +  } 
> +}
> +incr iter
> +
> +# Lastly, we do a single step through each instruction and generate a core file.
> +# Then, load the core file and verify that we can get a proper backtrace.
> +
> +if ![isnative] then {
> +  verbose "skipping core file tests on non-native configuration"
> +  return
> +}
> +
> +send_gdb "undisplay 1\n"
> +
> +proc do_core_test { count } {
> +  global gcorefile
> +  global gdb_prompt
> +  global hex
> +
> +  if { ! [ runto_main ] } then { gdb_suppress_tests; }
> +
> +  set baddr "0"
> +  send_gdb "print foo\n"
> +  gdb_expect {
> +    -re ".* = {int \\(int\\)} ($hex) <foo>.*$gdb_prompt $" {
> +      set baddr $expect_out(1,string)
> +      verbose "Breakpoint address at $baddr"
> +      pass "print foo = $baddr #$count"
> +    }
> +    default {
> +      fail "print foo #$count"
> +      gdb_suppress_tests
> +    } 
> +    timeout {
> +      fail "cannot get address of foo (timed out). #$count"
> +      gdb_suppress_tests
> +    }
> +  }
> +  gdb_breakpoint "*$baddr"
> +
> +  gdb_test "continue" ".*Breakpoint \[0-9\]*, foo \\(x=1\\) at .*pa-nullify.c:23.*" "break at first insn of foo #$count"
> +
> +  gdb_test "si $count" 
> +
> +  set gcore_works 0
> +  set escapedfilename [string_to_regexp $gcorefile]
> +  gdb_test_multiple "gcore $gcorefile" "gcore" {
> +    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
> +	pass "gcore #$count"
> +	set gcore_works 1
> +    }
> +    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
> +	unsupported "gcore"
> +    }
> +  }
> +
> +  if { $gcore_works } {
> +    gdb_test "core $gcorefile" "Core was generated by.*" \
> +      "load core file #$count" "A program is being debugged already.*" "y"
> +
> +    send_gdb "backtrace\n"
> +    gdb_expect {
> +      -re ".*foo \\(x=.*\\).* main \\(.*\\).*$gdb_prompt $" {
> +        pass "backtrace from foo #$count"
> +      }
> +      -re "$gdb_prompt $" {
> +        fail "backtrace from foo #$count"
> +        gdb_suppress_tests
> +      } 
> +      timeout {
> +        fail "(timeout) backtrace from foo in gcore #$count"
> +        gdb_suppress_tests
> +      }
> +    }
> +  }
> +}
> +
> +for { set i 1 } { $i <= $iter } {incr i} {
> +  do_core_test $i
> +}
> --- /dev/null	2004-08-25 14:59:25.000000000 -0700
> +++ pa-nullify.c	2004-12-01 21:19:51.324347136 -0800
> @@ -0,0 +1,30 @@
> +/* PA nullification test program.
> +
> +   Copyright 2004 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 2 of the License, or
> +   (at your option) any later version.
> +   
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +   
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 59 Temple Place - Suite 330,
> +   Boston, MA 02111-1307, USA.  */
> +
> +int foo(int x)
> +{ return x; }
> +
> +void bar(void) { }
> +
> +int main(int argc, char **argv)
> +{
> +	return foo(argc);
> +}
> 

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Anyway, more relevant, and as daniel asked, can it be done in assember? 
>  A starting point for that might be the gdb.asm test case which uses 
> assembly source code.

i'm working on it, but getting stuck with a weird problem... i'm seeing
a case where i do:

proc get_addr_of_sym { sym } {
  set addr 0
  global gdb_prompt
  global expect_out

  send_gdb "print $sym\n"
  gdb_expect 60 {
    -re ".*($hex) <$sym>.*$gdb_prompt $" {
      set addr $expect_out(1,string)
      pass "got address of $sym = $addr"
    }
    timeout {
      fail "cannot get address of $sym (timed out)."
      gdb_suppress_tests
    }
  }

  return $addr
}

set foo [get_addr_of_sym "foo"]
set bar [get_addr_of_sym "bar"]

i always get a FAIL on that, and from looking at the log (with --debug)
it seems like the gdb_expect is returning immediately without checking
for results from the previous send_gdb command....

viz:
send: sending "print foo\n" to { exp11 }^M
FAIL: gdb.arch/pa-nullify.exp: cannot get address of foo (timed out).
send: sending "print bar\n" to { exp11 }^M
FAIL: gdb.arch/pa-nullify.exp: cannot get address of bar (timed out).

am i doing something obviously wrong?

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Fri, Dec 03, 2004 at 10:15:05AM -0800, Randolph Chung wrote:
> > Anyway, more relevant, and as daniel asked, can it be done in assember? 
> >  A starting point for that might be the gdb.asm test case which uses 
> > assembly source code.
> 
> i'm working on it, but getting stuck with a weird problem... i'm seeing
> a case where i do:
> 
> proc get_addr_of_sym { sym } {
>   set addr 0
>   global gdb_prompt
>   global expect_out

Not sure but I don't think you need to declare expect_out as a global.

>   send_gdb "print $sym\n"
>   gdb_expect 60 {
>     -re ".*($hex) <$sym>.*$gdb_prompt $" {
>       set addr $expect_out(1,string)
>       pass "got address of $sym = $addr"
>     }
>     timeout {
>       fail "cannot get address of $sym (timed out)."
>       gdb_suppress_tests
>     }
>   }
> 
>   return $addr
> }
> 
> set foo [get_addr_of_sym "foo"]
> set bar [get_addr_of_sym "bar"]
> 
> i always get a FAIL on that, and from looking at the log (with --debug)
> it seems like the gdb_expect is returning immediately without checking
> for results from the previous send_gdb command....
> 
> viz:
> send: sending "print foo\n" to { exp11 }^M
> FAIL: gdb.arch/pa-nullify.exp: cannot get address of foo (timed out).
> send: sending "print bar\n" to { exp11 }^M
> FAIL: gdb.arch/pa-nullify.exp: cannot get address of bar (timed out).
> 
> am i doing something obviously wrong?

First, as Andrew mentioned, don't use gdb_expect this way.  Use
gdb_test_multiple.

Secondly, there are two possible causes of this.  One is a syntax
error, in either the regular expression or the code for the matching
case. That will invoke the "timeout" handler. The other is that you've
gotten out of sync somehow; doesn't look too likely from the above.

Try putting "exp_internal 1" in front of it to see what expect thinks
it is doing.

I just saw the bug: try "global hex" and see if that helps.

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> I just saw the bug: try "global hex" and see if that helps.

yes, thanks. i hate tcl...

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> Anyway, more relevant, and as daniel asked, can it be done in assember? 
>  A starting point for that might be the gdb.asm test case which uses 
> assembly source code.

ok, how about this. it's in asm and it only tests the relevant edge
cases...

2004-12-03  Randolph Chung  <tausq@debian.org>

testsuite/
	* gdb.arch/pa-nullify.exp: New file.
	* gdb.arch/pa-nullify.s: New file.

--- /dev/null	2004-08-25 14:59:25.000000000 -0700
+++ pa-nullify.s	2004-12-02 08:40:46.342945888 -0800
@@ -0,0 +1,59 @@
+	.LEVEL 1.1
+	.text
+	.align 4
+.globl foo
+	.type	foo, @function
+foo:
+	.PROC
+	.CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	stw %r26,-36(%r3)
+	ldw -36(%r3),%r19
+	copy %r19,%r28
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND
+
+.globl bar
+	.type	bar, @function
+bar:
+	.PROC
+	.CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND
+
+.globl main
+	.type	main, @function
+main:
+	.PROC
+	.CALLINFO FRAME=64,CALLS,SAVE_RP,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	stw %r2,-20(%r30)
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	stw %r26,-36(%r3)
+	stw %r25,-40(%r3)
+	ldw -36(%r3),%r26
+	bl foo,%r2
+	nop
+	copy %r28,%r19
+	copy %r19,%r28
+	ldw -20(%r3),%r2
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND
--- /dev/null	2004-08-25 14:59:25.000000000 -0700
+++ pa-nullify.exp	2004-12-03 12:16:52.044063224 -0800
@@ -0,0 +1,151 @@
+# Copyright 2004 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+#
+# This file is part of the gdb testsuite.
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+# Test handling of nullified instructions for the pa target.
+
+if ![istarget "hppa*-*-*"] then {
+    verbose "Skipping hppa nullification tests."
+    return
+}
+
+set testfile "pa-nullify"
+set srcfile ${testfile}.s
+set binfile ${objdir}/${subdir}/${testfile}
+set gcorefile ${objdir}/${subdir}/${testfile}.gcore
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {}] != "" } {
+    unsupported "Testcase compile failed."
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# In the first test, we do a "step" on a function whose last instruction 
+# contains a branch-with-nullify.  The instruction in the delay slot belongs
+# to the next function.  We verify that when we step off the first function
+# that we end up back at the caller and not at the second instruction.
+
+gdb_breakpoint foo
+gdb_test "run" ".*Breakpoint 1, .* in foo.*" "Breakpoint at foo"
+
+set test "stepi till main"
+gdb_test_multiple "stepi" "${test}" {
+  -re ".*in foo.*$gdb_prompt $" {
+    send_gdb "stepi\n"
+    exp_continue -continue_timer
+  }
+  -re ".*in bar.*$gdb_prompt $" {
+    fail "stepped into bar"
+  }
+  -re ".*in main.*$gdb_prompt $" {
+    pass "stepped into main"
+  }
+}
+
+# In the second test, we verify that we can get a proper backtrace 
+# even when we are in a nullified instruction that belongs to the next function.
+# We also verify that when stepping over a branch-with-nullify insn that we 
+# stay on the same insn for two steps.
+
+proc get_addr_of_sym { sym } {
+  set addr 0
+  global gdb_prompt
+  global hex
+
+  send_gdb "print $sym\n"
+  gdb_test_multiple "print $sym" "Get address of $sym" {
+    -re ".*($hex) <$sym>.*$gdb_prompt $" {
+      set addr $expect_out(1,string)
+      pass "got address of $sym = $addr"
+    }
+  }
+
+  return $addr
+}
+
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+
+set foo [get_addr_of_sym "foo"]
+set bar [get_addr_of_sym "bar"]
+set foo_last [expr $bar - 4]
+
+gdb_breakpoint "*$foo_last"
+
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
+gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from last insn in foo"
+gdb_test "stepi" ".*in foo.*" "stepi to nullified instruction stays in foo"
+gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from nullified insn"
+gdb_test "stepi" ".*in main.*" "stepi to main"
+
+# In the third test, we verify that backtraces from nullified instructions
+# work even in coredumps
+
+proc gen_core { test } {
+  global gcorefile
+  global gdb_prompt
+  set gcore_works 0
+  set escapedfilename [string_to_regexp $gcorefile]
+
+  gdb_test_multiple "gcore $gcorefile" "gcore" {
+    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
+      pass "$test: gcore"
+      set gcore_works 1
+    }
+    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
+      unsupported "gcore"
+    }
+  }
+
+  return $gcore_works
+}
+
+proc test_core_bt { test } {
+  global gcorefile
+
+  gdb_test "core $gcorefile" "Core was generated by.*" \
+    "$test: load core file" "A program is being debugged already.*" "y"
+
+  gdb_test "backtrace" ".*in foo.*in main.*" "$test: backtrace in gcore"
+}
+
+set test "core at last insn in foo"
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+gdb_breakpoint "*$foo_last"
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
+if [gen_core $test] {
+  test_core_bt $test
+}
+
+set test "core at nullified insn"
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+gdb_breakpoint "*$foo_last"
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
+gdb_test "stepi" ".*in foo.*"
+if [gen_core $test] {
+  test_core_bt $test
+}

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Andrew Cagney]

Randolph Chung wrote:

Just some nits, fix that, and assuming daniel doesn't notice anything 
(for a day or til monday?), commit it.

> +gdb_breakpoint foo
> +gdb_test "run" ".*Breakpoint 1, .* in foo.*" "Breakpoint at foo"

The leading .* in the pattern you've picked up from the existing code 
isn't actually needed (well at least I'm 99% certain of this as I've 
been stripping them out my self and it makes things more efficient) (the 
trailing .*s are needed :-):

> +  -re ".*in foo.*$gdb_prompt $" {

ditto here and for others.  Can you strip them out and check things are 
ok (i'd do it incrementally first :-).

> +    send_gdb "stepi\n"

Good one ...

> +    exp_continue -continue_timer

I can never remember to do this :-(

> +  }
> +  -re ".*in bar.*$gdb_prompt $" {
> +    fail "stepped into bar"

The pass / fail messages should all be identical (except perhaps for a 
fail where a trailing comment in paren can be added).  So:

	fail "$test (stepped into bar)"

> +  }
> +  -re ".*in main.*$gdb_prompt $" {
> +    pass "stepped into main"

and just:

	pass "$test"

check for the same problem elsewhere.

> +  gdb_test_multiple "print $sym" "Get address of $sym" {
> +    -re ".*($hex) <$sym>.*$gdb_prompt $" {
> +      set addr $expect_out(1,string)
> +      pass "got address of $sym = $addr"
> +    }
> +  }

Remove the address from the test result (I'm always being slapped for 
doing that one ;-).  It can make the comparison of subsequent runs, or 
runs on different systems, harder.

> +  return $addr
> +}
> +
> +if { ! [ runto_main ] } then { gdb_suppress_tests; }
> +
> +set foo [get_addr_of_sym "foo"]
> +set bar [get_addr_of_sym "bar"]
> +set foo_last [expr $bar - 4]
> +
> +gdb_breakpoint "*$foo_last"
> +
> +gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
> +gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from last insn in foo"
> +gdb_test "stepi" ".*in foo.*" "stepi to nullified instruction stays in foo"
> +gdb_test "backtrace" ".*in foo.*in main.*" "Backtrace from nullified insn"
> +gdb_test "stepi" ".*in main.*" "stepi to main"
> +
> +# In the third test, we verify that backtraces from nullified instructions
> +# work even in coredumps
> +
> +proc gen_core { test } {
> +  global gcorefile
> +  global gdb_prompt
> +  set gcore_works 0
> +  set escapedfilename [string_to_regexp $gcorefile]
> +
> +  gdb_test_multiple "gcore $gcorefile" "gcore" {
> +    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
> +      pass "$test: gcore"
> +      set gcore_works 1
> +    }
> +    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
> +      unsupported "gcore"
> +    }
> +  }

I'd kfail this.  Not supporting gcore is a bug.

If, on your system, gcore doesn't work then it might be better to steal 
code from bigcore.exp that uses SIGABRT to generate the core file.  Your 
call though.

Anyway, given the follow-on code, I guess it's working for you.

Finally, as a quick sanity check, can you take the gdb.sum output from 
just this test and check it for duplicate test names (sort < gdb.sum | 
uniq -d).  They should be unique.

Thanks!

Andrew

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Daniel Jacobowitz]

On Fri, Dec 03, 2004 at 04:09:50PM -0500, Andrew Cagney wrote:
> Randolph Chung wrote:
> 
> Just some nits, fix that, and assuming daniel doesn't notice anything 
> (for a day or til monday?), commit it.
> 
> >+gdb_breakpoint foo
> >+gdb_test "run" ".*Breakpoint 1, .* in foo.*" "Breakpoint at foo"
> 
> The leading .* in the pattern you've picked up from the existing code 
> isn't actually needed (well at least I'm 99% certain of this as I've 
> been stripping them out my self and it makes things more efficient) (the 
> trailing .*s are needed :-):

That's right.  gdb_test_multiple does:
    return [gdb_test_multiple $command $message {
        -re "\[\r\n\]*($pattern)\[\r\n\]+$gdb_prompt $" {

Hmm, that initial bit is completely pointless...

> >+  }
> >+  -re ".*in bar.*$gdb_prompt $" {
> >+    fail "stepped into bar"
> 
> The pass / fail messages should all be identical (except perhaps for a 
> fail where a trailing comment in paren can be added).  So:
> 
> 	fail "$test (stepped into bar)"
> 
> >+  }
> >+  -re ".*in main.*$gdb_prompt $" {
> >+    pass "stepped into main"
> 
> and just:
> 
> 	pass "$test"
> 
> check for the same problem elsewhere.
> 
> >+  gdb_test_multiple "print $sym" "Get address of $sym" {
> >+    -re ".*($hex) <$sym>.*$gdb_prompt $" {
> >+      set addr $expect_out(1,string)
> >+      pass "got address of $sym = $addr"
> >+    }
> >+  }
> 
> Remove the address from the test result (I'm always being slapped for 
> doing that one ;-).  It can make the comparison of subsequent runs, or 
> runs on different systems, harder.

[And make the different test name strings match, as earlier.]

These were the only problems I saw, too.  Fix them and it's OK with me.

> I'd kfail this.  Not supporting gcore is a bug.

Or just fail it for now.

-- 
Daniel Jacobowitz

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

> I'd kfail this.  Not supporting gcore is a bug.
> 
> If, on your system, gcore doesn't work then it might be better to steal 
> code from bigcore.exp that uses SIGABRT to generate the core file.  Your 
> call though.

mmm ok, i'll have to look. gcore doesn't work on hpux i think.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

I've committed the following.

randolph


2004-12-03  Randolph Chung  <tausq@debian.org>

	* gdbarch.sh (instruction_nullified): Delete.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* hppa-tdep.c (hppa_target_read_pc): Adjust pc if instruction is
	nullified.
	(hppa_unwind_pc): Likewise. 
	(hppa_instruction_nullified): Delete.
	(hppa_gdbarch_init): Don't set instruction_nullified method.
	* infrun.c (infwait_states): Remove infwait_nullified_state.
	(handle_inferior_event): Remove handling of infwait_nullified_state.
	Remove handling of nullified instructions.


testsuite/

	* gdb.arch/pa-nullify.exp: New file.
	* gdb.arch/pa-nullify.s: New file.

Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.315
diff -u -p -r1.315 gdbarch.c
--- gdbarch.c	23 Nov 2004 21:05:18 -0000	1.315
+++ gdbarch.c	3 Dec 2004 23:57:54 -0000
@@ -212,7 +212,6 @@ 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;
@@ -339,7 +338,6 @@ 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 */
@@ -437,7 +435,6 @@ 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;
@@ -594,7 +591,6 @@ 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 */
@@ -1199,9 +1195,6 @@ 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",
@@ -3373,23 +3366,6 @@ 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.276
diff -u -p -r1.276 gdbarch.h
--- gdbarch.h	23 Nov 2004 21:05:19 -0000	1.276
+++ gdbarch.h	3 Dec 2004 23:57:55 -0000
@@ -1227,16 +1227,6 @@ 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.351
diff -u -p -r1.351 gdbarch.sh
--- gdbarch.sh	23 Nov 2004 21:05:20 -0000	1.351
+++ gdbarch.sh	3 Dec 2004 23:57:55 -0000
@@ -614,12 +614,6 @@ 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: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.183
diff -u -p -r1.183 hppa-tdep.c
--- hppa-tdep.c	1 Dec 2004 06:54:56 -0000	1.183
+++ hppa-tdep.c	3 Dec 2004 23:57:56 -0000
@@ -1042,6 +1042,8 @@ static CORE_ADDR
 hppa_target_read_pc (ptid_t ptid)
 {
   int flags = read_register_pid (HPPA_FLAGS_REGNUM, ptid);
+  ULONGEST ipsw = read_register_pid (HPPA_IPSW_REGNUM, ptid);
+  CORE_ADDR pc;
 
   /* The following test does not belong here.  It is OS-specific, and belongs
      in native code.  */
@@ -1049,7 +1051,17 @@ hppa_target_read_pc (ptid_t ptid)
   if (flags & 2)
     return read_register_pid (31, ptid) & ~0x3;
 
-  return read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
+  pc = read_register_pid (HPPA_PCOQ_HEAD_REGNUM, ptid) & ~0x3;
+
+  /* If the current instruction is nullified, then we are effectively
+     still executing the previous instruction.  Pretend we are still
+     there.  This is needed when single stepping; if the nullified instruction
+     is on a different line, we don't want gdb to think we've stepped onto
+     that line.  */
+  if (ipsw & 0x00200000)
+    pc -= 4;
+
+  return pc;
 }
 
 /* Write out the PC.  If currently in a syscall, then also write the new
@@ -2185,7 +2197,21 @@ hppa_unwind_dummy_id (struct gdbarch *gd
 static CORE_ADDR
 hppa_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
-  return frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;
+  ULONGEST ipsw;
+  CORE_ADDR pc;
+
+  ipsw = frame_unwind_register_signed (next_frame, HPPA_IPSW_REGNUM);
+  pc = frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;
+
+  /* If the current instruction is nullified, then we are effectively
+     still executing the previous instruction.  Pretend we are still
+     there.  This is needed when single stepping; if the nullified instruction
+     is on a different line, we don't want gdb to think we've stepped onto
+     that line.  */
+  if (ipsw & 0x00200000)
+    pc -= 4;
+
+  return pc;
 }
 
 /* Instead of this nasty cast, add a method pvoid() that prints out a
@@ -2291,22 +2317,6 @@ hppa_pc_requires_run_before_use (CORE_AD
   return (!target_has_stack && (pc & 0xFF000000));
 }
 
-static int
-hppa_instruction_nullified (struct gdbarch *gdbarch, struct regcache *regcache)
-{
-  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));
-}
-
 /* Return the GDB type object for the "standard" data type of data
    in register N.  */
 
@@ -2573,7 +2583,6 @@ 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);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.183
diff -u -p -r1.183 infrun.c
--- infrun.c	23 Nov 2004 21:05:23 -0000	1.183
+++ infrun.c	3 Dec 2004 23:57:58 -0000
@@ -870,7 +870,6 @@ enum infwait_states
 {
   infwait_normal_state,
   infwait_thread_hop_state,
-  infwait_nullified_state,
   infwait_nonstep_watch_state
 };
 
@@ -1263,12 +1262,6 @@ handle_inferior_event (struct execution_
       stepped_after_stopped_by_watchpoint = 0;
       break;
 
-    case infwait_nullified_state:
-      if (debug_infrun)
-        printf_unfiltered ("infrun: infwait_nullified_state\n");
-      stepped_after_stopped_by_watchpoint = 0;
-      break;
-
     case infwait_nonstep_watch_state:
       if (debug_infrun)
         printf_unfiltered ("infrun: infwait_nonstep_watch_state\n");
@@ -1730,30 +1723,6 @@ handle_inferior_event (struct execution_
       singlestep_breakpoints_inserted_p = 0;
     }
 
-  /* If PC is pointing at a nullified instruction, then step beyond
-     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 (gdbarch_instruction_nullified (current_gdbarch, current_regcache))
-    {
-      if (debug_infrun)
-	printf_unfiltered ("infrun: instruction nullified\n");
-      registers_changed ();
-      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
-
-      /* We may have received a signal that we want to pass to
-         the inferior; therefore, we must not clobber the waitstatus
-         in WS. */
-
-      ecs->infwait_state = infwait_nullified_state;
-      ecs->waiton_ptid = ecs->ptid;
-      ecs->wp = &(ecs->tmpstatus);
-      prepare_to_wait (ecs);
-      return;
-    }
-
   /* It may not be necessary to disable the watchpoint to stop over
      it.  For example, the PA can (with some kernel cooperation)
      single step over a watchpoint without disabling the watchpoint.  */
Index: testsuite/gdb.arch/pa-nullify.exp
===================================================================
RCS file: testsuite/gdb.arch/pa-nullify.exp
diff -N testsuite/gdb.arch/pa-nullify.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.arch/pa-nullify.exp	3 Dec 2004 23:58:01 -0000
@@ -0,0 +1,152 @@
+# Copyright 2004 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+#
+# This file is part of the gdb testsuite.
+
+if $tracelevel {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+# Test handling of nullified instructions for the pa target.
+
+if ![istarget "hppa*-*-*"] then {
+    verbose "Skipping hppa nullification tests."
+    return
+}
+
+set testfile "pa-nullify"
+set srcfile ${testfile}.s
+set binfile ${objdir}/${subdir}/${testfile}
+set gcorefile ${objdir}/${subdir}/${testfile}.gcore
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {}] != "" } {
+    unsupported "Testcase compile failed."
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# In the first test, we do a "step" on a function whose last instruction 
+# contains a branch-with-nullify.  The instruction in the delay slot belongs
+# to the next function.  We verify that when we step off the first function
+# that we end up back at the caller and not at the second instruction.
+
+gdb_breakpoint foo
+gdb_test "run" "Breakpoint 1, .* in foo.*" "Breakpoint at foo"
+
+set test "stepi till main"
+gdb_test_multiple "stepi" "${test}" {
+  -re ".*in foo.*$gdb_prompt $" {
+    send_gdb "stepi\n"
+    exp_continue -continue_timer
+  }
+  -re ".*in bar.*$gdb_prompt $" {
+    fail $test
+  }
+  -re ".*in main.*$gdb_prompt $" {
+    pass $test
+  }
+}
+
+# In the second test, we verify that we can get a proper backtrace 
+# even when we are in a nullified instruction that belongs to the next function.
+# We also verify that when stepping over a branch-with-nullify insn that we 
+# stay on the same insn for two steps.
+
+proc get_addr_of_sym { sym } {
+  set addr 0
+  global gdb_prompt
+  global hex
+
+  set test "get address of $sym"
+  send_gdb "print $sym\n"
+  gdb_test_multiple "print $sym" $test {
+    -re ".*($hex) <$sym>.*$gdb_prompt $" {
+      set addr $expect_out(1,string)
+      pass $test
+    }
+  }
+
+  return $addr
+}
+
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+
+set foo [get_addr_of_sym "foo"]
+set bar [get_addr_of_sym "bar"]
+set foo_last [expr $bar - 4]
+
+gdb_breakpoint "*$foo_last"
+
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*"
+gdb_test "backtrace" "in foo.*in main.*" "Backtrace from last insn in foo"
+gdb_test "stepi" "in foo.*" "stepi to nullified instruction stays in foo"
+gdb_test "backtrace" "in foo.*in main.*" "Backtrace from nullified insn"
+gdb_test "stepi" "in main.*" "stepi to main"
+
+# In the third test, we verify that backtraces from nullified instructions
+# work even in coredumps
+
+proc gen_core { test } {
+  global gcorefile
+  global gdb_prompt
+  set gcore_works 0
+  set escapedfilename [string_to_regexp $gcorefile]
+
+  gdb_test_multiple "gcore $gcorefile" "$test: gcore" {
+    -re "Saved corefile ${escapedfilename}\[\r\n\]+$gdb_prompt $" {
+      pass "$test: gcore"
+      set gcore_works 1
+    }
+    -re "Can't create a corefile\[\r\n\]+$gdb_prompt $" {
+      fail "$test: gcore"
+    }
+  }
+
+  return $gcore_works
+}
+
+proc test_core_bt { test } {
+  global gcorefile
+
+  gdb_test "core $gcorefile" "Core was generated by.*" \
+    "$test: load core file" "A program is being debugged already.*" "y"
+
+  gdb_test "backtrace" ".*in foo.*in main.*" "$test: backtrace in gcore"
+}
+
+set test "core at last insn in foo"
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+gdb_breakpoint "*$foo_last"
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*" "$test: continue to breakpoint"
+if [gen_core $test] {
+  test_core_bt $test
+}
+
+set test "core at nullified insn"
+if { ! [ runto_main ] } then { gdb_suppress_tests; }
+gdb_breakpoint "*$foo_last"
+gdb_test "continue" "Breakpoint \[0-9\]*,.* in foo.*" "$test: continue to breakpoint"
+gdb_test "stepi" ".*in foo.*" "$test: step to nullified instruction"
+if [gen_core $test] {
+  test_core_bt $test
+}
Index: testsuite/gdb.arch/pa-nullify.s
===================================================================
RCS file: testsuite/gdb.arch/pa-nullify.s
diff -N testsuite/gdb.arch/pa-nullify.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.arch/pa-nullify.s	3 Dec 2004 23:58:01 -0000
@@ -0,0 +1,59 @@
+	.LEVEL 1.1
+	.text
+	.align 4
+.globl foo
+	.type	foo, @function
+foo:
+	.PROC
+	.CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	stw %r26,-36(%r3)
+	ldw -36(%r3),%r19
+	copy %r19,%r28
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND
+
+.globl bar
+	.type	bar, @function
+bar:
+	.PROC
+	.CALLINFO FRAME=64,NO_CALLS,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND
+
+.globl main
+	.type	main, @function
+main:
+	.PROC
+	.CALLINFO FRAME=64,CALLS,SAVE_RP,SAVE_SP,ENTRY_GR=3
+	.ENTRY
+	stw %r2,-20(%r30)
+	copy %r3,%r1
+	copy %r30,%r3
+	stwm %r1,64(%r30)
+	stw %r26,-36(%r3)
+	stw %r25,-40(%r3)
+	ldw -36(%r3),%r26
+	bl foo,%r2
+	nop
+	copy %r28,%r19
+	copy %r19,%r28
+	ldw -20(%r3),%r2
+	ldo 64(%r3),%r30
+	ldwm -64(%r30),%r3
+	bv,n %r0(%r2)
+	.EXIT
+	.PROCEND

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Mark Kettenis]

   Date: Fri, 3 Dec 2004 16:00:18 -0800
   From: Randolph Chung <randolph@tausq.org>

   I've committed the following.

Hmm.  There is a bit of an inconsistency with signed/unsigned in the code:

    static CORE_ADDR
    hppa_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
    {
   -  return frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;
   +  ULONGEST ipsw;
   +  CORE_ADDR pc;
   +
   +  ipsw = frame_unwind_register_signed (next_frame, HPPA_IPSW_REGNUM);
   +  pc = frame_unwind_register_signed (next_frame, HPPA_PCOQ_HEAD_REGNUM) & ~3;

Here you declare `ipsw' as ULONGEST (which is unsigned) but then use
frame_unwind_register_signed to read it.  Also, AFAIK, addresses
aren't signed on hppa (or at least aren't sign-extended) so there one
should also use frame_unwind_register_unsigned (but that problem was
already there).

These kind of things might cause problems in certain 32x64-bit
situations.

Mark

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Randolph Chung]

In-Reply-To: <BDD32F47.81A3%schlie@comcast.net>

> A point that may be worth considering is that on architectures which support
> jump/branch delay slots, the branch and delay slot is actually logically
> executed as single compound instruction, treated by the machine as a unified
> whole.

i don't think this is true for hppa. for instance you can do a hardware
singlestep into the delay slot of a branch insn by setting the recovery
counter appropriately. this will cause an hardware interruption which
the kernel can translate to a SIGTRAP.

nullification is also a more general concept on hppa which doesn't only
apply to branch instructions. A lot of ALU instructions can also
conditioanlly nullify the next insn, which potentially could belong to a
different line of source code. gcc uses this to implement if-conversion,
for example.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch/RFA] multiarch INSTRUCTION_NULLIFIED

[Paul Schlie]

> Randolph wrote:
>> >well, first i want to understand the problem. because i'm still not
>> >yet 100% convinced that step_through_delay will work. simply using the
>> >"instruction_nullified" method in hppa-tdep as the "step_through_delay"
>> >method certainly is not working...
>> 
>> When doing a stepi?  step_through_delay certainly won't help when it
>> comes to doing a backtrace from the nullified instruction.
>
> when doing a step.

A point that may be worth considering is that on architectures which support
jump/branch delay slots, the branch and delay slot is actually logically
executed as single compound instruction, treated by the machine as a unified
whole.

i.e. if an interrupt is received, the machine will break either at the
jump/branch, or at it's target (after logically completing execution of
it's delay slot, conditionally or otherwise, but never in between, as the
control flow through the delay slot is bound to the preceding jump/branch.)

Therefore it's likely ill advised to attempt to logically single step
through a corresponding instruction pair any differently, as although coded
as two separate instructions, execution can't be literally interrupted
between the jump/branch and it's delay slot on most machines, I believe.