Wrap TEMP_FAILURE_RETRY around system calls

BUG 19934827

Wrap TEMP_FAILURE_RETRY around system calls that can return EINTR
(waitpid, close).

Refactor fork/exec flows in various places into a utility function
and log errors so we can better understand failures in the test server.

Fix a small use-after-free issue in ScriptGroups.

Change-Id: I60b192f83c395a13c27cd6bd2289c44132b84791

3 files changed, 21 insertions, 123 deletions

diff --git a/cpu_ref/rsCpuExecutable.cpp b/cpu_ref/rsCpuExecutable.cpp
index 86e72945..98f9ef85 100644
--- a/cpu_ref/rsCpuExecutable.cpp
+++ b/cpu_ref/rsCpuExecutable.cpp
@@ -11,7 +11,6 @@
 #include <unistd.h>
 #else
 #include "bcc/Config/Config.h"
-#include <sys/wait.h>
 #endif
 
 #include <dlfcn.h>
@@ -132,42 +131,8 @@ bool SharedLibraryUtils::createSharedLibrary(const char *cacheDir, const char *r
         nullptr
     };
 
-    std::unique_ptr<const char> joined(
-        rsuJoinStrings(args.size()-1, args.data()));
-    std::string cmdLineStr (joined.get());
+    return rsuExecuteCommand(LD_EXE_PATH, args.size()-1, args.data());
 
-    pid_t pid = fork();
-
-    switch (pid) {
-    case -1: {  // Error occurred (we attempt no recovery)
-        ALOGE("Couldn't fork for linker (%s) execution", LD_EXE_PATH);
-        return false;
-    }
-    case 0: {  // Child process
-        ALOGV("Invoking ld.mc with args '%s'", cmdLineStr.c_str());
-        execv(LD_EXE_PATH, (char* const*) args.data());
-
-        ALOGE("execv() failed: %s", strerror(errno));
-        abort();
-        return false;
-    }
-    default: {  // Parent process (actual driver)
-        // Wait on child process to finish compiling the source.
-        int status = 0;
-        pid_t w = waitpid(pid, &status, 0);
-        if (w == -1) {
-            ALOGE("Could not wait for linker (%s)", LD_EXE_PATH);
-            return false;
-        }
-
-        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
-            return true;
-        }
-
-        ALOGE("Linker (%s) terminated unexpectedly", LD_EXE_PATH);
-        return false;
-    }
-    }
 }
 
 #endif  // RS_COMPATIBILITY_LIB
diff --git a/cpu_ref/rsCpuScript.cpp b/cpu_ref/rsCpuScript.cpp
index 481c54d9..6099cf4d 100644
--- a/cpu_ref/rsCpuScript.cpp
+++ b/cpu_ref/rsCpuScript.cpp
@@ -23,6 +23,8 @@
     #include <sys/stat.h>
     #include <unistd.h>
 #else
+    #include "rsCppUtils.h"
+
     #include <bcc/BCCContext.h>
     #include <bcc/Config/Config.h>
     #include <bcc/Renderscript/RSCompilerDriver.h>
@@ -32,7 +34,6 @@
     #include <zlib.h>
     #include <sys/file.h>
     #include <sys/types.h>
-    #include <sys/wait.h>
     #include <unistd.h>
 
     #include <string>
@@ -122,8 +123,7 @@ static void setCompileArguments(std::vector<const char*>* args,
 static bool compileBitcode(const std::string &bcFileName,
                            const char *bitcode,
                            size_t bitcodeSize,
-                           const char **compileArguments,
-                           const char *compileCommandLine) {
+                           std::vector<const char *> &compileArguments) {
     rsAssert(bitcode && bitcodeSize);
 
     FILE *bcfile = fopen(bcFileName.c_str(), "w");
@@ -139,39 +139,9 @@ static bool compileBitcode(const std::string &bcFileName,
         return false;
     }
 
-    pid_t pid = fork();
-
-    switch (pid) {
-    case -1: {  // Error occurred (we attempt no recovery)
-        ALOGE("Couldn't fork for bcc compiler execution");
-        return false;
-    }
-    case 0: {  // Child process
-        ALOGV("Invoking BCC with: %s", compileCommandLine);
-        execv(android::renderscript::RsdCpuScriptImpl::BCC_EXE_PATH,
-              (char* const*)compileArguments);
-
-        ALOGE("execv() failed: %s", strerror(errno));
-        abort();
-        return false;
-    }
-    default: {  // Parent process (actual driver)
-        // Wait on child process to finish compiling the source.
-        int status = 0;
-        pid_t w = waitpid(pid, &status, 0);
-        if (w == -1) {
-            ALOGE("Could not wait for bcc compiler");
-            return false;
-        }
-
-        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
-            return true;
-        }
-
-        ALOGE("bcc compiler terminated unexpectedly");
-        return false;
-    }
-    }
+    return android::renderscript::rsuExecuteCommand(
+                   android::renderscript::RsdCpuScriptImpl::BCC_EXE_PATH,
+                   compileArguments.size()-1, compileArguments.data());
 }
 
 bool isChecksumNeeded() {
@@ -200,7 +170,7 @@ bool addFileToChecksum(const char *fileName, uint32_t &checksum) {
             break;
     }
 
-    if (close(FD) != 0) {
+    if (TEMP_FAILURE_RETRY(close(FD)) != 0) {
         ALOGE("Cannot close file \'%s\' after computing checksum", fileName);
         return false;
     }
@@ -360,16 +330,17 @@ bool RsdCpuScriptImpl::init(char const *resName, char const *cacheDir,
     setCompileArguments(&compileArguments, bcFileName, cacheDir, resName, core_lib,
                         useRSDebugContext, bccPluginName);
 
-    // The last argument of compileArguments is a nullptr, so remove 1 from the
-    // size.
-    std::unique_ptr<const char> compileCommandLine(
-        rsuJoinStrings(compileArguments.size() - 1, compileArguments.data()));
-
     mChecksumNeeded = isChecksumNeeded();
     if (mChecksumNeeded) {
         std::vector<const char *> bccFiles = { BCC_EXE_PATH,
                                                core_lib,
                                              };
+
+        // The last argument of compileArguments is a nullptr, so remove 1 from
+        // the size.
+        std::unique_ptr<const char> compileCommandLine(
+            rsuJoinStrings(compileArguments.size()-1, compileArguments.data()));
+
         mBuildChecksum = constructBuildChecksum(bitcode, bitcodeSize,
                                                 compileCommandLine.get(),
                                                 bccFiles);
@@ -393,10 +364,6 @@ bool RsdCpuScriptImpl::init(char const *resName, char const *cacheDir,
     compileArguments.push_back(mBuildChecksum);
     compileArguments.push_back(nullptr);
 
-    // recompute compileCommandLine with the extra arguments
-    compileCommandLine.reset(
-        rsuJoinStrings(compileArguments.size() - 1, compileArguments.data()));
-
     if (!is_force_recompile() && !useRSDebugContext) {
         mScriptSO = SharedLibraryUtils::loadSharedLibrary(cacheDir, resName);
 
@@ -411,7 +378,7 @@ bool RsdCpuScriptImpl::init(char const *resName, char const *cacheDir,
     // again.
     if (mScriptSO == nullptr) {
         if (!compileBitcode(bcFileName, (const char*)bitcode, bitcodeSize,
-                            compileArguments.data(), compileCommandLine.get()))
+                            compileArguments))
         {
             ALOGE("bcc: FAILS to compile '%s'", resName);
             mCtx->unlockMutex();
diff --git a/cpu_ref/rsCpuScriptGroup2.cpp b/cpu_ref/rsCpuScriptGroup2.cpp
index 2e50ecb9..3a50221c 100644
--- a/cpu_ref/rsCpuScriptGroup2.cpp
+++ b/cpu_ref/rsCpuScriptGroup2.cpp
@@ -12,7 +12,6 @@
 
 #ifndef RS_COMPATIBILITY_LIB
 #include "bcc/Config/Config.h"
-#include <sys/wait.h>
 #endif
 
 #include "cpu_ref/rsCpuCore.h"
@@ -264,40 +263,6 @@ void setupCompileArguments(
     args->push_back(nullptr);
 }
 
-bool fuseAndCompile(const char** arguments,
-                    const string& commandLine) {
-    const pid_t pid = fork();
-
-    if (pid == -1) {
-        ALOGE("Couldn't fork for bcc execution");
-        return false;
-    }
-
-    if (pid == 0) {
-        // Child process
-        ALOGV("Invoking BCC with: %s", commandLine.c_str());
-        execv(RsdCpuScriptImpl::BCC_EXE_PATH, (char* const*)arguments);
-
-        ALOGE("execv() failed: %s", strerror(errno));
-        abort();
-        return false;
-    }
-
-    // Parent process
-    int status = 0;
-    const pid_t w = waitpid(pid, &status, 0);
-    if (w == -1) {
-        return false;
-    }
-
-    if (!WIFEXITED(status) || WEXITSTATUS(status) != 0 ) {
-        ALOGE("bcc terminated unexpectedly");
-        return false;
-    }
-
-    return true;
-}
-
 void generateSourceSlot(const Closure& closure,
                         const std::vector<std::string>& inputs,
                         std::stringstream& ss) {
@@ -384,13 +349,14 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
     const string& coreLibPath = getCoreLibPath(getCpuRefImpl()->getContext(),
                                                &coreLibRelaxedPath);
     vector<const char*> arguments;
-    setupCompileArguments(inputs, kernelBatches, invokeBatches, cacheDir,
+    string output_dir(cacheDir);
+    setupCompileArguments(inputs, kernelBatches, invokeBatches, output_dir,
                           outputFileName, coreLibPath, coreLibRelaxedPath, &arguments);
-    std::unique_ptr<const char> joined(
-        rsuJoinStrings(arguments.size() - 1, arguments.data()));
-    string commandLine (joined.get());
 
-    if (!fuseAndCompile(arguments.data(), commandLine)) {
+    bool compiled = rsuExecuteCommand(RsdCpuScriptImpl::BCC_EXE_PATH,
+                                     arguments.size()-1,
+                                     arguments.data());
+    if (!compiled) {
         unlink(objFilePath.c_str());
         return;
     }