diff options
author | Pirama Arumuga Nainar <pirama@google.com> | 2015-03-25 17:21:40 -0700 |
---|---|---|
committer | Pirama Arumuga Nainar <pirama@google.com> | 2015-03-25 18:16:26 -0700 |
commit | 2fa8a238dd69afebdeb757adcb1d674043d78e32 (patch) | |
tree | 4251f971d9e9bbf07587500830050ba8a78660a1 | |
parent | be8c89541d8b094d4a01a5539dac738003bf36cc (diff) | |
download | rs-2fa8a238dd69afebdeb757adcb1d674043d78e32.tar.gz |
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
-rw-r--r-- | cpu_ref/rsCpuExecutable.cpp | 37 | ||||
-rw-r--r-- | cpu_ref/rsCpuScript.cpp | 61 | ||||
-rw-r--r-- | cpu_ref/rsCpuScriptGroup2.cpp | 46 | ||||
-rw-r--r-- | rsCppUtils.cpp | 44 | ||||
-rw-r--r-- | rsCppUtils.h | 9 |
5 files changed, 74 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; } diff --git a/rsCppUtils.cpp b/rsCppUtils.cpp index c9a19c23..b55f9e6a 100644 --- a/rsCppUtils.cpp +++ b/rsCppUtils.cpp @@ -21,6 +21,10 @@ #include <string.h> +#ifndef RS_COMPATIBILITY_LIB +#include <sys/wait.h> +#endif + namespace android { namespace renderscript { @@ -46,5 +50,45 @@ const char* rsuJoinStrings(int n, const char* const* strs) { return strndup(tmp.c_str(), tmp.size()); } +#ifndef RS_COMPATIBILITY_LIB +bool rsuExecuteCommand(const char *exe, int nArgs, const char * const *args) { + std::unique_ptr<const char> joined(rsuJoinStrings(nArgs, args)); + + pid_t pid = fork(); + + switch (pid) { + case -1: { // Error occurred (we attempt no recovery) + ALOGE("Fork of \"%s\" failed with error %s", exe, strerror(errno)); + return false; + } + case 0: { // Child process + ALOGV("Invoking %s with args '%s'", exe, joined.get()); + execv(exe, (char * const *)args); + + ALOGE("execv() failed: %s", strerror(errno)); + abort(); + return false; + } + default: { // Parent process (actual driver) + // Wait on child process to finish execution. + int status = 0; + pid_t w = TEMP_FAILURE_RETRY(waitpid(pid, &status, 0)); + if (w == -1) { + ALOGE("Waitpid of \"%s\" failed with error %s", exe, + strerror(errno)); + return false; + } + + if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { + return true; + } + + ALOGE("Child process \"%s\" terminated with status %d", exe, status); + return false; + } + } +} +#endif // RS_COMPATIBILITY_LIB + } } diff --git a/rsCppUtils.h b/rsCppUtils.h index cc6d6cf6..7377a44c 100644 --- a/rsCppUtils.h +++ b/rsCppUtils.h @@ -283,6 +283,15 @@ static inline uint32_t rsBoxFilter8888(uint32_t i1, uint32_t i2, uint32_t i3, ui const char* rsuJoinStrings(int n, const char* const* strs); +#ifndef RS_COMPATIBILITY_LIB +// Utility to fork/exec a command. +// exe - Command to execute +// nArgs - Number of arguments (excluding the trailing nullptr in args) +// args - Arguments to the command +bool rsuExecuteCommand(const char *exe, int nArgs, const char * const *args); +#endif + + } } |