Dedup checksum calculation routines

I introduced a separate routine to cacluate checksum for ScriptGroup
in my previous CL, in addition to the one we use for regular scripts.
This CL removes the new one and uses the old one.

While I am on it, I made some other minor changes, e.g., changing
mBuildChecksum in RsdCpuScriptIml from char* to uint32_t, and a few
other minor cleanups in ScriptGroup2 implementation.

Change-Id: I168fdbb4e7bd14f1549a687e7b0d0ca6dd4da866

6 files changed, 101 insertions, 138 deletions

diff --git a/cpu_ref/rsCpuExecutable.cpp b/cpu_ref/rsCpuExecutable.cpp
index 98f9ef85..f45e8f0d 100644
--- a/cpu_ref/rsCpuExecutable.cpp
+++ b/cpu_ref/rsCpuExecutable.cpp
@@ -281,7 +281,7 @@ static char* strgets(char *s, int size, const char **ppstr) {
 }
 
 ScriptExecutable* ScriptExecutable::createFromSharedObject(
-    Context* RSContext, void* sharedObj) {
+    Context* RSContext, void* sharedObj, uint32_t expectedChecksum) {
     char line[MAXLINE];
 
     size_t varCount = 0;
@@ -299,7 +299,7 @@ ScriptExecutable* ScriptExecutable::createFromSharedObject(
     uint32_t* forEachSignatures = nullptr;
     const char ** pragmaKeys = nullptr;
     const char ** pragmaValues = nullptr;
-    char *checksum = nullptr;
+    uint32_t checksum = 0;
 
     const char *rsInfo = (const char *) dlsym(sharedObj, ".rs.info");
 
@@ -519,25 +519,21 @@ ScriptExecutable* ScriptExecutable::createFromSharedObject(
     }
 
     if (strgets(line, MAXLINE, &rsInfo) != nullptr) {
-        if (strncmp(line, CHECKSUM_STR, strlen(CHECKSUM_STR)) != 0) {
+        if (sscanf(line, CHECKSUM_STR "%08x", &checksum) != 1) {
             ALOGE("Invalid checksum flag!: %s", line);
             goto error;
         }
-
-        // consume trailing newline character
-        char *c = strrchr(line, '\n');
-        if (c) {
-            *c = '\0';
-        }
-
-        char *checksumStart = &line[strlen(CHECKSUM_STR)];
-        checksum = new char[strlen(checksumStart) + 1];
-        strcpy(checksum, checksumStart);
     } else {
         ALOGE("Missing checksum in shared obj file");
         goto error;
     }
 
+    if (expectedChecksum != 0 && checksum != expectedChecksum) {
+        ALOGE("Found invalid checksum.  Expected %08x, got %08x\n",
+              expectedChecksum, checksum);
+        goto error;
+    }
+
 #endif  // RS_COMPATIBILITY_LIB
 
     return new ScriptExecutable(
@@ -550,7 +546,6 @@ ScriptExecutable* ScriptExecutable::createFromSharedObject(
 error:
 
 #ifndef RS_COMPATIBILITY_LIB
-    delete[] checksum;
 
     for (size_t idx = 0; idx < pragmaCount; ++idx) {
         delete [] pragmaKeys[idx];
diff --git a/cpu_ref/rsCpuExecutable.h b/cpu_ref/rsCpuExecutable.h
index ed6904d0..785f53d1 100644
--- a/cpu_ref/rsCpuExecutable.h
+++ b/cpu_ref/rsCpuExecutable.h
@@ -64,7 +64,7 @@ public:
                      size_t forEachCount,
                      const char** pragmaKeys, const char** pragmaValues,
                      size_t pragmaCount,
-                     bool isThreadable, const char *buildChecksum) :
+                     bool isThreadable, uint32_t buildChecksum) :
         mFieldAddress(fieldAddress), mFieldIsObject(fieldIsObject),
         mFieldName(fieldName), mExportedVarCount(varCount),
         mInvokeFunctions(invokeFunctions), mFuncCount(funcCount),
@@ -86,7 +86,6 @@ public:
             }
         }
 
-        delete[] mBuildChecksum;
         for (size_t i = 0; i < mPragmaCount; ++i) {
             delete [] mPragmaKeys[i];
             delete [] mPragmaValues[i];
@@ -107,8 +106,13 @@ public:
         delete[] mFieldAddress;
     }
 
+    // Create an ScriptExecutable object from a shared object.
+    // If expectedChecksum is not zero, it will be compared to the checksum
+    // embedded in the shared object. A mismatch will cause a failure.
+    // If succeeded, returns the new object. Otherwise, returns nullptr.
     static ScriptExecutable*
-            createFromSharedObject(Context* RSContext, void* sharedObj);
+            createFromSharedObject(Context* RSContext, void* sharedObj,
+                                   uint32_t expectedChecksum = 0);
 
     size_t getExportedVariableCount() const { return mExportedVarCount; }
     size_t getExportedFunctionCount() const { return mFuncCount; }
@@ -130,11 +134,7 @@ public:
 
     bool getThreadable() const { return mIsThreadable; }
 
-    const char *getBuildChecksum() const { return mBuildChecksum; }
-    bool isChecksumValid(const char *checksum) const {
-      return (mBuildChecksum != nullptr &&
-              strcmp(checksum, mBuildChecksum) == 0);
-    }
+    uint32_t getBuildChecksum() const { return mBuildChecksum; }
 
 private:
     void** mFieldAddress;
@@ -154,7 +154,7 @@ private:
     size_t mPragmaCount;
 
     bool mIsThreadable;
-    const char *mBuildChecksum;
+    uint32_t mBuildChecksum;
 
     Context* mRS;
 };
diff --git a/cpu_ref/rsCpuScript.cpp b/cpu_ref/rsCpuScript.cpp
index fb87dd1a..4afcf2e4 100644
--- a/cpu_ref/rsCpuScript.cpp
+++ b/cpu_ref/rsCpuScript.cpp
@@ -46,6 +46,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <iostream>
+#include <sstream>
 
 #ifdef __LP64__
 #define SYSLIBPATH "/system/lib64"
@@ -150,38 +151,6 @@ bool isChecksumNeeded() {
     return (buf[0] == '1');
 }
 
-char *constructBuildChecksum(uint8_t const *bitcode, size_t bitcodeSize,
-                             const char *commandLine,
-                             const std::vector<const char *> &bccFiles) {
-    uint32_t checksum = adler32(0L, Z_NULL, 0);
-
-    // include checksum of bitcode
-    checksum = adler32(checksum, bitcode, bitcodeSize);
-
-    // include checksum of command line arguments
-    checksum = adler32(checksum, (const unsigned char *) commandLine,
-                       strlen(commandLine));
-
-    // include checksum of bccFiles
-    for (auto bccFile : bccFiles) {
-        if (!android::renderscript::addFileToChecksum(bccFile, checksum)) {
-            // return empty checksum instead of something partial/corrupt
-            return nullptr;
-        }
-    }
-
-    char *checksumStr = new char[9]();
-    sprintf(checksumStr, "%08x", checksum);
-    return checksumStr;
-}
-
-#endif  // !defined(RS_COMPATIBILITY_LIB)
-}  // namespace
-
-namespace android {
-namespace renderscript {
-
-#ifndef RS_COMPATIBILITY_LIB
 bool addFileToChecksum(const char *fileName, uint32_t &checksum) {
     int FD = open(fileName, O_RDONLY);
     if (FD == -1) {
@@ -208,6 +177,41 @@ bool addFileToChecksum(const char *fileName, uint32_t &checksum) {
     }
     return true;
 }
+
+#endif  // !defined(RS_COMPATIBILITY_LIB)
+}  // namespace
+
+namespace android {
+namespace renderscript {
+
+#ifndef RS_COMPATIBILITY_LIB
+
+uint32_t constructBuildChecksum(uint8_t const *bitcode, size_t bitcodeSize,
+                                const char *commandLine,
+                                const char** bccFiles, size_t numFiles) {
+    uint32_t checksum = adler32(0L, Z_NULL, 0);
+
+    // include checksum of bitcode
+    if (bitcode != nullptr && bitcodeSize > 0) {
+        checksum = adler32(checksum, bitcode, bitcodeSize);
+    }
+
+    // include checksum of command line arguments
+    checksum = adler32(checksum, (const unsigned char *) commandLine,
+                       strlen(commandLine));
+
+    // include checksum of bccFiles
+    for (size_t i = 0; i < numFiles; i++) {
+        const char* bccFile = bccFiles[i];
+        if (bccFile[0] != 0 && !addFileToChecksum(bccFile, checksum)) {
+            // return empty checksum instead of something partial/corrupt
+            return 0;
+        }
+    }
+
+    return checksum;
+}
+
 #endif  // !RS_COMPATIBILITY_LIB
 
 RsdCpuScriptImpl::RsdCpuScriptImpl(RsdCpuReferenceImpl *ctx, const Script *s) {
@@ -231,7 +235,7 @@ RsdCpuScriptImpl::RsdCpuScriptImpl(RsdCpuReferenceImpl *ctx, const Script *s) {
     mIntrinsicData = nullptr;
     mIsThreadable = true;
 
-    mBuildChecksum = nullptr;
+    mBuildChecksum = 0;
     mChecksumNeeded = false;
 }
 
@@ -239,20 +243,13 @@ bool RsdCpuScriptImpl::storeRSInfoFromSO() {
     // The shared object may have an invalid build checksum.
     // Validate and fail early.
     mScriptExec = ScriptExecutable::createFromSharedObject(
-            mCtx->getContext(), mScriptSO);
+            mCtx->getContext(), mScriptSO,
+            mChecksumNeeded ? mBuildChecksum : 0);
 
     if (mScriptExec == nullptr) {
         return false;
     }
 
-    if (mChecksumNeeded && !mScriptExec->isChecksumValid(mBuildChecksum)) {
-        ALOGE("Found invalid checksum.  Expected %s, got %s\n",
-                  mBuildChecksum, mScriptExec->getBuildChecksum());
-        delete mScriptExec;
-        mScriptExec = nullptr;
-        return false;
-    }
-
     mRoot = (RootFunc_t) dlsym(mScriptSO, "root");
     if (mRoot) {
         //ALOGE("Found root(): %p", mRoot);
@@ -345,9 +342,9 @@ bool RsdCpuScriptImpl::init(char const *resName, char const *cacheDir,
 
         mBuildChecksum = constructBuildChecksum(bitcode, bitcodeSize,
                                                 compileCommandLine.get(),
-                                                bccFiles);
+                                                bccFiles.data(), bccFiles.size());
 
-        if (mBuildChecksum == nullptr) {
+        if (mBuildChecksum == 0) {
             // cannot compute checksum but verification is enabled
             mCtx->unlockMutex();
             return false;
@@ -355,15 +352,16 @@ bool RsdCpuScriptImpl::init(char const *resName, char const *cacheDir,
     }
     else {
         // add a dummy/constant as a checksum if verification is disabled
-        mBuildChecksum = new char[9]();
-        strcpy(const_cast<char *>(mBuildChecksum), "abadcafe");
+        mBuildChecksum = 0xabadcafe;
     }
 
     // Append build checksum to commandline
     // Handle the terminal nullptr in compileArguments
     compileArguments.pop_back();
     compileArguments.push_back("-build-checksum");
-    compileArguments.push_back(mBuildChecksum);
+    std::stringstream ss;
+    ss << std::hex << mBuildChecksum;
+    compileArguments.push_back(ss.str().c_str());
     compileArguments.push_back(nullptr);
 
     if (!is_force_recompile() && !useRSDebugContext) {
@@ -842,8 +840,6 @@ RsdCpuScriptImpl::~RsdCpuScriptImpl() {
     if (mScriptSO) {
         dlclose(mScriptSO);
     }
-
-    delete[] mBuildChecksum;
 }
 
 Allocation * RsdCpuScriptImpl::getAllocationForPointer(const void *ptr) const {
diff --git a/cpu_ref/rsCpuScript.h b/cpu_ref/rsCpuScript.h
index f38158a4..22f973bc 100644
--- a/cpu_ref/rsCpuScript.h
+++ b/cpu_ref/rsCpuScript.h
@@ -136,7 +136,7 @@ public:
 
 private:
     String8 mBitcodeFilePath;
-    const char * mBuildChecksum;
+    uint32_t mBuildChecksum;
     bool mChecksumNeeded;
 };
 
@@ -145,7 +145,9 @@ Allocation * rsdScriptGetAllocationForPointer(
                         const Script *script,
                         const void *);
 
-bool addFileToChecksum(const char *fileName, uint32_t &checksum);
+uint32_t constructBuildChecksum(uint8_t const *bitcode, size_t bitcodeSize,
+                                const char *commandLine,
+                                const char ** bccFiles, size_t numFiles);
 
 }
 
diff --git a/cpu_ref/rsCpuScriptGroup2.cpp b/cpu_ref/rsCpuScriptGroup2.cpp
index df1172b7..1cc382f4 100644
--- a/cpu_ref/rsCpuScriptGroup2.cpp
+++ b/cpu_ref/rsCpuScriptGroup2.cpp
@@ -11,8 +11,6 @@
 #include <vector>
 
 #ifndef RS_COMPATIBILITY_LIB
-#include <zlib.h>
-
 #include "bcc/Config/Config.h"
 #endif
 
@@ -227,41 +225,11 @@ string getCoreLibPath(Context* context, string* coreLibRelaxedPath) {
 #endif
 }
 
-bool getChecksum(const std::vector<string>& inputBitcodeFilenames,
-                 const string& coreLibPath, const string& coreLibRelaxedPath,
-                 const char* commandLine,
-                 char* checksumStr) {
-    uint32_t checksum = adler32(0L, Z_NULL, 0);
-
-    for (const auto& bcFilename : inputBitcodeFilenames) {
-        if (!android::renderscript::addFileToChecksum(bcFilename.c_str(), checksum)) {
-            return false;
-        }
-    }
-
-    if (!android::renderscript::addFileToChecksum(coreLibPath.c_str(), checksum)) {
-        return false;
-    }
-
-    if (!coreLibRelaxedPath.empty() &&
-        !android::renderscript::addFileToChecksum(coreLibRelaxedPath.c_str(), checksum)) {
-        return false;
-    }
-
-    // include checksum of command line arguments
-    checksum = adler32(checksum, (const unsigned char *) commandLine,
-                       strlen(commandLine));
-
-    sprintf(checksumStr, "%08x", checksum);
-
-    return true;
-}
-
 void setupCompileArguments(
-        const vector<string>& inputs, const vector<string>& kernelBatches,
+        const vector<const char*>& inputs, const vector<string>& kernelBatches,
         const vector<string>& invokeBatches,
-        const string& output_dir, const string& output_filename,
-        const string& coreLibPath, const string& coreLibRelaxedPath,
+        const char* outputDir, const char* outputFileName,
+        const char* coreLibPath, const char* coreLibRelaxedPath,
         vector<const char*>* args) {
     args->push_back(RsdCpuScriptImpl::BCC_EXE_PATH);
     args->push_back("-fPIC");
@@ -269,11 +237,11 @@ void setupCompileArguments(
     args->push_back("-mtriple");
     args->push_back(DEFAULT_TARGET_TRIPLE_STRING);
     args->push_back("-bclib");
-    args->push_back(coreLibPath.c_str());
+    args->push_back(coreLibPath);
     args->push_back("-bclib_relaxed");
-    args->push_back(coreLibRelaxedPath.c_str());
-    for (const string& input : inputs) {
-        args->push_back(input.c_str());
+    args->push_back(coreLibRelaxedPath);
+    for (const char* input : inputs) {
+        args->push_back(input);
     }
     for (const string& batch : kernelBatches) {
         args->push_back("-merge");
@@ -284,13 +252,13 @@ void setupCompileArguments(
         args->push_back(batch.c_str());
     }
     args->push_back("-output_path");
-    args->push_back(output_dir.c_str());
+    args->push_back(outputDir);
     args->push_back("-o");
-    args->push_back(output_filename.c_str());
+    args->push_back(outputFileName);
 }
 
 void generateSourceSlot(const Closure& closure,
-                        const std::vector<std::string>& inputs,
+                        const std::vector<const char*>& inputs,
                         std::stringstream& ss) {
     const IDBase* funcID = (const IDBase*)closure.mFunctionID.get();
     const Script* script = funcID->mScript;
@@ -317,7 +285,11 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
         return;
     }
 
-    std::set<string> inputSet;
+    auto comparator = [](const char* str1, const char* str2) -> bool {
+        return strcmp(str1, str2) < 0;
+    };
+    std::set<const char*, decltype(comparator)> inputSet(comparator);
+
     for (Closure* closure : mGroup->mClosures) {
         const Script* script = closure->mFunctionID.get()->mScript;
 
@@ -328,11 +300,11 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
 
         const RsdCpuScriptImpl *cpuScript =
                 (const RsdCpuScriptImpl*)script->mHal.drv;
-        const string& bitcodeFilename = cpuScript->getBitcodeFilePath();
+        const char* bitcodeFilename = cpuScript->getBitcodeFilePath();
         inputSet.insert(bitcodeFilename);
     }
 
-    std::vector<string> inputs(inputSet.begin(), inputSet.end());
+    std::vector<const char*> inputs(inputSet.begin(), inputSet.end());
 
     std::vector<string> kernelBatches;
     std::vector<string> invokeBatches;
@@ -362,26 +334,32 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
     objFilePath.append(mGroup->mName);
     objFilePath.append(".o");
 
-    string outputFileName(mGroup->mName);
+    const char* resName = mGroup->mName;
     string coreLibRelaxedPath;
     const string& coreLibPath = getCoreLibPath(getCpuRefImpl()->getContext(),
                                                &coreLibRelaxedPath);
 
     vector<const char*> arguments;
-    string output_dir(cacheDir);
-    setupCompileArguments(inputs, kernelBatches, invokeBatches, output_dir,
-                          outputFileName, coreLibPath, coreLibRelaxedPath,
+    setupCompileArguments(inputs, kernelBatches, invokeBatches, cacheDir,
+                          resName, coreLibPath.c_str(), coreLibRelaxedPath.c_str(),
                           &arguments);
 
     std::unique_ptr<const char> cmdLine(rsuJoinStrings(arguments.size() - 1,
-                                                  arguments.data()));
+                                                       arguments.data()));
 
-    if (!getChecksum(inputs, coreLibPath, coreLibRelaxedPath, cmdLine.get(),
-                     mChecksum)) {
+    inputs.push_back(coreLibPath.c_str());
+    inputs.push_back(coreLibRelaxedPath.c_str());
+
+    uint32_t checksum = constructBuildChecksum(nullptr, 0, cmdLine.get(),
+                                               inputs.data(), inputs.size());
+
+    if (checksum == 0) {
         return;
     }
 
-    const char* resName = outputFileName.c_str();
+    std::stringstream ss;
+    ss << std::hex << checksum;
+    const char* checksumStr = ss.str().c_str();
 
     //===--------------------------------------------------------------------===//
     // Try to load a shared lib from code cache matching filename and checksum
@@ -390,16 +368,9 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
     mScriptObj = SharedLibraryUtils::loadSharedLibrary(cacheDir, resName);
     if (mScriptObj != nullptr) {
         mExecutable = ScriptExecutable::createFromSharedObject(
-            getCpuRefImpl()->getContext(), mScriptObj);
+            getCpuRefImpl()->getContext(), mScriptObj, checksum);
         if (mExecutable != nullptr) {
-            if (mExecutable->isChecksumValid(mChecksum)) {
-                return;
-            } else {
-                ALOGE("Invalid checksum from cached so: %s (expected: %s)",
-                      mExecutable->getBuildChecksum(), mChecksum);
-            }
-            delete mExecutable;
-            mExecutable = nullptr;
+            return;
         } else {
             ALOGE("Failed to create an executable object from so file");
         }
@@ -412,7 +383,7 @@ void CpuScriptGroup2Impl::compile(const char* cacheDir) {
     //===--------------------------------------------------------------------===//
 
     arguments.push_back("-build-checksum");
-    arguments.push_back(mChecksum);
+    arguments.push_back(checksumStr);
     arguments.push_back(nullptr);
 
     bool compiled = rsuExecuteCommand(RsdCpuScriptImpl::BCC_EXE_PATH,
diff --git a/cpu_ref/rsCpuScriptGroup2.h b/cpu_ref/rsCpuScriptGroup2.h
index e3f345e2..1883f903 100644
--- a/cpu_ref/rsCpuScriptGroup2.h
+++ b/cpu_ref/rsCpuScriptGroup2.h
@@ -74,7 +74,6 @@ private:
     RsdCpuReferenceImpl* mCpuRefImpl;
     const ScriptGroup2* mGroup;
     List<Batch*> mBatches;
-    char mChecksum[9];
     ScriptExecutable* mExecutable;
     void* mScriptObj;
 };