Re: buildbot vs --enable-targets=all

Hi Luis,

On Wed, 2022-08-03 at 12:35 +0100, Luis Machado wrote:
> On 8/3/22 11:38, Mark Wielaard wrote:
> > 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.
> > 
> 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.

Yes, if the code bytes were in the right order. Like when you read them
from disk or from a process running in that byte order. But here they
are constructed in memory in the byte order of the host architecture as
an uint16_t array. So when using the gdbarch_byte_order_for_code order
to reconstruct those instructions it is not using the host byte order
(if they don't match).

I think what Jeffrey means is that there is a different way to
construct the bytes in memory using .inst.n and .inst.w, but I don't
fully understand how that works.

Cheers,

Mark