Re: [RFA] Make the prec support signal better[3/4] -- i386-linux

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: Hui Zhu <teawater@gmail.com>
> Date: Wed, 9 Sep 2009 21:29:07 +0800
> 
> This patch make i386-linux support signal record.
> When signal happen, i386_linux_record_signal will record the change.
> When the signal handler want return, new code in
> "i386_linux_intx80_sysenter_record" will record the change.
> 
>  i386-linux-tdep.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> --- a/i386-linux-tdep.c
> +++ b/i386-linux-tdep.c
> @@ -354,6 +354,33 @@ i386_linux_write_pc (struct regcache *re
>    regcache_cooked_write_unsigned (regcache, I386_LINUX_ORIG_EAX_REGNUM, -1);
>  }
> 
> +/* Record all registers for process-record.  */

Not quite true isn't it?

> +static int
> +i386_all_but_ip_registers_record (struct regcache *regcache)
> +{
> +  if (record_arch_list_add_reg (regcache, I386_EAX_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_ECX_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_EDX_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_EBX_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_ESP_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_EBP_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_ESI_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_EDI_REGNUM))
> +    return -1;
> +  if (record_arch_list_add_reg (regcache, I386_EFLAGS_REGNUM))
> +    return -1;
> +
> +  return 0;
> +}

Why exactly is it that you're not recording the instruction pointer?

>  static struct linux_record_tdep i386_linux_record_tdep;
> 
>  /* i386_canonicalize_syscall maps from the native i386 Linux set
> @@ -388,6 +415,14 @@ i386_linux_intx80_sysenter_record (struc
> 
>    regcache_raw_read_signed (regcache, I386_EAX_REGNUM, &syscall_native);
> 
> +  if (syscall_native == 119 || syscall_native == 173)
> +   {

Ugh, magic numbers.  We have symbolic constants for them isn't it?
Any reason not to use them?

> +enum i386_linux_signal_stack {
> +  xstate = 270,
> +  frame_size = 732,
> +};

IMHO, these should be #define's, spelled with all capitals and have
I386_LINUX_ prefixes.