From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1585) id 5EF1E3850237; Wed, 7 Sep 2022 08:18:00 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5EF1E3850237 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1662538680; bh=cVsG7hK0BZ69tnBztaMEA0QZhJv9bFvvY1us/Tw5KVU=; h=From:To:Subject:Date:From; b=LWPcVexGqp3chBZuPArprAbGNWaOAK4gmdkZ1ZH9sYhQ72etpSqiSIDK8s+aVJgsT J2Ca2YsVngpfUOB5qHPMPt4PqZ7e18/u9ah5Fsd2QoC/e7pChYeP4FksoHuZqd4DNT tcYRq76kjTy9LyOv23J1l+ZOU1A38/8U1p8HOKB4= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Luis Machado To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Fix endianness handling for arm record self tests X-Act-Checkin: binutils-gdb X-Git-Author: Luis Machado X-Git-Refname: refs/heads/master X-Git-Oldrev: 6d0aebbcff0636fb11fb26116ef7ae53ecca314f X-Git-Newrev: 0833fb8f4bc8d8f369d0d76f604c248b4ad1de1d Message-Id: <20220907081800.5EF1E3850237@sourceware.org> Date: Wed, 7 Sep 2022 08:18:00 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D0833fb8f4bc8= d8f369d0d76f604c248b4ad1de1d commit 0833fb8f4bc8d8f369d0d76f604c248b4ad1de1d Author: Luis Machado Date: Thu Aug 4 00:00:26 2022 +0100 Fix endianness handling for arm record self tests =20 v2: =20 - 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. =20 The arm record tests handle 16-bit and 32-bit thumb instructions, but t= he code is laid out in a way that handles the 32-bit thumb instructions as two 16-bit parts. =20 This is fine, but it is prone to host-endianness issues given how the t= wo 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=3Dall and on a big endian host will run into the follo= wing: =20 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 =20 It turns out the abstract memory reader class is more generic than it n= eeds to be, and we can simplify the code a bit by assuming we have a simple ins= truction reader that only reads up to 4 bytes, which is the length of a 32-bit instruction. =20 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-A= rm hosts. =20 While at it, Tom suggested adding a 32-bit Arm instruction selftest to = increase the coverage of the selftests. =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D29432 =20 Co-authored-by: Tom de Vries Diff: --- gdb/arm-tdep.c | 150 ++++++++++++++++++++++++++---------------------------= ---- 1 file changed, 68 insertions(+), 82 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 0091d14708f..ead9bbf46c5 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -14252,59 +14252,39 @@ thumb2_record_decode_insn_handler (arm_insn_decod= e_record *thumb2_insn_r) } =20 namespace { -/* Abstract memory reader. */ +/* Abstract instruction reader. */ =20 -class abstract_memory_reader +class abstract_instruction_reader { public: - /* Read LEN bytes of target memory at address MEMADDR, placing the - results in GDB's memory at BUF. Return true on success. */ + /* Read one instruction of size LEN from address MEMADDR and using + BYTE_ORDER endianness. */ =20 - virtual bool read (CORE_ADDR memaddr, gdb_byte *buf, const size_t len) = =3D 0; + virtual ULONGEST read (CORE_ADDR memaddr, const size_t len, + enum bfd_endian byte_order) =3D 0; }; =20 /* Instruction reader from real target. */ =20 -class instruction_reader : public abstract_memory_reader +class instruction_reader : public abstract_instruction_reader { public: - bool read (CORE_ADDR memaddr, gdb_byte *buf, const size_t len) override + ULONGEST read (CORE_ADDR memaddr, const size_t len, + enum bfd_endian byte_order) override { - if (target_read_memory (memaddr, buf, len)) - return false; - else - return true; + return read_code_unsigned_integer (memaddr, len, byte_order); } }; =20 } // namespace =20 -/* Extracts arm/thumb/thumb2 insn depending on the size, and returns 0 on = success=20 -and positive val on failure. */ - -static int -extract_arm_insn (abstract_memory_reader& reader, - arm_insn_decode_record *insn_record, uint32_t insn_size) -{ - gdb_byte buf[insn_size]; - - memset (&buf[0], 0, insn_size); - =20 - if (!reader.read (insn_record->this_addr, buf, insn_size)) - return 1; - insn_record->arm_insn =3D (uint32_t) extract_unsigned_integer (&buf[0], - insn_size,=20 - gdbarch_byte_order_for_code (insn_record->gdbarch)); - return 0; -} - typedef int (*sti_arm_hdl_fp_t) (arm_insn_decode_record*); =20 /* Decode arm/thumb insn depending on condition cods and opcodes; and dispatch it. */ =20 static int -decode_insn (abstract_memory_reader &reader, +decode_insn (abstract_instruction_reader &reader, arm_insn_decode_record *arm_record, record_type_t record_type, uint32_t insn_size) { @@ -14339,20 +14319,12 @@ decode_insn (abstract_memory_reader &reader, =20 uint32_t ret =3D 0; /* return value: negative:failure 0:success. */ uint32_t insn_id =3D 0; + enum bfd_endian code_endian + =3D gdbarch_byte_order_for_code (arm_record->gdbarch); + arm_record->arm_insn + =3D reader.read (arm_record->this_addr, insn_size, code_endian); =20 - if (extract_arm_insn (reader, arm_record, insn_size)) - { - if (record_debug) - { - gdb_printf (gdb_stdlog, - _("Process record: error reading memory at " - "addr %s len =3D %d.\n"), - paddress (arm_record->gdbarch, - arm_record->this_addr), insn_size); - } - return -1; - } - else if (ARM_RECORD =3D=3D record_type) + if (ARM_RECORD =3D=3D record_type) { arm_record->cond =3D bits (arm_record->arm_insn, 28, 31); insn_id =3D bits (arm_record->arm_insn, 25, 27); @@ -14412,36 +14384,35 @@ decode_insn (abstract_memory_reader &reader, #if GDB_SELF_TEST namespace selftests { =20 -/* Provide both 16-bit and 32-bit thumb instructions. */ +/* Instruction reader class for selftests. + + For 16-bit Thumb instructions, an array of uint16_t should be used. =20 -class instruction_reader_thumb : public abstract_memory_reader + For 32-bit Thumb instructions and regular 32-bit Arm instructions, an a= rray + of uint32_t should be used. */ + +template +class instruction_reader_selftest : public abstract_instruction_reader { public: template - instruction_reader_thumb (enum bfd_endian endian, - const uint16_t (&insns)[SIZE]) - : m_endian (endian), m_insns (insns), m_insns_size (SIZE) + instruction_reader_selftest (const T (&insns)[SIZE]) + : m_insns (insns), m_insns_size (SIZE) {} =20 - bool read (CORE_ADDR memaddr, gdb_byte *buf, const size_t len) override + ULONGEST read (CORE_ADDR memaddr, const size_t length, + enum bfd_endian byte_order) override { - SELF_CHECK (len =3D=3D 4 || len =3D=3D 2); - SELF_CHECK (memaddr % 2 =3D=3D 0); - SELF_CHECK ((memaddr / 2) < m_insns_size); + SELF_CHECK (length =3D=3D sizeof (T)); + SELF_CHECK (memaddr % sizeof (T) =3D=3D 0); + SELF_CHECK ((memaddr / sizeof (T)) < m_insns_size); =20 - store_unsigned_integer (buf, 2, m_endian, m_insns[memaddr / 2]); - if (len =3D=3D 4) - { - store_unsigned_integer (&buf[2], 2, m_endian, - m_insns[memaddr / 2 + 1]); - } - return true; + return m_insns[memaddr / sizeof (T)]; } =20 private: - enum bfd_endian m_endian; - const uint16_t *m_insns; - size_t m_insns_size; + const T *m_insns; + const size_t m_insns_size; }; =20 static void @@ -14461,6 +14432,8 @@ arm_record_test (void) memset (&arm_record, 0, sizeof (arm_insn_decode_record)); arm_record.gdbarch =3D gdbarch; =20 + /* Use the endian-free representation of the instructions here. The t= est + will handle endianness conversions. */ static const uint16_t insns[] =3D { /* db b2 uxtb r3, r3 */ 0xb2db, @@ -14468,8 +14441,7 @@ arm_record_test (void) 0x58cd, }; =20 - enum bfd_endian endian =3D gdbarch_byte_order_for_code (arm_record.gdb= arch); - instruction_reader_thumb reader (endian, insns); + instruction_reader_selftest reader (insns); int ret =3D decode_insn (reader, &arm_record, THUMB_RECORD, THUMB_INSN_SIZE_BYTES); =20 @@ -14495,13 +14467,14 @@ arm_record_test (void) memset (&arm_record, 0, sizeof (arm_insn_decode_record)); arm_record.gdbarch =3D gdbarch; =20 - static const uint16_t insns[] =3D { - /* 1d ee 70 7f mrc 15, 0, r7, cr13, cr0, {3} */ - 0xee1d, 0x7f70, + /* Use the endian-free representation of the instruction here. The te= st + will handle endianness conversions. */ + static const uint32_t insns[] =3D { + /* mrc 15, 0, r7, cr13, cr0, {3} */ + 0x7f70ee1d, }; =20 - enum bfd_endian endian =3D gdbarch_byte_order_for_code (arm_record.gdb= arch); - instruction_reader_thumb reader (endian, insns); + instruction_reader_selftest reader (insns); int ret =3D decode_insn (reader, &arm_record, THUMB2_RECORD, THUMB2_INSN_SIZE_BYTES); =20 @@ -14510,6 +14483,27 @@ arm_record_test (void) SELF_CHECK (arm_record.reg_rec_count =3D=3D 1); SELF_CHECK (arm_record.arm_regs[0] =3D=3D 7); } + + /* 32-bit instructions. */ + { + arm_insn_decode_record arm_record; + + memset (&arm_record, 0, sizeof (arm_insn_decode_record)); + arm_record.gdbarch =3D gdbarch; + + /* Use the endian-free representation of the instruction here. The te= st + will handle endianness conversions. */ + static const uint32_t insns[] =3D { + /* mov r5, r0 */ + 0xe1a05000, + }; + + instruction_reader_selftest reader (insns); + int ret =3D decode_insn (reader, &arm_record, ARM_RECORD, + ARM_INSN_SIZE_BYTES); + + SELF_CHECK (ret =3D=3D 0); + } } =20 /* Instruction reader from manually cooked instruction sequences. */ @@ -14609,18 +14603,10 @@ arm_process_record (struct gdbarch *gdbarch, stru= ct regcache *regcache, } =20 instruction_reader reader; - if (extract_arm_insn (reader, &arm_record, 2)) - { - if (record_debug) - { - gdb_printf (gdb_stdlog, - _("Process record: error reading memory at " - "addr %s len =3D %d.\n"), - paddress (arm_record.gdbarch, - arm_record.this_addr), 2); - } - return -1; - } + enum bfd_endian code_endian + =3D gdbarch_byte_order_for_code (arm_record.gdbarch); + arm_record.arm_insn + =3D reader.read (arm_record.this_addr, 2, code_endian); =20 /* Check the insn, whether it is thumb or arm one. */