Re: buildbot vs --enable-targets=all

On 8/3/22 11:38, Mark Wielaard wrote:
> Hi Jeffrey,
> 
> On Mon, 2022-07-25 at 21:59 -0400, Jeffrey Walton wrote:
>> On Mon, Jul 25, 2022 at 8:26 AM Mark Wielaard <[email protected]> wrote:
>>> On fedora-s390x and debian-ppc64 one of the gdb selftests fails
>>> https://builder.sourceware.org/buildbot/#builders/75/builds/783
>>> https://builder.sourceware.org/buildbot/#builders/76/builds/772
>>>
>>> Running selftest arm-record.
>>> Process record and replay target doesn't support syscall number
>>> -2036195
>>> Process record does not support instruction 0x7f70ee1d at address 0x0.
>>> Self test failed: self-test failed at ../../binutils-gdb/gdb/arm-
>>> tdep.c:14407
>>>
>>> Which is:
>>>
>>>    /* 32-bit Thumb-2 instructions.  */
>>>    {
>>>      arm_insn_decode_record arm_record;
>>>
>>>      memset (&arm_record, 0, sizeof (arm_insn_decode_record));
>>>      arm_record.gdbarch = gdbarch;
>>>
>>>      static const uint16_t insns[] = {
>>>        /* 1d ee 70 7f     mrc    15, 0, r7, cr13, cr0, {3} */
>>>        0xee1d, 0x7f70,
>>>      };
>>>
>>>      enum bfd_endian endian = gdbarch_byte_order_for_code (arm_record.gdbarch);
>>>      instruction_reader_thumb reader (endian, insns);
>>>      int ret = decode_insn (reader, &arm_record, THUMB2_RECORD,
>>>                             THUMB2_INSN_SIZE_BYTES);
>>>
>>>      SELF_CHECK (ret == 0);
>>>      SELF_CHECK (arm_record.mem_rec_count == 0);
>>>      SELF_CHECK (arm_record.reg_rec_count == 1);
>>>      SELF_CHECK (arm_record.arm_regs[0] == 7);
>>>    }
>>>
>>> This seems a big endian issue given the instructions are given as two
>>> 16bit numbers.
>> [...]
> 
>>> For ARM, this does not look right (to me):
>>
>>>      static const uint16_t insns[] = {
>>>        /* 1d ee 70 7f     mrc    15, 0, r7, cr13, cr0, {3} */
>>>        0xee1d, 0x7f70,
>>>      };
>>
>> I think you are supposed to use .inst.n and .inst.w because they
>> handle endianness properly.
> 
> I couldn't figure out how to do that. Could you give an example?
> It looks like the instruction_reader_thumb only takes an array of
> uint16_t (even though the instructions are 32bit long). But I might be
> looking at the wrong code.
> 
> So the following fixes it for me on s390x and ppc64
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index d4c5beb5e06..ef0da73398d 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -14471,7 +14471,7 @@ arm_record_test (void)
>   
>       static const uint16_t insns[] = {
>         /* 1d ee 70 7f    mrc    15, 0, r7, cr13, cr0, {3} */
> -      0xee1d, 0x7f70,
> +      0x7f70, 0xee1d,
>       };
>   
>       enum bfd_endian endian = gdbarch_byte_order_for_code (arm_record.gdbarch);
> 
> But obviously that breaks things on little-endian architectures.
> We could define the insns[] differently using
> 
>   #if _BYTE_ORDER == _LITTLE_ENDIAN
>        0xee1d, 0x7f70,
>   #else
>        0x7f70, 0xee1d,
>   #endif
> 
> But that might not be what you meant.
> 
> Thanks,
> 
> Mark

I wonder what's going wrong here given we're using gdbarch_byte_order_for_code to read things.

Technically it should read in the right order.