* [PATCH 0/3] Bug fixes in arm reverse debugging
@ 2016-02-22 16:53 Yao Qi
2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw)
To: gdb-patches
This patch series extends test case gdb.reverse/aarch64.exp for general
test (it covers aarch64 and arm, and I plan to merge i386 tests to it
too) and fix some bugs on instruction decoding exposed the new tests.
*** BLURB HERE ***
Yao Qi (3):
Generalize gdb.reverse/aarch64.exp
Record right reg num of thumb special data instructions
Fix various bugs in arm_record_exreg_ld_st_insn
gdb/arm-tdep.c | 101 +++++++++++-------
gdb/testsuite/gdb.reverse/aarch64.c | 99 -----------------
gdb/testsuite/gdb.reverse/aarch64.exp | 115 --------------------
gdb/testsuite/gdb.reverse/insn-reverse.c | 164 +++++++++++++++++++++++++++++
gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++
5 files changed, 356 insertions(+), 253 deletions(-)
delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c
delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp
create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c
create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn 2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi @ 2016-02-22 16:53 ` Yao Qi 2016-02-23 19:59 ` Luis Machado 2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw) To: gdb-patches 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) gdb: 2016-02-22 Yao Qi <yao.qi@linaro.org> * arm-tdep.c (arm_record_exreg_ld_st_insn): Set 'single_reg' per bit 8. Check bit 20 instead of bit 4 for VMOV instruction. Record D registers for instructions changing S registers. Change of the order of length and address in record_buf_mem array. gdb/testsuite: 2016-02-22 Yao Qi <yao.qi@linaro.org> * gdb.reverse/insn-reverse.c [__arm__] (ext_reg_load): New. [__arm__] (ext_reg_mov, ext_reg_push_pop): New. (testcases): Update. --- gdb/arm-tdep.c | 93 ++++++++++++++++++++------------ gdb/testsuite/gdb.reverse/insn-reverse.c | 43 +++++++++++++++ 2 files changed, 101 insertions(+), 35 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 0c0e1a5..5c5524b 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -10909,13 +10909,13 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch); opcode = bits (arm_insn_r->arm_insn, 20, 24); - single_reg = bit (arm_insn_r->arm_insn, 8); + single_reg = !bit (arm_insn_r->arm_insn, 8); op_vldm_vstm = opcode & 0x1b; /* Handle VMOV instructions. */ if ((opcode & 0x1e) == 0x04) { - if (bit (arm_insn_r->arm_insn, 4)) + if (bit (arm_insn_r->arm_insn, 20)) /* to_arm_registers bit 20? */ { record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15); record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19); @@ -10923,18 +10923,29 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) } else { - uint8_t reg_m = ((bits (arm_insn_r->arm_insn, 0, 3) << 1) - | bit (arm_insn_r->arm_insn, 5)); + uint8_t reg_m = bits (arm_insn_r->arm_insn, 0, 3); + uint8_t bit_m = bit (arm_insn_r->arm_insn, 5); - if (!single_reg) + if (single_reg) { - record_buf[0] = num_regs + reg_m; - record_buf[1] = num_regs + reg_m + 1; - arm_insn_r->reg_rec_count = 2; + /* The first S register number m is REG_M:M (M is bit 5), + the corresponding D register number is REG_M:M / 2, which + is REG_M. */ + record_buf[arm_insn_r->reg_rec_count++] = ARM_D0_REGNUM + reg_m; + /* The second S register number is REG_M:M + 1, the + corresponding D register number is (REG_M:M + 1) / 2. + IOW, if bit M is 1, the first and second S registers + are mapped to different D registers, otherwise, they are + in the same D register. */ + if (bit_m) + { + record_buf[arm_insn_r->reg_rec_count++] + = ARM_D0_REGNUM + reg_m + 1; + } } else { - record_buf[0] = reg_m + ARM_D0_REGNUM; + record_buf[0] = ((bit_m << 4) + reg_m + ARM_D0_REGNUM); arm_insn_r->reg_rec_count = 1; } } @@ -10949,7 +10960,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) reg_rn = bits (arm_insn_r->arm_insn, 16, 19); regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval); imm_off8 = bits (arm_insn_r->arm_insn, 0, 7); - imm_off32 = imm_off8 << 24; + imm_off32 = imm_off8 << 2; memory_count = imm_off8; if (bit (arm_insn_r->arm_insn, 23)) @@ -10965,19 +10976,19 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) while (memory_count > 0) { - if (!single_reg) + if (single_reg) { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; start_address = start_address + 4; memory_index = memory_index + 2; } else { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; - record_buf_mem[memory_index + 2] = start_address + 4; - record_buf_mem[memory_index + 3] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; + record_buf_mem[memory_index + 2] = 4; + record_buf_mem[memory_index + 3] = start_address + 4; start_address = start_address + 8; memory_index = memory_index + 4; } @@ -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. + If the instruction loads memory to S register, divide 2 as well + 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). */ + reg_count++; + } + while (reg_count > 0) + { + record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1; reg_count--; } arm_insn_r->reg_rec_count = reg_index; @@ -11023,7 +11045,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) reg_rn = bits (arm_insn_r->arm_insn, 16, 19); regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval); imm_off8 = bits (arm_insn_r->arm_insn, 0, 7); - imm_off32 = imm_off8 << 24; + imm_off32 = imm_off8 << 2; if (bit (arm_insn_r->arm_insn, 23)) start_address = u_regval + imm_off32; @@ -11032,16 +11054,16 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) if (single_reg) { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; arm_insn_r->mem_rec_count = 1; } else { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; - record_buf_mem[memory_index + 2] = start_address + 4; - record_buf_mem[memory_index + 3] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; + record_buf_mem[memory_index + 2] = 4; + record_buf_mem[memory_index + 3] = start_address + 4; arm_insn_r->mem_rec_count = 2; } } @@ -11058,7 +11080,8 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) else { reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22); - record_buf[0] = num_regs + reg_vd; + /* Record register D rather than pseudo register S. */ + record_buf[0] = ARM_D0_REGNUM + reg_vd / 2; } arm_insn_r->reg_rec_count = 1; } diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c index 1bfb8b0..22cd267 100644 --- a/gdb/testsuite/gdb.reverse/insn-reverse.c +++ b/gdb/testsuite/gdb.reverse/insn-reverse.c @@ -88,6 +88,45 @@ adv_simd_vect_shift (void) { asm ("fcvtzs s0, s0, #1"); } +#elif (defined __arm__) +static void +ext_reg_load (void) +{ + char in[8]; + + asm ("vldr d0, [%0]" : : "r" (in)); + asm ("vldr s3, [%0]" : : "r" (in)); + + asm ("vldm %0, {d3-d4}" : : "r" (in)); + asm ("vldm %0, {s9-s11}" : : "r" (in)); +} + +static void +ext_reg_mov (void) +{ + int i, j; + double d; + + i = 1; + j = 2; + + asm ("vmov s4, s5, %0, %1" : "=r" (i), "=r" (j): ); + asm ("vmov s7, s8, %0, %1" : "=r" (i), "=r" (j): ); + asm ("vmov %0, %1, s10, s11" : : "r" (i), "r" (j)); + asm ("vmov %0, %1, s1, s2" : : "r" (i), "r" (j)); + + asm ("vmov %P2, %0, %1" : "=r" (i), "=r" (j): "w" (d)); + asm ("vmov %1, %2, %P0" : "=w" (d) : "r" (i), "r" (j)); +} + +static void +ext_reg_push_pop (void) +{ + double d; + + asm ("vpush {%P0}" : : "w" (d)); + asm ("vpop {%P0}" : : "w" (d)); +} #endif typedef void (*testcase_ftype) (void); @@ -104,6 +143,10 @@ static testcase_ftype testcases[] = adv_simd_scalar_index, adv_simd_smlal, adv_simd_vect_shift, +#elif (defined __arm__) + ext_reg_load, + ext_reg_mov, + ext_reg_push_pop, #endif }; -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn 2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi @ 2016-02-23 19:59 ` Luis Machado 2016-02-26 14:54 ` Yao Qi 0 siblings, 1 reply; 11+ messages in thread From: Luis Machado @ 2016-02-23 19:59 UTC (permalink / raw) To: Yao Qi, gdb-patches 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..."? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn 2016-02-23 19:59 ` Luis Machado @ 2016-02-26 14:54 ` Yao Qi 0 siblings, 0 replies; 11+ messages in thread From: Yao Qi @ 2016-02-26 14:54 UTC (permalink / raw) To: Luis Machado; +Cc: Yao Qi, gdb-patches Luis Machado <lgustavo@codesourcery.com> writes: >> - 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..."? Hi Luis, Thanks for the review. I fixed them. -- Yao (齐尧) From 8600280115853d508d82ae47932b3cbb8d4c22ac Mon Sep 17 00:00:00 2001 From: Yao Qi <yao.qi@linaro.org> Date: Tue, 16 Feb 2016 12:59:03 +0000 Subject: [PATCH] Fix various bugs in arm_record_exreg_ld_st_insn 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) gdb: 2016-02-26 Yao Qi <yao.qi@linaro.org> * arm-tdep.c (arm_record_exreg_ld_st_insn): Set 'single_reg' per bit 8. Check bit 20 instead of bit 4 for VMOV instruction. Record D registers for instructions changing S registers. Change of the order of length and address in record_buf_mem array. gdb/testsuite: 2016-02-26 Yao Qi <yao.qi@linaro.org> * gdb.reverse/insn-reverse.c [__arm__] (ext_reg_load): New. [__arm__] (ext_reg_mov, ext_reg_push_pop): New. (testcases): Update. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index bd0ee97..2eb7bb1 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -10909,13 +10909,13 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch); opcode = bits (arm_insn_r->arm_insn, 20, 24); - single_reg = bit (arm_insn_r->arm_insn, 8); + single_reg = !bit (arm_insn_r->arm_insn, 8); op_vldm_vstm = opcode & 0x1b; /* Handle VMOV instructions. */ if ((opcode & 0x1e) == 0x04) { - if (bit (arm_insn_r->arm_insn, 4)) + if (bit (arm_insn_r->arm_insn, 20)) /* to_arm_registers bit 20? */ { record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15); record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19); @@ -10923,18 +10923,29 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) } else { - uint8_t reg_m = ((bits (arm_insn_r->arm_insn, 0, 3) << 1) - | bit (arm_insn_r->arm_insn, 5)); + uint8_t reg_m = bits (arm_insn_r->arm_insn, 0, 3); + uint8_t bit_m = bit (arm_insn_r->arm_insn, 5); - if (!single_reg) + if (single_reg) { - record_buf[0] = num_regs + reg_m; - record_buf[1] = num_regs + reg_m + 1; - arm_insn_r->reg_rec_count = 2; + /* The first S register number m is REG_M:M (M is bit 5), + the corresponding D register number is REG_M:M / 2, which + is REG_M. */ + record_buf[arm_insn_r->reg_rec_count++] = ARM_D0_REGNUM + reg_m; + /* The second S register number is REG_M:M + 1, the + corresponding D register number is (REG_M:M + 1) / 2. + IOW, if bit M is 1, the first and second S registers + are mapped to different D registers, otherwise, they are + in the same D register. */ + if (bit_m) + { + record_buf[arm_insn_r->reg_rec_count++] + = ARM_D0_REGNUM + reg_m + 1; + } } else { - record_buf[0] = reg_m + ARM_D0_REGNUM; + record_buf[0] = ((bit_m << 4) + reg_m + ARM_D0_REGNUM); arm_insn_r->reg_rec_count = 1; } } @@ -10949,7 +10960,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) reg_rn = bits (arm_insn_r->arm_insn, 16, 19); regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval); imm_off8 = bits (arm_insn_r->arm_insn, 0, 7); - imm_off32 = imm_off8 << 24; + imm_off32 = imm_off8 << 2; memory_count = imm_off8; if (bit (arm_insn_r->arm_insn, 23)) @@ -10965,19 +10976,19 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) while (memory_count > 0) { - if (!single_reg) + if (single_reg) { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; start_address = start_address + 4; memory_index = memory_index + 2; } else { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; - record_buf_mem[memory_index + 2] = start_address + 4; - record_buf_mem[memory_index + 3] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; + record_buf_mem[memory_index + 2] = 4; + record_buf_mem[memory_index + 3] = start_address + 4; start_address = start_address + 8; memory_index = memory_index + 4; } @@ -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 + be divided by 2, according to the ARM Architecture Reference + Manual. If the instruction loads memory to S register, divide by + 2 as well 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 + an odd number (bit d is one). */ + reg_count++; + } + while (reg_count > 0) + { + record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1; reg_count--; } arm_insn_r->reg_rec_count = reg_index; @@ -11023,7 +11045,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) reg_rn = bits (arm_insn_r->arm_insn, 16, 19); regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval); imm_off8 = bits (arm_insn_r->arm_insn, 0, 7); - imm_off32 = imm_off8 << 24; + imm_off32 = imm_off8 << 2; if (bit (arm_insn_r->arm_insn, 23)) start_address = u_regval + imm_off32; @@ -11032,16 +11054,16 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) if (single_reg) { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; arm_insn_r->mem_rec_count = 1; } else { - record_buf_mem[memory_index] = start_address; - record_buf_mem[memory_index + 1] = 4; - record_buf_mem[memory_index + 2] = start_address + 4; - record_buf_mem[memory_index + 3] = 4; + record_buf_mem[memory_index] = 4; + record_buf_mem[memory_index + 1] = start_address; + record_buf_mem[memory_index + 2] = 4; + record_buf_mem[memory_index + 3] = start_address + 4; arm_insn_r->mem_rec_count = 2; } } @@ -11058,7 +11080,8 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r) else { reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22); - record_buf[0] = num_regs + reg_vd; + /* Record register D rather than pseudo register S. */ + record_buf[0] = ARM_D0_REGNUM + reg_vd / 2; } arm_insn_r->reg_rec_count = 1; } diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c index 1bfb8b0..22cd267 100644 --- a/gdb/testsuite/gdb.reverse/insn-reverse.c +++ b/gdb/testsuite/gdb.reverse/insn-reverse.c @@ -88,6 +88,45 @@ adv_simd_vect_shift (void) { asm ("fcvtzs s0, s0, #1"); } +#elif (defined __arm__) +static void +ext_reg_load (void) +{ + char in[8]; + + asm ("vldr d0, [%0]" : : "r" (in)); + asm ("vldr s3, [%0]" : : "r" (in)); + + asm ("vldm %0, {d3-d4}" : : "r" (in)); + asm ("vldm %0, {s9-s11}" : : "r" (in)); +} + +static void +ext_reg_mov (void) +{ + int i, j; + double d; + + i = 1; + j = 2; + + asm ("vmov s4, s5, %0, %1" : "=r" (i), "=r" (j): ); + asm ("vmov s7, s8, %0, %1" : "=r" (i), "=r" (j): ); + asm ("vmov %0, %1, s10, s11" : : "r" (i), "r" (j)); + asm ("vmov %0, %1, s1, s2" : : "r" (i), "r" (j)); + + asm ("vmov %P2, %0, %1" : "=r" (i), "=r" (j): "w" (d)); + asm ("vmov %1, %2, %P0" : "=w" (d) : "r" (i), "r" (j)); +} + +static void +ext_reg_push_pop (void) +{ + double d; + + asm ("vpush {%P0}" : : "w" (d)); + asm ("vpop {%P0}" : : "w" (d)); +} #endif typedef void (*testcase_ftype) (void); @@ -104,6 +143,10 @@ static testcase_ftype testcases[] = adv_simd_scalar_index, adv_simd_smlal, adv_simd_vect_shift, +#elif (defined __arm__) + ext_reg_load, + ext_reg_mov, + ext_reg_push_pop, #endif }; ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Record right reg num of thumb special data instructions 2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi 2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi @ 2016-02-22 16:53 ` Yao Qi 2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi 2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi 3 siblings, 0 replies; 11+ messages in thread From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw) To: gdb-patches When GDB decodes these thumb special data instructions, such as 'mov sp, r7' the Rd is got incorrectly. According to the arch reference manual, the Rd is DN:Rdn, in which DN is bit 7 and Rdn is bits 0 to 2. This patch fixes it. gdb: 2016-02-22 Yao Qi <yao.qi@linaro.org> * arm-tdep.c (thumb_record_ld_st_reg_offset): Fix the register number of Rd. --- gdb/arm-tdep.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 1a3a209..0c0e1a5 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -11512,10 +11512,10 @@ thumb_record_ld_st_reg_offset (insn_decode_record *thumb_insn_r) } else { - /* Format 8; special data processing insns. */ - reg_src1 = bits (thumb_insn_r->arm_insn, 0, 2); - record_buf[0] = ARM_PS_REGNUM; - record_buf[1] = reg_src1; + /* Format 8; special data processing insns. */ + record_buf[0] = ARM_PS_REGNUM; + record_buf[1] = (bit (thumb_insn_r->arm_insn, 7) << 3 + | bits (thumb_insn_r->arm_insn, 0, 2)); thumb_insn_r->reg_rec_count = 2; } } -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Generalize gdb.reverse/aarch64.exp 2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi 2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi 2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi @ 2016-02-22 16:53 ` Yao Qi 2016-02-24 11:22 ` Pedro Alves 2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi 3 siblings, 1 reply; 11+ messages in thread From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw) To: gdb-patches I said we can generialize gdb.reverse/aarch64.exp for other architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html and here is the patch to change aarch64.exp to insn-reverse.exp. gdb/testsuite: 2016-02-22 Yao Qi <yao.qi@linaro.org> * gdb.reverse/aarch64.c: Rename it ... * gdb.reverse/insn-reverse.c: to ... [__aarch64__] Include arm_neon.h. (testcase_ftype): New. (testcases): New array. (n_testcases): New. (main): Call each element in testcases. * gdb.reverse/aarch64.exp: Rename it ... * gdb.reverse/insn-reverse.exp: to ... Remove is_aarch64_target check. (read_testcase): New. Do the tests in a loop. --- gdb/testsuite/gdb.reverse/aarch64.c | 99 ---------------------- gdb/testsuite/gdb.reverse/aarch64.exp | 115 ------------------------- gdb/testsuite/gdb.reverse/insn-reverse.c | 121 +++++++++++++++++++++++++++ gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 214 deletions(-) delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c deleted file mode 100644 index ae8509a..0000000 --- a/gdb/testsuite/gdb.reverse/aarch64.c +++ /dev/null @@ -1,99 +0,0 @@ -/* This testcase is part of GDB, the GNU debugger. - - Copyright 2015-2016 Free Software Foundation, Inc. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. */ - -#include <arm_neon.h> - -static void -load (void) -{ - int buf[8]; - - asm ("ld1 { v1.8b }, [%[buf]]\n" - "ld1 { v2.8b, v3.8b }, [%[buf]]\n" - "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n" - : - : [buf] "r" (buf) - : /* No clobbers */); -} - -static void -move (void) -{ - float32x2_t b1_ = vdup_n_f32(123.0f); - float32_t a1_ = 0; - float64x1_t b2_ = vdup_n_f64(456.0f); - float64_t a2_ = 0; - - asm ("ins %0.s[0], %w1\n" - : "=w"(b1_) - : "r"(a1_), "0"(b1_) - : /* No clobbers */); - - asm ("ins %0.d[1], %x1\n" - : "=w"(b2_) - : "r"(a2_), "0"(b2_) - : /* No clobbers */); -} - -static void -adv_simd_mod_imm (void) -{ - float32x2_t a1 = {2.0, 4.0}; - - asm ("bic %0.2s, #1\n" - "bic %0.2s, #1, lsl #8\n" - : "=w"(a1) - : "0"(a1) - : /* No clobbers */); -} - -static void -adv_simd_scalar_index (void) -{ - float64x2_t b_ = {0.0, 0.0}; - float64_t a_ = 1.0; - float64_t result; - - asm ("fmla %d0,%d1,%2.d[1]" - : "=w"(result) - : "w"(a_), "w"(b_) - : /* No clobbers */); -} - -static void -adv_simd_smlal (void) -{ - asm ("smlal v13.2d, v8.2s, v0.2s"); -} - -static void -adv_simd_vect_shift (void) -{ - asm ("fcvtzs s0, s0, #1"); -} - -int -main () -{ - load (); - move (); - adv_simd_mod_imm (); - adv_simd_scalar_index (); - adv_simd_smlal (); - adv_simd_vect_shift (); - return 0; -} diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp deleted file mode 100644 index 2906d4b..0000000 --- a/gdb/testsuite/gdb.reverse/aarch64.exp +++ /dev/null @@ -1,115 +0,0 @@ -# Copyright (C) 2015-2016 Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. - -if ![supports_reverse] { - return -} - -# Test aarch64 instruction recording. - -if {![is_aarch64_target]} then { - verbose "Skipping aarch64 instruction recording tests." - return -} - -standard_testfile - -if {[prepare_for_testing $testfile.exp $testfile $srcfile \ - [list debug]]} { - untested ${testfile}.exp - return -1 -} -if { ![runto main] } then { - fail "run to main" - return -} - -# In each function FUNC, GDB turns on process record, and single step -# until program goes to the end of the function. Then, single step -# backward. In each of forward single step and backward single step, -# the contents of registers are saved, and test compares them. If -# there is any differences, a FAIL is emitted. - -proc test { func } { - global hex decimal - global gdb_prompt - - with_test_prefix "$func" { - gdb_breakpoint $func - gdb_test "continue" - - set last_insn "" - set test "disassemble $func" - gdb_test_multiple $test $test { - -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" { - set last_insn $expect_out(1,string) - } - } - if { $last_insn == "" } { - fail "find the last instruction of function $func" - } - - # Activate process record/replay - gdb_test_no_output "record" "Turn on process record" - - # Registers contents before each forward single step. - set count 0 - for {} {$count < 500} {incr count} { - gdb_test_multiple "x/i \$pc" "" { - -re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" { - set insn_addr $expect_out(1,string) - - if [expr {$last_insn == $insn_addr}] { - break - } - - set insn_array($count) $expect_out(2,string) - } - } - - set pre_regs($count) [capture_command_output "info all-registers" ""] - gdb_test "si" "" "" - } - - incr count -1 - # Registers contents after each backward single step. - for {set i $count} {$i >= 0} {incr i -1} { - gdb_test "reverse-stepi" "" "" - set post_regs($i) [capture_command_output "info all-registers" ""] - } - - # Compare the register contents. - for {set i 0} {$i < $count} {incr i} { - if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \ - "compare registers on insn $i:$insn_array($i)"] } { - - foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] { - if { [string compare $pre_line $post_line] } { - verbose -log " -:$pre_line" - verbose -log " +:$post_line" - } - } - } - } - gdb_test "record stop" - } -} - -test "load" -test "move" -test "adv_simd_mod_imm" -test "adv_simd_scalar_index" -test "adv_simd_smlal" -test "adv_simd_vect_shift" diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c new file mode 100644 index 0000000..1bfb8b0 --- /dev/null +++ b/gdb/testsuite/gdb.reverse/insn-reverse.c @@ -0,0 +1,121 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015-2016 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#if (defined __aarch64__) +#include <arm_neon.h> +#endif + +#if (defined __aarch64__) +static void +load (void) +{ + int buf[8]; + + asm ("ld1 { v1.8b }, [%[buf]]\n" + "ld1 { v2.8b, v3.8b }, [%[buf]]\n" + "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n" + : + : [buf] "r" (buf) + : /* No clobbers */); +} + +static void +move (void) +{ + float32x2_t b1_ = vdup_n_f32(123.0f); + float32_t a1_ = 0; + float64x1_t b2_ = vdup_n_f64(456.0f); + float64_t a2_ = 0; + + asm ("ins %0.s[0], %w1\n" + : "=w"(b1_) + : "r"(a1_), "0"(b1_) + : /* No clobbers */); + + asm ("ins %0.d[1], %x1\n" + : "=w"(b2_) + : "r"(a2_), "0"(b2_) + : /* No clobbers */); +} + +static void +adv_simd_mod_imm (void) +{ + float32x2_t a1 = {2.0, 4.0}; + + asm ("bic %0.2s, #1\n" + "bic %0.2s, #1, lsl #8\n" + : "=w"(a1) + : "0"(a1) + : /* No clobbers */); +} + +static void +adv_simd_scalar_index (void) +{ + float64x2_t b_ = {0.0, 0.0}; + float64_t a_ = 1.0; + float64_t result; + + asm ("fmla %d0,%d1,%2.d[1]" + : "=w"(result) + : "w"(a_), "w"(b_) + : /* No clobbers */); +} + +static void +adv_simd_smlal (void) +{ + asm ("smlal v13.2d, v8.2s, v0.2s"); +} + +static void +adv_simd_vect_shift (void) +{ + asm ("fcvtzs s0, s0, #1"); +} +#endif + +typedef void (*testcase_ftype) (void); + +/* Functions testing instruction decodings. GDB will read n_testcases + to know how many functions to test. */ + +static testcase_ftype testcases[] = +{ +#if (defined __aarch64__) + load, + move, + adv_simd_mod_imm, + adv_simd_scalar_index, + adv_simd_smlal, + adv_simd_vect_shift, +#endif +}; + +static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype)); + +int +main () +{ + int i = 0; + + for (i = 0; i < n_testcases; i++) + testcases[i] (); + + return 0; +} diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.exp b/gdb/testsuite/gdb.reverse/insn-reverse.exp new file mode 100644 index 0000000..f52b40c --- /dev/null +++ b/gdb/testsuite/gdb.reverse/insn-reverse.exp @@ -0,0 +1,130 @@ +# Copyright (C) 2015-2016 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +if ![supports_reverse] { + return +} + +standard_testfile + +if {[prepare_for_testing $testfile.exp $testfile $srcfile \ + [list debug]]} { + untested ${testfile}.exp + return -1 +} +if { ![runto main] } then { + fail "run to main" + return +} + +# Read function name from testcases[N]. + +proc read_testcase { n } { + global gdb_prompt + + set result -1 + gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" { + -re "\[$\].*= .*<(.*)>.*$gdb_prompt $" { + set result $expect_out(1,string) + } + -re "$gdb_prompt $" { } + } + + return $result +} + +# In each function FUNC, GDB turns on process record, and single step +# until program goes to the end of the function. Then, single step +# backward. In each of forward single step and backward single step, +# the contents of registers are saved, and test compares them. If +# there is any differences, a FAIL is emitted. + +proc test { func } { + global hex decimal + global gdb_prompt + + with_test_prefix "$func" { + gdb_breakpoint $func + gdb_test "continue" + + set last_insn "" + set test "disassemble $func" + gdb_test_multiple $test $test { + -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" { + set last_insn $expect_out(1,string) + } + } + if { $last_insn == "" } { + fail "find the last instruction of function $func" + } + + # Activate process record/replay + gdb_test_no_output "record" "Turn on process record" + + # Registers contents before each forward single step. + set count 0 + for {} {$count < 500} {incr count} { + gdb_test_multiple "x/i \$pc" "" { + -re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" { + set insn_addr $expect_out(1,string) + + if [expr {$last_insn == $insn_addr}] { + break + } + + set insn_array($count) $expect_out(2,string) + } + } + + set pre_regs($count) [capture_command_output "info all-registers" ""] + gdb_test "si" "" "" + } + + incr count -1 + # Registers contents after each backward single step. + for {set i $count} {$i >= 0} {incr i -1} { + gdb_test "reverse-stepi" "" "" + set post_regs($i) [capture_command_output "info all-registers" ""] + } + + # Compare the register contents. + for {set i 0} {$i < $count} {incr i} { + if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \ + "compare registers on insn $i:$insn_array($i)"] } { + + foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] { + if { [string compare $pre_line $post_line] } { + verbose -log " -:$pre_line" + verbose -log " +:$post_line" + } + } + } + } + gdb_test "record stop" + } +} + +set n_testcases [get_integer_valueof "n_testcases" 0] + +if { ${n_testcases} == 0 } { + untested "No test" + return 1 +} + +for { set i 0 } { ${i} < ${n_testcases} } { incr i } { + set testcase [read_testcase $i] + + test $testcase +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Generalize gdb.reverse/aarch64.exp 2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi @ 2016-02-24 11:22 ` Pedro Alves 2016-02-24 14:40 ` Yao Qi 0 siblings, 1 reply; 11+ messages in thread From: Pedro Alves @ 2016-02-24 11:22 UTC (permalink / raw) To: Yao Qi, gdb-patches On 02/22/2016 04:53 PM, Yao Qi wrote: > I said we can generialize gdb.reverse/aarch64.exp for other > architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html > and here is the patch to change aarch64.exp to insn-reverse.exp. > > gdb/testsuite: > > 2016-02-22 Yao Qi <yao.qi@linaro.org> > > * gdb.reverse/aarch64.c: Rename it ... > * gdb.reverse/insn-reverse.c: to ... > [__aarch64__] Include arm_neon.h. > (testcase_ftype): New. > (testcases): New array. > (n_testcases): New. > (main): Call each element in testcases. > * gdb.reverse/aarch64.exp: Rename it ... > * gdb.reverse/insn-reverse.exp: to ... > Remove is_aarch64_target check. > (read_testcase): New. > Do the tests in a loop. > --- > gdb/testsuite/gdb.reverse/aarch64.c | 99 ---------------------- > gdb/testsuite/gdb.reverse/aarch64.exp | 115 ------------------------- > gdb/testsuite/gdb.reverse/insn-reverse.c | 121 +++++++++++++++++++++++++++ > gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++ Can you send a "git diff -M" so we can easily see the actual changes? (You can make that the default with git config.) Though I think even better would be to do the rename in one patch, and the generalization changes in a separate patch. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Generalize gdb.reverse/aarch64.exp 2016-02-24 11:22 ` Pedro Alves @ 2016-02-24 14:40 ` Yao Qi 2016-02-24 14:40 ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi 2016-02-24 19:37 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves 0 siblings, 2 replies; 11+ messages in thread From: Yao Qi @ 2016-02-24 14:40 UTC (permalink / raw) To: gdb-patches I said we can generialize gdb.reverse/aarch64.exp for other architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html and here is the patch to change aarch64.exp so that it can be used to test for other architectures as well. gdb/testsuite: 2016-02-24 Yao Qi <yao.qi@linaro.org> * gdb.reverse/aarch64.c: [__aarch64__] Include arm_neon.h. (testcase_ftype): New. (testcases): New array. (n_testcases): New. (main): Call each element in testcases. * gdb.reverse/aarch64.exp: Remove is_aarch64_target check. (read_testcase): New. Do the tests in a loop. --- gdb/testsuite/gdb.reverse/aarch64.c | 34 ++++++++++++++++++++++++----- gdb/testsuite/gdb.reverse/aarch64.exp | 41 ++++++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c index ae8509a..1bfb8b0 100644 --- a/gdb/testsuite/gdb.reverse/aarch64.c +++ b/gdb/testsuite/gdb.reverse/aarch64.c @@ -15,8 +15,11 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#if (defined __aarch64__) #include <arm_neon.h> +#endif +#if (defined __aarch64__) static void load (void) { @@ -85,15 +88,34 @@ adv_simd_vect_shift (void) { asm ("fcvtzs s0, s0, #1"); } +#endif + +typedef void (*testcase_ftype) (void); + +/* Functions testing instruction decodings. GDB will read n_testcases + to know how many functions to test. */ + +static testcase_ftype testcases[] = +{ +#if (defined __aarch64__) + load, + move, + adv_simd_mod_imm, + adv_simd_scalar_index, + adv_simd_smlal, + adv_simd_vect_shift, +#endif +}; + +static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype)); int main () { - load (); - move (); - adv_simd_mod_imm (); - adv_simd_scalar_index (); - adv_simd_smlal (); - adv_simd_vect_shift (); + int i = 0; + + for (i = 0; i < n_testcases; i++) + testcases[i] (); + return 0; } diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp index 2906d4b..f52b40c 100644 --- a/gdb/testsuite/gdb.reverse/aarch64.exp +++ b/gdb/testsuite/gdb.reverse/aarch64.exp @@ -17,13 +17,6 @@ if ![supports_reverse] { return } -# Test aarch64 instruction recording. - -if {![is_aarch64_target]} then { - verbose "Skipping aarch64 instruction recording tests." - return -} - standard_testfile if {[prepare_for_testing $testfile.exp $testfile $srcfile \ @@ -36,6 +29,22 @@ if { ![runto main] } then { return } +# Read function name from testcases[N]. + +proc read_testcase { n } { + global gdb_prompt + + set result -1 + gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" { + -re "\[$\].*= .*<(.*)>.*$gdb_prompt $" { + set result $expect_out(1,string) + } + -re "$gdb_prompt $" { } + } + + return $result +} + # In each function FUNC, GDB turns on process record, and single step # until program goes to the end of the function. Then, single step # backward. In each of forward single step and backward single step, @@ -107,9 +116,15 @@ proc test { func } { } } -test "load" -test "move" -test "adv_simd_mod_imm" -test "adv_simd_scalar_index" -test "adv_simd_smlal" -test "adv_simd_vect_shift" +set n_testcases [get_integer_valueof "n_testcases" 0] + +if { ${n_testcases} == 0 } { + untested "No test" + return 1 +} + +for { set i 0 } { ${i} < ${n_testcases} } { incr i } { + set testcase [read_testcase $i] + + test $testcase +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} 2016-02-24 14:40 ` Yao Qi @ 2016-02-24 14:40 ` Yao Qi 2016-02-24 19:37 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves 1 sibling, 0 replies; 11+ messages in thread From: Yao Qi @ 2016-02-24 14:40 UTC (permalink / raw) To: gdb-patches gdb/testsuite: 2016-02-24 Yao Qi <yao.qi@linaro.org> * gdb.reverse/aarch64.c: Rename to ... * gdb.reverse/insn-reverse.c: ... it. * gdb.reverse/aarch64.exp: Rename to ... * gdb.reverse/insn-reverse.exp: ... it. --- gdb/testsuite/gdb.reverse/aarch64.c | 121 --------------------------- gdb/testsuite/gdb.reverse/aarch64.exp | 130 ----------------------------- gdb/testsuite/gdb.reverse/insn-reverse.c | 121 +++++++++++++++++++++++++++ gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 251 deletions(-) delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c deleted file mode 100644 index 1bfb8b0..0000000 --- a/gdb/testsuite/gdb.reverse/aarch64.c +++ /dev/null @@ -1,121 +0,0 @@ -/* This testcase is part of GDB, the GNU debugger. - - Copyright 2015-2016 Free Software Foundation, Inc. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. */ - -#if (defined __aarch64__) -#include <arm_neon.h> -#endif - -#if (defined __aarch64__) -static void -load (void) -{ - int buf[8]; - - asm ("ld1 { v1.8b }, [%[buf]]\n" - "ld1 { v2.8b, v3.8b }, [%[buf]]\n" - "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n" - : - : [buf] "r" (buf) - : /* No clobbers */); -} - -static void -move (void) -{ - float32x2_t b1_ = vdup_n_f32(123.0f); - float32_t a1_ = 0; - float64x1_t b2_ = vdup_n_f64(456.0f); - float64_t a2_ = 0; - - asm ("ins %0.s[0], %w1\n" - : "=w"(b1_) - : "r"(a1_), "0"(b1_) - : /* No clobbers */); - - asm ("ins %0.d[1], %x1\n" - : "=w"(b2_) - : "r"(a2_), "0"(b2_) - : /* No clobbers */); -} - -static void -adv_simd_mod_imm (void) -{ - float32x2_t a1 = {2.0, 4.0}; - - asm ("bic %0.2s, #1\n" - "bic %0.2s, #1, lsl #8\n" - : "=w"(a1) - : "0"(a1) - : /* No clobbers */); -} - -static void -adv_simd_scalar_index (void) -{ - float64x2_t b_ = {0.0, 0.0}; - float64_t a_ = 1.0; - float64_t result; - - asm ("fmla %d0,%d1,%2.d[1]" - : "=w"(result) - : "w"(a_), "w"(b_) - : /* No clobbers */); -} - -static void -adv_simd_smlal (void) -{ - asm ("smlal v13.2d, v8.2s, v0.2s"); -} - -static void -adv_simd_vect_shift (void) -{ - asm ("fcvtzs s0, s0, #1"); -} -#endif - -typedef void (*testcase_ftype) (void); - -/* Functions testing instruction decodings. GDB will read n_testcases - to know how many functions to test. */ - -static testcase_ftype testcases[] = -{ -#if (defined __aarch64__) - load, - move, - adv_simd_mod_imm, - adv_simd_scalar_index, - adv_simd_smlal, - adv_simd_vect_shift, -#endif -}; - -static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype)); - -int -main () -{ - int i = 0; - - for (i = 0; i < n_testcases; i++) - testcases[i] (); - - return 0; -} diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp deleted file mode 100644 index f52b40c..0000000 --- a/gdb/testsuite/gdb.reverse/aarch64.exp +++ /dev/null @@ -1,130 +0,0 @@ -# Copyright (C) 2015-2016 Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. - -if ![supports_reverse] { - return -} - -standard_testfile - -if {[prepare_for_testing $testfile.exp $testfile $srcfile \ - [list debug]]} { - untested ${testfile}.exp - return -1 -} -if { ![runto main] } then { - fail "run to main" - return -} - -# Read function name from testcases[N]. - -proc read_testcase { n } { - global gdb_prompt - - set result -1 - gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" { - -re "\[$\].*= .*<(.*)>.*$gdb_prompt $" { - set result $expect_out(1,string) - } - -re "$gdb_prompt $" { } - } - - return $result -} - -# In each function FUNC, GDB turns on process record, and single step -# until program goes to the end of the function. Then, single step -# backward. In each of forward single step and backward single step, -# the contents of registers are saved, and test compares them. If -# there is any differences, a FAIL is emitted. - -proc test { func } { - global hex decimal - global gdb_prompt - - with_test_prefix "$func" { - gdb_breakpoint $func - gdb_test "continue" - - set last_insn "" - set test "disassemble $func" - gdb_test_multiple $test $test { - -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" { - set last_insn $expect_out(1,string) - } - } - if { $last_insn == "" } { - fail "find the last instruction of function $func" - } - - # Activate process record/replay - gdb_test_no_output "record" "Turn on process record" - - # Registers contents before each forward single step. - set count 0 - for {} {$count < 500} {incr count} { - gdb_test_multiple "x/i \$pc" "" { - -re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" { - set insn_addr $expect_out(1,string) - - if [expr {$last_insn == $insn_addr}] { - break - } - - set insn_array($count) $expect_out(2,string) - } - } - - set pre_regs($count) [capture_command_output "info all-registers" ""] - gdb_test "si" "" "" - } - - incr count -1 - # Registers contents after each backward single step. - for {set i $count} {$i >= 0} {incr i -1} { - gdb_test "reverse-stepi" "" "" - set post_regs($i) [capture_command_output "info all-registers" ""] - } - - # Compare the register contents. - for {set i 0} {$i < $count} {incr i} { - if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \ - "compare registers on insn $i:$insn_array($i)"] } { - - foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] { - if { [string compare $pre_line $post_line] } { - verbose -log " -:$pre_line" - verbose -log " +:$post_line" - } - } - } - } - gdb_test "record stop" - } -} - -set n_testcases [get_integer_valueof "n_testcases" 0] - -if { ${n_testcases} == 0 } { - untested "No test" - return 1 -} - -for { set i 0 } { ${i} < ${n_testcases} } { incr i } { - set testcase [read_testcase $i] - - test $testcase -} diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c new file mode 100644 index 0000000..1bfb8b0 --- /dev/null +++ b/gdb/testsuite/gdb.reverse/insn-reverse.c @@ -0,0 +1,121 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015-2016 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#if (defined __aarch64__) +#include <arm_neon.h> +#endif + +#if (defined __aarch64__) +static void +load (void) +{ + int buf[8]; + + asm ("ld1 { v1.8b }, [%[buf]]\n" + "ld1 { v2.8b, v3.8b }, [%[buf]]\n" + "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n" + : + : [buf] "r" (buf) + : /* No clobbers */); +} + +static void +move (void) +{ + float32x2_t b1_ = vdup_n_f32(123.0f); + float32_t a1_ = 0; + float64x1_t b2_ = vdup_n_f64(456.0f); + float64_t a2_ = 0; + + asm ("ins %0.s[0], %w1\n" + : "=w"(b1_) + : "r"(a1_), "0"(b1_) + : /* No clobbers */); + + asm ("ins %0.d[1], %x1\n" + : "=w"(b2_) + : "r"(a2_), "0"(b2_) + : /* No clobbers */); +} + +static void +adv_simd_mod_imm (void) +{ + float32x2_t a1 = {2.0, 4.0}; + + asm ("bic %0.2s, #1\n" + "bic %0.2s, #1, lsl #8\n" + : "=w"(a1) + : "0"(a1) + : /* No clobbers */); +} + +static void +adv_simd_scalar_index (void) +{ + float64x2_t b_ = {0.0, 0.0}; + float64_t a_ = 1.0; + float64_t result; + + asm ("fmla %d0,%d1,%2.d[1]" + : "=w"(result) + : "w"(a_), "w"(b_) + : /* No clobbers */); +} + +static void +adv_simd_smlal (void) +{ + asm ("smlal v13.2d, v8.2s, v0.2s"); +} + +static void +adv_simd_vect_shift (void) +{ + asm ("fcvtzs s0, s0, #1"); +} +#endif + +typedef void (*testcase_ftype) (void); + +/* Functions testing instruction decodings. GDB will read n_testcases + to know how many functions to test. */ + +static testcase_ftype testcases[] = +{ +#if (defined __aarch64__) + load, + move, + adv_simd_mod_imm, + adv_simd_scalar_index, + adv_simd_smlal, + adv_simd_vect_shift, +#endif +}; + +static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype)); + +int +main () +{ + int i = 0; + + for (i = 0; i < n_testcases; i++) + testcases[i] (); + + return 0; +} diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.exp b/gdb/testsuite/gdb.reverse/insn-reverse.exp new file mode 100644 index 0000000..f52b40c --- /dev/null +++ b/gdb/testsuite/gdb.reverse/insn-reverse.exp @@ -0,0 +1,130 @@ +# Copyright (C) 2015-2016 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +if ![supports_reverse] { + return +} + +standard_testfile + +if {[prepare_for_testing $testfile.exp $testfile $srcfile \ + [list debug]]} { + untested ${testfile}.exp + return -1 +} +if { ![runto main] } then { + fail "run to main" + return +} + +# Read function name from testcases[N]. + +proc read_testcase { n } { + global gdb_prompt + + set result -1 + gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" { + -re "\[$\].*= .*<(.*)>.*$gdb_prompt $" { + set result $expect_out(1,string) + } + -re "$gdb_prompt $" { } + } + + return $result +} + +# In each function FUNC, GDB turns on process record, and single step +# until program goes to the end of the function. Then, single step +# backward. In each of forward single step and backward single step, +# the contents of registers are saved, and test compares them. If +# there is any differences, a FAIL is emitted. + +proc test { func } { + global hex decimal + global gdb_prompt + + with_test_prefix "$func" { + gdb_breakpoint $func + gdb_test "continue" + + set last_insn "" + set test "disassemble $func" + gdb_test_multiple $test $test { + -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" { + set last_insn $expect_out(1,string) + } + } + if { $last_insn == "" } { + fail "find the last instruction of function $func" + } + + # Activate process record/replay + gdb_test_no_output "record" "Turn on process record" + + # Registers contents before each forward single step. + set count 0 + for {} {$count < 500} {incr count} { + gdb_test_multiple "x/i \$pc" "" { + -re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" { + set insn_addr $expect_out(1,string) + + if [expr {$last_insn == $insn_addr}] { + break + } + + set insn_array($count) $expect_out(2,string) + } + } + + set pre_regs($count) [capture_command_output "info all-registers" ""] + gdb_test "si" "" "" + } + + incr count -1 + # Registers contents after each backward single step. + for {set i $count} {$i >= 0} {incr i -1} { + gdb_test "reverse-stepi" "" "" + set post_regs($i) [capture_command_output "info all-registers" ""] + } + + # Compare the register contents. + for {set i 0} {$i < $count} {incr i} { + if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \ + "compare registers on insn $i:$insn_array($i)"] } { + + foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] { + if { [string compare $pre_line $post_line] } { + verbose -log " -:$pre_line" + verbose -log " +:$post_line" + } + } + } + } + gdb_test "record stop" + } +} + +set n_testcases [get_integer_valueof "n_testcases" 0] + +if { ${n_testcases} == 0 } { + untested "No test" + return 1 +} + +for { set i 0 } { ${i} < ${n_testcases} } { incr i } { + set testcase [read_testcase $i] + + test $testcase +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Generalize gdb.reverse/aarch64.exp 2016-02-24 14:40 ` Yao Qi 2016-02-24 14:40 ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi @ 2016-02-24 19:37 ` Pedro Alves 1 sibling, 0 replies; 11+ messages in thread From: Pedro Alves @ 2016-02-24 19:37 UTC (permalink / raw) To: Yao Qi, gdb-patches On 02/24/2016 02:40 PM, Yao Qi wrote: > I said we can generialize gdb.reverse/aarch64.exp for other > architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html > and here is the patch to change aarch64.exp so that it can be used to > test for other architectures as well. > > gdb/testsuite: > > 2016-02-24 Yao Qi <yao.qi@linaro.org> > > * gdb.reverse/aarch64.c: [__aarch64__] Include arm_neon.h. > (testcase_ftype): New. > (testcases): New array. > (n_testcases): New. > (main): Call each element in testcases. > * gdb.reverse/aarch64.exp: Remove is_aarch64_target check. > (read_testcase): New. > Do the tests in a loop. Thanks for splitting. LGTM. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Bug fixes in arm reverse debugging 2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi ` (2 preceding siblings ...) 2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi @ 2016-02-26 15:01 ` Yao Qi 3 siblings, 0 replies; 11+ messages in thread From: Yao Qi @ 2016-02-26 15:01 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches Yao Qi <qiyaoltc@gmail.com> writes: > This patch series extends test case gdb.reverse/aarch64.exp for general > test (it covers aarch64 and arm, and I plan to merge i386 tests to it > too) and fix some bugs on instruction decoding exposed the new tests. I pushed them in. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-02-26 15:01 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
2016-02-23 19:59 ` Luis Machado
2016-02-26 14:54 ` Yao Qi
2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi
2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi
2016-02-24 11:22 ` Pedro Alves
2016-02-24 14:40 ` Yao Qi
2016-02-24 14:40 ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi
2016-02-24 19:37 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves
2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).