Re: [PATCH] Handle vfork in thread with follow-fork-mode child

[Tom de Vries <tdevries@suse.de>]

On 17-04-19 19:45, Pedro Alves wrote:
> On 4/16/19 4:06 PM, Tom de Vries wrote:
>> Hi,
> 
> Hi!
> 
> Comments below.  As I was reviewing this, I kept experimenting,
> so I ended up addressing my own comments myself.  See updated patch
> at the bottom.
> 

Hi Pedro,

thanks for the comments and updated patch.

>> @@ -982,7 +983,17 @@ handle_vfork_child_exec_or_exit (int exec)
>>  		}
>>  	    }
>>  
>> -	  target_detach (inf->vfork_parent, 0);
>> +	  /* Now that the vfork child has terminated, make sure during detach
> 
> This path is also reached if the vfork child execs, so the reference to
> "terminated" above would better be exec.  But also, the other paths in
> the function already clear vfork_parent/vfork_child, so I think it's better
> to refactor things a bit so that all paths share the code.
> 

> +      inf->vfork_parent = NULL;

I was checking this and found dereferences of inf->vfork_parent after it
was set to NULL here:
...
      else if (exec)
        {
          ...
          resume_parent = inf->vfork_parent->pid;
...
and here:
...
      else
        {
          ...
          clone_program_space (pspace, inf->vfork_parent->pspace);
...

To confirm, I did another testrun with aborts at the start of the
blocks, and I found no regressions. So, either this is dead code, or we
need test-cases that trigger these paths.

Thanks,
- Tom