From fd913fe365ac937d76ae3d2c79edf48923badd3b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 3 Dec 2018 08:23:34 +0800 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/24950 Reviewed-By: Anna Henningsen --- src/node_code_cache_stub.cc | 4 -- src/node_native_module.cc | 41 -------------------- src/node_native_module.h | 5 --- test/code-cache/test-code-cache-generator.js | 6 --- tools/generate_code_cache.js | 27 ++----------- tools/js2c.py | 21 +--------- 6 files changed, 4 insertions(+), 100 deletions(-) diff --git a/src/node_code_cache_stub.cc b/src/node_code_cache_stub.cc index 95997bb120bbf2..4fffa8e0c29557 100644 --- 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 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 index 85f8d83d63a162..98cc128b735cca 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -105,9 +105,7 @@ Local NativeModuleLoader::GetSource(Isolate* isolate, NativeModuleLoader::NativeModuleLoader() : config_(GetConfig()) { LoadJavaScriptSource(); - LoadJavaScriptHash(); LoadCodeCache(); - LoadCodeCacheHash(); } void NativeModuleLoader::CompileCodeCache( @@ -168,29 +166,6 @@ MaybeLocal NativeModuleLoader::CompileAsModule( env->context(), id, ¶meters, 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 index 02c769ef023907..54c5f388f7480f 100644 --- 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 index b440d4e58c7c88..78a2b1c9a639a0 100644 --- 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 index b185f6246d0ef6..3e21743f2c71b6 100644 --- 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 index d103a3d2364980..46cf918ba977f6 100755 --- 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():