Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Hui Zhu <teawater@gmail.com>
To: gdb-patches ml <gdb-patches@sourceware.org>
Cc: Michael Snyder <msnyder@vmware.com>,
	Anthony Green <green@moxielogic.com>
Subject: Re: [RFA/Prec] Add signal support for process record
Date: Mon, 31 Aug 2009 08:23:00 -0000	[thread overview]
Message-ID: <daef60380908310009r22a6fe91m5d00d9e9e4349e21@mail.gmail.com> (raw)
In-Reply-To: <daef60380908292003n208cd9e2q8f9db45e918000f0@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 16998 bytes --]

On Sun, Aug 30, 2009 at 11:03, Hui Zhu<teawater@gmail.com> wrote:
> This code can be use to test this patch.
>
> Thanks,
> Hui
>
> On Thu, Aug 27, 2009 at 23:35, Hui Zhu<teawater@gmail.com> wrote:
>> Hi guys,
>>
>> I found that prec with signal has some bugs and support is not very
>> well.  So I make a patch for it.
>> What it include is:
>> 1. Fix the bug in record_wait, it always send the same sig to the
>> resume, it will make sig send to inferior again and agin.
>> 2. Add a new argument sig to arch interface "process_record".  Then
>> each arch can handle with sig with itself.  With linux,
>> TARGET_SIGNAL_0 means not send a sig.  Update the each process_record
>> of i386 and moxie.
>> 3.  Add a new interface i386_signal_record in i386 to let each taget
>> can add code to handle sig.
>>
>> Please post your comments about it.  Thanks a lot.  :)
>>
>> Hui
>>
>> 2009-08-27  Hui Zhu  <teawater@gmail.com>
>>
>>        * gdbarch.sh (process_record): Add a argument "siggnal".
>>        * record.c (record_message): Ditto.
>>        (record_wait): Set record_resume_siggnal to TARGET_SIGNAL_0.
>>        Change the last argument of record_beneath_to_resume to
>>        TARGET_SIGNAL_0.
>>        * i386-tdep.c (i386_process_record): Add a argument "siggnal".
>>        Call "i386_signal_record".
>>        * i386-tdep.h (gdbarch_tdep): Add i386_signal_record.
>>        * i386-linux-tdep.c (i386_linux_all_register_record): New
>>        function.
>>        (i386_linux_intx80_sysenter_record): Call
>>        "i386_linux_all_register_record" if the syscall is
>>        sys_sigreturn or sys_rt_sigreturn.
>>        (i386_linux_signal_record): New function.
>>        (i386_linux_init_abi): Set tdep->i386_signal_record to
>>        i386_linux_signal_record.
>>        * amd64-linux-tdep.c (amd64_linux_all_register_record): New
>>        function.
>>        (amd64_linux_syscall_record): Call
>>        "amd64_linux_all_register_record" if syscall is
>>        sys_rt_sigreturn.
>>        (amd64_linux_signal_record): New function.
>>        (amd64_linux_init_abi): Set tdep->i386_signal_record to
>>        amd64_linux_signal_record.
>>        * moxie-tdep.c (moxie_process_record): Add a argument
>>        "siggnal".
>>
>> ---
>>  amd64-linux-tdep.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  gdbarch.c          |    4 +--
>>  gdbarch.h          |    4 +--
>>  gdbarch.sh         |    2 -
>>  i386-linux-tdep.c  |   51 ++++++++++++++++++++++++++++++++++++++++++
>>  i386-tdep.c        |   21 +++++++++++++++--
>>  i386-tdep.h        |    6 ++++-
>>  moxie-tdep.c       |    7 +++--
>>  record.c           |   10 +++++---
>>  9 files changed, 152 insertions(+), 16 deletions(-)
>>
>> --- a/amd64-linux-tdep.c
>> +++ b/amd64-linux-tdep.c
>> @@ -275,6 +275,47 @@ static struct linux_record_tdep amd64_li
>>  #define RECORD_ARCH_GET_GS     0x1004
>>
>>  static int
>> +amd64_linux_all_register_record (struct regcache *regcache)
>> +{
>> +  if (record_arch_list_add_reg (regcache, AMD64_RAX_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RCX_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RDX_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RBX_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RSP_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RBP_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RSI_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_RDI_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R8_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R9_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R10_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R11_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R12_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R13_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R14_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_R15_REGNUM))
>> +    return -1;
>> +  if (record_arch_list_add_reg (regcache, AMD64_EFLAGS_REGNUM))
>> +    return -1;
>> +
>> +  return 0;
>> +}
>> +
>> +static int
>>  amd64_linux_syscall_record (struct regcache *regcache)
>>  {
>>   int ret, num = -1;
>> @@ -347,7 +388,9 @@ amd64_linux_syscall_record (struct regca
>>       break;
>>       /* sys_rt_sigreturn */
>>     case 15:
>> -      num = 173;
>> +      if (amd64_linux_all_register_record (regcache))
>> +        return -1;
>> +      return 0;
>>       break;
>>       /* sys_ioctl */
>>     case 16:
>> @@ -1384,6 +1427,22 @@ amd64_linux_syscall_record (struct regca
>>   return 0;
>>  }
>>
>> +static int
>> +amd64_linux_signal_record (struct regcache *regcache,
>> +                           enum target_signal signal)
>> +{
>> +  if (signal == TARGET_SIGNAL_0)
>> +    return 100;
>> +
>> +  if (amd64_linux_all_register_record (regcache))
>> +    return -1;
>> +
>> +  if (record_arch_list_add_reg (regcache, AMD64_RIP_REGNUM))
>> +    return -1;
>> +
>> +  return 0;
>> +}
>> +
>>  static void
>>  amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>  {
>> @@ -1599,6 +1658,8 @@ amd64_linux_init_abi (struct gdbarch_inf
>>   amd64_linux_record_tdep.arg6 = AMD64_R9_REGNUM;
>>
>>   tdep->i386_syscall_record = amd64_linux_syscall_record;
>> +
>> +  tdep->i386_signal_record = amd64_linux_signal_record;
>>  }
>>
>>
>> --- a/gdbarch.c
>> +++ b/gdbarch.c
>> @@ -3282,13 +3282,13 @@ gdbarch_process_record_p (struct gdbarch
>>  }
>>
>>  int
>> -gdbarch_process_record (struct gdbarch *gdbarch, struct regcache
>> *regcache, CORE_ADDR addr)
>> +gdbarch_process_record (struct gdbarch *gdbarch, struct regcache
>> *regcache, CORE_ADDR addr, enum target_signal siggnal)
>>  {
>>   gdb_assert (gdbarch != NULL);
>>   gdb_assert (gdbarch->process_record != NULL);
>>   if (gdbarch_debug >= 2)
>>     fprintf_unfiltered (gdb_stdlog, "gdbarch_process_record called\n");
>> -  return gdbarch->process_record (gdbarch, regcache, addr);
>> +  return gdbarch->process_record (gdbarch, regcache, addr, siggnal);
>>  }
>>
>>  void
>> --- a/gdbarch.h
>> +++ b/gdbarch.h
>> @@ -817,8 +817,8 @@ extern void set_gdbarch_sofun_address_ma
>>
>>  extern int gdbarch_process_record_p (struct gdbarch *gdbarch);
>>
>> -typedef int (gdbarch_process_record_ftype) (struct gdbarch *gdbarch,
>> struct regcache *regcache, CORE_ADDR addr);
>> -extern int gdbarch_process_record (struct gdbarch *gdbarch, struct
>> regcache *regcache, CORE_ADDR addr);
>> +typedef int (gdbarch_process_record_ftype) (struct gdbarch *gdbarch,
>> struct regcache *regcache, CORE_ADDR addr, enum target_signal
>> siggnal);
>> +extern int gdbarch_process_record (struct gdbarch *gdbarch, struct
>> regcache *regcache, CORE_ADDR addr, enum target_signal siggnal);
>>  extern void set_gdbarch_process_record (struct gdbarch *gdbarch,
>> gdbarch_process_record_ftype *process_record);
>>
>>  /* Signal translation: translate inferior's signal (host's) number into
>> --- a/gdbarch.sh
>> +++ b/gdbarch.sh
>> @@ -707,7 +707,7 @@ v:int:sofun_address_maybe_missing:::0:0:
>>  # the registers REGCACHE and memory ranges that will be affected when
>>  # the instruction executes, along with their current values.
>>  # Return -1 if something goes wrong, 0 otherwise.
>> -M:int:process_record:struct regcache *regcache, CORE_ADDR addr:regcache, addr
>> +M:int:process_record:struct regcache *regcache, CORE_ADDR addr, enum
>> target_signal siggnal:regcache, addr, siggnal
>>
>>  # Signal translation: translate inferior's signal (host's) number into
>>  # GDB's representation.
>> --- a/i386-linux-tdep.c
>> +++ b/i386-linux-tdep.c
>> @@ -364,6 +364,31 @@ i386_linux_write_pc (struct regcache *re
>>  static struct linux_record_tdep i386_linux_record_tdep;
>>
>>  static int
>> +i386_linux_all_register_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;
>> +}
>> +
>> +static int
>>  i386_linux_intx80_sysenter_record (struct regcache *regcache)
>>  {
>>   int ret;
>> @@ -378,6 +403,14 @@ i386_linux_intx80_sysenter_record (struc
>>       return -1;
>>     }
>>
>> +  if (tmpu32 == 119 || tmpu32 == 173)
>> +    {
>> +      /* sys_sigreturn sys_rt_sigreturn */
>> +      if (i386_linux_all_register_record (regcache))
>> +        return -1;
>> +      return 0;
>> +    }
>> +
>>   ret = record_linux_system_call (tmpu32, regcache,
>>                                  &i386_linux_record_tdep);
>>   if (ret)
>> @@ -389,6 +422,22 @@ i386_linux_intx80_sysenter_record (struc
>>
>>   return 0;
>>  }
>> +
>> +static int
>> +i386_linux_signal_record (struct regcache *regcache,
>> +                          enum target_signal signal)
>> +{
>> +  if (signal == TARGET_SIGNAL_0)
>> +    return 100;
>> +
>> +  if (i386_linux_all_register_record (regcache))
>> +    return -1;
>> +
>> +  if (record_arch_list_add_reg (regcache, I386_EIP_REGNUM))
>> +    return -1;
>> +
>> +  return 0;
>> +}
>>
>>
>>  /* The register sets used in GNU/Linux ELF core-dumps are identical to
>> @@ -647,6 +696,8 @@ i386_linux_init_abi (struct gdbarch_info
>>   tdep->i386_intx80_record = i386_linux_intx80_sysenter_record;
>>   tdep->i386_sysenter_record = i386_linux_intx80_sysenter_record;
>>
>> +  tdep->i386_signal_record = i386_linux_signal_record;
>> +
>>   /* N_FUN symbols in shared libaries have 0 for their values and need
>>      to be relocated. */
>>   set_gdbarch_sofun_address_maybe_missing (gdbarch, 1);
>> --- a/i386-tdep.c
>> +++ b/i386-tdep.c
>> @@ -3205,7 +3205,7 @@ i386_record_push (struct i386_record_s *
>>
>>  int
>>  i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
>> -                    CORE_ADDR addr)
>> +                    CORE_ADDR addr, enum target_signal signal)
>>  {
>>   int prefixes = 0;
>>   uint8_t tmpu8;
>> @@ -3230,8 +3230,23 @@ i386_process_record (struct gdbarch *gdb
>>
>>   if (record_debug > 1)
>>     fprintf_unfiltered (gdb_stdlog, "Process record: i386_process_record "
>> -                                   "addr = %s\n",
>> -                       paddress (gdbarch, ir.addr));
>> +                                   "addr = %s signal = %s\n",
>> +                       paddress (gdbarch, ir.addr),
>> +                        target_signal_to_name (signal));
>> +
>> +  if (gdbarch_tdep (gdbarch)->i386_signal_record != NULL)
>> +    {
>> +      int ret;
>> +
>> +      /* If this signal is not be handled, return 100.  */
>> +      ret = gdbarch_tdep (gdbarch)->i386_signal_record (ir.regcache, signal);
>> +      if (ret < 100)
>> +        {
>> +          if (record_arch_list_add_end ())
>> +            return -1;
>> +          return ret;
>> +        }
>> +    }
>>
>>   /* prefixes */
>>   while (1)
>> --- a/i386-tdep.h
>> +++ b/i386-tdep.h
>> @@ -114,6 +114,9 @@ struct gdbarch_tdep
>>   /* The map for registers because the AMD64's registers order
>>      in GDB is not same as I386 instructions.  */
>>   const int *record_regmap;
>> +  /* Parse signal args.  */
>> +  int (*i386_signal_record) (struct regcache *regcache,
>> +                             enum target_signal signal);
>>   /* Parse intx80 args.  */
>>   int (*i386_intx80_record) (struct regcache *regcache);
>>   /* Parse sysenter args.  */
>> @@ -261,7 +264,8 @@ extern void i386_elf_init_abi (struct gd
>>  extern void i386_svr4_init_abi (struct gdbarch_info, struct gdbarch *);
>>
>>  extern int i386_process_record (struct gdbarch *gdbarch,
>> -                                struct regcache *regcache, CORE_ADDR addr);
>> +                                struct regcache *regcache, CORE_ADDR addr,
>> +                                enum target_signal signal);
>>
>>
>>  /* Functions and variables exported from i386bsd-tdep.c.  */
>> --- a/moxie-tdep.c
>> +++ b/moxie-tdep.c
>> @@ -501,7 +501,7 @@ moxie_process_readu (CORE_ADDR addr, cha
>>
>>  int
>>  moxie_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
>> -                     CORE_ADDR addr)
>> +                     CORE_ADDR addr, enum target_signal signal)
>>  {
>>   gdb_byte buf[4];
>>   uint16_t inst;
>> @@ -510,8 +510,9 @@ moxie_process_record (struct gdbarch *gd
>>
>>   if (record_debug > 1)
>>     fprintf_unfiltered (gdb_stdlog, "Process record: moxie_process_record "
>> -                                   "addr = 0x%s\n",
>> -                       paddress (target_gdbarch, addr));
>> +                                   "addr = 0x%s signal = %s\n",
>> +                       paddress (target_gdbarch, addr),
>> +                       target_signal_to_name (signal));
>>
>>   inst = (uint16_t) moxie_process_readu (addr, buf, 2, byte_order);
>>
>> --- a/record.c
>> +++ b/record.c
>> @@ -92,6 +92,8 @@ static int record_stop_at_limit = 1;
>>  static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
>>  static int record_insn_num = 0;
>>
>> +static enum target_signal record_resume_siggnal;
>> +
>>  /* The target_ops of process record.  */
>>  static struct target_ops record_ops;
>>
>> @@ -373,7 +375,8 @@ record_message (void *args)
>>
>>   ret = gdbarch_process_record (get_regcache_arch (regcache),
>>                                regcache,
>> -                               regcache_read_pc (regcache));
>> +                               regcache_read_pc (regcache),
>> +                                record_resume_siggnal);
>>   if (ret > 0)
>>     error (_("Process record: inferior program stopped."));
>>   if (ret < 0)
>> @@ -514,7 +517,6 @@ record_close (int quitting)
>>  }
>>
>>  static int record_resume_step = 0;
>> -static enum target_signal record_resume_siggnal;
>>  static int record_resume_error;
>>
>>  static void
>> @@ -606,6 +608,8 @@ record_wait (struct target_ops *ops,
>>          ptid_t ret;
>>          CORE_ADDR tmp_pc;
>>
>> +          record_resume_siggnal = TARGET_SIGNAL_0;
>> +
>>          while (1)
>>            {
>>              ret = record_beneath_to_wait (record_beneath_to_wait_ops,
>> @@ -638,7 +642,7 @@ record_wait (struct target_ops *ops,
>>                        }
>>                      record_beneath_to_resume (record_beneath_to_resume_ops,
>>                                                ptid, 1,
>> -                                               record_resume_siggnal);
>> +                                               TARGET_SIGNAL_0);
>>                      continue;
>>                    }
>>                }
>>
>

Hi guys,

After discussion with Michael,  we decide to deal with this issue step by step.
This is the fix bug patch.  It will fix the after get sig, inferior
get a lot of sig.  Please help me with it.

Thanks,
Hui


2009-08-31  Hui Zhu  <teawater@gmail.com>

	* record.c (record_resume_siggnal): Deleted.
	(record_resume): Ditto.
	(record_wait): Change record_resume_siggnal to
	TARGET_SIGNAL_0.

---
 record.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- a/record.c
+++ b/record.c
@@ -514,7 +514,6 @@ record_close (int quitting)
 }

 static int record_resume_step = 0;
-static enum target_signal record_resume_siggnal;
 static int record_resume_error;

 static void
@@ -522,7 +521,6 @@ record_resume (struct target_ops *ops, p
                enum target_signal siggnal)
 {
   record_resume_step = step;
-  record_resume_siggnal = siggnal;

   if (!RECORD_IS_REPLAY)
     {
@@ -638,7 +636,7 @@ record_wait (struct target_ops *ops,
 			}
 		      record_beneath_to_resume (record_beneath_to_resume_ops,
 						ptid, 1,
-						record_resume_siggnal);
+						TARGET_SIGNAL_0);
 		      continue;
 		    }
 		}

[-- Attachment #2: prec-fix-signal-bug.txt --]
[-- Type: text/plain, Size: 734 bytes --]

---
 record.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--- a/record.c
+++ b/record.c
@@ -514,7 +514,6 @@ record_close (int quitting)
 }
 
 static int record_resume_step = 0;
-static enum target_signal record_resume_siggnal;
 static int record_resume_error;
 
 static void
@@ -522,7 +521,6 @@ record_resume (struct target_ops *ops, p
                enum target_signal siggnal)
 {
   record_resume_step = step;
-  record_resume_siggnal = siggnal;
 
   if (!RECORD_IS_REPLAY)
     {
@@ -638,7 +636,7 @@ record_wait (struct target_ops *ops,
 			}
 		      record_beneath_to_resume (record_beneath_to_resume_ops,
 						ptid, 1,
-						record_resume_siggnal);
+						TARGET_SIGNAL_0);
 		      continue;
 		    }
 		}

  reply	other threads:[~2009-08-31  7:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-27 16:50 Hui Zhu
2009-08-30  3:09 ` Hui Zhu
2009-08-31  8:23   ` Hui Zhu [this message]
2009-08-31 17:54     ` Michael Snyder
2009-09-01  4:35       ` Hui Zhu

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=daef60380908310009r22a6fe91m5d00d9e9e4349e21@mail.gmail.com \
    --to=teawater@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=green@moxielogic.com \
    --cc=msnyder@vmware.com \
    /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