From patchwork Wed Oct 22 00:51:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Machado X-Patchwork-Id: 3314 Received: (qmail 1985 invoked by alias); 22 Oct 2014 00:52:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1586 invoked by uid 89); 22 Oct 2014 00:52:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 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; Wed, 22 Oct 2014 00:51:57 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1Xgk9a-0001MT-7D from Luis_Gustavo@mentor.com for gdb-patches@sourceware.org; Tue, 21 Oct 2014 17:51:54 -0700 Received: from [172.30.14.191] (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.3.181.6; Tue, 21 Oct 2014 17:51:53 -0700 Message-ID: <5446FFA5.2040209@codesourcery.com> Date: Tue, 21 Oct 2014 22:51:49 -0200 From: Luis Machado Reply-To: User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Yao Qi CC: "'gdb-patches@sourceware.org'" Subject: Re: [PATCH, v2] Fix ARM machine state testcase failures References: <54464B82.6030703@codesourcery.com> <87k33tingn.fsf@codesourcery.com> In-Reply-To: <87k33tingn.fsf@codesourcery.com> X-IsSubscribed: yes On 10/21/2014 11:03 AM, Yao Qi wrote: > Luis Machado writes: > >> + /* Handle bit P. */ >> + if (bit (arm_insn_r->arm_insn, 24)) >> + { >> + record_buf_mem[1] = tgt_mem_addr; >> + } >> + else >> + { >> + record_buf_mem[1] = (uint32_t) u_regval; >> + } > > These braces are not necessary. > Thanks. Comments addressed in the attached updated patch. I added another check for wback so we only save the base register when needed. I plan to commit this version in a couple days unless there are objections. Luis 2014-10-21 Luis Machado * arm-tdep.c (INSN_S_L_BIT_NUM): Document. (arm_record_ld_st_imm_offset): Reimplement to cover all load/store cases for ARM opcode 010. (arm_record_ld_st_multiple): Reimplement to cover all load/store cases for ARM opcode 100. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index e2559ec..ef75fe2 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -10676,6 +10676,8 @@ vfp - VFP co-processor."), #define THUMB2_INSN_SIZE_BYTES 4 +/* Position of the bit within a 32-bit ARM instruction + that defines whether the instruction is a load or store. */ #define INSN_S_L_BIT_NUM 20 #define REG_ALLOC(REGS, LENGTH, RECORD_BUF) \ @@ -11473,110 +11475,90 @@ arm_record_data_proc_imm (insn_decode_record *arm_insn_r) return 0; } -/* Handling opcode 010 insns. */ +/* Handle ARM mode instructions with opcode 010. */ static int arm_record_ld_st_imm_offset (insn_decode_record *arm_insn_r) { struct regcache *reg_cache = arm_insn_r->regcache; - uint32_t reg_src1 = 0 , reg_dest = 0; - uint32_t offset_12 = 0, tgt_mem_addr = 0; + uint32_t reg_base , reg_dest; + uint32_t offset_12, tgt_mem_addr; uint32_t record_buf[8], record_buf_mem[8]; + unsigned char wback; + ULONGEST u_regval; - ULONGEST u_regval = 0; + /* Calculate wback. */ + wback = (bit (arm_insn_r->arm_insn, 24) == 0) + || (bit (arm_insn_r->arm_insn, 21) == 1); - arm_insn_r->opcode = bits (arm_insn_r->arm_insn, 21, 24); - arm_insn_r->decode = bits (arm_insn_r->arm_insn, 4, 7); + arm_insn_r->reg_rec_count = 0; + reg_base = bits (arm_insn_r->arm_insn, 16, 19); if (bit (arm_insn_r->arm_insn, INSN_S_L_BIT_NUM)) { + /* LDR (immediate), LDR (literal), LDRB (immediate), LDRB (literal), LDRBT + and LDRT. */ + reg_dest = bits (arm_insn_r->arm_insn, 12, 15); - /* LDR insn has a capability to do branching, if - MOV LR, PC is precedded by LDR insn having Rn as R15 - in that case, it emulates branch and link insn, and hence we - need to save CSPR and PC as well. */ - if (ARM_PC_REGNUM != reg_dest) - { - record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15); - arm_insn_r->reg_rec_count = 1; - } - else - { - record_buf[0] = reg_dest; - record_buf[1] = ARM_PS_REGNUM; - arm_insn_r->reg_rec_count = 2; - } + record_buf[arm_insn_r->reg_rec_count++] = reg_dest; + + /* The LDR instruction is capable of doing branching. If MOV LR, PC + preceeds a LDR instruction having R15 as reg_base, it + emulates a branch and link instruction, and hence we need to save + CPSR and PC as well. */ + if (ARM_PC_REGNUM == reg_dest) + record_buf[arm_insn_r->reg_rec_count++] = ARM_PS_REGNUM; + + /* If wback is true, also save the base register, which is going to be + written to. */ + if (wback) + record_buf[arm_insn_r->reg_rec_count++] = reg_base; } else { - /* Store, immediate offset, immediate pre-indexed, - immediate post-indexed. */ - reg_src1 = bits (arm_insn_r->arm_insn, 16, 19); + /* STR (immediate), STRB (immediate), STRBT and STRT. */ + offset_12 = bits (arm_insn_r->arm_insn, 0, 11); - regcache_raw_read_unsigned (reg_cache, reg_src1, &u_regval); - /* U == 1 */ + regcache_raw_read_unsigned (reg_cache, reg_base, &u_regval); + + /* Handle bit U. */ if (bit (arm_insn_r->arm_insn, 23)) - { - tgt_mem_addr = u_regval + offset_12; - } + { + /* U == 1: Add the offset. */ + tgt_mem_addr = (uint32_t) u_regval + offset_12; + } else - { - tgt_mem_addr = u_regval - offset_12; - } + { + /* U == 0: subtract the offset. */ + tgt_mem_addr = (uint32_t) u_regval - offset_12; + } + + /* Bit 22 tells us whether the store instruction writes 1 byte or 4 + bytes. */ + if (bit (arm_insn_r->arm_insn, 22)) + { + /* STRB and STRBT: 1 byte. */ + record_buf_mem[0] = 1; + } + else + { + /* STR and STRT: 4 bytes. */ + record_buf_mem[0] = 4; + } + + /* Handle bit P. */ + if (bit (arm_insn_r->arm_insn, 24)) + record_buf_mem[1] = tgt_mem_addr; + else + record_buf_mem[1] = (uint32_t) u_regval; - switch (arm_insn_r->opcode) - { - /* STR. */ - case 8: - case 12: - /* STR. */ - case 9: - case 13: - /* STRT. */ - case 1: - case 5: - /* STR. */ - case 4: - case 0: - record_buf_mem[0] = 4; - break; - - /* STRB. */ - case 10: - case 14: - /* STRB. */ - case 11: - case 15: - /* STRBT. */ - case 3: - case 7: - /* STRB. */ - case 2: - case 6: - record_buf_mem[0] = 1; - break; - - default: - gdb_assert_not_reached ("no decoding pattern found"); - break; - } - record_buf_mem[1] = tgt_mem_addr; arm_insn_r->mem_rec_count = 1; - if (9 == arm_insn_r->opcode || 11 == arm_insn_r->opcode - || 13 == arm_insn_r->opcode || 15 == arm_insn_r->opcode - || 0 == arm_insn_r->opcode || 2 == arm_insn_r->opcode - || 4 == arm_insn_r->opcode || 6 == arm_insn_r->opcode - || 1 == arm_insn_r->opcode || 3 == arm_insn_r->opcode - || 5 == arm_insn_r->opcode || 7 == arm_insn_r->opcode - ) - { - /* We are handling pre-indexed mode; post-indexed mode; - where Rn is going to be changed. */ - record_buf[0] = reg_src1; - arm_insn_r->reg_rec_count = 1; - } + /* If wback is true, also save the base register, which is going to be + written to. */ + if (wback) + record_buf[arm_insn_r->reg_rec_count++] = reg_base; } REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf); @@ -11847,134 +11829,99 @@ arm_record_ld_st_reg_offset (insn_decode_record *arm_insn_r) return 0; } -/* Handling opcode 100 insns. */ +/* Handle ARM mode instructions with opcode 100. */ static int arm_record_ld_st_multiple (insn_decode_record *arm_insn_r) { struct regcache *reg_cache = arm_insn_r->regcache; - - uint32_t register_list[16] = {0}, register_count = 0, register_bits = 0; - uint32_t reg_src1 = 0, addr_mode = 0, no_of_regs = 0; - uint32_t start_address = 0, index = 0; + uint32_t register_count = 0, register_bits; + uint32_t reg_base, addr_mode; uint32_t record_buf[24], record_buf_mem[48]; + uint32_t wback; + ULONGEST u_regval; - ULONGEST u_regval[2] = {0}; + /* Fetch the list of registers. */ + register_bits = bits (arm_insn_r->arm_insn, 0, 15); + arm_insn_r->reg_rec_count = 0; + + /* Fetch the base register that contains the address we are loading data + to. */ + reg_base = bits (arm_insn_r->arm_insn, 16, 19); - /* This mode is exclusively for load and store multiple. */ - /* Handle incremenrt after/before and decrment after.before mode; - Rn is changing depending on W bit, but as of now we store Rn too - without optimization. */ + /* Calculate wback. */ + wback = (bit (arm_insn_r->arm_insn, 21) == 1); if (bit (arm_insn_r->arm_insn, INSN_S_L_BIT_NUM)) { - /* LDM (1,2,3) where LDM (3) changes CPSR too. */ + /* LDM/LDMIA/LDMFD, LDMDA/LDMFA, LDMDB and LDMIB. */ - if (bit (arm_insn_r->arm_insn, 20) && !bit (arm_insn_r->arm_insn, 22)) - { - register_bits = bits (arm_insn_r->arm_insn, 0, 15); - no_of_regs = 15; - } - else - { - register_bits = bits (arm_insn_r->arm_insn, 0, 14); - no_of_regs = 14; - } - /* Get Rn. */ - reg_src1 = bits (arm_insn_r->arm_insn, 16, 19); + /* Find out which registers are going to be loaded from memory. */ while (register_bits) - { - if (register_bits & 0x00000001) - record_buf[index++] = register_count; - register_bits = register_bits >> 1; - register_count++; - } + { + if (register_bits & 0x00000001) + record_buf[arm_insn_r->reg_rec_count++] = register_count; + register_bits = register_bits >> 1; + register_count++; + } - /* Extra space for Base Register and CPSR; wihtout optimization. */ - record_buf[index++] = reg_src1; - record_buf[index++] = ARM_PS_REGNUM; - arm_insn_r->reg_rec_count = index; + + /* If wback is true, also save the base register, which is going to be + written to. */ + if (wback) + record_buf[arm_insn_r->reg_rec_count++] = reg_base; + + /* Save the CPSR register. */ + record_buf[arm_insn_r->reg_rec_count++] = ARM_PS_REGNUM; } else { - /* It handles both STM(1) and STM(2). */ - addr_mode = bits (arm_insn_r->arm_insn, 23, 24); + /* STM (STMIA, STMEA), STMDA (STMED), STMDB (STMFD) and STMIB (STMFA). */ - register_bits = bits (arm_insn_r->arm_insn, 0, 15); - /* Get Rn. */ - reg_src1 = bits (arm_insn_r->arm_insn, 16, 19); - regcache_raw_read_unsigned (reg_cache, reg_src1, &u_regval[0]); + addr_mode = bits (arm_insn_r->arm_insn, 23, 24); + + regcache_raw_read_unsigned (reg_cache, reg_base, &u_regval); + + /* Find out how many registers are going to be stored to memory. */ while (register_bits) - { - if (register_bits & 0x00000001) - register_count++; - register_bits = register_bits >> 1; - } + { + if (register_bits & 0x00000001) + register_count++; + register_bits = register_bits >> 1; + } switch (addr_mode) - { - /* Decrement after. */ - case 0: - start_address = (u_regval[0]) - (register_count * 4) + 4; - arm_insn_r->mem_rec_count = register_count; - while (register_count) - { - record_buf_mem[(register_count * 2) - 1] = start_address; - record_buf_mem[(register_count * 2) - 2] = 4; - start_address = start_address + 4; - register_count--; - } - break; - - /* Increment after. */ - case 1: - start_address = u_regval[0]; - arm_insn_r->mem_rec_count = register_count; - while (register_count) - { - record_buf_mem[(register_count * 2) - 1] = start_address; - record_buf_mem[(register_count * 2) - 2] = 4; - start_address = start_address + 4; - register_count--; - } - break; - - /* Decrement before. */ - case 2: - - start_address = (u_regval[0]) - (register_count * 4); - arm_insn_r->mem_rec_count = register_count; - while (register_count) - { - record_buf_mem[(register_count * 2) - 1] = start_address; - record_buf_mem[(register_count * 2) - 2] = 4; - start_address = start_address + 4; - register_count--; - } - break; - - /* Increment before. */ - case 3: - start_address = u_regval[0] + 4; - arm_insn_r->mem_rec_count = register_count; - while (register_count) - { - record_buf_mem[(register_count * 2) - 1] = start_address; - record_buf_mem[(register_count * 2) - 2] = 4; - start_address = start_address + 4; - register_count--; - } - break; - - default: - gdb_assert_not_reached ("no decoding pattern found"); - break; - } + { + /* STMDA (STMED): Decrement after. */ + case 0: + record_buf_mem[1] = (uint32_t) u_regval + - register_count * INT_REGISTER_SIZE + 4; + break; + /* STM (STMIA, STMEA): Increment after. */ + case 1: + record_buf_mem[1] = (uint32_t) u_regval; + break; + /* STMDB (STMFD): Decrement before. */ + case 2: + record_buf_mem[1] = (uint32_t) u_regval + - register_count * INT_REGISTER_SIZE; + break; + /* STMIB (STMFA): Increment before. */ + case 3: + record_buf_mem[1] = (uint32_t) u_regval + INT_REGISTER_SIZE; + break; + default: + gdb_assert_not_reached ("no decoding pattern found"); + break; + } - /* Base register also changes; based on condition and W bit. */ - /* We save it anyway without optimization. */ - record_buf[0] = reg_src1; - arm_insn_r->reg_rec_count = 1; + record_buf_mem[0] = register_count * INT_REGISTER_SIZE; + arm_insn_r->mem_rec_count = 1; + + /* If wback is true, also save the base register, which is going to be + written to. */ + if (wback) + record_buf[arm_insn_r->reg_rec_count++] = reg_base; } REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);