[PATCH] unwind: initialize unwind context only if given tcb is initialized

[Dmitry V. Levin]

On Fri, Apr 20, 2018 at 09:47:44AM +0900, Masatake YAMATO wrote:
> On Fri, 20 Apr 2018 01:44:51 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
> >> --- a/strace.c
> >> +++ b/strace.c
> >> @@ -1783,6 +1783,8 @@ init(int argc, char *argv[])
> >>  
> >>  		unwind_init();
> >>  		for (tcbi = 0; tcbi < tcbtabsize; ++tcbi) {
> >> +			if (!tcbtab[tcbi]->pid)
> >> +				continue;
> >>  			unwind_tcb_init(tcbtab[tcbi]);
> >>  		}
> >>  	}
> > 
> > Apparently, we don't need this loop at all, there is no use to call
> > unwind_tcb_init until the tracee is actually attached.  The proper place
> > for unwind_tcb_init invocation is the same as for newoutf.
> > I've pushed a few commits to implement this change.
> 
> Much better fix.
> The function name is also improved.
> 
> One thing I cannot be convince is the following code block:
> 
>     static struct tcb *
>     maybe_switch_tcbs(struct tcb *tcp, const int pid)
>     {
> 	    ...	
> 	    tcp->pid = pid;
> 
> pid of tcb is updated. I wonder whether unwind context can be attached
> well to the modified(?) tcb.

Good catch, looks like this code is out of date as more fields were added
to struct tcb.  I suppose switching whole structures might be a better
alternative to cherry-picking relevant fields.

> I could not write a test executable that makes strace execute the line.

According to https://codecov.io/gh/strace/strace/src/master/strace.c#L2079
this line is covered by a test.  I think the test is threads-execve.test


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180420/6f3db246/attachment.bin>