Repository navigation
vm.compileFunction is crashing the node process with unknown error #27256
Description
- Version: 10.15.3
- Platform: Windows 10
- Subsystem: vm
The following simple script crashes the node process with no apparent reason:
const vm = require('vm');
console.log(process.version);
vm.compileFunction('return', ['ab'.replace('b', 'b'.repeat(12)).split('').join('')]);
console.log('compiled ok 1');
vm.compileFunction('return', ['ab'.replace('b', 'b'.repeat(11))]);
console.log('compiled ok 2');
try {
vm.compileFunction('return', ['ab'.replace('b', 'b'.repeat(12))]); // crash here
console.log('compiled ok 3');
} catch (e) { // no exception was caught
console.error(e);
}Output:
v10.15.3
compiled ok 1
compiled ok 2
Activity
addaleax commentedon Apr 16, 2019
This seems to have been fixed between v11.10.1 and v11.11.0, so it’s probably just a question of figuring out the bugfix and backporting it. I’ll try to bisect.
kalinkrustev commentedon Apr 16, 2019
I am curious to see the reason behind this. If you find the fix, can you please post a reference here?
addaleax commentedon Apr 16, 2019
Ok, so, there are two issues here:
The crash is being addressed by 7b19893; in particular, 7b19893#diff-0cf206672499c2f86db4ffb0cc5b668bL1101 would previously have crashed, because there was no exception caught by the
try_catchobject (which lead to a nullptr dereference).However, this does not actually work on newer released versions of Node.js, it just returns
undefinedinstead of a compiled function. That seems like a V8 bug, which is fixed on Node.js master but not on Node v11.x; I’ll try to investigate a bit more.addaleax commentedon Apr 16, 2019
Okay, digging a bit more: Until v8/v8@61f4c22, V8 assumed that the argument name string was not internally represented as a concatenated string. Apparently, V8 has a cutoff value here that decides whether the result of your string concatenation is internally represented as a contiguous block of memory, or as the concatenation of two or more contiguous blocks of memory. The “is this a valid identifier” check would simply fail in the latter case. This is fixed by the linked V8 commit.
It’s also not ideal that V8 doesn’t throw an exception in this case, which imo it should when returning an empty
MaybeLocal<>fromScriptCompiler::CompileFunctionInContext()… @hashseed Should that be added? I’d be happy to do so.deps: V8: backport 61f4c22
addaleax commentedon Apr 16, 2019
#27259 should address the main issue here for v11.x, and could be backported to v10.x.
hashseed commentedon Apr 17, 2019
Yes. I think that it should throw an exception indeed, similar to other compile APIs.
deps: V8: backport 61f4c22
juanarbol commentedon May 7, 2021
I'll proceed to close this issue, this seems to be fixed on Fermiun (LTS), and Dubnium is sadly no longer maintained.