Skip to content

Commit

Permalink
src: remove code cache integrity check
Browse files Browse the repository at this point in the history
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: nodejs#24950
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
joyeecheung authored and refack committed Jan 10, 2019
1 parent 1370d95 commit f7da469
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 100 deletions.
4 changes: 0 additions & 4 deletions src/node_code_cache_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
41 changes: 0 additions & 41 deletions src/node_native_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ Local<String> NativeModuleLoader::GetSource(Isolate* isolate,

NativeModuleLoader::NativeModuleLoader() : config_(GetConfig()) {
LoadJavaScriptSource();
LoadJavaScriptHash();
LoadCodeCache();
LoadCodeCacheHash();
}

void NativeModuleLoader::CompileCodeCache(
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}

Expand Down
5 changes: 0 additions & 5 deletions src/node_native_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -105,9 +103,6 @@ class NativeModuleLoader {
NativeModuleRecordMap source_;
NativeModuleRecordMap code_cache_;
UnionBytes config_;

NativeModuleHashMap source_hash_;
NativeModuleHashMap code_cache_hash_;
};

} // namespace native_module
Expand Down
6 changes: 0 additions & 6 deletions test/code-cache/test-code-cache-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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);
}));
27 changes: 3 additions & 24 deletions tools/generate_code_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// of `configure`.

const {
getSource,
getCodeCache,
cachableBuiltins
} = require('internal/bootstrap/cache');
Expand All @@ -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];
Expand Down Expand Up @@ -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) {
Expand All @@ -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"
Expand All @@ -131,10 +114,6 @@ void NativeModuleLoader::LoadCodeCache() {
${cacheInitializers.join('\n ')}
}
void NativeModuleLoader::LoadCodeCacheHash() {
${cacheHashInitializers.join('\n ')}
}
} // namespace native_module
} // namespace node
`;
Expand Down
21 changes: 1 addition & 20 deletions tools/js2c.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
}}
Expand All @@ -218,13 +214,6 @@ def ReadMacros(lines):
);
"""

HASH_INITIALIZER = """\
source_hash_.emplace(
"{module}",
"{hash_value}"
);
"""

DEPRECATED_DEPS = """\
'use strict';
process.emitWarning(
Expand All @@ -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.
Expand All @@ -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))
Expand Down Expand Up @@ -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():
Expand Down

0 comments on commit f7da469

Please sign in to comment.