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

[Masatake YAMATO]

On Fri, 20 Apr 2018 17:50:50 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
> On Fri, Apr 20, 2018 at 04:38:15AM +0300, Dmitry V. Levin wrote:
>> 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.
> 
> On second thought, maybe_switch_tcbs already swaps whole structures
> except a few pid-related fields.  So the question is, do we have
> any more pid-related fields that should be excluded?

What I know are unwind_ctx and mmap_cache.

How about introducing hooks where a subsystem can register itself to
like:

diff --git a/strace.c b/strace.c
index 7b79b0c5..f3adf547 100644
--- a/strace.c
+++ b/strace.c
@@ -2080,8 +2080,14 @@ maybe_switch_tcbs(struct tcb *tcp, const int pid)
 	/* Drop leader, but close execve'd thread outfile (if -ff) */
 	droptcb(tcp);
 	/* Switch to the thread, reusing leader's outfile and pid */
+
+	/* Give a chance to cleanup resources attached to tcb to subsystems. */
+	run_subsystem_changing_pid_pre_hook(tcp, pid);
 	tcp = execve_thread;
+	old_pid = tcp->pid;
 	tcp->pid = pid;
+	/* Give a chance to attach newly allocate resources to subsystems.  */
+	run_subsystem_changing_pid_post_hook(tcp, old_pid);
 	if (cflag != CFLAG_ONLY_STATS) {
 		printleader(tcp);
 		tprintf("+++ superseded by execve in pid %lu +++\n", old_pid);

Masatake YAMATO