Broken debug build

[targos]

09:42:54 + '[' -x out/Debug/node ']'
09:42:54 ++ out/Debug/node -pe process.config.target_defaults.default_configuration
09:42:54 
09:42:54 
09:42:54 #
09:42:54 # Fatal error in ../deps/v8/src/init/bootstrapper.cc, line 4251
09:42:54 # Debug check failed: promise_fun->HasFastProperties().
09:42:54 #
09:42:54 #
09:42:54 #
09:42:54 #FailureMessage Object: 0x7ffd36653db0
09:42:54  1: 0x5569d6e6152d node::DumpBacktrace(_IO_FILE*) [out/Debug/node]
09:42:54  2: 0x5569d6fe657f  [out/Debug/node]
09:42:54  3: 0x5569d6fe659f  [out/Debug/node]
09:42:54  4: 0x5569d8ab53b6 V8_Fatal(char const*, int, char const*, ...) [out/Debug/node]
09:42:54  5: 0x5569d8ab53e5  [out/Debug/node]
09:42:54  6: 0x5569d761e8f8  [out/Debug/node]
09:42:54  7: 0x5569d761ea80 v8::internal::Genesis::InitializeExperimentalGlobal() [out/Debug/node]
09:42:54  8: 0x5569d762bb48 v8::internal::Genesis::Genesis(v8::internal::Isolate*, v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>, v8::Local<v8::ObjectTemplate>, unsigned long, v8::DeserializeInternalFieldsCallback, v8::MicrotaskQueue*) [out/Debug/node]
09:42:54  9: 0x5569d762ce21 v8::internal::Bootstrapper::CreateEnvironment(v8::internal::MaybeHandle<v8::internal::JSGlobalProxy>, v8::Local<v8::ObjectTemplate>, v8::ExtensionConfiguration*, unsigned long, v8::DeserializeInternalFieldsCallback, v8::MicrotaskQueue*) [out/Debug/node]
09:42:54 10: 0x5569d71b3c6a v8::NewContext(v8::Isolate*, v8::ExtensionConfiguration*, v8::MaybeLocal<v8::ObjectTemplate>, v8::MaybeLocal<v8::Value>, unsigned long, v8::DeserializeInternalFieldsCallback, v8::MicrotaskQueue*) [out/Debug/node]
09:42:54 11: 0x5569d71b4672 v8::Context::FromSnapshot(v8::Isolate*, unsigned long, v8::DeserializeInternalFieldsCallback, v8::ExtensionConfiguration*, v8::MaybeLocal<v8::Value>, v8::MicrotaskQueue*) [out/Debug/node]
09:42:54 12: 0x5569d6f92888 node::NodeMainInstance::CreateMainEnvironment(int*) [out/Debug/node]
09:42:54 13: 0x5569d6f92335 node::NodeMainInstance::Run() [out/Debug/node]
09:42:54 14: 0x5569d6ec6261 node::Start(int, char**) [out/Debug/node]
09:42:54 15: 0x5569d8714b72 main [out/Debug/node]
09:42:54 16: 0x7fdeac840b97 __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6]
09:42:54 17: 0x5569d6e0f06a _start [out/Debug/node]
09:42:54 + BUILD_TYPE=

[addaleax]

This was added in v8/v8@6f994a0

/cc @marjakh

[marjakh]

Yikes, this is unexpected. How can I repro this?

[addaleax]

@marjakh The following should work:

• Check out this repo (current HEAD is ec7f9a6)
• ./configure --v8-with-dchecks or ./configure --debug
• make -j8
• Run ./node (or ./node_g in the --debug case)

(You probably want to use --debug and possibly also pass --v8-non-optimized-debug to configure in order to get a reasonable debugging experience, it just takes a long, long time to build.)

[marjakh]

Thanks for the repro instructions!

Here are some initial findings (I haven't had the chance to look into this too much):

When promise_fun is initially created (at snapshot generation time), it has fast properties as expected:

0x24ec23148999: [Function] in OldSpace
 - map: 0x0b2f70a44ba9 <Map(HOLEY_ELEMENTS)> [FastProperties]
 - prototype: 0x24ec23140991 <JSFunction (sfi = 0xe38194c9ff9)>
 - elements: 0x1fd3c5ec0b29 <FixedArray[0]> [HOLEY_ELEMENTS]
 - function prototype: 0x24ec23148c39 <Object map = 0xb2f70a41099>
 - initial_map: 0x0b2f70a41051 <Map(HOLEY_ELEMENTS)>
 - shared_info: 0x0e38194d08c9 <SharedFunctionInfo Promise>
 - name: 0x1fd3c5ec3da1 <String[7]: #Promise>
 - builtin: PromiseConstructor
 - formal_parameter_count: 1
 - kind: NormalFunction
 - context: 0x24ec23140121 <NativeContext[240]>
 - code: 0x20e18df51d81 <Code BUILTIN PromiseConstructor>

(Relevant line: map has FastProperties.)

But when node is starting, and tries to use this promise_fun, it already has dictionary properties (even before adding Promise.any):

0x12dc0e87a79: [Function] in OldSpace
 - map: 0x118de0140f79 <Map(HOLEY_ELEMENTS)> [DictionaryProperties]
 - prototype: 0x012dc0e80991 <JSFunction (sfi = 0x3d76780a519)>
 - elements: 0x1c7e79f40b29 <FixedArray[0]> [HOLEY_ELEMENTS]
 - function prototype: 0x012dc0e87da1 <Object map = 0x118de0141009>
 - initial_map: 0x118de0140fc1 <Map(HOLEY_ELEMENTS)>
 - shared_info: 0x03d767814e71 <SharedFunctionInfo Promise>
 - name: 0x1c7e79f43da1 <String[7]: #Promise>
 - builtin: PromiseConstructor
 - formal_parameter_count: 1
 - kind: NormalFunction
 - context: 0x012dc0e80121 <NativeContext[240]>
 - code: 0x3ae288611d81 <Code BUILTIN PromiseConstructor>

(Relevant line: map has DictionaryProperties.)

Does node do something nonstandard with the snapshot? Does it do something that would cause the function to turn to dictionary mode somewhere before creating a snapshot or just after reading it in?

If this is blocking you, I think it's ok to just remove the failing DCHECK. As it's already dictionary properties before adding the Function for Promise.any, it doesn't change anything. This should still be investigated of course, just that at the moment it doesn't look like it's a regression (meaning: promise_fun was dictionary props before, we just didn't have a check asserting it's not).

[targos]

/cc @joyeecheung

[marjakh]

Here's a stack trace for the moment where the map turns dictionary props during node_mksnapshot time:

node-v8/out/Debug/node_mksnapshot(v8::internal::JSObject::MigrateToMap(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::Map>, int)+0x3e5) [0x563d702e0265]
node-v8/out/Debug/node_mksnapshot(v8::internal::JSObject::NormalizeProperties(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSObject>, v8::internal::PropertyNormalizationMode, int, char const*)+0x72) [0x563d702e2e22]
node-v8/out/Debug/node_mksnapshot(v8::internal::JSObject::OptimizeAsPrototype(v8::internal::Handle<v8::internal::JSObject>, bool)+0x2cc) [0x563d702e30fc]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::SetPrototype(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::HeapObject>, bool)+0x85) [0x563d70318d15]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::RawCopy(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, int, int)+0x5d) [0x563d70318fcd]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::CopyDropDescriptors(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>)+0x48) [0x563d703191c8]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::CopyReplaceDescriptors(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::DescriptorArray>, v8::internal::Handle<v8::internal::LayoutDescriptor>, v8::internal::TransitionFlag, v8::internal::MaybeHandle<v8::internal::Name>, char const*, v8::internal::SimpleTransitionFlag)+0x47) [0x563d7031bfd7]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::Copy(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, char const*)+0x66) [0x563d7031c316]
node-v8/out/Debug/node_mksnapshot(v8::internal::JSObject::OptimizeAsPrototype(v8::internal::Handle<v8::internal::JSObject>, bool)+0xbe) [0x563d702e2eee]
node-v8/out/Debug/node_mksnapshot(v8::internal::Map::SetPrototype(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::HeapObject>, bool)+0x85) [0x563d70318d15]
node-v8/out/Debug/node_mksnapshot(+0x24597d9) [0x563d708947d9]
node-v8/out/Debug/node_mksnapshot(v8::internal::Runtime_DefineClass(int, unsigned long*, v8::internal::Isolate*)+0x6a) [0x563d70895caa]
node-v8/out/Debug/node_mksnapshot(+0xdfa13b) [0x563d6f23513b]

[marjakh]

My current understanding: it's debatable whether this is the expected behavior or not, but at least, given the current state of the things, the DCHECK is wrong. I'll remove it on V8 side.

[marjakh]

https://chromium-review.googlesource.com/c/v8/v8/+/2246577 is in the works

[marjakh]

The fix landed, do you need anything more from our side?

[ryzokuken]

Thanks a ton, @marjakh. No, once the commit lands on LKGR, we'd pick it up.

[joyeecheung]

Does node do something nonstandard with the snapshot? Does it do something that would cause the function to turn to dictionary mode somewhere before creating a snapshot or just after reading it in?

My guess is this has something to do with primordials baking which is done during snapshot building (we now extend Promise with a SafePromise class and copy the methods/prototype methods over before freezing it)

[marjakh]

Yep, that looks like it's causing this. (In particular: using Promise as a prototype.) It's ok, just has this side effect that Promise will now have dictionary properties in your snapshot. :)