diff mbox

Clear *VAL in regcache_raw_read_unsigned

Message ID 56BCBE08.6040306@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Feb. 11, 2016, 4:59 p.m. UTC 
  Hi Pedro,

On 11/02/16 15:51, Pedro Alves wrote:
> I think it's only going to bite us back in the future.
>
>  From the perspective of potentially making it easier to share more
> code between gdb and gdbserver, I'd prefer that.  Would you object it?
>

No, I am not against code sharing between GDB and GDBserver in general,
but sharing extract_unsigned_integer is not necessary for GDBserver.

>> >
>>> >>
>>> >>Or maybe better, just byte the bullet and make gdbserver use
>>> >>extract_unsigned_integer, like gdb.
>>> >>
>>> >>The problem with that is that gdbserver can't currently use 'enum bfd_endian',
>>> >>which IIRC, was already an issue in the get-next-pcs stuff.  I've attached a
>> >
>> >get-next-pcs stuff needs endianness in GDB side.  In GDBserver,
>> >endianness is not needed.
> The get-next-pcs stuff does have endianness bits, but it works
> around the lack of 'enum bfd_endian' by hacking it through an int instead:

IMO, it was a design issue to have endianness in get-next-pcs stuff,
because endianess is only used by GDB, so we shouldn't bring endianess
into get-next-pcs at all.  I pointed it out during the review
https://sourceware.org/ml/gdb-patches/2015-12/msg00040.html but I
didn't insist on this, because I didn't want this block the whole patch
set and I can fix this issue later.

In fact, I've already had a patch to remove byte_order and
byte_oder_for_code from get-next-pcs, but have no change to post it out
due to the recent bug fixing for 7.11 release.

>
> void
> arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
> 		       struct arm_get_next_pcs_ops *ops,
> 		       int byte_order,
> 		       int byte_order_for_code,
> 		       int has_thumb2_breakpoint,
> 		       struct regcache *regcache)
> {
>
> $ grep byte_order *
>
> arm-get-next-pcs.c:                    int byte_order,
> arm-get-next-pcs.c:                    int byte_order_for_code,
> arm-get-next-pcs.c:  self->byte_order = byte_order;
> arm-get-next-pcs.c:  self->byte_order_for_code = byte_order_for_code;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  insn1 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:  insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:      insn1 = self->ops->read_mem_uint (loc, 2,byte_order_for_code);
> arm-get-next-pcs.c:       insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
> arm-get-next-pcs.c:      insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
> arm-get-next-pcs.c:  int byte_order = self->byte_order;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:           inst1 = self->ops->read_mem_uint (pc, 2,byte_order_for_code);
> arm-get-next-pcs.c:               inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:               inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
> arm-get-next-pcs.c:      nextpc = self->ops->read_mem_uint (sp + offset, 4, byte_order);
> arm-get-next-pcs.c:      inst2 = self->ops->read_mem_uint (pc + 2, 2, byte_order_for_code);
> arm-get-next-pcs.c:           nextpc = self->ops->read_mem_uint (addr + offset, 4, byte_order);
> arm-get-next-pcs.c:           = self->ops->read_mem_uint (base, 4, byte_order);
> arm-get-next-pcs.c:       length = 2 * self->ops->read_mem_uint (table + offset, 1, byte_order);
> arm-get-next-pcs.c:       length = 2 * self->ops->read_mem_uint (table + offset, 2, byte_order);
> arm-get-next-pcs.c:  int byte_order = self->byte_order;
> arm-get-next-pcs.c:  int byte_order_for_code = self->byte_order_for_code;
> arm-get-next-pcs.c:  this_instr = self->ops->read_mem_uint (pc, 4, byte_order_for_code);
> arm-get-next-pcs.c:                                                         4, byte_order);
> arm-get-next-pcs.c:                                                              4, byte_order);
> arm-get-next-pcs.h:  ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order);
> arm-get-next-pcs.h:  int byte_order;
> arm-get-next-pcs.h:  int byte_order_for_code;
> arm-get-next-pcs.h:                         int byte_order,
> arm-get-next-pcs.h:                         int byte_order_for_code,
>
>

All of them can be removed by the patch below,

Comments

Pedro Alves Feb. 11, 2016, 5:24 p.m. UTC | #1 
On 02/11/2016 04:59 PM, Yao Qi wrote:
> Hi Pedro,
> 
> On 11/02/16 15:51, Pedro Alves wrote:
>> I think it's only going to bite us back in the future.
>>
>>  From the perspective of potentially making it easier to share more
>> code between gdb and gdbserver, I'd prefer that.  Would you object it?
>>
> 
> No, I am not against code sharing between GDB and GDBserver in general,
> but sharing extract_unsigned_integer is not necessary for GDBserver.

It's not like like we'd be bringing in some big complicated piece of
code.  The routines in question are quite small.  It's about the
same code as the other solution with the switch.  Compare,

gdb's:

ULONGEST
extract_unsigned_integer (const gdb_byte *addr, int len,
                         enum bfd_endian byte_order)
{
  ULONGEST retval;
  const unsigned char *p;
  const unsigned char *startaddr = addr;
  const unsigned char *endaddr = startaddr + len;

  if (len > (int) sizeof (ULONGEST))
    error (_("\
That operation is not available on integers of more than %d bytes."),
          (int) sizeof (ULONGEST));

  /* Start at the most significant end of the integer, and work towards
     the least significant.  */
  retval = 0;
  if (byte_order == BFD_ENDIAN_BIG)
    {
      for (p = startaddr; p < endaddr; ++p)
       retval = (retval << 8) | *p;
    }
  else
    {
      for (p = endaddr - 1; p >= startaddr; --p)
       retval = (retval << 8) | *p;
    }
  return retval;
}

vs

gdbserver's, using a switch:

ULONGEST
extract_unsigned_integer (const gdb_byte *addr, int len)
{
  uint8_t u8;
  uint16_t u16;
  uint32_t u32;
  uint64_t u64;

  if (len > (int) sizeof (ULONGEST))
    error (_("That operation is not available on integers of more than"
            "%d bytes."),
          (int) sizeof (ULONGEST));

  switch (len)
  {
    case 1:
      memcpy (&u8, addr, len);
      return u8;
   case 2:
     memcpy (&u16, addr, len);
     return u16;
   case 4:
     memcpy (&u32, addr, len);
     return u32;
   case 8:
     memcpy (&u64, addr, len);
     return u64;
  }
}

I just don't see what the worry is.  OTOH, sharing the routines
would pave to way to share more, and have one less diverging detail
to worry about, avoiding proliferation of "bridging" interfaces like
regcache_raw_read_unsigned or the need to hook in more entry points
to get_next_pcs and whatever else we put in shared code areas.

Thanks,
Pedro Alves
Yao Qi Feb. 12, 2016, 11:15 a.m. UTC | #2 
Pedro Alves <palves@redhat.com> writes:

Hi Pedro,

> I just don't see what the worry is.  OTOH, sharing the routines
> would pave to way to share more, and have one less diverging detail
> to worry about, avoiding proliferation of "bridging" interfaces like
> regcache_raw_read_unsigned or the need to hook in more entry points
> to get_next_pcs and whatever else we put in shared code areas.

I don't worry about the code sharing.  I am OK with your patches moving
bfd_endian to a separate header and fix the issue in
regcache_raw_read_unsigned with extract_unsigned_integer.
diff mbox

Patch

From e1ea49a05e595a35973ed390d4ae9f6e8f1ed226 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 13 Jan 2016 14:49:56 +0000
Subject: [PATCH] Remove fields byte_order and byte_order_for_code from struct
 arm_get_next_pcs

We have fields byte_order and byte_order_for_code in
"struct arm_get_next_pcs", but we don't use them at all in GDBserver.
This patch adds a sub-class of arm_get_next_pcs for GDB and move fields
byte_order and byte_order_for_code there.  In order to differentiate
reading from data and instruction (due to the different endianess of
data and instruction), field read_mem_uint is split to two,
read_code_uint and read_data_uint.

gdb:

2016-01-13  Yao Qi  <yao.qi@linaro.org>

	* arch/arm-get-next-pcs.c (arm_get_next_pcs_ctor): Remove
	arguments byte_order and byte_order_for_code.  Remove code
	setting self->byte_order and self->byte_order_for_code.
	Callers updated.
	(thumb_deal_with_atomic_sequence_raw): Remove local variable
	'byte_order_for_code'.  Replace function pointer
	read_mem_uint with read_code_uint and read_data_unint.
	(arm_deal_with_atomic_sequence_raw): Likewise.
	(thumb_get_next_pcs_raw): Likewise.
	(arm_get_next_pcs_raw): Likewise.
	* arch/arm-get-next-pcs.h (struct arm_get_next_pcs_ops)
	<read_mem_uint>: Remove.
	<read_code_uint>, <read_data_uint>: New fields.
	(struct arm_get_next_pcs) <byte_order>: New field.
	<byte_order_for_code>: New field.
	(arm_get_next_pcs_ctor): Update declaration.
	* arm-linux-tdep.c (arm_linux_get_next_pcs_ops): Update
	initialization.
	(arm_linux_software_single_step): Use struct arm_gdb_get_next_pcs
	and set byte_order and byte_order_for_code.
	* arm-tdep.c (arm_get_next_pcs_ops): Update
	initialization.
	(arm_get_next_pcs_read_memory_unsigned_integer): Remove.
	(arm_get_next_pcs_read_code_unsigned_integer): New function.
	(arm_get_next_pcs_read_data_unsigned_integer): New function.
	(arm_software_single_step): Use struct arm_gdb_get_next_pcs and
	set byte_order and byte_order_for_code.
	* arm-tdep.h (struct get_next_pcs): Remove declaration.
	(struct arm_get_next_pcs, struct gdb_get_next_pcs): Likewise.
	Include arch/arm-get-next-pcs.h.
	(arm_get_next_pcs_read_memory_unsigned_integer): Remove declaration.
	(struct arm_gdb_get_next_pcs): New.
	(arm_get_next_pcs_read_code_unsigned_integer): Declare.
	(arm_get_next_pcs_read_data_unsigned_integer): Likewise.

gdb/gdbserver:

2016-01-13  Yao Qi  <yao.qi@linaro.org>

	* linux-arm-low.c (get_next_pcs_read_memory_unsigned_integer):
	Update declaration.
	(get_next_pcs_ops): Update initialization.
	(get_next_pcs_read_memory_unsigned_integer): Remove argument byte_order.
	Add argument self.
---
 gdb/arch/arm-get-next-pcs.c   | 57 +++++++++++++++++++------------------------
 gdb/arch/arm-get-next-pcs.h   | 11 +++------
 gdb/arm-linux-tdep.c          | 13 +++++-----
 gdb/arm-tdep.c                | 34 +++++++++++++++++---------
 gdb/arm-tdep.h                | 24 +++++++++++++-----
 gdb/gdbserver/linux-arm-low.c | 21 +++++++---------
 6 files changed, 85 insertions(+), 75 deletions(-)

diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index e840147..be640b6 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -28,14 +28,10 @@ 
 void
 arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 		       struct arm_get_next_pcs_ops *ops,
-		       int byte_order,
-		       int byte_order_for_code,
 		       int has_thumb2_breakpoint,
 		       struct regcache *regcache)
 {
   self->ops = ops;
-  self->byte_order = byte_order;
-  self->byte_order_for_code = byte_order_for_code;
   self->has_thumb2_breakpoint = has_thumb2_breakpoint;
   self->regcache = regcache;
 }
@@ -48,7 +44,6 @@  arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 static VEC (CORE_ADDR) *
 thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
 {
-  int byte_order_for_code = self->byte_order_for_code;
   CORE_ADDR breaks[2] = {-1, -1};
   CORE_ADDR pc = regcache_read_pc (self->regcache);
   CORE_ADDR loc = pc;
@@ -67,13 +62,13 @@  thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
     return NULL;
 
   /* Assume all atomic sequences start with a ldrex{,b,h,d} instruction.  */
-  insn1 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
+  insn1 = self->ops->read_code_uint (self, loc, 2);
 
   loc += 2;
   if (thumb_insn_size (insn1) != 4)
     return NULL;
 
-  insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
+  insn2 = self->ops->read_code_uint (self, loc, 2);
 
   loc += 2;
   if (!((insn1 & 0xfff0) == 0xe850
@@ -84,7 +79,7 @@  thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
      instructions.  */
   for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count)
     {
-      insn1 = self->ops->read_mem_uint (loc, 2,byte_order_for_code);
+      insn1 = self->ops->read_code_uint (self, loc, 2);
       loc += 2;
 
       if (thumb_insn_size (insn1) != 4)
@@ -111,7 +106,7 @@  thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
 	}
       else
 	{
-	  insn2 = self->ops->read_mem_uint (loc, 2, byte_order_for_code);
+	  insn2 = self->ops->read_code_uint (self, loc, 2);
 
 	  loc += 2;
 
@@ -185,7 +180,6 @@  thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
 static VEC (CORE_ADDR) *
 arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
 {
-  int byte_order_for_code = self->byte_order_for_code;
   CORE_ADDR breaks[2] = {-1, -1};
   CORE_ADDR pc = regcache_read_pc (self->regcache);
   CORE_ADDR loc = pc;
@@ -199,7 +193,7 @@  arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
   /* Assume all atomic sequences start with a ldrex{,b,h,d} instruction.
      Note that we do not currently support conditionally executed atomic
      instructions.  */
-  insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
+  insn = self->ops->read_code_uint (self, loc, 4);
 
   loc += 4;
   if ((insn & 0xff9000f0) != 0xe1900090)
@@ -209,7 +203,7 @@  arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
      instructions.  */
   for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count)
     {
-      insn = self->ops->read_mem_uint (loc, 4, byte_order_for_code);
+      insn = self->ops->read_code_uint (self, loc, 4);
 
       loc += 4;
 
@@ -263,8 +257,6 @@  arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
 static VEC (CORE_ADDR) *
 thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 {
-  int byte_order = self->byte_order;
-  int byte_order_for_code = self->byte_order_for_code;
   CORE_ADDR pc = regcache_read_pc (self->regcache);
   unsigned long pc_val = ((unsigned long) pc) + 4;	/* PC after prefetch */
   unsigned short inst1;
@@ -277,7 +269,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
   nextpc = MAKE_THUMB_ADDR (nextpc);
   pc_val = MAKE_THUMB_ADDR (pc_val);
 
-  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
+  inst1 = self->ops->read_code_uint (self, pc, 2);
 
   /* Thumb-2 conditional execution support.  There are eight bits in
      the CPSR which describe conditional execution state.  Once
@@ -310,7 +302,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 
 	  while (itstate != 0 && ! condition_true (itstate >> 4, status))
 	    {
-	      inst1 = self->ops->read_mem_uint (pc, 2,byte_order_for_code);
+	      inst1 = self->ops->read_code_uint (self, pc, 2);
 	      pc += thumb_insn_size (inst1);
 	      itstate = thumb_advance_itstate (itstate);
 	    }
@@ -329,7 +321,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 
 	      while (itstate != 0 && ! condition_true (itstate >> 4, status))
 		{
-		  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
+		  inst1 = self->ops->read_code_uint (self, pc, 2);
 
 		  pc += thumb_insn_size (inst1);
 		  itstate = thumb_advance_itstate (itstate);
@@ -372,7 +364,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 		 the instruction after the IT block.  */
 	      do
 		{
-		  inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
+		  inst1 = self->ops->read_code_uint (self, pc, 2);
 		  pc += thumb_insn_size (inst1);
 		  itstate = thumb_advance_itstate (itstate);
 		}
@@ -410,7 +402,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
          all of the other registers.  */
       offset = bitcount (bits (inst1, 0, 7)) * INT_REGISTER_SIZE;
       sp = regcache_raw_get_unsigned (regcache, ARM_SP_REGNUM);
-      nextpc = self->ops->read_mem_uint (sp + offset, 4, byte_order);
+      nextpc = self->ops->read_data_uint (self, sp + offset, 4);
     }
   else if ((inst1 & 0xf000) == 0xd000)	/* conditional branch */
     {
@@ -429,7 +421,8 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
   else if (thumb_insn_size (inst1) == 4) /* 32-bit instruction */
     {
       unsigned short inst2;
-      inst2 = self->ops->read_mem_uint (pc + 2, 2, byte_order_for_code);
+
+      inst2 = self->ops->read_code_uint (self, pc + 2, 2);
 
       /* Default to the next instruction.  */
       nextpc = pc + 4;
@@ -519,7 +512,8 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 	  if (load_pc)
 	    {
 	      CORE_ADDR addr = regcache_raw_get_unsigned (regcache, rn);
-	      nextpc = self->ops->read_mem_uint	(addr + offset, 4, byte_order);
+
+	      nextpc = self->ops->read_data_uint (self, addr + offset, 4);
 	    }
 	}
       else if ((inst1 & 0xffef) == 0xea4f && (inst2 & 0xfff0) == 0x0f00)
@@ -566,8 +560,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 	    load_pc = 0;
 
 	  if (load_pc)
-	    nextpc
-	      = self->ops->read_mem_uint (base, 4, byte_order);
+	    nextpc = self->ops->read_data_uint (self, base, 4);
 	}
       else if ((inst1 & 0xfff0) == 0xe8d0 && (inst2 & 0xfff0) == 0xf000)
 	{
@@ -581,7 +574,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 	    table = regcache_raw_get_unsigned (regcache, tbl_reg);
 
 	  offset = regcache_raw_get_unsigned (regcache, bits (inst2, 0, 3));
-	  length = 2 * self->ops->read_mem_uint (table + offset, 1, byte_order);
+	  length = 2 * self->ops->read_data_uint (self, table + offset, 1);
 	  nextpc = pc_val + length;
 	}
       else if ((inst1 & 0xfff0) == 0xe8d0 && (inst2 & 0xfff0) == 0xf010)
@@ -596,7 +589,7 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 	    table = regcache_raw_get_unsigned (regcache, tbl_reg);
 
 	  offset = 2 * regcache_raw_get_unsigned (regcache, bits (inst2, 0, 3));
-	  length = 2 * self->ops->read_mem_uint (table + offset, 2, byte_order);
+	  length = 2 * self->ops->read_data_uint (self, table + offset, 2);
 	  nextpc = pc_val + length;
 	}
     }
@@ -644,8 +637,6 @@  thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
 static VEC (CORE_ADDR) *
 arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
 {
-  int byte_order = self->byte_order;
-  int byte_order_for_code = self->byte_order_for_code;
   unsigned long pc_val;
   unsigned long this_instr = 0;
   unsigned long status;
@@ -655,7 +646,7 @@  arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
   VEC (CORE_ADDR) *next_pcs = NULL;
 
   pc_val = (unsigned long) pc;
-  this_instr = self->ops->read_mem_uint (pc, 4, byte_order_for_code);
+  this_instr = self->ops->read_code_uint (self, pc, 4);
 
   status = regcache_raw_get_unsigned (regcache, ARM_PS_REGNUM);
   nextpc = (CORE_ADDR) (pc_val + 4);	/* Default case */
@@ -838,8 +829,9 @@  arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
 			base -= offset;
 		    }
 		  nextpc
-		    = (CORE_ADDR) self->ops->read_mem_uint ((CORE_ADDR) base,
-							    4, byte_order);
+		    = (CORE_ADDR) self->ops->read_data_uint (self,
+							     (CORE_ADDR) base,
+							     4);
 		}
 	    }
 	  break;
@@ -870,8 +862,9 @@  arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
 		    offset = -4;
 
 		  rn_val_offset = rn_val + offset;
-		  nextpc = (CORE_ADDR) self->ops->read_mem_uint (rn_val_offset,
-								 4, byte_order);
+		  nextpc = (CORE_ADDR) self->ops->read_data_uint (self,
+								  rn_val_offset,
+								 4);
 		}
 	    }
 	  break;
diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
index 7cb0858..daa674b 100644
--- a/gdb/arch/arm-get-next-pcs.h
+++ b/gdb/arch/arm-get-next-pcs.h
@@ -26,7 +26,10 @@  struct arm_get_next_pcs;
 /* get_next_pcs operations.  */
 struct arm_get_next_pcs_ops
 {
-  ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order);
+  ULONGEST (*read_code_uint) (struct arm_get_next_pcs *self,
+			      CORE_ADDR memaddr, int len);
+  ULONGEST (*read_data_uint) (struct arm_get_next_pcs *self,
+			      CORE_ADDR memaddr, int len);
   CORE_ADDR (*syscall_next_pc) (struct arm_get_next_pcs *self, CORE_ADDR pc);
   CORE_ADDR (*addr_bits_remove) (struct arm_get_next_pcs *self, CORE_ADDR val);
   int (*is_thumb) (struct arm_get_next_pcs *self);
@@ -37,10 +40,6 @@  struct arm_get_next_pcs
 {
   /* Operations implementations.  */
   struct arm_get_next_pcs_ops *ops;
-  /* Byte order for data.  */
-  int byte_order;
-  /* Byte order for code.  */
-  int byte_order_for_code;
   /* Whether the target has 32-bit thumb-2 breakpoint defined or
      not.  */
   int has_thumb2_breakpoint;
@@ -51,8 +50,6 @@  struct arm_get_next_pcs
 /* Initialize arm_get_next_pcs.  */
 void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
 			    struct arm_get_next_pcs_ops *ops,
-			    int byte_order,
-			    int byte_order_for_code,
 			    int has_thumb2_breakpoint,
 			    struct regcache *regcache);
 
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 3421f3b..f5a9a53 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -271,7 +271,8 @@  static CORE_ADDR
 
 /* Operation function pointers for get_next_pcs.  */
 static struct arm_get_next_pcs_ops arm_linux_get_next_pcs_ops = {
-  arm_get_next_pcs_read_memory_unsigned_integer,
+  arm_get_next_pcs_read_code_unsigned_integer,
+  arm_get_next_pcs_read_data_unsigned_integer,
   arm_linux_get_next_pcs_syscall_next_pc,
   arm_get_next_pcs_addr_bits_remove,
   arm_get_next_pcs_is_thumb
@@ -929,7 +930,7 @@  arm_linux_software_single_step (struct frame_info *frame)
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   struct address_space *aspace = get_regcache_aspace (regcache);
-  struct arm_get_next_pcs next_pcs_ctx;
+  struct arm_gdb_get_next_pcs next_pcs_ctx;
   CORE_ADDR pc;
   int i;
   VEC (CORE_ADDR) *next_pcs = NULL;
@@ -940,14 +941,14 @@  arm_linux_software_single_step (struct frame_info *frame)
   if (target_can_do_single_step () == 1)
     return 0;
 
-  arm_get_next_pcs_ctor (&next_pcs_ctx,
+  arm_get_next_pcs_ctor ((struct arm_get_next_pcs *) &next_pcs_ctx,
 			 &arm_linux_get_next_pcs_ops,
-			 gdbarch_byte_order (gdbarch),
-			 gdbarch_byte_order_for_code (gdbarch),
 			 1,
 			 regcache);
+  next_pcs_ctx.byte_order = gdbarch_byte_order (gdbarch);
+  next_pcs_ctx.byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  next_pcs = arm_get_next_pcs (&next_pcs_ctx);
+  next_pcs = arm_get_next_pcs ((struct arm_get_next_pcs *) &next_pcs_ctx);
 
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
     {
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 6ac05f0..1b4c4c4 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -244,7 +244,8 @@  static CORE_ADDR
 
 /* get_next_pcs operations.  */
 static struct arm_get_next_pcs_ops arm_get_next_pcs_ops = {
-  arm_get_next_pcs_read_memory_unsigned_integer,
+  arm_get_next_pcs_read_code_unsigned_integer,
+  arm_get_next_pcs_read_data_unsigned_integer,
   arm_get_next_pcs_syscall_next_pc,
   arm_get_next_pcs_addr_bits_remove,
   arm_get_next_pcs_is_thumb
@@ -6125,15 +6126,24 @@  thumb2_copy_block_xfer (struct gdbarch *gdbarch, uint16_t insn1, uint16_t insn2,
   return 0;
 }
 
-/* Wrapper over read_memory_unsigned_integer for use in arm_get_next_pcs.
- This is used to avoid a dependency on BFD's bfd_endian enum.  */
+/* Wrapper over read_memory_unsigned_integer for use in arm_get_next_pcs.  */
 
 ULONGEST
-arm_get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr, int len,
-					       int byte_order)
+arm_get_next_pcs_read_code_unsigned_integer (struct arm_get_next_pcs *self,
+					     CORE_ADDR memaddr, int len)
 {
-  return read_memory_unsigned_integer (memaddr, len,
-				       (enum bfd_endian) byte_order);
+  struct arm_gdb_get_next_pcs *gdb = (struct arm_gdb_get_next_pcs *) self;
+
+  return read_memory_unsigned_integer (memaddr, len, gdb->byte_order_for_code);
+}
+
+ULONGEST
+arm_get_next_pcs_read_data_unsigned_integer (struct arm_get_next_pcs *self,
+					     CORE_ADDR memaddr, int len)
+{
+  struct arm_gdb_get_next_pcs *gdb = (struct arm_gdb_get_next_pcs *) self;
+
+  return read_memory_unsigned_integer (memaddr, len, gdb->byte_order);
 }
 
 /* Wrapper over gdbarch_addr_bits_remove for use in arm_get_next_pcs.  */
@@ -6173,20 +6183,20 @@  arm_software_single_step (struct frame_info *frame)
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   struct address_space *aspace = get_regcache_aspace (regcache);
-  struct arm_get_next_pcs next_pcs_ctx;
+  struct arm_gdb_get_next_pcs next_pcs_ctx;
   CORE_ADDR pc;
   int i;
   VEC (CORE_ADDR) *next_pcs = NULL;
   struct cleanup *old_chain = make_cleanup (VEC_cleanup (CORE_ADDR), &next_pcs);
 
-  arm_get_next_pcs_ctor (&next_pcs_ctx,
+  arm_get_next_pcs_ctor ((struct arm_get_next_pcs *) &next_pcs_ctx,
 			 &arm_get_next_pcs_ops,
-			 gdbarch_byte_order (gdbarch),
-			 gdbarch_byte_order_for_code (gdbarch),
 			 0,
 			 regcache);
+  next_pcs_ctx.byte_order = gdbarch_byte_order (gdbarch);
+  next_pcs_ctx.byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  next_pcs = arm_get_next_pcs (&next_pcs_ctx);
+  next_pcs = arm_get_next_pcs ((struct arm_get_next_pcs *) &next_pcs_ctx);
 
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
     arm_insert_single_step_breakpoint (gdbarch, aspace, pc);
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 1306cbb..41adffd 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -23,11 +23,9 @@ 
 struct gdbarch;
 struct regset;
 struct address_space;
-struct get_next_pcs;
-struct arm_get_next_pcs;
-struct gdb_get_next_pcs;
 
 #include "arch/arm.h"
+#include "arch/arm-get-next-pcs.h"
 
 /* Say how long FP registers are.  Used for documentation purposes and
    code readability in this header.  IEEE extended doubles are 80
@@ -250,9 +248,23 @@  extern void
 
 CORE_ADDR arm_skip_stub (struct frame_info *, CORE_ADDR);
 
-ULONGEST arm_get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
-							int len,
-							int byte_order);
+struct arm_gdb_get_next_pcs
+{
+  struct arm_get_next_pcs base;
+
+  enum bfd_endian byte_order;
+  enum bfd_endian byte_order_for_code;
+};
+
+ULONGEST
+  arm_get_next_pcs_read_code_unsigned_integer (struct arm_get_next_pcs *self,
+					       CORE_ADDR memaddr,
+					       int len);
+
+ULONGEST
+  arm_get_next_pcs_read_data_unsigned_integer (struct arm_get_next_pcs *self,
+					       CORE_ADDR memaddr,
+					       int len);
 
 CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 					     CORE_ADDR val);
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 0f62706..8ca7e6c 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -140,9 +140,10 @@  static int arm_regmap[] = {
 };
 
 /* Forward declarations needed for get_next_pcs ops.  */
-static ULONGEST get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
-							   int len,
-							   int byte_order);
+static ULONGEST
+  get_next_pcs_read_memory_unsigned_integer (struct arm_get_next_pcs *self,
+					     CORE_ADDR memaddr,
+					     int len);
 
 static CORE_ADDR get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 						CORE_ADDR val);
@@ -155,6 +156,7 @@  static int get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
 /* get_next_pcs operations.  */
 static struct arm_get_next_pcs_ops get_next_pcs_ops = {
   get_next_pcs_read_memory_unsigned_integer,
+  get_next_pcs_read_memory_unsigned_integer,
   get_next_pcs_syscall_next_pc,
   get_next_pcs_addr_bits_remove,
   get_next_pcs_is_thumb
@@ -252,13 +254,11 @@  get_next_pcs_is_thumb (struct arm_get_next_pcs *self)
   return arm_is_thumb_mode ();
 }
 
-/* Read memory from the inferiror.
-   BYTE_ORDER is ignored and there to keep compatiblity with GDB's
-   read_memory_unsigned_integer. */
+/* Read memory from the inferior.  */
 static ULONGEST
-get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
-					   int len,
-					   int byte_order)
+get_next_pcs_read_memory_unsigned_integer (struct arm_get_next_pcs *self,
+					   CORE_ADDR memaddr,
+					   int len)
 {
   ULONGEST res;
 
@@ -930,9 +930,6 @@  arm_gdbserver_get_next_pcs (struct regcache *regcache)
 
   arm_get_next_pcs_ctor (&next_pcs_ctx,
 			 &get_next_pcs_ops,
-			 /* Byte order is ignored assumed as host.  */
-			 0,
-			 0,
 			 1,
 			 regcache);
 
-- 
1.9.1