From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2f6v-0006qt-1R for qemu-devel@nongnu.org; Fri, 04 Nov 2016 10:04:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2f6p-0008CG-Pr for qemu-devel@nongnu.org; Fri, 04 Nov 2016 10:04:49 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:43919) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2f6p-00089p-L2 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 10:04:43 -0400 Date: Fri, 4 Nov 2016 14:04:24 +0000 From: Julian Brown Message-ID: <20161104140424.51369668@squid.athome> In-Reply-To: References: <1478194258-75276-1-git-send-email-julian@codesourcery.com> <1478194258-75276-3-git-send-email-julian@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit 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: Peter Maydell Cc: QEMU Developers On Fri, 4 Nov 2016 13:30:12 +0000 Peter Maydell wrote: > 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.) Hmm, not sure. I'll try to think about whether that can be done nicely. Julian