From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2eZq-00088a-4d for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:30:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2eZp-0005Cb-1T for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:30:38 -0400 Received: from mail-vk0-x229.google.com ([2607:f8b0:400c:c05::229]:35232) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c2eZo-0005BM-JY for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:30:36 -0400 Received: by mail-vk0-x229.google.com with SMTP id w194so66739632vkw.2 for ; Fri, 04 Nov 2016 06:30:36 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1478194258-75276-3-git-send-email-julian@codesourcery.com> References: <1478194258-75276-1-git-send-email-julian@codesourcery.com> <1478194258-75276-3-git-send-email-julian@codesourcery.com> From: Peter Maydell Date: Fri, 4 Nov 2016 13:30:12 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 2/5] Fix Thumb-1 BE32 execution and disassembly. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Julian Brown Cc: QEMU Developers On 3 November 2016 at 17:30, Julian Brown wrote: > Thumb-1 code has some issues in BE32 mode (as currently implemented). In > short, since bytes are swapped within words at load time for BE32 > executables, this also swaps pairs of adjacent Thumb-1 instructions. > > This patch un-swaps those pairs of instructions again, both for execution, > and for disassembly. > > Signed-off-by: Julian Brown > --- > disas/arm.c | 46 +++++++++++++++++++++++++++++++++++----------- > include/disas/bfd.h | 1 + > target-arm/arm_ldst.h | 10 +++++++++- > target-arm/cpu.c | 4 ++++ > 4 files changed, 49 insertions(+), 12 deletions(-) > > diff --git a/disas/arm.c b/disas/arm.c > index 93c6503..4807ba3 100644 > --- a/disas/arm.c > +++ b/disas/arm.c > @@ -3863,10 +3863,11 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info) > int is_data = false; > unsigned int size = 4; > void (*printer) (bfd_vma, struct disassemble_info *, long); > - int little; > + int little, is_thumb1_be32 = false; > > little = (info->endian == BFD_ENDIAN_LITTLE); > is_thumb |= (pc & 1); > + is_thumb1_be32 = (info->flags & INSN_ARM_THUMB1_BE32) != 0; > pc &= ~(bfd_vma)1; > > if (force_thumb) > @@ -3915,11 +3916,22 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info) > info->bytes_per_chunk = 2; > size = 2; > > - status = info->read_memory_func (pc, (bfd_byte *)b, 2, info); > - if (little) > - given = (b[0]) | (b[1] << 8); > - else > - given = (b[1]) | (b[0] << 8); > + if (is_thumb1_be32) { > + status = info->read_memory_func(pc & ~3, (bfd_byte *)b, 4, info); > + assert(little); > + if ((pc & 2) == 0) { > + given = b[2] | (b[3] << 8); > + } else { > + given = b[0] | (b[1] << 8); > + } > + } else { > + status = info->read_memory_func(pc, (bfd_byte *)b, 2, info); > + if (little) { > + given = (b[0]) | (b[1] << 8); > + } else { > + given = (b[1]) | (b[0] << 8); > + } > + } Could we do this instead by changing the read_memory_func() so that it did the appropriate XORing of addresses ? (Chaining through to the original read_memory_func would be a bit irritating as you'd need to find a place to stash that function pointer where you could get at it again from the new read_memory_func.) thanks -- PMM