Patchwork Make arm_record_test work on big-endian machines

login
register
mail settings
Submitter Yao Qi
Date March 12, 2018, 2:15 p.m.
Message ID <86tvtle5wo.fsf@gmail.com>
Download mbox | patch
Permalink /patch/26283/
State New
Headers show

Comments

Yao Qi - March 12, 2018, 2:15 p.m.
Simon Marchi <simon.marchi@polymtl.ca> writes:

> While running selftests on a big-endian PPC, I noticed arm_record_test
> failed a test.  The reason is that the gdbarch_find_by_info call is done
> with BFD_ENDIAN_UNKNOWN byte order in the info structure.  In that case,
> it uses the byte order of the current default BFD, which happened to be
> big-endian on that GDB build, and the gdbarch returned is a big-endian
> one.  The instruction used for the 32-bits part of the test is written
> in little-endian form, so GDB fails to decode the instruction
> properly.

The test instructions are endianess-neutral, because they are written as
uint16_t, and that is why you didn't have to adjust 16-bit thumb
instructions in the tests.

>
> Since ARM supports both endiannesses, and it should be possible to debug
> using an host of both endiannesses, I changed the test to check with
> gdbarches of both endiannesses.

Although ARM supports both endianesses, the big endian has two variants,
BE-32 and BE-8.  Before ARMv6, it was BE-32.  In ARMv6, BE-8 must be
supported, but support of BE-32 is implementation defined.  ARMv7 drops
the BE-32 support.  With BE-8, data is big endian and code is little
endian.  Thumb-2 instruction was introduced in armv6t2, so it is still
likely to have both big endian and little endian thumb-2 instructions
code.

As I said, the test case is written in an endianess-neutral way,

    static const uint16_t insns[] = {
      /* 1d ee 70 7f	 mrc	15, 0, r7, cr13, cr0, {3} */
      0xee1d, 0x7f70,
    };

but the arm process recording processes thumb-2 instruction as one
32-bit instruction, so that it has this,

	  /* Swap first half of 32bit thumb instruction with second half.  */
	  arm_record->arm_insn
	    = (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16);

I wonder this is the cause of the test fail, that is, we don't need to
swap them in big endian.  Since I've never run GDB tests on arm big
endian target, I am not very confident on my fix below.  It fixes the
test fail on ppc64, and also the fail with your patch to change the test
with two different endianess.

Patch

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index ef7e66b..153f568 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -13188,10 +13188,13 @@  decode_insn (abstract_memory_reader &reader, insn_decode_record *arm_record,
       /* As thumb does not have condition codes, we set negative.  */
       arm_record->cond = -1;
 
-      /* Swap first half of 32bit thumb instruction with second half.  */
-      arm_record->arm_insn
-	= (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16);
-
+      if (gdbarch_byte_order_for_code (arm_record->gdbarch)
+	  == BFD_ENDIAN_LITTLE)
+	{
+	  /* Swap first half of 32bit thumb instruction with second half.  */
+	  arm_record->arm_insn
+	    = (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16);
+	}
       ret = thumb2_record_decode_insn_handler (arm_record);
 
       if (ret != ARM_RECORD_SUCCESS)