Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [RFA 1/2] Fix decoding of ARM VFP instructions
Date: Sat, 21 Apr 2018 21:47:00 -0000	[thread overview]
Message-ID: <20180421214721.7232-2-tom@tromey.com> (raw)
In-Reply-To: <20180421214721.7232-1-tom@tromey.com>

-Wduplicated-cond pointed out that arm_record_vfp_data_proc_insn
checks "opc1 == 0x0b" twice.  I filed this a while ago as
PR tdep/20362.

Based on the ARM instruction manual at
https://www.scss.tcd.ie/~waldroj/3d1/arm_arm.pdf, I think the
instruction decoding in this function has two bugs.

First, opc1 is computed as:

  opc1 = bits (arm_insn_r->arm_insn, 20, 23);
[...]
  opc1 = opc1 & 0x04;

This means that tests like:

  else if (opc1 == 0x01)

can never be true.

In the ARM manual, "opc1" corresponds to these bits:

    name   bit
    r      20
    q      21
    D      22
    p      23

... where the D bit is not used for VFP instruction decoding.

So, I believe this code should use ~0x04 instead.

Second, VDIV is recognized by the bits "pqrs" being equal to "1000".
This tranlates to opc1 == 0x08 -- not 0x0b.  Note that pqrs==1001 is
an undefined encoding, which is probably why opc2 is not checked here;
this code doesn't seem to really deal with undefined encodings in
general, so I've left that as is.

I don't have an ARM machine or any reasonable way to test this.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	PR tdep/20362:
	* arm-tdep.c (arm_record_vfp_data_proc_insn): Properly mask off D
	bit.  Use correct value for VDIV.
---
 gdb/ChangeLog  | 6 ++++++
 gdb/arm-tdep.c | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index f64df4c574..98bbb0244c 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -11420,7 +11420,8 @@ arm_record_vfp_data_proc_insn (insn_decode_record *arm_insn_r)
   opc3 = bits (arm_insn_r->arm_insn, 6, 7);
   dp_op_sz = bit (arm_insn_r->arm_insn, 8);
   bit_d = bit (arm_insn_r->arm_insn, 22);
-  opc1 = opc1 & 0x04;
+  /* Mask off the "D" bit.  */
+  opc1 = opc1 & ~0x04;
 
   /* Handle VMLA, VMLS.  */
   if (opc1 == 0x00)
@@ -11485,7 +11486,7 @@ arm_record_vfp_data_proc_insn (insn_decode_record *arm_insn_r)
         }
     }
   /* Handle VDIV.  */
-  else if (opc1 == 0x0b)
+  else if (opc1 == 0x08)
     {
       if (dp_op_sz)
         curr_insn_type = INSN_T1;
-- 
2.13.6


  reply	other threads:[~2018-04-21 21:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-21 21:47 [RFA 0/2] Add -Wduplicated-cond Tom Tromey
2018-04-21 21:47 ` Tom Tromey [this message]
2018-04-24 21:10   ` [RFA 1/2] Fix decoding of ARM VFP instructions Omair Javaid
2018-05-04 17:07     ` Tom Tromey
2018-05-07 14:35       ` Pedro Alves
2018-04-21 21:47 ` [RFA 2/2] Add -Wduplicated-cond Tom Tromey
2018-04-22 15:47   ` Pedro Alves
2018-04-24 23:07     ` Tom Tromey
2018-04-23 15:36 ` [RFA 0/2] " Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180421214721.7232-2-tom@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox