From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37022 invoked by alias); 24 Apr 2018 21:10:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 37010 invoked by uid 89); 24 Apr 2018 21:10:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-HELO:sk:mail-qk X-HELO: mail-qk0-f181.google.com Received: from mail-qk0-f181.google.com (HELO mail-qk0-f181.google.com) (209.85.220.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Apr 2018 21:10:23 +0000 Received: by mail-qk0-f181.google.com with SMTP id b39so20861375qkb.5 for ; Tue, 24 Apr 2018 14:10:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=d46OikMX25Wuschume7McFfiHisGKKwPcKgJmXWVfGY=; b=d9nmoKmabHO1gOmjTVBvmqH3/BIkWRa+QgHkP+N0RJWvCCyXegLlF3uQofQVIyQD4C zFVmss4FvGjbbeFvaXfKTXxOBmgqF6YwMVwGGBAJV42LBKXNTWJ87hx816TgU4iYdfin 3v4fHdwcZzyRqj3jrzU82WnJkAVx7+dxL3eQ55jXMxZJ5MQtNxfQN9KvlwXrXgxdR77z UXn9BsjDoT9emeD7G8OLDpyQkPu/ZFU1g5pQQjnqC+czlZCR4PxOXNDY4Ke+HSFKd9Tu 1BH7ocA8mX3G3zZuK6IrG+Cvy7pIzISqpW4Bz9KFHLICC1agpa7AoPRtNWiw4jNs8nhi M4Ig== X-Gm-Message-State: ALQs6tCsx32CA8lIOtf4qYhWjLyEeR0wKnghScmif1FCAfzl/voPlYUk YutNRh2ML19elF032ctlXWID7Rccd/VZWyKW+YwETLm5 X-Google-Smtp-Source: AB8JxZprKOy5txSPF2Fi1j9Ha0XWJ3JBbs3ZWd41lw3C7EsPdCozqoGNh2TaH7L0Mb5tiJJdnpNnUVodlrP3gIlgM7k= X-Received: by 10.55.16.207 with SMTP id 76mr28522021qkq.16.1524604221429; Tue, 24 Apr 2018 14:10:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.39.46 with HTTP; Tue, 24 Apr 2018 14:09:51 -0700 (PDT) In-Reply-To: <20180421214721.7232-2-tom@tromey.com> References: <20180421214721.7232-1-tom@tromey.com> <20180421214721.7232-2-tom@tromey.com> From: Omair Javaid Date: Tue, 24 Apr 2018 21:10:00 -0000 Message-ID: Subject: Re: [RFA 1/2] Fix decoding of ARM VFP instructions To: Tom Tromey Cc: GDB Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-04/txt/msg00491.txt.bz2 On 22 April 2018 at 02:47, Tom Tromey wrote: > -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 > > 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 > Seems LGTM. Let me get back to you after running testsuite for regressions.