Re: buildbot vs --enable-targets=all

[Mark Wielaard <mark@klomp.org>]

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 <mark@klomp.org> 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