From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9038 invoked by alias); 23 Feb 2016 19:59:20 -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 8360 invoked by uid 89); 23 Feb 2016 19:59:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:2561 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Feb 2016 19:59:18 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1aYJ75-0001Ec-4e from Luis_Gustavo@mentor.com ; Tue, 23 Feb 2016 11:59:15 -0800 Received: from [172.30.1.130] (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.3.224.2; Tue, 23 Feb 2016 11:59:14 -0800 Subject: Re: [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn References: <1456159999-5644-1-git-send-email-yao.qi@linaro.org> <1456159999-5644-4-git-send-email-yao.qi@linaro.org> To: Yao Qi , Reply-To: Luis Machado From: Luis Machado Message-ID: <56CCBA0F.3000205@codesourcery.com> Date: Tue, 23 Feb 2016 19:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1456159999-5644-4-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00697.txt.bz2 Just internal comment nits. On 02/22/2016 01:53 PM, Yao Qi wrote: > This patch fixes various bugs in arm_record_exreg_ld_st_insn, and use > gdb.reverse/insn-reverse.c to test more arm instructions. > > - Set flag SINGLE_REG correctly. In the arch reference manual, > SING_REG is true when the bit 8 of instruction is zero. > - Record the right D registers for instructions changing S registers. > - Fix the order of length and address in record_buf_mem array. > - Shift the offset by 2 instead of by 24. > > This patch also fixes one internal error, > > (gdb) PASS: gdb.reverse/finish-precsave.exp: BP at end of main > continue^M > Continuing.^M > ../../binutils-gdb/gdb/utils.c:1072: internal-error: virtual memory exhausted.^M > A problem internal to GDB has been detected,FAIL: gdb.reverse/finish-precsave.exp: run to end of main (GDB internal error) > ... > @@ -10991,25 +11002,36 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) > { > uint32_t reg_count, reg_vd; > uint32_t reg_index = 0; > + uint32_t bit_d = bit (arm_insn_r->arm_insn, 22); > > reg_vd = bits (arm_insn_r->arm_insn, 12, 15); > reg_count = bits (arm_insn_r->arm_insn, 0, 7); > > - if (single_reg) > - reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 22) << 4); > - else > - reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22); > + /* REG_VD is the first D register number. If the instruction > + loads memory to S registers (SINGLE_REG is TRUE), the register > + number is (REG_VD << 1 | bit D), so the corresponding D > + register number is (REG_VD << 1 | bit D) / 2 = REG_VD. */ > + if (!single_reg) > + reg_vd = reg_vd | (bit_d << 4); > > - if (bit (arm_insn_r->arm_insn, 21)) > + if (bit (arm_insn_r->arm_insn, 21) /* write back */) > record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19); > > - while (reg_count > 0) > + /* If the instruction loads memory to D register, REG_COUNT should > + divide 2, according to the ARM Architecture Reference Manual. "...should be divided by 2..."? > + If the instruction loads memory to S register, divide 2 as well "... divide by 2..." > + because two S registers are mapped to D register. */ > + reg_count = reg_count / 2; > + if (single_reg && bit_d) > { > - if (single_reg) > - record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1; > - else > - record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1; > + /* Increase the register count if S register list starts from > + odd number (bit d is one). */ "...from an odd number..."?