Skip to content

Commit fd913fe

Browse files
joyeecheungaddaleax
authored andcommitted
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: #24950 Reviewed-By: Anna Henningsen <[email protected]>
1 parent 0e2fbe4 commit fd913fe

6 files changed

+4
-100
lines changed

src/node_code_cache_stub.cc

-4
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,5 @@ namespace native_module {
1111
// into native_module_loader.code_cache_.
1212
void NativeModuleLoader::LoadCodeCache() {}
1313

14-
// The generated source code would instert <std::string, std::string> pairs
15-
// into native_module_loader.code_cache_hash_.
16-
void NativeModuleLoader::LoadCodeCacheHash() {}
17-
1814
} // namespace native_module
1915
} // namespace node

src/node_native_module.cc

-41
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,7 @@ Local<String> NativeModuleLoader::GetSource(Isolate* isolate,
105105

106106
NativeModuleLoader::NativeModuleLoader() : config_(GetConfig()) {
107107
LoadJavaScriptSource();
108-
LoadJavaScriptHash();
109108
LoadCodeCache();
110-
LoadCodeCacheHash();
111109
}
112110

113111
void NativeModuleLoader::CompileCodeCache(
@@ -168,29 +166,6 @@ MaybeLocal<Value> NativeModuleLoader::CompileAsModule(
168166
env->context(), id, &parameters, result, env);
169167
}
170168

171-
// Currently V8 only checks that the length of the source code is the
172-
// same as the code used to generate the hash, so we add an additional
173-
// check here:
174-
// 1. During compile time, when generating node_javascript.cc and
175-
// node_code_cache.cc, we compute and include the hash of the
176-
// JavaScript source in both.
177-
// 2. At runtime, we check that the hash of the code being compiled
178-
// and the hash of the code used to generate the cache
179-
// (without the parameters) is the same.
180-
// This is based on the assumptions:
181-
// 1. `code_cache_hash` must be in sync with `code_cache`
182-
// (both defined in node_code_cache.cc)
183-
// 2. `source_hash` must be in sync with `source`
184-
// (both defined in node_javascript.cc)
185-
// 3. If `source_hash` is in sync with `code_cache_hash`,
186-
// then the source code used to generate `code_cache`
187-
// should be in sync with the source code in `source`
188-
// The only variable left, then, are the parameters passed to the
189-
// CompileFunctionInContext. If the parameters used generate the cache
190-
// is different from the one used to compile modules at run time, then
191-
// there could be false postivies, but that should be rare and should fail
192-
// early in the bootstrap process so it should be easy to detect and fix.
193-
194169
// Returns nullptr if there is no code cache corresponding to the id
195170
ScriptCompiler::CachedData* NativeModuleLoader::GetCachedData(
196171
const char* id) const {
@@ -204,22 +179,6 @@ ScriptCompiler::CachedData* NativeModuleLoader::GetCachedData(
204179
const uint8_t* code_cache_value = it->second.one_bytes_data();
205180
size_t code_cache_length = it->second.length();
206181

207-
const auto it2 = code_cache_hash_.find(id);
208-
CHECK_NE(it2, code_cache_hash_.end());
209-
const std::string& code_cache_hash_value = it2->second;
210-
211-
const auto it3 = source_hash_.find(id);
212-
CHECK_NE(it3, source_hash_.end());
213-
const std::string& source_hash_value = it3->second;
214-
215-
// It may fail when any of the inputs of the `node_js2c` target in
216-
// node.gyp is modified but the tools/generate_code_cache.js
217-
// is not re run.
218-
// FIXME(joyeecheung): Figure out how to resolve the dependency issue.
219-
// When the code cache was introduced we were at a point where refactoring
220-
// node.gyp may not be worth the effort.
221-
CHECK_EQ(code_cache_hash_value, source_hash_value);
222-
223182
return new ScriptCompiler::CachedData(code_cache_value, code_cache_length);
224183
}
225184

src/node_native_module.h

-5
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,12 @@ class NativeModuleLoader {
7575

7676
// Generated by tools/js2c.py as node_javascript.cc
7777
void LoadJavaScriptSource(); // Loads data into source_
78-
void LoadJavaScriptHash(); // Loads data into source_hash_
7978
UnionBytes GetConfig(); // Return data for config.gypi
8079

8180
// Generated by tools/generate_code_cache.js as node_code_cache.cc when
8281
// the build is configured with --code-cache-path=.... They are noops
8382
// in node_code_cache_stub.cc
8483
void LoadCodeCache(); // Loads data into code_cache_
85-
void LoadCodeCacheHash(); // Loads data into code_cache_hash_
8684

8785
v8::ScriptCompiler::CachedData* GetCachedData(const char* id) const;
8886

@@ -105,9 +103,6 @@ class NativeModuleLoader {
105103
NativeModuleRecordMap source_;
106104
NativeModuleRecordMap code_cache_;
107105
UnionBytes config_;
108-
109-
NativeModuleHashMap source_hash_;
110-
NativeModuleHashMap code_cache_hash_;
111106
};
112107

113108
} // namespace native_module

test/code-cache/test-code-cache-generator.js

-6
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ if (child.status !== 0) {
3131

3232
// Verifies that:
3333
// - node::LoadCodeCache()
34-
// - node::LoadCodeCacheHash()
3534
// are defined in the generated code.
3635
// See src/node_native_module.h for explanations.
3736

@@ -41,18 +40,13 @@ const rl = readline.createInterface({
4140
});
4241

4342
let hasCacheDef = false;
44-
let hasHashDef = false;
4543

4644
rl.on('line', common.mustCallAtLeast((line) => {
4745
if (line.includes('LoadCodeCache(')) {
4846
hasCacheDef = true;
4947
}
50-
if (line.includes('LoadCodeCacheHash(')) {
51-
hasHashDef = true;
52-
}
5348
}, 2));
5449

5550
rl.on('close', common.mustCall(() => {
5651
assert.ok(hasCacheDef);
57-
assert.ok(hasHashDef);
5852
}));

tools/generate_code_cache.js

+3-24
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// of `configure`.
99

1010
const {
11-
getSource,
1211
getCodeCache,
1312
cachableBuiltins
1413
} = require('internal/bootstrap/cache');
@@ -19,13 +18,6 @@ const {
1918
}
2019
} = require('util');
2120

22-
function hash(str) {
23-
if (process.versions.openssl) {
24-
return require('crypto').createHash('sha256').update(str).digest('hex');
25-
}
26-
return '';
27-
}
28-
2921
const fs = require('fs');
3022

3123
const resultPath = process.argv[2];
@@ -65,26 +57,18 @@ function getInitalizer(key, cache) {
6557
const defName = `${key.replace(/\//g, '_').replace(/-/g, '_')}_raw`;
6658
const definition = `static const uint8_t ${defName}[] = {\n` +
6759
`${cache.join(',')}\n};`;
68-
const source = getSource(key);
69-
const sourceHash = hash(source);
7060
const initializer =
7161
'code_cache_.emplace(\n' +
7262
` "${key}",\n` +
7363
` UnionBytes(${defName}, arraysize(${defName}))\n` +
7464
');';
75-
const hashIntializer =
76-
'code_cache_hash_.emplace(\n' +
77-
` "${key}",\n` +
78-
` "${sourceHash}"\n` +
79-
');';
8065
return {
81-
definition, initializer, hashIntializer, sourceHash
66+
definition, initializer
8267
};
8368
}
8469

8570
const cacheDefinitions = [];
8671
const cacheInitializers = [];
87-
const cacheHashInitializers = [];
8872
let totalCacheSize = 0;
8973

9074
function lexical(a, b) {
@@ -107,13 +91,12 @@ for (const key of cachableBuiltins.sort(lexical)) {
10791
const size = cachedData.byteLength;
10892
totalCacheSize += size;
10993
const {
110-
definition, initializer, hashIntializer, sourceHash
94+
definition, initializer,
11195
} = getInitalizer(key, cachedData);
11296
cacheDefinitions.push(definition);
11397
cacheInitializers.push(initializer);
114-
cacheHashInitializers.push(hashIntializer);
11598
console.log(`Generated cache for '${key}', size = ${formatSize(size)}` +
116-
`, hash = ${sourceHash}, total = ${formatSize(totalCacheSize)}`);
99+
`, total = ${formatSize(totalCacheSize)}`);
117100
}
118101

119102
const result = `#include "node_native_module.h"
@@ -131,10 +114,6 @@ void NativeModuleLoader::LoadCodeCache() {
131114
${cacheInitializers.join('\n ')}
132115
}
133116
134-
void NativeModuleLoader::LoadCodeCacheHash() {
135-
${cacheHashInitializers.join('\n ')}
136-
}
137-
138117
} // namespace native_module
139118
} // namespace node
140119
`;

tools/js2c.py

+1-20
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,6 @@ def ReadMacros(lines):
189189
{initializers}
190190
}}
191191
192-
void NativeModuleLoader::LoadJavaScriptHash() {{
193-
{hash_initializers}
194-
}}
195-
196192
UnionBytes NativeModuleLoader::GetConfig() {{
197193
return UnionBytes(config_raw, arraysize(config_raw)); // config.gypi
198194
}}
@@ -218,13 +214,6 @@ def ReadMacros(lines):
218214
);
219215
"""
220216

221-
HASH_INITIALIZER = """\
222-
source_hash_.emplace(
223-
"{module}",
224-
"{hash_value}"
225-
);
226-
"""
227-
228217
DEPRECATED_DEPS = """\
229218
'use strict';
230219
process.emitWarning(
@@ -251,8 +240,6 @@ def JS2C(source, target):
251240
# Build source code lines
252241
definitions = []
253242
initializers = []
254-
hash_initializers = []
255-
config_initializers = []
256243

257244
def GetDefinition(var, source):
258245
# Treat non-ASCII as UTF-8 and convert it to UTF-16.
@@ -267,15 +254,11 @@ def GetDefinition(var, source):
267254

268255
def AddModule(module, source):
269256
var = '%s_raw' % (module.replace('-', '_').replace('/', '_'))
270-
source_hash = hashlib.sha256(source).hexdigest()
271257
definition = GetDefinition(var, source)
272258
initializer = INITIALIZER.format(module=module,
273259
var=var)
274-
hash_initializer = HASH_INITIALIZER.format(module=module,
275-
hash_value=source_hash)
276260
definitions.append(definition)
277261
initializers.append(initializer)
278-
hash_initializers.append(hash_initializer)
279262

280263
for name in modules:
281264
lines = ReadFile(str(name))
@@ -320,9 +303,7 @@ def AddModule(module, source):
320303
output = open(str(target[0]), "w")
321304
output.write(
322305
TEMPLATE.format(definitions=''.join(definitions),
323-
initializers=''.join(initializers),
324-
hash_initializers=''.join(hash_initializers),
325-
config_initializers=''.join(config_initializers)))
306+
initializers=''.join(initializers)))
326307
output.close()
327308

328309
def main():

0 commit comments

Comments
 (0)