Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: for debug mode set v8_optimized_debug #23704

Merged
merged 2 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,9 @@

'openssl_fips%': '',

# Default to -O0 for debug builds.
'v8_optimized_debug%': 0,

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.7',
'v8_embedder_string': '-node.8',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
8 changes: 7 additions & 1 deletion configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,12 @@
default=False,
help='get more output from this script')

parser.add_option('--v8-non-optimized-debug',
action='store_true',
dest='v8_non_optimized_debug',
default=False,
help='compile V8 with minimal optimizations and with runtime checks')

# Create compile_commands.json in out/Debug and out/Release.
parser.add_option('-C',
action='store_true',
Expand Down Expand Up @@ -1138,7 +1144,7 @@ def configure_library(lib, output):
def configure_v8(o):
o['variables']['v8_enable_gdbjit'] = 1 if options.gdb else 0
o['variables']['v8_no_strict_aliasing'] = 1 # Work around compiler bugs.
o['variables']['v8_optimized_debug'] = 0 # Compile with -O0 in debug builds.
o['variables']['v8_optimized_debug'] = 0 if options.v8_non_optimized_debug else 1
o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables.
o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks.
o['variables']['v8_use_snapshot'] = 'false' if options.without_snapshot else 'true'
Expand Down
258 changes: 110 additions & 148 deletions deps/v8/gypfiles/toolchain.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1134,121 +1134,7 @@
}],
], # conditions
'configurations': {
# Abstract configuration for v8_optimized_debug == 0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am reading this correctly, this switches the configuration-based conditional to a variable-based conditional (v8_optimized_debug) to fix the bug? Can you paste the PR message into the commit message of the second commit, since it is a bit difficult to tell what the diff is doing due to all the indentations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the tl;dr if I flattened the inheritance structure. form:

}, # DebugBaseCommon
'Debug': {
'inherit_from': ['DebugBaseCommon'],
'conditions': [
['v8_optimized_debug==0', {
'inherit_from': ['DebugBase0'],
}, {
'inherit_from': ['DebugBase1'],
}],
],
}, # Debug

to
https://github.com/nodejs/node/blob/693c7bbe621f4ab8686397e301e1f8027e734dff/deps/v8/gypfiles/toolchain.gypi#L1136-L1137
...
https://github.com/nodejs/node/blob/693c7bbe621f4ab8686397e301e1f8027e734dff/deps/v8/gypfiles/toolchain.gypi#L1200

I'll document that

'DebugBase0': {
'abstract': 1,
'msvs_settings': {
'VCCLCompilerTool': {
'Optimization': '0',
'conditions': [
['component=="shared_library" or force_dynamic_crt==1', {
'RuntimeLibrary': '3', # /MDd
}, {
'RuntimeLibrary': '1', # /MTd
}],
],
},
'VCLinkerTool': {
'LinkIncremental': '2',
},
},
'variables': {
'v8_enable_slow_dchecks%': 1,
},
'conditions': [
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \
OS=="qnx" or OS=="aix"', {
'cflags!': [
'-O3',
'-O2',
'-O1',
'-Os',
],
'cflags': [
'-fdata-sections',
'-ffunction-sections',
],
}],
['OS=="mac"', {
'xcode_settings': {
'GCC_OPTIMIZATION_LEVEL': '0', # -O0
},
}],
['v8_enable_slow_dchecks==1', {
'defines': [
'ENABLE_SLOW_DCHECKS',
],
}],
],
}, # DebugBase0
# Abstract configuration for v8_optimized_debug == 1.
'DebugBase1': {
'abstract': 1,
'msvs_settings': {
'VCCLCompilerTool': {
'Optimization': '2',
'InlineFunctionExpansion': '2',
'EnableIntrinsicFunctions': 'true',
'FavorSizeOrSpeed': '0',
'StringPooling': 'true',
'BasicRuntimeChecks': '0',
'conditions': [
['component=="shared_library" or force_dynamic_crt==1', {
'RuntimeLibrary': '3', #/MDd
}, {
'RuntimeLibrary': '1', #/MTd
}],
],
},
'VCLinkerTool': {
'LinkIncremental': '1',
'OptimizeReferences': '2',
'EnableCOMDATFolding': '2',
},
},
'variables': {
'v8_enable_slow_dchecks%': 0,
},
'conditions': [
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \
OS=="qnx" or OS=="aix"', {
'cflags!': [
'-O0',
'-O1',
'-Os',
],
'cflags': [
'-fdata-sections',
'-ffunction-sections',
],
'conditions': [
# Don't use -O3 with sanitizers.
['asan==0 and msan==0 and lsan==0 \
and tsan==0 and ubsan==0 and ubsan_vptr==0', {
'cflags': ['-O3'],
'cflags!': ['-O2'],
}, {
'cflags': ['-O2'],
'cflags!': ['-O3'],
}],
],
}],
['OS=="mac"', {
'xcode_settings': {
'GCC_OPTIMIZATION_LEVEL': '3', # -O3
'GCC_STRICT_ALIASING': 'YES',
},
}],
['v8_enable_slow_dchecks==1', {
'defines': [
'ENABLE_SLOW_DCHECKS',
],
}],
],
}, # DebugBase1
# Common settings for the Debug configuration.
'DebugBaseCommon': {
'abstract': 1,
'Debug': {
'defines': [
'ENABLE_DISASSEMBLER',
'V8_ENABLE_CHECKS',
Expand Down Expand Up @@ -1311,27 +1197,126 @@
}],
],
}],
],
}, # DebugBaseCommon
'Debug': {
'inherit_from': ['DebugBaseCommon'],
'conditions': [
['v8_optimized_debug==0', {
'inherit_from': ['DebugBase0'],
'msvs_settings': {
'VCCLCompilerTool': {
'Optimization': '0',
'conditions': [
['component=="shared_library" or force_dynamic_crt==1', {
'RuntimeLibrary': '3', # /MDd
}, {
'RuntimeLibrary': '1', # /MTd
}],
],
},
'VCLinkerTool': {
'LinkIncremental': '2',
},
},
'variables': {
'v8_enable_slow_dchecks%': 1,
},
'conditions': [
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \
OS=="qnx" or OS=="aix"', {
'cflags!': [
'-O3',
'-O2',
'-O1',
'-Os',
],
'cflags': [
'-fdata-sections',
'-ffunction-sections',
],
}],
['OS=="mac"', {
'xcode_settings': {
'GCC_OPTIMIZATION_LEVEL': '0', # -O0
},
}],
['v8_enable_slow_dchecks==1', {
'defines': [
'ENABLE_SLOW_DCHECKS',
],
}],
],
}, {
'inherit_from': ['DebugBase1'],
'msvs_settings': {
'VCCLCompilerTool': {
'Optimization': '2',
'InlineFunctionExpansion': '2',
'EnableIntrinsicFunctions': 'true',
'FavorSizeOrSpeed': '0',
'StringPooling': 'true',
'BasicRuntimeChecks': '0',
'conditions': [
['component=="shared_library" or force_dynamic_crt==1', {
'RuntimeLibrary': '3', #/MDd
}, {
'RuntimeLibrary': '1', #/MTd
}],
],
},
'VCLinkerTool': {
'LinkIncremental': '1',
'OptimizeReferences': '2',
'EnableCOMDATFolding': '2',
},
},
'variables': {
'v8_enable_slow_dchecks%': 0,
},
'conditions': [
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \
OS=="qnx" or OS=="aix"', {
'cflags!': [
'-O0',
'-O1',
'-Os',
],
'cflags': [
'-fdata-sections',
'-ffunction-sections',
],
'conditions': [
# Don't use -O3 with sanitizers.
['asan==0 and msan==0 and lsan==0 \
and tsan==0 and ubsan==0 and ubsan_vptr==0', {
'cflags': ['-O3'],
'cflags!': ['-O2'],
}, {
'cflags': ['-O2'],
'cflags!': ['-O3'],
}],
],
}],
['OS=="mac"', {
'xcode_settings': {
'GCC_OPTIMIZATION_LEVEL': '3', # -O3
'GCC_STRICT_ALIASING': 'YES',
},
}],
['v8_enable_slow_dchecks==1', {
'defines': [
'ENABLE_SLOW_DCHECKS',
],
}],
],
}],
# Temporary refs: https://github.com/nodejs/node/pull/23801
['v8_enable_handle_zapping==1', {
'defines': ['ENABLE_HANDLE_ZAPPING',],
}],
],
}, # Debug
'ReleaseBase': {
'abstract': 1,

}, # DebugBaseCommon
'Release': {
'variables': {
'v8_enable_slow_dchecks%': 0,
},
# Temporary refs: https://github.com/nodejs/node/pull/23801
'defines!': ['ENABLE_HANDLE_ZAPPING',],
'conditions': [
['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" \
or OS=="aix"', {
Expand Down Expand Up @@ -1407,29 +1392,6 @@
}],
], # conditions
}, # Release
'Release': {
'inherit_from': ['ReleaseBase'],
# Temporary refs: https://github.com/nodejs/node/pull/23801
'defines!': ['ENABLE_HANDLE_ZAPPING',],
}, # Debug
'conditions': [
[ 'OS=="win"', {
# TODO(bradnelson): add a gyp mechanism to make this more graceful.
'Debug_x64': {
'inherit_from': ['DebugBaseCommon'],
'conditions': [
['v8_optimized_debug==0', {
'inherit_from': ['DebugBase0'],
}, {
'inherit_from': ['DebugBase1'],
}],
],
},
'Release_x64': {
'inherit_from': ['ReleaseBase'],
},
}],
],
}, # configurations
'msvs_disabled_warnings': [
4245, # Conversion with signed/unsigned mismatch.
Expand Down
21 changes: 4 additions & 17 deletions deps/v8/gypfiles/v8.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -2539,23 +2539,10 @@
'_HAS_EXCEPTIONS=0',
'BUILDING_V8_SHARED=1',
],
# This is defined trough `configurations` for GYP+ninja compatibility
'configurations': {
'Debug': {
'msvs_settings': {
'VCCLCompilerTool': {
'RuntimeTypeInfo': 'true',
'ExceptionHandling': 1,
},
}
},
'Release': {
'msvs_settings': {
'VCCLCompilerTool': {
'RuntimeTypeInfo': 'true',
'ExceptionHandling': 1,
},
}
'msvs_settings': {
'VCCLCompilerTool': {
'RuntimeTypeInfo': 'true',
'ExceptionHandling': 1,
},
},
'sources': [
Expand Down