Re: [PATCH] Make arm_record_test work on big-endian machines

[Yao Qi <qiyaoltc@gmail.com>]

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.

-- 
Yao (齐尧)
From 4f1aa44d6688ab2bc8885e3c0b02c49c15eb4bb7 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 12 Mar 2018 14:12:58 +0000
Subject: [PATCH] Fix process record on big endian

gdb:

2018-03-12  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (decode_insn): Swap thumb-2 instruction on little
	endian.

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)