src: remove code cache integrity check

In preparation of sharing code cache among different threads - we simply rely on v8 to reject invalid cache, since there isn't any serious consequence when the cache is invalid anyway.
nodejs · joyeecheung · Dec 3, 2018 · Dec 3, 2018 · Dec 11, 2018 · Dec 11, 2018
commit 216d5725da96b5f99898de57f051e27db15d35bc
diff --git a/src/node_code_cache_stub.cc b/src/node_code_cache_stub.cc
@@ -11,9 +11,5 @@ namespace native_module {
 // into native_module_loader.code_cache_.
 void NativeModuleLoader::LoadCodeCache() {}
 
-// The generated source code would instert <std::string, std::string> pairs
-// into native_module_loader.code_cache_hash_.
-void NativeModuleLoader::LoadCodeCacheHash() {}
-
 }  // namespace native_module
 }  // namespace node
diff --git a/src/node_native_module.cc b/src/node_native_module.cc
@@ -105,9 +105,7 @@ Local<String> NativeModuleLoader::GetSource(Isolate* isolate,
 
 NativeModuleLoader::NativeModuleLoader() : config_(GetConfig()) {
   LoadJavaScriptSource();
-  LoadJavaScriptHash();
   LoadCodeCache();
-  LoadCodeCacheHash();
 }
 
 void NativeModuleLoader::CompileCodeCache(
@@ -168,29 +166,6 @@ MaybeLocal<Value> NativeModuleLoader::CompileAsModule(
       env->context(), id, &parameters, result, env);
 }
 
-// Currently V8 only checks that the length of the source code is the
-// same as the code used to generate the hash, so we add an additional
-// check here:
-// 1. During compile time, when generating node_javascript.cc and
-//    node_code_cache.cc, we compute and include the hash of the
-//    JavaScript source in both.
-// 2. At runtime, we check that the hash of the code being compiled
-//   and the hash of the code used to generate the cache
-//   (without the parameters) is the same.
-// This is based on the assumptions:
-// 1. `code_cache_hash` must be in sync with `code_cache`
-//     (both defined in node_code_cache.cc)
-// 2. `source_hash` must be in sync with `source`
-//     (both defined in node_javascript.cc)
-// 3. If `source_hash` is in sync with `code_cache_hash`,
-//    then the source code used to generate `code_cache`
-//    should be in sync with the source code in `source`
-// The only variable left, then, are the parameters passed to the
-// CompileFunctionInContext. If the parameters used generate the cache
-// is different from the one used to compile modules at run time, then
-// there could be false postivies, but that should be rare and should fail
-// early in the bootstrap process so it should be easy to detect and fix.
-
 // Returns nullptr if there is no code cache corresponding to the id
 ScriptCompiler::CachedData* NativeModuleLoader::GetCachedData(
     const char* id) const {
@@ -204,22 +179,6 @@ ScriptCompiler::CachedData* NativeModuleLoader::GetCachedData(
   const uint8_t* code_cache_value = it->second.one_bytes_data();
   size_t code_cache_length = it->second.length();
 
-  const auto it2 = code_cache_hash_.find(id);
-  CHECK_NE(it2, code_cache_hash_.end());
-  const std::string& code_cache_hash_value = it2->second;
-
-  const auto it3 = source_hash_.find(id);
-  CHECK_NE(it3, source_hash_.end());
-  const std::string& source_hash_value = it3->second;
-
-  // It may fail when any of the inputs of the `node_js2c` target in
-  // node.gyp is modified but the tools/generate_code_cache.js
-  // is not re run.
-  // FIXME(joyeecheung): Figure out how to resolve the dependency issue.
-  // When the code cache was introduced we were at a point where refactoring
-  // node.gyp may not be worth the effort.
-  CHECK_EQ(code_cache_hash_value, source_hash_value);
-
   return new ScriptCompiler::CachedData(code_cache_value, code_cache_length);
 }
 

diff --git a/src/node_native_module.h b/src/node_native_module.h
@@ -75,14 +75,12 @@ class NativeModuleLoader {
 
   // Generated by tools/js2c.py as node_javascript.cc
   void LoadJavaScriptSource();  // Loads data into source_
-  void LoadJavaScriptHash();    // Loads data into source_hash_
   UnionBytes GetConfig();       // Return data for config.gypi
 
   // Generated by tools/generate_code_cache.js as node_code_cache.cc when
   // the build is configured with --code-cache-path=.... They are noops
   // in node_code_cache_stub.cc
   void LoadCodeCache();      // Loads data into code_cache_
-  void LoadCodeCacheHash();  // Loads data into code_cache_hash_
 
   v8::ScriptCompiler::CachedData* GetCachedData(const char* id) const;
 
@@ -105,9 +103,6 @@ class NativeModuleLoader {
   NativeModuleRecordMap source_;
   NativeModuleRecordMap code_cache_;
   UnionBytes config_;
-
-  NativeModuleHashMap source_hash_;
-  NativeModuleHashMap code_cache_hash_;
 };
 
 }  // namespace native_module

diff --git a/test/code-cache/test-code-cache-generator.js b/test/code-cache/test-code-cache-generator.js
@@ -31,7 +31,6 @@ if (child.status !== 0) {
 
 // Verifies that:
 // - node::LoadCodeCache()
-// - node::LoadCodeCacheHash()
 // are defined in the generated code.
 // See src/node_native_module.h for explanations.
 
@@ -41,18 +40,13 @@ const rl = readline.createInterface({
 });
 
 let hasCacheDef = false;
-let hasHashDef = false;
 
 rl.on('line', common.mustCallAtLeast((line) => {
   if (line.includes('LoadCodeCache(')) {
     hasCacheDef = true;
   }
-  if (line.includes('LoadCodeCacheHash(')) {
-    hasHashDef = true;
-  }
 }, 2));
 
 rl.on('close', common.mustCall(() => {
   assert.ok(hasCacheDef);
-  assert.ok(hasHashDef);
 }));
diff --git a/tools/generate_code_cache.js b/tools/generate_code_cache.js
@@ -8,7 +8,6 @@
 // of `configure`.
 
 const {
-  getSource,
   getCodeCache,
   cachableBuiltins
 } = require('internal/bootstrap/cache');
@@ -19,13 +18,6 @@ const {
   }
 } = require('util');
 
-function hash(str) {
-  if (process.versions.openssl) {
-    return require('crypto').createHash('sha256').update(str).digest('hex');
-  }
-  return '';
-}
-
 const fs = require('fs');
 
 const resultPath = process.argv[2];
@@ -65,26 +57,18 @@ function getInitalizer(key, cache) {
   const defName = `${key.replace(/\//g, '_').replace(/-/g, '_')}_raw`;
   const definition = `static const uint8_t ${defName}[] = {\n` +
                      `${cache.join(',')}\n};`;
-  const source = getSource(key);
-  const sourceHash = hash(source);
   const initializer =
     'code_cache_.emplace(\n' +
     `  "${key}",\n` +
     `  UnionBytes(${defName}, arraysize(${defName}))\n` +
     ');';
-  const hashIntializer =
-    'code_cache_hash_.emplace(\n' +
-    `  "${key}",\n` +
-    `  "${sourceHash}"\n` +
-    ');';
   return {
-    definition, initializer, hashIntializer, sourceHash
+    definition, initializer
   };
 }
 
 const cacheDefinitions = [];
 const cacheInitializers = [];
-const cacheHashInitializers = [];
 let totalCacheSize = 0;
 
 function lexical(a, b) {
@@ -107,13 +91,12 @@ for (const key of cachableBuiltins.sort(lexical)) {
   const size = cachedData.byteLength;
   totalCacheSize += size;
   const {
-    definition, initializer, hashIntializer, sourceHash
+    definition, initializer,
   } = getInitalizer(key, cachedData);
   cacheDefinitions.push(definition);
   cacheInitializers.push(initializer);
-  cacheHashInitializers.push(hashIntializer);
   console.log(`Generated cache for '${key}', size = ${formatSize(size)}` +
-              `, hash = ${sourceHash}, total = ${formatSize(totalCacheSize)}`);
+              `, total = ${formatSize(totalCacheSize)}`);
 }
 
 const result = `#include "node_native_module.h"
@@ -131,10 +114,6 @@ void NativeModuleLoader::LoadCodeCache() {
   ${cacheInitializers.join('\n  ')}
 }
 
-void NativeModuleLoader::LoadCodeCacheHash() {
-  ${cacheHashInitializers.join('\n  ')}
-}
-
 }  // namespace native_module
 }  // namespace node
 `;

diff --git a/tools/js2c.py b/tools/js2c.py
@@ -189,10 +189,6 @@ def ReadMacros(lines):
   {initializers}
 }}
 
-void NativeModuleLoader::LoadJavaScriptHash() {{
-  {hash_initializers}
-}}
-
 UnionBytes NativeModuleLoader::GetConfig() {{
   return UnionBytes(config_raw, arraysize(config_raw));  // config.gypi
 }}
@@ -218,13 +214,6 @@ def ReadMacros(lines):
 );
 """
 
-HASH_INITIALIZER = """\
-source_hash_.emplace(
-    "{module}",
-    "{hash_value}"
-);
-"""
-
 DEPRECATED_DEPS = """\
 'use strict';
 process.emitWarning(
@@ -251,8 +240,6 @@ def JS2C(source, target):
   # Build source code lines
   definitions = []
   initializers = []
-  hash_initializers = []
-  config_initializers = []
 
   def GetDefinition(var, source):
     # Treat non-ASCII as UTF-8 and convert it to UTF-16.
@@ -267,15 +254,11 @@ def GetDefinition(var, source):
 
   def AddModule(module, source):
     var = '%s_raw' % (module.replace('-', '_').replace('/', '_'))
-    source_hash = hashlib.sha256(source).hexdigest()
     definition = GetDefinition(var, source)
     initializer = INITIALIZER.format(module=module,
                                      var=var)
-    hash_initializer = HASH_INITIALIZER.format(module=module,
-                                               hash_value=source_hash)
     definitions.append(definition)
     initializers.append(initializer)
-    hash_initializers.append(hash_initializer)
 
   for name in modules:
     lines = ReadFile(str(name))
@@ -320,9 +303,7 @@ def AddModule(module, source):
   output = open(str(target[0]), "w")
   output.write(
     TEMPLATE.format(definitions=''.join(definitions),
-                    initializers=''.join(initializers),
-                    hash_initializers=''.join(hash_initializers),
-                    config_initializers=''.join(config_initializers)))
+                    initializers=''.join(initializers)))
   output.close()
 
 def main():