[PATCH] Use PTRACE_GETREGS on i386

Denys Vlasenko dvlasenk at redhat.com
Fri Aug 26 09:50:15 UTC 2011


On Thu, 2011-08-25 at 21:10 +0400, Dmitry V. Levin wrote:
> On Thu, Aug 25, 2011 at 12:10:06PM +0200, Denys Vlasenko wrote:
> > While discussing ptrace speedup, in one email Linus said
> > that it's stupid that strace doesn't even use already existing
> > speedups, such as PTRACE_GETREGS.
> 
> Yes, use of PTRACE_GETREGS is a long awaited change.
> I expect a significant reduction of ptrace() syscall invocations.
> 
> > I guess we need to add a check on startup to make sure PTRACE_GETREGS works?
> 
> PTRACE_GETREGS is quite ancient thing, it is supported even by linux-2.2.26,
> so I see no need in the runtime check this time.
> 
> > --- strace.5/syscall.c	2011-08-25 10:39:36.000000000 +0200
> > +++ strace.6/syscall.c	2011-08-25 11:51:32.148543603 +0200
> > @@ -716,7 +716,28 @@ struct tcb *tcp_last = NULL;
> >  
> >  #ifdef LINUX
> >  # if defined (I386)
> > -static long eax;
> > +struct i386_user_regs_struct {
> > +	long ebx;
> > +	long ecx;
> > +	long edx;
> > +	long esi;
> > +	long edi;
> > +	long ebp;
> > +	long eax;
> > +	long xds;
> > +	long xes;
> > +	long xfs;
> > +	long xgs;
> > +	long orig_eax;
> > +	long eip;
> > +	long xcs;
> > +	long eflags;
> > +	long esp;
> > +	long xss;
> > +	/* Just in case we forgot a few fields and kernel would write more... */
> > +	long paranoia[8];
> > +};
> > +static struct i386_user_regs_struct i386_regs;
> 
> I suppose we shouldn't hurry with this.  There is a <asm/ptrace.h> where
> all this stuff is defined and supported for all linux architectures.
> So I think migrating from <sys/ptrace.h> to <linux/ptrace.h> is the right
> way to go.

Something tells me that we _will_ get bitten by header include problems
if we start including more asm/* and linux/* stuff. Every now and again
as new kernels are released there are breakage in their headers.
Typically easily fixable, yes, but still, it's a PITA.

Defining a correct structure ourselves has no such problems.
It's not like this structure is going to ever change.
 

> > @@ -1291,21 +1313,17 @@ syscall_fixup_on_sysenter(struct tcb *tc
> >  #ifdef LINUX
> >  	/* A common case of "not a syscall entry" is post-execve SIGTRAP */
> >  #if defined (I386)
> > +	if (i386_regs.eax != -ENOSYS) {
> > +		if (debug)
> > +			fprintf(stderr, "not a syscall entry (eax = %ld)\n", i386_regs.eax);
> > +		return 0;
> > +	}
> 
> Looks like the check for !(ptrace_setoptions & PTRACE_O_TRACEEXEC) was
> removed here.

Yes, and it's intended. Since we no longer need to perform a ptrace()
call to get EAX, this check becomes ~500 times less expensive.
And it's not impossible to imagine that there are as-yet unknown
cases when we misinterpret some ptrace-stop as syscall entry
while it is not one.

Therefore I decided to do this check regardless of PTRACE_O_TRACEEXEC.

-- 
vda






More information about the Strace-devel mailing list