[ARM] Remove field syscall_next_pc in struct gdbarch_tdep
Commit Message
Field syscall_next_pc in struct gdbarch_tdep was to calculate the
next pc of syscall instruction. On linux target, syscall_next_pc
is set to arm_linux_syscall_next_pc, to do linux specific things.
However, after we have struct arm_get_next_pcs_ops, we can do the
same thing in struct arm_get_next_pcs_ops field syscall_next_pc,
so syscall_next_pc in struct gdbarch_tdep is not needed any more.
No regressions on arm-linux.
gdb:
2016-01-13 Yao Qi <yao.qi@linaro.org>
* arm-linux-tdep.c (arm_linux_get_next_pcs_syscall_next_pc):
Declare.
(arm_linux_get_next_pcs_ops): Install
arm_linux_get_next_pcs_syscall_next_pc.
(arm_linux_syscall_next_pc): Change to ...
(arm_linux_get_next_pcs_syscall_next_pc): ... it.
(arm_linux_init_abi): Don't set tdep->syscall_next_pc.
* arm-tdep.c (arm_get_next_pcs_syscall_next_pc): Declare.
(arm_get_next_pcs_syscall_next_pc): Make it static. Don't
call tdep->syscall_next_pc.
* arm-tdep.h (struct gdbarch_tdep) <syscall_next_pc>: Remove.
(arm_get_next_pcs_syscall_next_pc): Remove.
---
gdb/arm-linux-tdep.c | 27 +++++++++++++--------------
gdb/arm-tdep.c | 16 ++++++++--------
gdb/arm-tdep.h | 7 -------
3 files changed, 21 insertions(+), 29 deletions(-)
Comments
On 01/13/2016 11:53 AM, Yao Qi wrote:
> Field syscall_next_pc in struct gdbarch_tdep was to calculate the
> next pc of syscall instruction. On linux target, syscall_next_pc
> is set to arm_linux_syscall_next_pc, to do linux specific things.
> However, after we have struct arm_get_next_pcs_ops, we can do the
> same thing in struct arm_get_next_pcs_ops field syscall_next_pc,
> so syscall_next_pc in struct gdbarch_tdep is not needed any more.
>
> No regressions on arm-linux.
>
> gdb:
>
> 2016-01-13 Yao Qi <yao.qi@linaro.org>
>
> * arm-linux-tdep.c (arm_linux_get_next_pcs_syscall_next_pc):
> Declare.
> (arm_linux_get_next_pcs_ops): Install
> arm_linux_get_next_pcs_syscall_next_pc.
> (arm_linux_syscall_next_pc): Change to ...
> (arm_linux_get_next_pcs_syscall_next_pc): ... it.
> (arm_linux_init_abi): Don't set tdep->syscall_next_pc.
> * arm-tdep.c (arm_get_next_pcs_syscall_next_pc): Declare.
> (arm_get_next_pcs_syscall_next_pc): Make it static. Don't
> call tdep->syscall_next_pc.
> * arm-tdep.h (struct gdbarch_tdep) <syscall_next_pc>: Remove.
> (arm_get_next_pcs_syscall_next_pc): Remove.
> ---
> gdb/arm-linux-tdep.c | 27 +++++++++++++--------------
> gdb/arm-tdep.c | 16 ++++++++--------
> gdb/arm-tdep.h | 7 -------
> 3 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index 6050bdf..2306bda 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -265,10 +265,14 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
> /* Syscall number for rt_sigreturn. */
> #define ARM_RT_SIGRETURN 173
>
> +static CORE_ADDR
> + arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
> + CORE_ADDR pc);
> +
Indent looks wrong on function signature line.
> /* Operation function pointers for get_next_pcs. */
> static struct arm_get_next_pcs_ops arm_linux_get_next_pcs_ops = {
> arm_get_next_pcs_read_memory_unsigned_integer,
> - arm_get_next_pcs_syscall_next_pc,
> + arm_linux_get_next_pcs_syscall_next_pc,
> arm_get_next_pcs_addr_bits_remove,
> arm_get_next_pcs_is_thumb
> };
> @@ -859,26 +863,23 @@ arm_linux_get_syscall_number (struct gdbarch *gdbarch,
> return svc_number;
> }
>
> -/* When the processor is at a syscall instruction, return the PC of the
> - next instruction to be executed. */
> -
> static CORE_ADDR
> -arm_linux_syscall_next_pc (struct regcache *regcache)
> +arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
> + CORE_ADDR pc)
> {
> - CORE_ADDR pc = regcache_read_pc (regcache);
> CORE_ADDR next_pc = 0;
> - int is_thumb = arm_is_thumb (regcache);
> + int is_thumb = arm_is_thumb (self->regcache);
> ULONGEST svc_number = 0;
> - struct gdbarch *gdbarch = get_regcache_arch (regcache);
> + struct gdbarch *gdbarch = get_regcache_arch (self->regcache);
>
> if (is_thumb)
> {
> - svc_number = regcache_raw_get_unsigned (regcache, 7);
> + svc_number = regcache_raw_get_unsigned (self->regcache, 7);
> next_pc = pc + 2;
> }
> else
> {
> - struct gdbarch *gdbarch = get_regcache_arch (regcache);
> + struct gdbarch *gdbarch = get_regcache_arch (self->regcache);
> enum bfd_endian byte_order_for_code =
> gdbarch_byte_order_for_code (gdbarch);
> unsigned long this_instr =
> @@ -891,14 +892,14 @@ arm_linux_syscall_next_pc (struct regcache *regcache)
> }
> else /* EABI. */
> {
> - svc_number = regcache_raw_get_unsigned (regcache, 7);
> + svc_number = regcache_raw_get_unsigned (self->regcache, 7);
> }
>
> next_pc = pc + 4;
> }
>
> if (svc_number == ARM_SIGRETURN || svc_number == ARM_RT_SIGRETURN)
> - next_pc = arm_linux_sigreturn_next_pc (regcache, svc_number);
> + next_pc = arm_linux_sigreturn_next_pc (self->regcache, svc_number);
>
> /* Addresses for calling Thumb functions have the bit 0 set. */
> if (is_thumb)
> @@ -1485,8 +1486,6 @@ arm_linux_init_abi (struct gdbarch_info info,
> set_gdbarch_stap_parse_special_token (gdbarch,
> arm_stap_parse_special_token);
>
> - tdep->syscall_next_pc = arm_linux_syscall_next_pc;
> -
> /* `catch syscall' */
> set_xml_syscall_file_name (gdbarch, "syscalls/arm-linux.xml");
> set_gdbarch_get_syscall_number (gdbarch, arm_linux_get_syscall_number);
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 8874ec8..ccc2a03 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -237,6 +237,11 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch,
> struct regcache *regcache,
> int regnum, const gdb_byte *buf);
>
> +static CORE_ADDR
> + arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
> + CORE_ADDR pc);
> +
> +
Ditto.
> /* get_next_pcs operations. */
> static struct arm_get_next_pcs_ops arm_get_next_pcs_ops = {
> arm_get_next_pcs_read_memory_unsigned_integer,
> @@ -6142,15 +6147,10 @@ arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
>
> /* Wrapper over syscall_next_pc for use in get_next_pcs. */
>
> -CORE_ADDR
> -arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self, CORE_ADDR pc)
> +static CORE_ADDR
> +arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
> + CORE_ADDR pc)
> {
> - struct gdbarch_tdep *tdep;
> -
> - tdep = gdbarch_tdep (get_regcache_arch (self->regcache));
> - if (tdep->syscall_next_pc != NULL)
> - return tdep->syscall_next_pc (self->regcache);
> -
> return 0;
> }
>
> diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
> index faa543e..1306cbb 100644
> --- a/gdb/arm-tdep.h
> +++ b/gdb/arm-tdep.h
> @@ -138,10 +138,6 @@ struct gdbarch_tdep
> struct type *neon_double_type;
> struct type *neon_quad_type;
>
> - /* Return the expected next PC if the program is stopped at a syscall
> - instruction. */
> - CORE_ADDR (*syscall_next_pc) (struct regcache *regcache);
> -
> /* syscall record. */
> int (*arm_syscall_record) (struct regcache *regcache, unsigned long svc_number);
> };
> @@ -261,9 +257,6 @@ ULONGEST arm_get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
> CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
> CORE_ADDR val);
>
> -CORE_ADDR arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
> - CORE_ADDR pc);
> -
> int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
>
> void arm_insert_single_step_breakpoint (struct gdbarch *,
>
Otherwise LGTM, thanks for the patch.
Antoine
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>> +static CORE_ADDR
>> + arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
>> + CORE_ADDR pc);
>> +
>
> Indent looks wrong on function signature line.
What is wrong?
On 01/14/2016 04:56 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>>> +static CORE_ADDR
>>> + arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
>>> + CORE_ADDR pc);
>>> +
>>
>> Indent looks wrong on function signature line.
>
> What is wrong?
>
Maybe it's only in the email ? but it seems like it's
static CORE_ADDR
arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
When it should be :
static CORE_ADDR
arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
Regards,
Antoine
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
> Maybe it's only in the email ? but it seems like it's
>
> static CORE_ADDR
> arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
>
> When it should be :
>
> static CORE_ADDR
> arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
No, it is intended. We don't write declaration at column zero. See
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
On 01/14/2016 08:43 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> Maybe it's only in the email ? but it seems like it's
>>
>> static CORE_ADDR
>> arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
>>
>> When it should be :
>>
>> static CORE_ADDR
>> arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
>
> No, it is intended. We don't write declaration at column zero. See
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
>
Right by bad, thanks,
Antoine
Yao Qi <qiyaoltc@gmail.com> writes:
> gdb:
>
> 2016-01-13 Yao Qi <yao.qi@linaro.org>
>
> * arm-linux-tdep.c (arm_linux_get_next_pcs_syscall_next_pc):
> Declare.
> (arm_linux_get_next_pcs_ops): Install
> arm_linux_get_next_pcs_syscall_next_pc.
> (arm_linux_syscall_next_pc): Change to ...
> (arm_linux_get_next_pcs_syscall_next_pc): ... it.
> (arm_linux_init_abi): Don't set tdep->syscall_next_pc.
> * arm-tdep.c (arm_get_next_pcs_syscall_next_pc): Declare.
> (arm_get_next_pcs_syscall_next_pc): Make it static. Don't
> call tdep->syscall_next_pc.
> * arm-tdep.h (struct gdbarch_tdep) <syscall_next_pc>: Remove.
> (arm_get_next_pcs_syscall_next_pc): Remove.
Patch is pushed in.
@@ -265,10 +265,14 @@ static const gdb_byte arm_linux_thumb2_le_breakpoint[] = { 0xf0, 0xf7, 0x00, 0xa
/* Syscall number for rt_sigreturn. */
#define ARM_RT_SIGRETURN 173
+static CORE_ADDR
+ arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
+ CORE_ADDR pc);
+
/* Operation function pointers for get_next_pcs. */
static struct arm_get_next_pcs_ops arm_linux_get_next_pcs_ops = {
arm_get_next_pcs_read_memory_unsigned_integer,
- arm_get_next_pcs_syscall_next_pc,
+ arm_linux_get_next_pcs_syscall_next_pc,
arm_get_next_pcs_addr_bits_remove,
arm_get_next_pcs_is_thumb
};
@@ -859,26 +863,23 @@ arm_linux_get_syscall_number (struct gdbarch *gdbarch,
return svc_number;
}
-/* When the processor is at a syscall instruction, return the PC of the
- next instruction to be executed. */
-
static CORE_ADDR
-arm_linux_syscall_next_pc (struct regcache *regcache)
+arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
+ CORE_ADDR pc)
{
- CORE_ADDR pc = regcache_read_pc (regcache);
CORE_ADDR next_pc = 0;
- int is_thumb = arm_is_thumb (regcache);
+ int is_thumb = arm_is_thumb (self->regcache);
ULONGEST svc_number = 0;
- struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch *gdbarch = get_regcache_arch (self->regcache);
if (is_thumb)
{
- svc_number = regcache_raw_get_unsigned (regcache, 7);
+ svc_number = regcache_raw_get_unsigned (self->regcache, 7);
next_pc = pc + 2;
}
else
{
- struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch *gdbarch = get_regcache_arch (self->regcache);
enum bfd_endian byte_order_for_code =
gdbarch_byte_order_for_code (gdbarch);
unsigned long this_instr =
@@ -891,14 +892,14 @@ arm_linux_syscall_next_pc (struct regcache *regcache)
}
else /* EABI. */
{
- svc_number = regcache_raw_get_unsigned (regcache, 7);
+ svc_number = regcache_raw_get_unsigned (self->regcache, 7);
}
next_pc = pc + 4;
}
if (svc_number == ARM_SIGRETURN || svc_number == ARM_RT_SIGRETURN)
- next_pc = arm_linux_sigreturn_next_pc (regcache, svc_number);
+ next_pc = arm_linux_sigreturn_next_pc (self->regcache, svc_number);
/* Addresses for calling Thumb functions have the bit 0 set. */
if (is_thumb)
@@ -1485,8 +1486,6 @@ arm_linux_init_abi (struct gdbarch_info info,
set_gdbarch_stap_parse_special_token (gdbarch,
arm_stap_parse_special_token);
- tdep->syscall_next_pc = arm_linux_syscall_next_pc;
-
/* `catch syscall' */
set_xml_syscall_file_name (gdbarch, "syscalls/arm-linux.xml");
set_gdbarch_get_syscall_number (gdbarch, arm_linux_get_syscall_number);
@@ -237,6 +237,11 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, const gdb_byte *buf);
+static CORE_ADDR
+ arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
+ CORE_ADDR pc);
+
+
/* get_next_pcs operations. */
static struct arm_get_next_pcs_ops arm_get_next_pcs_ops = {
arm_get_next_pcs_read_memory_unsigned_integer,
@@ -6142,15 +6147,10 @@ arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
/* Wrapper over syscall_next_pc for use in get_next_pcs. */
-CORE_ADDR
-arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self, CORE_ADDR pc)
+static CORE_ADDR
+arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
+ CORE_ADDR pc)
{
- struct gdbarch_tdep *tdep;
-
- tdep = gdbarch_tdep (get_regcache_arch (self->regcache));
- if (tdep->syscall_next_pc != NULL)
- return tdep->syscall_next_pc (self->regcache);
-
return 0;
}
@@ -138,10 +138,6 @@ struct gdbarch_tdep
struct type *neon_double_type;
struct type *neon_quad_type;
- /* Return the expected next PC if the program is stopped at a syscall
- instruction. */
- CORE_ADDR (*syscall_next_pc) (struct regcache *regcache);
-
/* syscall record. */
int (*arm_syscall_record) (struct regcache *regcache, unsigned long svc_number);
};
@@ -261,9 +257,6 @@ ULONGEST arm_get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
CORE_ADDR val);
-CORE_ADDR arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self,
- CORE_ADDR pc);
-
int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
void arm_insert_single_step_breakpoint (struct gdbarch *,