[WIP] Update V8 to 5.2

[targos]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

v8

Description of change

This is almost ready, but I am still waiting for https://bugs.chromium.org/p/v8/issues/detail?id=5174 to be fixed in the 5.2 branch.
I'm creating the PR now to start a CI run.

CI: https://ci.nodejs.org/job/node-test-pull-request/3370/
CI: https://ci.nodejs.org/job/node-test-pull-request/3383/
CI: https://ci.nodejs.org/job/node-test-pull-request/3420/

[bricss]

In which next node version it is possible to see that's new shiny V8? 🤔

[mscdex]

@bricss Probably not until v7.0.0, unless the ABI incompatibility issues can be smoothed over before August.

[targos]

Noting some errors.

freebsd

 c++ '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_I18N_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' '-DUCONFIG_NO_LEGACY_CONVERSION=1' '-DUCONFIG_NO_CONVERSION=1' -I../deps/v8 -I../deps -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common -pthread -Wall -Wextra -Wno-unused-parameter -m64 -fno-strict-aliasing -I/usr/local/include -fdata-sections -ffunction-sections -O3 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/.deps//usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/sampling-heap-profiler.o.d.raw -c -o /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/sampling-heap-profiler.o ../deps/v8/src/profiler/sampling-heap-profiler.cc
../deps/v8/src/profiler/sampler.cc:216:10: error: static_cast from 'pthread_t' (aka 'pthread *') to 'uint32_t' (aka 'unsigned int') is not allowed
return static_cast<uint32_t>(thread_id);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
deps/v8/src/v8_base.target.mk:506: recipe for target '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/sampler.o' failed

smartos 64

 g++ '-DV8_TARGET_ARCH_X64' '-D__C99FEATURES__=1' '-DENABLE_DISASSEMBLER' '-DV8_I18N_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' '-DUCONFIG_NO_LEGACY_CONVERSION=1' '-DUCONFIG_NO_CONVERSION=1' -I../deps/v8 -I../deps -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common -Wall -Wextra -Wno-unused-parameter -m64 -pthreads -fno-strict-aliasing -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/.deps//home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/interface-descriptors.o.d.raw -c -o /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/interface-descriptors.o ../deps/v8/src/interface-descriptors.cc
../deps/v8/src/ic/ic.cc: In member function 'v8::internal::MaybeHandle<v8::internal::Object> v8::internal::BinaryOpIC::Transition(v8::internal::Handle<v8::internal::AllocationSite>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>)':
../deps/v8/src/ic/ic.cc:2547:68: error: 'alloca' was not declared in this scope
byte* instructions = static_cast<byte*>(alloca(instruction_size));
^
deps/v8/src/v8_base.target.mk:498: recipe for target '/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/ic/ic.o' failed

V8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=5227

smartos 32

parallel/test-tick-processor.js failed

[mhdawson]

v8 CI run - https://ci.nodejs.org/job/node-test-commit-v8-linux/228/. To check for PPC, s390, should be ok there.

[i5ting]

nice job

[ofrobots]

@targos I independently ran into the the ICU transliteration issue. Here's the fix I put together with help from @srl295: ofrobots@fcd0595. We should reconcile the two fixes.

[targos]

@ofrobots thanks, it's done.

[targos]

I made an attempt to fix the compiling issues.
New CI: https://ci.nodejs.org/job/node-test-pull-request/3420/ with the following errors:

smartos-64

g++ '-DV8_TARGET_ARCH_X64' '-D__C99FEATURES__=1' '-DENABLE_DISASSEMBLER' '-DV8_I18N_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' '-DUCONFIG_NO_LEGACY_CONVERSION=1' '-DUCONFIG_NO_CONVERSION=1' -I../deps/v8 -I../deps -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common -Wall -Wextra -Wno-unused-parameter -m64 -pthreads -fno-strict-aliasing -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/.deps//home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/strings-storage.o.d.raw -c -o /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/strings-storage.o ../deps/v8/src/profiler/strings-storage.cc
../deps/v8/src/profiler/sampler.cc: In member function 'virtual void v8::internal::SamplerThread::Run()':
../deps/v8/src/profiler/sampler.cc:600:69: error: cast from 'void*' to 'pthread_t {aka unsigned int}' loses precision [-fpermissive]
pthread_t thread_id = reinterpret_cast<pthread_t>(p->key);
^
deps/v8/src/v8_base.target.mk:498: recipe for target '/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-64/out/Release/obj.target/v8_base/deps/v8/src/profiler/sampler.o' failed

freebsd

not ok 914 parallel/test-tick-processor
# # # #
# # Fatal error in ../deps/v8/src/base/platform/semaphore.cc, line 88
# # Check failed: ( 0)==(reinterpret_cast<uintptr_t>(&native_handle_) & kSemaphoreAlignmentMask).
# #
# # ==== C stack trace ===============================
# # 1: _Fatal volatile
# 2: v8::base::Semaphore::Semaphore(int)
# 3: v8::internal::Logger::SetUp(v8::internal::Isolate*)
# 4: v8::internal::Isolate::Init(v8::internal::Deserializer*)
# 5: v8::internal::Snapshot::Initialize(v8::internal::Isolate*)
# 6: v8::Isolate::New(v8::Isolate::CreateParams const&)
# 7: node::Start(int, char**)
# 8: _start
# # assert.js:89
# throw new assert.AssertionError({
# ^
# AssertionError: /LazyCompile.*\[eval\]:1|.*% UNKNOWN/ not matching Statistical profiling result from isolate-0x803c4d000-v8.log, (0 ticks, 0 unaccounted, 0 excluded).
# # at runTest (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/parallel/test-tick-processor.js:58:3)
# at Object.<anonymous> (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/parallel/test-tick-processor.js:21:1)
# at Module._compile (module.js:541:32)
# at Object.Module._extensions..js (module.js:550:10)
# at Module.load (module.js:458:32)
# at tryModuleLoad (module.js:417:12)
# at Function.Module._load (module.js:409:3)
# at Module.runMain (module.js:575:10)
# at run (bootstrap_node.js:340:7)
# at startup (bootstrap_node.js:132:9)

Edit: V8 bug ? https://bugs.chromium.org/p/chromium/issues/detail?id=605349

[targos]

While playing with this branch in one of my projects, I found a bug:

asyncfunctiontest(){await1;thrownewError('bad');}test().catch(function(e){e.stack;});

This code fails at a check:

#
# Fatal error in ../deps/v8/src/frames-inl.h, line 163
# Check failed: -1 <= index && index < param_count.
#
==== C stack trace ===============================
1: V8_Fatal
2: v8::internal::JavaScriptFrame::GetParameterSlot(int) const
3: v8::internal::JavaScriptFrame::GetParameter(int) const
4: v8::internal::JavaScriptFrame::receiver() const
5: v8::internal::JavaScriptFrame::Summarize(v8::internal::List<v8::internal::FrameSummary, v8::internal::FreeStoreAllocationPolicy>*, v8::internal::FrameSummary::Mode) const
6: v8::internal::OptimizedFrame::Summarize(v8::internal::List<v8::internal::FrameSummary, v8::internal::FreeStoreAllocationPolicy>*, v8::internal::FrameSummary::Mode) const
7: v8::internal::Isolate::CaptureSimpleStackTrace(v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>)
8: v8::internal::Isolate::CaptureAndSetSimpleStackTrace(v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>)
9: 0x1b21737
10: v8::internal::Runtime_CollectStackTrace(int, v8::internal::Object**, v8::internal::Isolate*)
11: 0x1bd2a4008bc7

The bug is fixed in V8's master branch but not in 5.2 or 5.3.
We might want to try and backport the fix, as 5.3 will probably be in the v7 release and I'm sure a lot of people will start using --harmony-async-await as soon as it is available.

[ofrobots]

I am hoping we can get V8 5.4 in Node v7.x, but I guess it depends on the exact timing of its release.

[srl295]

Yes let's please coordinate. @ofrobots is there a separate issue tracking this?

[srl295]

yeah - "locales" may not be quite right here in fact. It's more complex which is why I'd like a separate issue tracking this (translit enablement for v8) For now, I'd actually remove the translit line altogether from this file.

[srl295]

I commented on some outdated diffs for discussion.

• It's not clear exactly what translit data is needed. either locales or deleting the line will probably add 640k to the minimal node build, most or all of which (99% or 100%)is unneeded and not beneficial to node.

What I'd like to see is a separate PR that enables transliteration support if to the v8 version requires it. This could go in prior to v8 going in and would make node.js always build ICU the right way.

Related here is 7de59ef#diff-eda374a7b1c3fccd439785b07caa1372 from @jasnell ’s #7355 which also enabled some additional ICU code and data, but for a core feature and not for a v8 requirement.

[ilkkao]

The V8 crash bug mentioned above makes debugging challenging in bigger projects. Programming errors and typos in async functions, e.g. something that normally prints errorTypeError: Cannot read property 'foo' of null leads to a crash without any information about the location in V8 5.2.

If possible, it'd be nice to see the fix backported.

[ilkkao]

More specifically I keep seeing the crash in this situation:

#!/usr/bin/env node --harmony_async_await
process.on('unhandledRejection', function(reason, p) {
console.log("Unhandled Rejection at: Promise ", p, " reason: ", reason); // [1]
process.exit(1);
});
async function foo() {
await 42;
consXole.log('Typo on this line');
}
(async function() {
await foo();
})();

If I comment out [1] then the app doesn't crash anymore. But then you don't know what the problem is. This looks like a variation of the test case @targos already posted.

[jasnell]

meta: given that v5 is EOL and there will be no further releases in that line, I'm going to go ahead and remove the dont-land-on-v5.x label and likely delete it.

[Jessidhia]

This (and a bug 5174 fix in 5.2) appear to be going nowhere? Could be better to skip 5.2 and go straight to 5.3? It has been out for a month now, and there will soon be a 5.4...

[targos]

@Kovensky I plan to skip directly to 5.4 when the branch is cut.

[targos]

Closing (superseded by #8317)