SyntheticModule aborts with illegal hardware instruction during linking

[SimenB]

• Version: v13.2.0
• Platform: macOS
• Subsystem: VM (ESM)

What steps will reproduce the bug?

// file.mjs
import {SyntheticModule} from 'vm';
import * as fs from 'fs';

const m = new SyntheticModule(['default', ...Object.keys(fs)], function () {});

// this aborts
m.link(() => {});

$ node --experimental-vm-modules file.mjs
(node:92375) ExperimentalWarning: The ESM module loader is experimental.


#
# Fatal error in , line 0
# Check failed: exports->Lookup(name).IsTheHole(isolate).
#
#
#
#FailureMessage Object: 0x7ffeefbf5e60
 1: 0x1001019c2 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 2: 0x100f4a442 V8_Fatal(char const*, ...) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 3: 0x1005ab307 v8::internal::SyntheticModule::PrepareInstantiate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SyntheticModule>, v8::Local<v8::Context>, v8::MaybeLocal<v8::Module> (*)(v8::Local<v8::Context>, v8::Local<v8::String>, v8::Local<v8::Module>)) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 4: 0x10055a2fa v8::internal::Module::Instantiate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Module>, v8::Local<v8::Context>, v8::MaybeLocal<v8::Module> (*)(v8::Local<v8::Context>, v8::Local<v8::String>, v8::Local<v8::Module>)) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 5: 0x1001dff79 v8::Module::InstantiateModule(v8::Local<v8::Context>, v8::MaybeLocal<v8::Module> (*)(v8::Local<v8::Context>, v8::Local<v8::String>, v8::Local<v8::Module>)) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 6: 0x10006a534 node::loader::ModuleWrap::Instantiate(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 7: 0x100243068 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 8: 0x1002425fc v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
 9: 0x100241d22 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
10: 0x1009d4bd9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
11: 0x100960c7b Builtins_InterpreterEntryTrampoline [/Users/simen/.nvm/versions/node/v13.12.0/bin/node]
[1]    92375 illegal hardware instruction  node --experimental-vm-modules file.mjs

How often does it reproduce? Is there a required condition?

It always happens

What is the expected behavior?

Node process shouldn't abort

What do you see instead?

It aborts

Additional information

Import fs via require rather than import doesn't abort.

import {createRequire} from 'module';
import {SyntheticModule} from 'vm';

const require = createRequire(import.meta.url);
const fs = require('fs');

const m = new SyntheticModule(['default', ...Object.keys(fs)], function () {});

m.link(() => {});

Is it not allowed to do Object.keys on the imported module? I find it weird it explodes only if doing link and not otherwise

[himself65]

I think the reason is caused by node core modules have default export

node/lib/internal/modules/esm/translators.js

Lines 103 to 115 in 0f96dc2

	if (module && module.loaded) {
	const exports = module.exports;
	return new ModuleWrap(url, undefined, ['default'], function() {
	this.setExport('default', exports);
	});
	}
	return new ModuleWrap(url, undefined, ['default'], function() {
	debug(`Loading CJSModule ${url}`);
	// We don't care about the return val of _load here because Module#load
	// will handle it for us by checking the loader registry and filling the
	// exports like above
	CJSModule._load(pathname, undefined, isMain);
	});

[himself65]

as has been said above. This will crash too

// a.mjs
export default {

}

// file.mjs
import {SyntheticModule} from 'vm';
import * as fs from './a.mjs';

const m = new SyntheticModule(['default', ...Object.keys(fs)], function () {});

// this aborts
m.link(() => {
  console.log('link')
});

[SimenB]

The docs state that all builtins provide both named and default exports: https://nodejs.org/api/esm.html#esm_builtin_modules

However, the issue seems to be duplicate 'default' in the exportNames argument.

// file.mjs
import {SyntheticModule} from 'vm';

const m = new SyntheticModule(['default', 'default'], function () {});

// this aborts
m.link(() => {
  console.log('link')
});

[himself65]

oh, you're right. use a Set to remove duplicate name seems ok

[SimenB]

I'd expect node to yell at me for having duplicate names rather than silently dedupe them in this case.

For my (real) use case, require works fine, I got this error when playing with the API. So I'm not blocked by this whatsoever 🙂

[himself65]

I'm working on this.

[targos]

@himself65 I think we should make a copy of the exportNames array while it is being validated to avoid bad values at the C++ layer.