[Support][AIX] Implement wait4 wrapper to fix build

[hubert.reinterpretcast]

Summary

While wait4 is not documented for AIX, it is available; however, even on systems where it is available, the system headers do not always provide a declaration of the function.

This patch firstly provides a declaration of wait4 for AIX. Secondly, it provides a wrapper for wait4 to work around issues that occur when using it directly with WNOHANG.

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Program.inc

332

Is this meant to protect against wait4 as a function-like macro or also as an object-like macro? If the former, an alternate approach (see comment below) would work.

333

I asked around and the kwaitpid interface may be subject to change.

I was told that wait4 is actually available (just not listed in the documentation), and I was able to find the symbol in the system libc. Declaring the signature below with extern "C" should be enough.

If you prefer, I will put that fix in. Please let me know; thanks.

335

To avoid interpretation of wait4 as a function-like macro, it can be placed in parentheses: (wait4).

[sepavloff]

Could you please select 'Commandeer Revision' in the 'Add Action' field? You will be able to rule this patch.

llvm/lib/Support/Unix/Program.inc

332

Yes, it is protection against definition of wait4 as a macro. Such conflicts may give weird diagnostics.

333

Yes, please. I don't have access to AIX and cannot check patch.

335

Interesting solution, thank you!

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Program.inc

333

Got it; working on it.

[hubert.reinterpretcast]

Add declaration of wait4 to fix AIX build

[daltenty]

LGTM apart from minor nit

llvm/lib/Support/Unix/Program.inc

332

nit: should this be #if defined(_AIX) && !defined(_ALL_SOURCE), we'd actually get this definition if we weren't in a POSIX compliant mode

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Program.inc

410

Got a build, but it hangs. Will need to check this loop.

[ RUN      ] ProgramEnvTest.TestExecuteNoWait
Note: Google Test filter = ProgramEnvTest.TestExecuteNoWait
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ProgramEnvTest
[ RUN      ] ProgramEnvTest.TestExecuteNoWait
Note: Google Test filter = ProgramEnvTest.TestExecuteNoWait
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ProgramEnvTest
[ RUN      ] ProgramEnvTest.TestExecuteNoWait

[hubert.reinterpretcast]

llvm/lib/Support/Unix/Program.inc

410

AIX wait4 does not seem to work with WNOHANG. Attempting workaround...

[hubert.reinterpretcast]

• Implement wait4 wrapper for WNOHANG

[daltenty]

Confirming wrapper LGTM

llvm/lib/Support/Unix/Program.inc

#endif
   }
 
   PI.Pid = child;
   PI.Process = child;
 
   return true;
 }
 
 namespace llvm {
+namespace sys {
 
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
+#ifndef _AIX
+using ::wait4;
+#else
+static pid_t (wait4)(pid_t pid, int *status, int options, struct rusage *usage);
+#endif
+
+} // namespace sys
+} // namespace llvm
+
+#ifdef _AIX
+#ifndef _ALL_SOURCE
+extern "C" pid_t (wait4)(pid_t pid, int *status, int options,
+                         struct rusage *usage);
+#endif
+pid_t (llvm::sys::wait4)(pid_t pid, int *status, int options,
+                         struct rusage *usage) {
+  assert(pid > 0 && "Only expecting to handle actual PID values!");
+  assert((options & ~WNOHANG) == 0 && "Expecting WNOHANG at most!");
+  assert(usage && "Expecting usage collection!");
+
+  // AIX wait4 does not work well with WNOHANG.
+  if (!(options & WNOHANG))
+    return ::wait4(pid, status, options, usage);
+
+  // For WNOHANG, we use waitid (which supports WNOWAIT) until the child process
+  // has terminated.
+  siginfo_t WaitIdInfo;
+  WaitIdInfo.si_pid = 0;
+  int WaitIdRetVal =
+      waitid(P_PID, pid, &WaitIdInfo, WNOWAIT | WEXITED | options);
+
+  if (WaitIdRetVal == -1 || WaitIdInfo.si_pid == 0)
+    return WaitIdRetVal;
+
+  assert(WaitIdInfo.si_pid == pid);
+
+  // The child has already terminated, so a blocking wait on it is okay in the
+  // absence of indiscriminate `wait` calls from the current process (which
+  // would cause the call here to fail with ECHILD).
+  return ::wait4(pid, status, options & ~WNOHANG, usage);
+}
+#endif
+
+ProcessInfo llvm::sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
                             bool WaitUntilTerminates, std::string *ErrMsg,
                             Optional<ProcessStatistics> *ProcStat) {
   struct sigaction Act, Old;
   assert(PI.Pid && "invalid pid to wait on, process not started?");
 
   int WaitPidOptions = 0;
   pid_t ChildPid = PI.Pid;
   if (WaitUntilTerminates) {
     SecondsToWait = 0;
   } else if (SecondsToWait) {
     // Install a timeout handler.  The handler itself does nothing, but the
     // simple fact of having a handler at all causes the wait below to return
     // with EINTR, unlike if we used SIG_IGN.
     memset(&Act, 0, sizeof(Act));
     Act.sa_handler = TimeOutHandler;
     sigemptyset(&Act.sa_mask);
     sigaction(SIGALRM, &Act, &Old);
+    // FIXME The alarm signal may be delivered to another thread.
     alarm(SecondsToWait);
   } else if (SecondsToWait == 0)
     WaitPidOptions = WNOHANG;
 
   // Parent process: Wait for the child process to terminate.
   int status;
   ProcessInfo WaitResult;
   rusage Info;
   if (ProcStat)
     ProcStat->reset();
 
   do {
-    WaitResult.Pid = wait4(ChildPid, &status, WaitPidOptions, &Info);
+    WaitResult.Pid = sys::wait4(ChildPid, &status, WaitPidOptions, &Info);
   } while (WaitUntilTerminates && WaitResult.Pid == -1 && errno == EINTR);
 
   if (WaitResult.Pid != PI.Pid) {
     if (WaitResult.Pid == 0) {
       // Non-blocking wait.
       return WaitResult;
     } else {
       if (SecondsToWait && errno == EINTR) {
         // Kill the child.
         kill(PI.Pid, SIGKILL);
 
         // Turn off the alarm and restore the signal handler
         alarm(0);
         sigaction(SIGALRM, &Old, nullptr);
 
         // Wait for child to die
+        // FIXME This could grab some other child process out from another
+        // waiting thread and then leave a zombie anyway.
         if (wait(&status) != ChildPid)
           MakeErrMsg(ErrMsg, "Child timed out but wouldn't die");
         else
           MakeErrMsg(ErrMsg, "Child timed out", 0);
 
         WaitResult.ReturnCode = -2; // Timeout detected
         return WaitResult;
       } else if (errno != EINTR) {
#endif
     }
     // Return a special value to indicate that the process received an unhandled
     // signal during execution as opposed to failing to execute.
     WaitResult.ReturnCode = -2;
   }
   return WaitResult;
 }
 
-std::error_code sys::ChangeStdinToBinary() {
+std::error_code llvm::sys::ChangeStdinToBinary() {
   // Do nothing, as Unix doesn't differentiate between text and binary.
   return std::error_code();
 }
 
-std::error_code sys::ChangeStdoutToBinary() {
+std::error_code llvm::sys::ChangeStdoutToBinary() {
   // Do nothing, as Unix doesn't differentiate between text and binary.
   return std::error_code();
 }
 
 std::error_code
 llvm::sys::writeFileWithEncoding(StringRef FileName, StringRef Contents,
                                  WindowsEncodingMethod Encoding /*unused*/) {
   std::error_code EC;
for (StringRef Arg : Args) {
     ArgLength += Arg.size() + 1;
     if (ArgLength > size_t(HalfArgMax)) {
       return false;
     }
   }
 
   return true;
 }
-}