From: Yao Qi <yao@codesourcery.com>
To: Omair Javaid <omair.javaid@linaro.org>
Cc: <gdb-patches@sourceware.org>, Patch Tracking <patches@linaro.org>
Subject: Re: [PATCH 1/2] GDB process record and reverse debugging improvements for arm*-linux*
Date: Thu, 24 Oct 2013 02:25:00 -0000 [thread overview]
Message-ID: <526884D5.309@codesourcery.com> (raw)
In-Reply-To: <CANW4E-2g0ty_BP7EK2CHV1h9ZmDuKR_wBC5qsqWMNxHbZHAwSw@mail.gmail.com>
On 10/24/2013 08:09 AM, Omair Javaid wrote:
> gdb:
>
> 2013-10-24 Omair Javaid<omair.javaid@linaro.org>
>
> * arm-tdep.c (struct arm_mem_r): Update.
"Update" is too general. Please describe what is changed.
> (arm_record_coproc_data_proc): Update.
> (thumb_record_ldm_stm_swi): Update.
> (thumb2_record_ld_st_multiple): New function.
> (thumb2_record_ld_st_dual_ex_tbb): New function.
> (thumb2_record_data_proc_sreg_mimm): New function.
> (thumb2_record_unsupported_insn): New function.
> (thumb2_record_ps_dest_generic): New function.
> (thumb2_record_branch_misc_cntrl): New function.
> (thumb2_record_str_single_data): New function.
> (thumb2_record_ld_mem_hints): New function.
> (thumb2_record_ld_word): New function.
> (thumb2_record_lmul_lmla_div): New function.
> (thumb2_record_decode_inst_id): New function.
> (decode_insn): Update.
Each line of changelog should be prefixed by tab.
> + {
> + if (tdep->arm_syscall_record != NULL)
> + {
> + ULONGEST svc_operand, svc_number;
>
> - printf_unfiltered (_("Process record does not support instruction "
> + svc_operand = (0x00ffffff & arm_insn_r->arm_insn);
> +
> + if (svc_operand) /* OABI. */
> + {
> + svc_number = svc_operand - 0x900000;
> + }
Unnecessary braces.
> + else /* EABI. */
> + {
> + regcache_raw_read_unsigned (reg_cache, 7, &svc_number);
> + }
Unnecessary braces.
> +
> + ret = tdep->arm_syscall_record(reg_cache, svc_number);
> + }
> + else
> + {
> + printf_unfiltered (_("no syscall record support\n"));
> + ret = -1;
> + }
> + }
> + else
> + {
> + printf_unfiltered (_("Process record does not support instruction "
> "0x%0x at address %s.\n"),arm_insn_r->arm_insn,
> paddress (arm_insn_r->gdbarch, arm_insn_r->this_addr));
I find this statements appear many times, probably, we can move it into
a function named arm_record_unsupported_insn.
> + ret = -1;
> + }
> return ret;
> }
>
> @@ -12361,9 +12377,10 @@ thumb_record_ldm_stm_swi (insn_decode_re
> else if (0x1F == opcode1)
> {
> /* Handle arm syscall insn. */
> - if (tdep->arm_swi_record != NULL)
> + if (tdep->arm_syscall_record != NULL)
> {
> - ret = tdep->arm_swi_record(reg_cache);
> + regcache_raw_read_unsigned (reg_cache, 7, &u_regval);
> + ret = tdep->arm_syscall_record(reg_cache, u_regval);
Space is needed before "(". This change belongs to support recording
syscall instructions, and IWBN to move this part to patch 2/2.
> }
> else
> {
> @@ -12414,6 +12431,630 @@ thumb_record_branch (insn_decode_record
> return 0;
> }
>
> +/* Handler for thumb2 load/store multiple instructions. */
> +
> +static int
> +thumb2_record_ld_st_multiple (insn_decode_record *thumb2_insn_r)
> +{
> + struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> + uint32_t reg_rn, op;
> + uint32_t register_bits = 0, register_count = 0;
> + uint32_t index = 0, start_address = 0;
> + uint32_t record_buf[24], record_buf_mem[48];
> +
> + ULONGEST u_regval = 0;
> +
> + reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> + op = bits (thumb2_insn_r->arm_insn, 23, 24);
> +
> + if (0 == op || 3 == op)
> + {
> + if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> + {
> + /* Handle RFE instruction. */
> + record_buf[0] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 1;
> + }
> + else
> + {
> + /* Handle SRS instruction after reading banked SP. */
> + printf_unfiltered (_("Process record does not support instruction "
> + "0x%0x at address %s.\n"),
> + thumb2_insn_r->arm_insn,
> + paddress (thumb2_insn_r->gdbarch,
> + thumb2_insn_r->this_addr));
Use thumb2_record_unsupported_insn?
> + return -1;
> + }
> + }
> + else if(1 == op || 2 == op)
> + {
> + if (bit (thumb2_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> + {
> + /* Handle LDM/LDMIA/LDMFD and LDMDB/LDMEA instructions. */
> + register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> + while (register_bits)
> + {
> + if (register_bits & 0x00000001)
> + {
> + record_buf[index++] = register_count;
> + }
Unnecessary braces.
> + register_count++;
> + register_bits = register_bits >> 1;
> + }
> + record_buf[index++] = reg_rn;
> + record_buf[index++] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = index;
> + }
> + else
> + {
> + /* Handle STM/STMIA/STMEA and STMDB/STMFD. */
> + register_bits = bits (thumb2_insn_r->arm_insn, 0, 15);
> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> + while (register_bits)
> + {
> + if (register_bits & 0x00000001)
> + {
> + register_count++;
> + }
Unnecessary braces.
> + register_bits = register_bits >> 1;
> + }
> +
> + if (1 == op)
> + {
> + /* Start address calculation for LDMDB/LDMEA. */
> + start_address = u_regval;
> + }
> + else if (2 == op)
> + {
> + /* Start address calculation for LDMDB/LDMEA. */
> + start_address = (u_regval) - (register_count * 4);
> + }
> +
> + thumb2_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--;
> + }
> + record_buf[0] = reg_rn;
> + record_buf[1] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 2;
> + }
> + }
> +
> + MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> + record_buf_mem);
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> + return 0;
> +}
> +
> +/* Handler for thumb2 ld/st dual, ld/st exclusive, table branch
> instructions. */
Looks your mailer wraps the patch. See
https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches
on how to adjust your mailer.
> +
> +/* Handler for thumb2 data processing (shift register and modified immediate)
> + instructions. */
> +
> +static int
> +thumb2_record_data_proc_sreg_mimm (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t ret = 0; /* Return value: -1:record failure ; 0:success. */
This function always return 0.
> + uint32_t reg_rd, op;
> + uint32_t record_buf[8];
> +
> + op = bits (thumb2_insn_r->arm_insn, 21, 24);
> + reg_rd = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> + if ((0 == op || 4 == op || 8 == op || 13 == op) && 15 == reg_rd)
> + {
> + record_buf[0] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 1;
> + }
> + else
> + {
> + record_buf[0] = reg_rd;
> + record_buf[1] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 2;
> + }
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> +
> + return ret;
> +}
> +
> +
> +/* Generic handler for thumb2 instructions which need to save PS and
> + destination registers is saved. Handles multiply, multiply accumulate,
Two spaces after period.
> + and absolute difference instructions. Also handles data-processing
and here.
> + (register and plain binary immediate) instructions. */
> +
> +static int
> +thumb2_record_ps_dest_generic (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t ret = 0; /* Return value: -1:record failure ; 0:success. */
Document the meaning of return value in function header comment and
remove this line of comment.
> +
> +/* Handler for thumb2 branch and miscellaneous control instructions. */
> +
> +static int
> +thumb2_record_branch_misc_cntrl (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t op, op1, op2;
> + uint32_t record_buf[8];
> +
> + op = bits (thumb2_insn_r->arm_insn, 20, 26);
> + op1 = bits (thumb2_insn_r->arm_insn, 12, 14);
> + op2 = bits (thumb2_insn_r->arm_insn, 8, 11);
> +
> + /* Handle MSR insn. */
> + if (!(op1 & 0x2) && 0x38 == op)
> + {
> + if (!(op2 & 0x3))
> + {
> + /* CPSR is going to be changed. */
> + record_buf[0] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 1;
> + }
> + else
> + {
> + /* SPSR is going to be changed. */
> + printf_unfiltered (_("Process record does not support instruction "
> + "0x%0x at address %s.\n"),
> + thumb2_insn_r->arm_insn,
> + paddress (thumb2_insn_r->gdbarch,
> + thumb2_insn_r->this_addr));
Use thumb2_record_unsupported_insn?
> + return -1;
> + }
> + }
> + else if (4 == (op1 & 0x5) || 5 == (op1 & 0x5))
> + {
> + /* BLX. */
> + record_buf[0] = ARM_PS_REGNUM;
> + record_buf[1] = ARM_LR_REGNUM;
> + thumb2_insn_r->reg_rec_count = 2;
> + }
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> +
> + return 0;
> +}
> +
> +/* Handler for thumb2 store single data item instructions. */
> +
> +static int
> +thumb2_record_str_single_data (insn_decode_record *thumb2_insn_r)
> +{
> + struct regcache *reg_cache = thumb2_insn_r->regcache;
> +
> + uint32_t reg_rn, reg_rm, offset_imm, shift_imm;
> + uint32_t address, offset_addr;
> + uint32_t record_buf[8], record_buf_mem[8];
> + uint32_t op1, op2;
> +
> + ULONGEST u_regval[2];
> +
> + op1 = bits (thumb2_insn_r->arm_insn, 21, 23);
> + op2 = bits (thumb2_insn_r->arm_insn, 6, 11);
> + reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval[0]);
> +
> + if (bit (thumb2_insn_r->arm_insn, 23))
> + {
> + /* T2 encoding. */
> + offset_imm = bits (thumb2_insn_r->arm_insn, 0, 11);
> + offset_addr = u_regval[0] + offset_imm;
> + address = offset_addr;
> + }
> + else
> + {
> + /* T3 encoding. */
> + if ((0 == op1 || 1 == op1 || 2 == op1) && !(op2 & 0x20))
> + {
> + /* Handle STRB (register). */
> + reg_rm = bits (thumb2_insn_r->arm_insn, 0, 3);
> + regcache_raw_read_unsigned (reg_cache, reg_rm, &u_regval[1]);
> + shift_imm = bits (thumb2_insn_r->arm_insn, 4, 5);
> + offset_addr = u_regval[1] << shift_imm;
> + address = u_regval[0] + offset_addr;
> + }
> + else
> + {
> + offset_imm = bits (thumb2_insn_r->arm_insn, 0, 7);
> + if (bit (thumb2_insn_r->arm_insn, 10))
> + {
> + if (bit (thumb2_insn_r->arm_insn, 9))
> + {
> + offset_addr = u_regval[0] + offset_imm;
> + }
Unnecessary braces.
> + else
> + {
> + offset_addr = u_regval[0] - offset_imm;
> + }
> + address = offset_addr;
> + }
> + else
> + {
> + address = u_regval[0];
> + }
Unnecessary braces.
> + }
> + }
> +
> + switch (op1)
> + {
> + /* Store byte instructions. */
> + case 4:
> + case 0:
> + record_buf_mem[0] = 1;
> + break;
> + /* Store half word instructions. */
> + case 1:
> + case 5:
> + record_buf_mem[0] = 2;
> + break;
> + /* Store word instructions. */
> + case 2:
> + case 6:
> + record_buf_mem[0] = 4;
> + break;
> +
> + default:
> + gdb_assert_not_reached ("no decoding pattern found");
> + break;
> + }
> +
> + record_buf_mem[1] = address;
> + thumb2_insn_r->mem_rec_count = 1;
> + record_buf[0] = reg_rn;
> + thumb2_insn_r->reg_rec_count = 1;
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> + MEM_ALLOC (thumb2_insn_r->arm_mems, thumb2_insn_r->mem_rec_count,
> + record_buf_mem);
> + return 0;
> +}
> +
> +/* Handler for thumb2 load memory hints instructions. */
> +
> +static int
> +thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t record_buf[8];
> + uint32_t reg_rt, reg_rn;
> +
> + reg_rt = bits (thumb2_insn_r->arm_insn, 12, 15);
> + reg_rn = bits (thumb2_insn_r->arm_insn, 16, 19);
> +
> + if (15 != reg_rt)
Replace 15 with ARM_PC_REGNUM.
> + {
> + record_buf[0] = reg_rt;
> + record_buf[1] = reg_rn;
> + record_buf[2] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 3;
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> + return 0;
> + }
> +
> + return -1;
Is PC allowed to be used in load memory hints? If reference manual says
PC is not allowed, we don't have to worry about "reg_rt == 15".
> +}
> +
> +/* Handler for thumb2 load word instructions. */
> +
> +static int
> +thumb2_record_ld_word (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t ret = 0; /* Return value: -1:record failure ; 0:success. */
Looks this function always return 0.
> + uint32_t opcode1 = 0, opcode2 = 0;
> + uint32_t record_buf[8];
> +
> + record_buf[0] = bits (thumb2_insn_r->arm_insn, 12, 15);
> + record_buf[1] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 2;
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> +
> + return ret;
"return 0;"
> +}
> +
> +/* Handler for thumb2 long multiply, long multiply accumulate, and
> + divide instructions. */
> +
> +static int
> +thumb2_record_lmul_lmla_div (insn_decode_record *thumb2_insn_r)
> +{
> + uint32_t ret = 0; /* Return value: -1:record failure ; 0:success. */
Move this line of comment to function header comment.
> + uint32_t opcode1 = 0, opcode2 = 0;
> + uint32_t record_buf[8];
> + uint32_t reg_src1 = 0;
> +
> + opcode1 = bits (thumb2_insn_r->arm_insn, 20, 22);
> + opcode2 = bits (thumb2_insn_r->arm_insn, 4, 7);
> +
> + if (0 == opcode1 || 2 == opcode1 || (opcode1 >= 4 && opcode1 <= 6))
> + {
> + /* Handle SMULL, UMULL, SMULAL. */
> + /* Handle SMLAL(S), SMULL(S), UMLAL(S), UMULL(S). */
> + record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> + record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> + record_buf[2] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 3;
> + }
> + else if (1 == opcode1 || 3 == opcode2)
> + {
> + /* Handle SDIV and UDIV. */
> + record_buf[0] = bits (thumb2_insn_r->arm_insn, 16, 19);
> + record_buf[1] = bits (thumb2_insn_r->arm_insn, 12, 15);
> + record_buf[2] = ARM_PS_REGNUM;
> + thumb2_insn_r->reg_rec_count = 3;
> + }
> + else
> + {
> + ret = -1;
> + }
Unncessary braces.
> +
> + REG_ALLOC (thumb2_insn_r->arm_regs, thumb2_insn_r->reg_rec_count,
> + record_buf);
> +
> + return ret;
> +}
> +
>
> /* Extracts arm/thumb/thumb2 insn depending on the size, and returns
> 0 on success
> and positive val on fauilure. */
> @@ -12469,6 +13110,27 @@ decode_insn (insn_decode_record *arm_rec
> thumb_record_branch /* 111. */
> };
>
> + /* (Starting from numerical 0); bits 13,14,15 decodes type of thumb
> instruction. */
> + static const sti_arm_hdl_fp_t const thumb2_handle_insn[16] =
> + { \
> + thumb2_record_ld_st_multiple, /* 00. */
> + thumb2_record_ld_st_dual_ex_tbb, /* 01. */
> + thumb2_record_data_proc_sreg_mimm, /* 02. */
> + thumb2_record_unsupported_insn, /* 03. */
> + thumb2_record_data_proc_sreg_mimm, /* 04. */
> + thumb2_record_ps_dest_generic, /* 05. */
> + thumb2_record_branch_misc_cntrl, /* 06. */
> + thumb2_record_str_single_data, /* 07. */
> + thumb2_record_unsupported_insn, /* 08. */
> + thumb2_record_ld_mem_hints, /* 09. */
> + thumb2_record_ld_mem_hints, /* 10. */
> + thumb2_record_ld_word, /* 11. */
> + thumb2_record_ps_dest_generic, /* 12. */
> + thumb2_record_ps_dest_generic, /* 13. */
> + thumb2_record_lmul_lmla_div, /* 14. */
> + thumb2_record_unsupported_insn /* 15. */
> + };
> +
> uint32_t ret = 0; /* return value: negative:failure 0:success. */
> uint32_t insn_id = 0;
>
> @@ -12503,11 +13165,27 @@ decode_insn (insn_decode_record *arm_rec
> }
> else if (THUMB2_RECORD == record_type)
> {
> - printf_unfiltered (_("Process record doesnt support thumb32 instruction "
> - "0x%0x at address %s.\n"),arm_record->arm_insn,
> - paddress (arm_record->gdbarch,
> - arm_record->this_addr));
> - ret = -1;
> + /* As thumb does not have condition codes, we set negative. */
> + arm_record->cond = -1;
> +
> + /* Swap first half of 32bit thumb instruction with second half. */
> + arm_record->arm_insn = (arm_record->arm_insn >> 16) |
> + (arm_record->arm_insn << 16);
> +
> + insn_id = thumb2_record_decode_inst_id (arm_record->arm_insn);
> +
> + if (insn_id >= 0)
> + {
> + ret = thumb2_handle_insn[insn_id] (arm_record);
> + }
Unnessary braces.
> + else
> + {
> + printf_unfiltered (_("Process record doesnt support instruction "
> + "0x%0x at address %s.\n"),arm_record->arm_insn,
> + paddress (arm_record->gdbarch,
> + arm_record->this_addr));
> + ret = -1;
> + }
Use thumb2_record_unsupported_insn?
--
Yao (é½å°§)
next prev parent reply other threads:[~2013-10-24 2:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-24 0:09 Omair Javaid
2013-10-24 2:25 ` Yao Qi [this message]
2013-11-08 3:20 ` Omair Javaid
2013-11-11 10:53 ` Yao Qi
2013-11-25 0:05 ` Omair Javaid
2013-11-25 14:23 ` Oza Pawandeep
2013-11-27 23:58 ` Omair Javaid
2013-11-28 12:30 ` Yao Qi
2013-12-17 10:22 ` Omair Javaid
2013-12-20 12:37 ` Pedro Alves
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=526884D5.309@codesourcery.com \
--to=yao@codesourcery.com \
--cc=gdb-patches@sourceware.org \
--cc=omair.javaid@linaro.org \
--cc=patches@linaro.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