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
diff --git a/cpu_ref/rsCpuExecutable.cpp b/cpu_ref/rsCpuExecutable.cpp
index 86e7294..98f9ef8 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 @@
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 481c54d..6099cf4 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 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 2e50ecb..3a50221 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 @@
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 @@
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;
}