[PATCH v2 22/46] vfork: Use fork if arch does not have the vfork syscall

[Vineet Gupta]

On Wednesday 28 November 2012 04:57 AM, Rich Felker wrote:
> On Tue, Nov 27, 2012 at 02:09:36PM +0530, Vineet Gupta wrote:
>>> diff --git a/libc/sysdeps/linux/common/vfork.c b/libc/sysdeps/linux/common/vfork.c
>>> index e7c9208..377418c 100644
>>> --- a/libc/sysdeps/linux/common/vfork.c
>>> +++ b/libc/sysdeps/linux/common/vfork.c
>>> @@ -4,13 +4,27 @@
>>>   * Licensed under the LGPL v2.1, see the file COPYING.LIB in this tarball.
>>>   */
>>>  
>>> +#include <signal.h>
>>>  #include <unistd.h>
>>>  #include <sys/types.h>
>>>  #include <sys/syscall.h>
>>>  
>>>  extern __typeof(vfork) __vfork attribute_hidden;
>>>  
>>> -#ifdef __NR_vfork
>>> +#if defined(__NR_fork) && !defined(__NR_vfork)
>>> +pid_t __vfork(void)
>>> +{
>>> +	pid_t pid = INLINE_SYSCALL(fork, 0);
>>> +
>>> +	if (pid<0)
>>> +		return -1
>>> +
>>> +	return pid;
>>> +}
>>> +weak_alias(__vfork, vfork)
>>> +libc_hidden_weak(vfork)
>>> +
>>> +#elif defined(__NR_vfork)
>>>  
>>>  # define __NR___vfork __NR_vfork
>>>  _syscall0(pid_t, __vfork)
>>>
>>  [...]
>> While v1 of this particular patch made sense to me - this one doesn't.
>> kernel's asm-generic unistd.h only defines clone (and doesn't define
>> vfork/fork). Thus this file will compile to nothing as opposed to v1
>> where vfork would correctly call clone syscall with correct args -
>> something which the vfork syscall handler itself in kernel typically
>> does. And since that would be gone - we need to do same thing in user
>> space wrapper.
> 
> The new patch is wrong too. The correct way to do vfork from C on a
> no-legacy-syscalls kernel is to use the clone syscall with the flags
> combination that makes it equivalent to plain fork (i.e. just
> SIGCHLD). Using CLONE_VFORK from C is incorrect because there's no way
> to preserve the return address for the parent to use after the child
> runs.

But for a any future no-legacy syscalls + no-MMU system, won't this be
semantically wrong. Shouldn't we define "C" vfork to return ERROR (for
!__NR_vfork) so that arch guys know to define a asm stub which calls
clone with correct args - or even put a #error in "C" file for that case.

-Vineet