osproc: make failed execv an exception (when using fork or clone)

startProcessAuxFork creates a pipe, which is used by a child to pass
an error code if execv fails.

2 files changed, 64 insertions, 23 deletions

diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim
index 41260b36f..138df1aec 100644
--- a/lib/posix/posix.nim
+++ b/lib/posix/posix.nim
@@ -2066,6 +2066,7 @@ proc pthread_spin_unlock*(a1: ptr Tpthread_spinlock): cint {.
 proc pthread_testcancel*() {.importc, header: "<pthread.h>".}
 
 
+proc exitnow*(code: int): void {.importc: "_exit", header: "<unistd.h>".}
 proc access*(a1: cstring, a2: cint): cint {.importc, header: "<unistd.h>".}
 proc alarm*(a1: cint): cint {.importc, header: "<unistd.h>".}
 proc chdir*(a1: cstring): cint {.importc, header: "<unistd.h>".}
diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim
index 40877f638..aa2f6f937 100644
--- a/lib/pure/osproc.nim
+++ b/lib/pure/osproc.nim
@@ -600,13 +600,16 @@ elif not defined(useNimRtl):
     sysArgs: cstringArray
     sysEnv: cstringArray
     workingDir: cstring
-    pStdin, pStdout, pStderr: array[0..1, cint]
+    pStdin, pStdout, pStderr, pErrorPipe: array[0..1, cint]
     optionPoUsePath: bool
     optionPoParentStreams: bool
     optionPoStdErrToStdOut: bool
 
   proc startProcessAuxSpawn(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].}
-  proc startProcessAfterFork(data: ptr TStartProcessData) {.tags: [FExecIO, FReadEnv], cdecl.}
+
+  proc startProcessAuxFork(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].}
+  proc startProcessAfterFork(data: ptr TStartProcessData) {.
+    tags: [FExecIO, FReadEnv], noStackFrame, cdecl.}
 
   proc startProcess(command: string,
                  workingDir: string = "",
@@ -658,23 +661,11 @@ elif not defined(useNimRtl):
     data.optionPoStdErrToStdOut = poStdErrToStdOut in options
     data.workingDir = workingDir
 
-    when defined(useClone):
-      const stackSize = 8096
-      let stackEnd = cast[clong](alloc(stackSize))
-      let stack = cast[pointer](stackEnd + stackSize)
-      let fn: pointer = startProcessAfterFork
-      pid = clone(fn, stack,
-                  cint(CLONE_VM or CLONE_VFORK or SIGCHLD),
-                  pointer(addr data), nil, nil, nil)
 
-      if pid < 0: osError(osLastError())
-    elif defined(posix_spawn) and not defined(useFork):
+    when defined(posix_spawn) and not defined(useFork) and not defined(useClone):
       pid = startProcessAuxSpawn(data)
     else:
-      pid = fork()
-      if pid < 0: osError(osLastError())
-      if pid == 0:
-        startProcessAfterFork(addr(data))
+      pid = startProcessAuxFork(data)
 
     # Parent process. Copy process information.
     if poEchoCmd in options:
@@ -747,30 +738,79 @@ elif not defined(useNimRtl):
     chck res
     return pid
 
+  proc startProcessAuxFork(data: TStartProcessData): TPid =
+    if pipe(data.pErrorPipe) != 0:
+        osError(osLastError())
+
+    finally:
+      discard close(data.pErrorPipe[readIdx])
+
+    var pid: TPid
+    var dataCopy = data
+
+    if defined(useClone):
+      const stackSize = 8096
+      let stackEnd = cast[clong](alloc(stackSize))
+      let stack = cast[pointer](stackEnd + stackSize)
+      let fn: pointer = startProcessAfterFork
+      pid = clone(fn, stack,
+                  cint(CLONE_VM or CLONE_VFORK or SIGCHLD),
+                  pointer(addr dataCopy), nil, nil, nil)
+      discard close(data.pErrorPipe[writeIdx])
+      dealloc(stack)
+    else:
+      pid = fork()
+      if pid == 0:
+        startProcessAfterFork(addr(dataCopy))
+        exitnow(1)
+
+    discard close(data.pErrorPipe[writeIdx])
+    if pid < 0: osError(osLastError())
+
+    var error: cint
+    let sizeRead = read(data.pErrorPipe[readIdx], addr error, sizeof(error))
+    if sizeRead == sizeof(error):
+      osError($strerror(error))
+
+    return pid
+
+  proc startProcessFail(data: ptr TStartProcessData) {.noStackFrame.} =
+    var error: cint = errno
+    discard write(data.pErrorPipe[writeIdx], addr error, sizeof(error))
+    exitnow(1)
+
   proc startProcessAfterFork(data: ptr TStartProcessData) =
     # Warning: no GC here!
+    # Or anythink that touches global structures - all called nimrod procs
+    # must be marked with noStackFrame. Inspect C code after making changes.
     if not data.optionPoParentStreams:
       discard close(data.pStdin[writeIdx])
-      if dup2(data.pStdin[readIdx], readIdx) < 0: osError(osLastError())
+      if dup2(data.pStdin[readIdx], readIdx) < 0:
+        startProcessFail(data)
       discard close(data.pStdout[readIdx])
-      if dup2(data.pStdout[writeIdx], writeIdx) < 0: osError(osLastError())
+      if dup2(data.pStdout[writeIdx], writeIdx) < 0:
+        startProcessFail(data)
       discard close(data.pStderr[readIdx])
       if data.optionPoStdErrToStdOut:
-        if dup2(data.pStdout[writeIdx], 2) < 0: osError(osLastError())
+        if dup2(data.pStdout[writeIdx], 2) < 0:
+          startProcessFail(data)
       else:
-        if dup2(data.pStderr[writeIdx], 2) < 0: osError(osLastError())
+        if dup2(data.pStderr[writeIdx], 2) < 0:
+          startProcessFail(data)
 
     if data.workingDir.len > 0:
       if chdir(data.workingDir) < 0:
-        quit("chdir failed")
+        startProcessFail(data)
+
+    discard close(data.pErrorPipe[readIdx])
+    discard fcntl(data.pErrorPipe[writeIdx], F_SETFD, FD_CLOEXEC)
 
     if data.optionPoUsePath:
       discard execvpe(data.sysCommand, data.sysArgs, data.sysEnv)
     else:
       discard execve(data.sysCommand, data.sysArgs, data.sysEnv)
 
-    # too risky to raise an exception here:
-    quit("execve call failed: " & $strerror(errno))
+    startProcessFail(data)
 
   proc close(p: PProcess) =
     if p.inStream != nil: close(p.inStream)