Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Michael Snyder <msnyder@vmware.com>
To: Hui Zhu <teawater@gmail.com>
Cc: gdb-patches ml <gdb-patches@sourceware.org>
Subject: Re: [RFA/RFC] Prec multi-thread support [2/4] arch support
Date: Wed, 25 Nov 2009 20:09:00 -0000	[thread overview]
Message-ID: <4B0D8DF3.8090307@vmware.com> (raw)
In-Reply-To: <daef60380911250005x35202669i1851165239f2f64e@mail.gmail.com>

Hui Zhu wrote:
> 1. In record_linux_system_call gdb_sys_clone, I set record_step to 0.
> Then the record target will let inferior continue until to a
> breakpoint.  Most of time, record just let inferior step.  But
> sys_clone is very special.  It cannot work OK with
> PTRACE_SINGLESTEP.(It make me hang this patch a log of months.)
> 
> 2. Add new argument "addr" that point to the next code that need to be
> analyzed to linux syscall record function "record_linux_system_call".

I'm OK with #2.
I need to spend some time looking at #1.
But there's another change in here that you didn't mention --
you added "if (record_debug) around some warnings.

Would  you take that change out, please, and submit it separately?
It isn't really a part of adding multi-thread support, since this
warning shows up frequently in single-thread prec debugging.


> 
> 2009-11-25  Hui Zhu  <teawater@gmail.com>
> 
> 	* amd64-linux-tdep.c (amd64_linux_syscall_record): Add new
> 	argument "addr".
> 	* i386-linux-tdep.c (i386_linux_intx80_sysenter_record): Ditto.
> 	* i386-tdep.c (i386_record_lea_modrm): Add "if (record_debug)"
> 	to segment register warning.
> 	(i386_process_record): Ditto.  Add new argument "addr" when
> 	call i386_intx80_record, i386_sysenter_record and
> 	i386_syscall_record.
> 	* i386-tdep.h (gdbarch_tdep): Add new argument "addr" when
> 	call i386_intx80_record, i386_sysenter_record and
> 	i386_syscall_record.
> 	* linux-record.c (inferior.h): New include.
> 	(record_linux_system_call): Add new argument "addr".
> 	Update code for gdb_sys_clone.
> 	* linux-record.h (record_linux_system_call): Add new argument
> 	"addr".
> 
> ---
>  amd64-linux-tdep.c |    4 ++--
>  i386-linux-tdep.c  |    4 ++--
>  i386-tdep.c        |   52 +++++++++++++++++++++++++++++-----------------------
>  i386-tdep.h        |    6 +++---
>  linux-record.c     |   13 +++++++++----
>  linux-record.h     |    2 +-
>  6 files changed, 46 insertions(+), 35 deletions(-)
> 
> --- a/amd64-linux-tdep.c
> +++ b/amd64-linux-tdep.c
> @@ -1155,7 +1155,7 @@ static struct linux_record_tdep amd64_li
>  #define RECORD_ARCH_GET_GS	0x1004
> 
>  static int
> -amd64_linux_syscall_record (struct regcache *regcache)
> +amd64_linux_syscall_record (struct regcache *regcache, CORE_ADDR addr)
>  {
>    int ret;
>    ULONGEST syscall_native;
> @@ -1205,7 +1205,7 @@ amd64_linux_syscall_record (struct regca
>      }
>    else
>      {
> -      ret = record_linux_system_call (syscall_gdb, regcache,
> +      ret = record_linux_system_call (syscall_gdb, addr, regcache,
>                                        &amd64_linux_record_tdep);
>        if (ret)
>          return ret;
> --- a/i386-linux-tdep.c
> +++ b/i386-linux-tdep.c
> @@ -411,7 +411,7 @@ i386_canonicalize_syscall (int syscall)
>  static struct linux_record_tdep i386_linux_record_tdep;
> 
>  static int
> -i386_linux_intx80_sysenter_record (struct regcache *regcache)
> +i386_linux_intx80_sysenter_record (struct regcache *regcache, CORE_ADDR addr)
>  {
>    int ret;
>    LONGEST syscall_native;
> @@ -437,7 +437,7 @@ i386_linux_intx80_sysenter_record (struc
>       return 0;
>     }
> 
> -  ret = record_linux_system_call (syscall_gdb, regcache,
> +  ret = record_linux_system_call (syscall_gdb, addr, regcache,
>  				  &i386_linux_record_tdep);
>    if (ret)
>      return ret;
> --- a/i386-tdep.c
> +++ b/i386-tdep.c
> @@ -3160,10 +3160,11 @@ i386_record_lea_modrm (struct i386_recor
> 
>    if (irp->override >= 0)
>      {
> -      warning (_("Process record ignores the memory change "
> -                 "of instruction at address %s because it "
> -                 "can't get the value of the segment register."),
> -               paddress (gdbarch, irp->orig_addr));
> +      if (record_debug)
> +        warning (_("Process record ignores the memory change "
> +                   "of instruction at address %s because it "
> +                   "can't get the value of the segment register."),
> +                 paddress (gdbarch, irp->orig_addr));
>        return 0;
>      }
> 
> @@ -4042,11 +4043,12 @@ reswitch:
>      case 0xa3:
>        if (ir.override >= 0)
>          {
> -	  warning (_("Process record ignores the memory change "
> -                     "of instruction at address %s because "
> -                     "it can't get the value of the segment "
> -                     "register."),
> -                   paddress (gdbarch, ir.orig_addr));
> +          if (record_debug)
> +	    warning (_("Process record ignores the memory change "
> +                       "of instruction at address %s because "
> +                       "it can't get the value of the segment "
> +                       "register."),
> +                     paddress (gdbarch, ir.orig_addr));
>  	}
>        else
>  	{
> @@ -4467,11 +4469,12 @@ reswitch:
>            if (ir.aflag && (es != ds))
>              {
>                /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
> -              warning (_("Process record ignores the memory "
> -                         "change of instruction at address %s "
> -                         "because it can't get the value of the "
> -                         "ES segment register."),
> -                       paddress (gdbarch, ir.orig_addr));
> +              if (record_debug)
> +                warning (_("Process record ignores the memory "
> +                           "change of instruction at address %s "
> +                           "because it can't get the value of the "
> +                           "ES segment register."),
> +                         paddress (gdbarch, ir.orig_addr));
>              }
>            else
>              {
> @@ -4872,7 +4875,7 @@ reswitch:
>  	    ir.addr -= 2;
>  	    goto no_support;
>  	  }
> -	ret = gdbarch_tdep (gdbarch)->i386_intx80_record (ir.regcache);
> +	ret = gdbarch_tdep (gdbarch)->i386_intx80_record (ir.regcache, ir.addr);
>  	if (ret)
>  	  return ret;
>        }
> @@ -4974,7 +4977,8 @@ reswitch:
>  	    ir.addr -= 2;
>  	    goto no_support;
>  	  }
> -	ret = gdbarch_tdep (gdbarch)->i386_sysenter_record (ir.regcache);
> +	ret = gdbarch_tdep (gdbarch)->i386_sysenter_record (ir.regcache,
> +                                                            ir.addr);
>  	if (ret)
>  	  return ret;
>        }
> @@ -4999,7 +5003,8 @@ reswitch:
>  	    ir.addr -= 2;
>  	    goto no_support;
>  	  }
> -	ret = gdbarch_tdep (gdbarch)->i386_syscall_record (ir.regcache);
> +	ret = gdbarch_tdep (gdbarch)->i386_syscall_record (ir.regcache,
> +                                                           ir.addr);
>  	if (ret)
>  	  return ret;
>        }
> @@ -5135,12 +5140,13 @@ reswitch:
>  	      /* sidt */
>  	      if (ir.override >= 0)
>  		{
> -		  warning (_("Process record ignores the memory "
> -                             "change of instruction at "
> -                             "address %s because it can't get "
> -                             "the value of the segment "
> -                             "register."),
> -                           paddress (gdbarch, ir.orig_addr));
> +                  if (record_debug)
> +		    warning (_("Process record ignores the memory "
> +                               "change of instruction at "
> +                               "address %s because it can't get "
> +                               "the value of the segment "
> +                               "register."),
> +                             paddress (gdbarch, ir.orig_addr));
>  		}
>  	      else
>  		{
> --- a/i386-tdep.h
> +++ b/i386-tdep.h
> @@ -115,11 +115,11 @@ struct gdbarch_tdep
>       in GDB is not same as I386 instructions.  */
>    const int *record_regmap;
>    /* Parse intx80 args.  */
> -  int (*i386_intx80_record) (struct regcache *regcache);
> +  int (*i386_intx80_record) (struct regcache *regcache, CORE_ADDR addr);
>    /* Parse sysenter args.  */
> -  int (*i386_sysenter_record) (struct regcache *regcache);
> +  int (*i386_sysenter_record) (struct regcache *regcache, CORE_ADDR addr);
>    /* Parse syscall args.  */
> -  int (*i386_syscall_record) (struct regcache *regcache);
> +  int (*i386_syscall_record) (struct regcache *regcache, CORE_ADDR addr);
>  };
> 
>  /* Floating-point registers.  */
> --- a/linux-record.c
> +++ b/linux-record.c
> @@ -21,6 +21,7 @@
>  #include "target.h"
>  #include "gdbtypes.h"
>  #include "regcache.h"
> +#include "inferior.h"
>  #include "record.h"
>  #include "linux-record.h"
> 
> @@ -222,7 +223,7 @@ record_linux_msghdr (struct regcache *re
>     Return -1 if something wrong.  */
> 
>  int
> -record_linux_system_call (enum gdb_syscall syscall,
> +record_linux_system_call (enum gdb_syscall syscall, CORE_ADDR addr,
>  			  struct regcache *regcache,
>                            struct linux_record_tdep *tdep)
>  {
> @@ -242,8 +243,9 @@ record_linux_system_call (enum gdb_sysca
>          int q;
>          target_terminal_ours ();
>          q = yquery (_("The next instruction is syscall exit.  "
> -                      "It will make the program exit.  "
> -                      "Do you want to stop the program?"));
> +                      "It will make the thread %s exit.  "
> +                      "Do you want to stop the program?"),
> +                    target_pid_to_str (inferior_ptid));
>          target_terminal_inferior ();
>          if (q)
>            return 1;
> @@ -1209,10 +1211,13 @@ record_linux_system_call (enum gdb_sysca
> 
>      case gdb_sys_fsync:
>      case gdb_sys_sigreturn:
> -    case gdb_sys_clone:
>      case gdb_sys_setdomainname:
>        break;
> 
> +    case gdb_sys_clone:
> +      record_step = 0;
> +      break;
> +
>      case gdb_sys_newuname:
>        regcache_raw_read_unsigned (regcache, tdep->arg1, &tmpulongest);
>        if (record_arch_list_add_mem ((CORE_ADDR) tmpulongest,
> --- a/linux-record.h
> +++ b/linux-record.h
> @@ -534,7 +534,7 @@ enum gdb_syscall {
> 
>  /* Record a linux syscall.  */
> 
> -extern int record_linux_system_call (enum gdb_syscall num,
> +extern int record_linux_system_call (enum gdb_syscall num, CORE_ADDR addr,
>  				     struct regcache *regcache,
>  				     struct linux_record_tdep *tdep);
>  #endif /* _LINUX_RECORD_H_ */


  reply	other threads:[~2009-11-25 20:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-25  8:06 Hui Zhu
2009-11-25 20:09 ` Michael Snyder [this message]
2009-11-26  7:20   ` Hui Zhu
2009-11-26 18:26     ` Michael Snyder
2009-12-07 19:00 ` Michael Snyder
2009-12-08  5:11   ` 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=4B0D8DF3.8090307@vmware.com \
    --to=msnyder@vmware.com \
    --cc=gdb-patches@sourceware.org \
    --cc=teawater@gmail.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