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

On Thu, Nov 26, 2009 at 04:05, Michael Snyder <msnyder@vmware.com> wrote:
> 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.

To reproduce this issue:
cat 2.c
#include <stdio.h>
#include <pthread.h>
#include <assert.h>

void td1(void * i)
{
	while (1)
	{
		printf ("1\n");
		sleep (1);
	}

	return;
}

void td2(void * i)
{
	while (1)
	{
		printf ("2\n");
		sleep (1);
	}

	return;
}

int
main(int argc,char *argv[],char *envp[])
{
	pthread_t	t1,t2;

	pthread_create(&t1, NULL, (void*)td1, NULL);
	pthread_create(&t2, NULL, (void*)td2, NULL);

	while (1)
	{
		printf ("0\n");
		sleep (1);
	}

	return (0);
}

teawater@pek-hzhu:~/gdb$ gcc -lpthread -g 2.c
teawater@pek-hzhu:~/gdb$ gdb ./a.out
GNU gdb (GDB) 7.0.50.20091126-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/teawater/gdb/a.out...done.
(gdb) start
Temporary breakpoint 1 at 0x8048495: file 2.c, line 32.
Starting program: /home/teawater/gdb/a.out
[Thread debugging using libthread_db enabled]

Temporary breakpoint 1, main (argc=<value optimized out>, argv=<value
optimized out>, envp=<value optimized out>)
    at 2.c:32
32		pthread_create(&t1, NULL, (void*)td1, NULL);
(gdb) b clone
Breakpoint 2 at 0xb7f2ae00
(gdb) c
Continuing.

Breakpoint 2, 0xb7f2ae00 in clone () from /lib/tls/i686/cmov/libc.so.6
(gdb) disas
Dump of assembler code for function clone:
=> 0xb7f2ae00 <+0>:	mov    $0xffffffea,%eax
   0xb7f2ae05 <+5>:	mov    0x4(%esp),%ecx
   0xb7f2ae09 <+9>:	jecxz  0xb7f2ae78 <clone+120>
   0xb7f2ae0b <+11>:	mov    0x8(%esp),%ecx
   0xb7f2ae0f <+15>:	jecxz  0xb7f2ae78 <clone+120>
   0xb7f2ae11 <+17>:	and    $0xfffffff0,%ecx
   0xb7f2ae14 <+20>:	sub    $0x1c,%ecx
   0xb7f2ae17 <+23>:	mov    0x10(%esp),%eax
   0xb7f2ae1b <+27>:	mov    %eax,0xc(%ecx)
   0xb7f2ae1e <+30>:	mov    0x4(%esp),%eax
   0xb7f2ae22 <+34>:	mov    %eax,0x8(%ecx)
   0xb7f2ae25 <+37>:	movl   $0x0,0x4(%ecx)
   0xb7f2ae2c <+44>:	push   %ebx
   0xb7f2ae2d <+45>:	push   %esi
   0xb7f2ae2e <+46>:	push   %edi
   0xb7f2ae2f <+47>:	mov    0x24(%esp),%esi
   0xb7f2ae33 <+51>:	mov    0x20(%esp),%edx
   0xb7f2ae37 <+55>:	mov    0x18(%esp),%ebx
   0xb7f2ae3b <+59>:	mov    0x28(%esp),%edi
   0xb7f2ae3f <+63>:	mov    $0x78,%eax
   0xb7f2ae44 <+68>:	mov    %ebx,(%ecx)
   0xb7f2ae46 <+70>:	int    $0x80
   0xb7f2ae48 <+72>:	pop    %edi
   0xb7f2ae49 <+73>:	pop    %esi
   0xb7f2ae4a <+74>:	pop    %ebx
   0xb7f2ae4b <+75>:	test   %eax,%eax
   0xb7f2ae4d <+77>:	jl     0xb7f2ae78 <clone+120>
   0xb7f2ae4f <+79>:	je     0xb7f2ae52 <clone+82>
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) b *0xb7f2ae46
Breakpoint 3 at 0xb7f2ae46
(gdb) c
Continuing.

Breakpoint 3, 0xb7f2ae46 in clone () from /lib/tls/i686/cmov/libc.so.6
(gdb) dis
(gdb) si

Program terminated with signal SIGTRAP, Trace/breakpoint trap.
The program no longer exists.

> 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.
>

Without this change, when debug the multi-thread inferior, gdb will
output a lot of warning like a big rain.  :(  And it looks didn't
affect revere debug.
I will post a separate patch for it.

Thanks,
Hui


>
>>
>> 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-26  7:20 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
2009-11-26  7:20   ` Hui Zhu [this message]
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=daef60380911252319w5621e869r86a91bcbd7abeed@mail.gmail.com \
    --to=teawater@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --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