Re: [PATCH 1/2] arm-tdep.c: Replace arguments to relocation functions by a structure

Luis Machado
2016-02-26T16:12:44Z

Hi,

I like this idea. If it ends up being accepted, we should eventually
extend it for other architectures so we can decouple this type of code
from GDB and shared it more.

On 02/25/2016 12:47 PM, Simon Marchi wrote:
> This patch prepares the functions that do the instruction
> decoding/relocating so they can be moved out of arm-tdep.c and shared
> with gdbserver. Some arguments passed around are necessary for
> relocating the instructions, but specific to GDB and the displaced
> stepping use case. Instead of passing those values by parameters
> directly, we now carry a pointer to a data structure containing them.
> This allows decoupling the decoding phase (analyzing the opcode), which
> will be shared with gdbserver, from the relocation phase (generating a
> modified instruction). When gdbserver uses those functions, it will be
> able to specify its own data structure, with different fields.
>
> gdb/ChangeLog:
>
> * arm-tdep.c (struct arm_insn_reloc_data): New structure.
> (arm_copy_unmodified): Replace args with pointer to struct
> arm_insn_reloc_data.
> (thumb_copy_unmodified_32bit): Likewise.
> (thumb_copy_unmodified_16bit):
> (install_preload): Likewise.
> (arm_copy_preload): Likewise.
> (thumb2_copy_preload): Likewise.
> (install_preload_reg): Likewise.
> (arm_copy_preload_reg): Likewise.
> (install_copro_load_store): Likewise.
> (arm_copy_copro_load_store): Likewise.
> (thumb2_copy_copro_load_store): Likewise.
> (install_b_bl_blx): Likewise.
> (arm_copy_b_bl_blx): Likewise.
> (thumb2_copy_b_bl_blx): Likewise.
> (thumb_copy_b): Likewise.
> (install_bx_blx_reg): Likewise.
> (arm_copy_bx_blx_reg): Likewise.
> (thumb_copy_bx_blx_reg): Likewise.
> (arm_copy_alu_imm): Likewise.
> (thumb2_copy_alu_imm): Likewise.
> (install_alu_reg): Likewise.
> (arm_copy_alu_reg): Likewise.
> (thumb_copy_alu_reg): Likewise.
> (install_alu_shifted_reg): Likewise.
> (arm_copy_alu_shifted_reg): Likewise.
> (arm_copy_extra_ld_st): Likewise.
> (install_load_store): Likewise.
> (thumb2_copy_load_literal): Likewise.
> (thumb2_copy_load_reg_imm): Likewise.
> (arm_copy_ldr_str_ldrb_strb): Likewise.
> (arm_copy_block_xfer): Likewise.
> (thumb2_copy_block_xfer): Likewise.
> (install_svc): Likewise.
> (arm_copy_svc): Likewise.
> (arm_copy_unpred): Likewise.
> (arm_decode_misc_memhint_neon): Likewise.
> (arm_decode_ld_st_word_ubyte): Likewise.
> (arm_decode_b_bl_ldmstm): Likewise.
> (thumb2_decode_dp_shift_reg): Likewise.
> (thumb2_decode_ext_reg_ld_st): Likewise.
> (arm_decode_svc_copro): Likewise.
> (thumb_copy_pc_relative_16bit): Likewise.
> (thumb_decode_pc_relative_16bit): Likewise.
> (thumb_copy_pc_relative_32bit): Likewise.
> (thumb_copy_16bit_ldr_literal): Likewise.
> (thumb_copy_cbnz_cbz): Likewise.
> (thumb2_copy_table_branch): Likewise.
> (thumb_copy_pop_pc_16bit): Likewise.
> (thumb_process_displaced_16bit_insn): Likewise.
> (decode_thumb_32bit_ld_mem_hints): Likewise.
> (thumb_process_displaced_32bit_insn): Likewise.
> (thumb_process_displaced_insn): Likewise.
> (arm_process_displaced_insn): Allocate a strict

Allocate a struct.

The rest looks mostly like mechanical changes, so OK to my eyes.