Debug build doesn't work on Windows

[seishun]

• Version: master
• Platform: Windows
• Subsystem: n/a

A debug build built using vcbuild debug and the latest VS2017 exits immediately. It seems to be a compiler bug: the EnvironmentOptionsParser instance is initialized before the DebugOptionsParser instance, causing a segfault. I would like someone to confirm the issue before I continue looking into it.

[seishun]

cc @nodejs/platform-windows

[refack]

Confirmed.
I'm not sure it's a Compiler bug, it might be Undefined Behavior. I think I was able to fix this by reordering the singleton declarations (*OptionsParser::instance) in node_options.cc by their order of dependency. e.g.

node/src/node_options.cc

Line 121 in 17c6b1d

const DebugOptionsParser DebugOptionsParser::instance;

[seishun]

They are already in the dependency order, aren't they?

[refack]

They are already in the dependency order, aren't they?

I think there is something more subtle that is related to static init order. But I didn't fully grok this.

I found my patch: #25596

[seishun]

The static init order thing is irrelevant, they are all in the same translation unit here.

[refack]

they are all in the same translation unit here.

IIUC, not necessarily since they are used in two other places:

node/src/node_worker.cc

Line 440 in 01cd219

options_parser::PerIsolateOptionsParser::instance.Parse(

and

node/src/node.cc

Line 908 in 55e0ad9

options_parser::PerProcessOptionsParser::instance.Parse(

[seishun]

That's a usage, not a definition.

[refack]

Are we sure those two lines are not part of some other static construction?
Or alternatively that

node/src/node_options.cc

Line 121 in 17c6b1d

const DebugOptionsParser DebugOptionsParser::instance;

isn't optimized away?

[addaleax]

Are we sure those two lines are not part of some other static construction?

Based on grepping the source code for cli_options, I would say that this is not the case, yes. The way in which @seishun narrowed down the issue might tell us more?

[seishun]

Narrowing it down was easy, I just pressed F5 and saw that the DebugOptionsParser instance is full of zeroes inside the EnvironmentOptionsParser constructor, and then confirmed it by setting a breakpoint in the DebugOptionsParser and seeing that it's never hit.

I'm now trying to come up with a minimal test case.

[seishun]

From dumpbin /all node_options.obj output:

                                                Symbol    Symbol
 Offset    Type              Applied To         Index     Name
 --------  ----------------  -----------------  --------  ------
 00000000  ADDR64            00000000 00000000      5219  ??__E?instance@EnvironmentOptionsParser@options_parser@node@@2V123@B@@YAXXZ (void __cdecl `dynamic initializer for 'public: static class node::options_parser::EnvironmentOptionsParser const node::options_parser::EnvironmentOptionsParser::instance''(void))
 00000008  ADDR64            00000000 00000000      521C  ??__E?instance@PerIsolateOptionsParser@options_parser@node@@2V123@B@@YAXXZ (void __cdecl `dynamic initializer for 'public: static class node::options_parser::PerIsolateOptionsParser const node::options_parser::PerIsolateOptionsParser::instance''(void))
 00000010  ADDR64            00000000 00000000      521F  ??__E?instance@PerProcessOptionsParser@options_parser@node@@2V123@B@@YAXXZ (void __cdecl `dynamic initializer for 'public: static class node::options_parser::PerProcessOptionsParser const node::options_parser::PerProcessOptionsParser::instance''(void))
 00000018  ADDR64            00000000 00000000      5208  ??__Ecli_options_mutex@per_process@node@@YAXXZ (void __cdecl node::per_process::`dynamic initializer for 'cli_options_mutex''(void))
 00000020  ADDR64            00000000 00000000      520F  ??__Ecli_options@per_process@node@@YAXXZ (void __cdecl node::per_process::`dynamic initializer for 'cli_options''(void))
 00000028  ADDR64            00000000 00000000      5216  ??__E?instance@DebugOptionsParser@options_parser@node@@2V123@B@@YAXXZ (void __cdecl `dynamic initializer for 'public: static class node::options_parser::DebugOptionsParser const node::options_parser::DebugOptionsParser::instance''(void))

I wonder if cli_options_mutex or cli_options is throwing it off somehow?

[addaleax]

@seishun I think this means that DebugOptionsParser is actually initialized last, right? If so, I think this is pretty clearly a compiler bug…

[seishun]

Correct, this is from the .CRT$XCU section, which defines the dynamic initialization order (CRT Initialization).