Fix endianness handling for arm record self tests
authorLuis Machado <luis.machado@arm.com>
Wed, 3 Aug 2022 23:00:26 +0000 (00:00 +0100)
committerLuis Machado <luis.machado@arm.com>
Wed, 7 Sep 2022 08:17:32 +0000 (09:17 +0100)
commit0833fb8f4bc8d8f369d0d76f604c248b4ad1de1d
tree72edcfce64b8951b958fbef81cf678191c5ceb7c
parent6d0aebbcff0636fb11fb26116ef7ae53ecca314f
Fix endianness handling for arm record self tests

v2:

- Add 32-bit Arm instruction selftest
- Refactored abstract memory reader into abstract instruction reader
- Adjusted code to use templated type and to use host endianness as
  opposed to target endianness.

The arm record tests handle 16-bit and 32-bit thumb instructions, but the
code is laid out in a way that handles the 32-bit thumb instructions as
two 16-bit parts.

This is fine, but it is prone to host-endianness issues given how the two
16-bit parts are stored and how they are accessed later on. Arm is
little-endian by default, so running this test with a GDB built with
--enable-targets=all and on a big endian host will run into the following:

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:14482

It turns out the abstract memory reader class is more generic than it needs to
be, and we can simplify the code a bit by assuming we have a simple instruction
reader that only reads up to 4 bytes, which is the length of a 32-bit
instruction.

Instead of returning a bool, we return instead the instruction that has been
read. This way we avoid having to deal with the endianness conversion, and use
the host endianness instead. The Arm selftests can be executed on non-Arm
hosts.

While at it, Tom suggested adding a 32-bit Arm instruction selftest to increase
the coverage of the selftests.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29432

Co-authored-by: Tom de Vries <tdevries@suse.de>
gdb/arm-tdep.c