deps: update V8 to 6.3

[targos]

Because the earlier the better!

I kept the HACK in the commit message for de0d28b because I'm not sure it's the way we should implement it (SystemClockTimeMillis comes from V8 AFAIK).

This branch is based on the 6.2 PR. I will update once it is merged.

/cc @nodejs/v8

Checklist

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

Affected core subsystem(s)

V8

[MylesBorins]

I'll work on getting 5479f8b backported upstream tomorrow

/cc @nodejs/v8 so this PR can be on their radar

CI: https://ci.nodejs.org/job/node-test-pull-request/10799/
V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/997/

[targos]

@MylesBorins I opened a merge request bug at https://bugs.chromium.org/p/v8/issues/detail?id=6960

[targos]

New CI: https://ci.nodejs.org/job/node-test-pull-request/10839/

[targos]

AIX seems broken:

  g++ '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=0' '-DV8_TARGET_ARCH_PPC' '-DV8_TARGET_ARCH_PPC64' '-DV8_TARGET_ARCH_PPC_BE' '-D_LINUX_SOURCE_COMPAT=1' '-D__STDC_FORMAT_MACROS' '-D_ALL_SOURCE=1' '-DV8_EMBEDDER_STRING="-node.0"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DV8_CONCURRENT_MARKING' -I../deps/v8 -I../. -I../deps/v8/include  -pthread -Wall -Wextra -Wno-unused-parameter -maix64 -mcpu=power5+ -mfprnd -mno-popcntb -fno-strict-aliasing -maix64 -fdata-sections -ffunction-sections -O3 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++0x -MMD -MF /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/out/Release/.deps//home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/out/Release/obj.target/v8_libplatform/deps/v8/src/libplatform/tracing/trace-buffer.o.d.raw   -c -o /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/out/Release/obj.target/v8_libplatform/deps/v8/src/libplatform/tracing/trace-buffer.o ../deps/v8/src/libplatform/tracing/trace-buffer.cc
../deps/v8/src/base/platform/platform-aix.cc: In static member function 'static void* v8::base::OS::ReserveAlignedRegion(size_t, size_t, void*, size_t*)':
../deps/v8/src/base/platform/platform-aix.cc:129:3: error: 'address_' was not declared in this scope
   address_ = static_cast<void*>(aligned_base);
   ^
../deps/v8/src/base/platform/platform-aix.cc:130:3: error: 'size_' was not declared in this scope
   size_ = aligned_size;
   ^

/cc @nodejs/platform-aix

[targos]

Windows test failure:

not ok 431 parallel/test-windows-failed-heap-allocation
  ---
  duration_ms: 0.372
  severity: fail
  stack: |-
    assert.js:45
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: Wrong exit code of 3221225477! Expected 134 for abort
        at common.mustCall (c:\workspace\node-test-binary-windows\test\parallel\test-windows-failed-heap-allocation.js:23:10)
        at c:\workspace\node-test-binary-windows\test\common\index.js:520:15
        at ChildProcess.exithandler (child_process.js:279:5)
        at emitTwo (events.js:136:13)
        at ChildProcess.emit (events.js:227:7)
        at maybeClose (internal/child_process.js:943:16)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:220:5)
  ...

[gireeshpunathil]

Need to dig to figure out the actual reason, but on the face of I can say a missing PR - there were efforts to normalize the windows error codes match with other platforms by adding the signal number to 128. This means a platform specific code change to that effect and a test case to test that. I think only one of them is merged.

[gireeshpunathil]

looking at AIX break now.

[gireeshpunathil]

ok, looks like the memory reservation performed in VirtualMemory class (until v6.1*)

class V8_BASE_EXPORT VirtualMemory {
  private:
  void* address_;  // Start address of the virtual memory.
  size_t size_;  // Size of the virtual memory.
};

which is now moved to

class V8_BASE_EXPORT OS {
void* OS::ReserveAlignedRegion(size_t size, size_t alignment, void* hint, size_t* allocated)
};

that does not have the two variables address_ and size_. Moreover, the assignment to these variable at the failing site does not seem to be required. Don't know how the build succeeded in v6.3 .

This code cannot be integrated in its current form.

/cc @nodejs/platform-ppc @jbajwa ?

[gireeshpunathil]

re: windows failure
Commit 80ebb42 introduced the aforementioned change, but all the required changes are present together in one PR.

The test expects 134 as the error code on heap exhaustion, as ABORT_NO_BACKTRACE has been redefined to exit with 134 return code. So here the actual exit was 3221225477 which would mean that node exited through a different route than ABORT_NO_BACKTRACE

/cc @nodejs/platform-windows @refack

[bnoordhuis]

3221225477 == 0xC0000005 == ACCESS_VIOLATION so that's probably a genuine bug.

[bzoz]

A NULL pointer (context) gets dereferenced in https://github.com/targos/node/blob/v8-6.3/deps/v8/src/frames.cc#L1890.

FWIW, FatalProcessOutOfMemory is in the call stack:

node.exe!v8::internal::JavaScriptFrame::Print(v8::internal::StringStream   * accumulator, v8::internal::StackFrame::PrintMode mode, int index) Line 1890
node.exe!v8::internal::PrintFrames(v8::internal::Isolate   * isolate, v8::internal::StringStream * accumulator,   v8::internal::StackFrame::PrintMode mode) Line 824
node.exe!v8::internal::Isolate::PrintStack(v8::internal::StringStream   * accumulator, v8::internal::Isolate::PrintStackMode mode) Line 845
node.exe!v8::internal::Heap::RecordStats(v8::internal::HeapStats   * stats, bool) Line 5095
node.exe!v8::internal::V8::FatalProcessOutOfMemory(const   char * location, bool is_heap_oom) Line 392
node.exe!v8::internal::Heap::FatalProcessOutOfMemory(const   char * location, bool is_heap_oom) Line 5828
node.exe!v8::internal::Factory::NewUninitializedFixedArray(int   size) Line 214
[Inline Frame]   node.exe!v8::internal::`anonymous-namespace'::ElementsAccessorBase<v8::internal::`anonymous   namespace'::FastHoleySmiElementsAccessor,v8::internal::A0xc02eecb3::ElementsKindTraits<1>   >::ConvertElementsWithCapacity(v8::internal::Handle<v8::internal::JSObject>)   Line 856
[Inline Frame]   node.exe!v8::internal::`anonymous-namespace'::ElementsAccessorBase<v8::internal::`anonymous   namespace'::FastHoleySmiElementsAccessor,v8::internal::A0xc02eecb3::ElementsKindTraits<1>   >::ConvertElementsWithCapacity(v8::internal::Handle<v8::internal::JSObject>)   Line 835
[Inline Frame]   node.exe!v8::internal::`anonymous-namespace'::ElementsAccessorBase<v8::internal::`anonymous   namespace'::FastHoleySmiElementsAccessor,v8::internal::A0xc02eecb3::ElementsKindTraits<1>   >::BasicGrowCapacityAndConvertImpl(v8::internal::Handle<v8::internal::JSObject>)   Line 930
node.exe!v8::internal::`anonymous   namespace'::ElementsAccessorBase<v8::internal::`anonymous   namespace'::FastHoleySmiElementsAccessor,v8::internal::A0xc02eecb3::ElementsKindTraits<1>   >::GrowCapacityAndConvertImpl(v8::internal::Handle<v8::internal::JSObject>   object, unsigned int capacity) Line 922
node.exe!v8::internal::`anonymous   namespace'::ElementsAccessorBase<v8::internal::`anonymous   namespace'::FastHoleySmiElementsAccessor,v8::internal::A0xc02eecb3::ElementsKindTraits<1>   >::SetLengthImpl(v8::internal::Isolate * isolate, v8::internal::Handle<v8::internal::JSArray>   array, unsigned int length,   v8::internal::Handle<v8::internal::FixedArrayBase> backing_store) Line   805
node.exe!v8::internal::ArrayConstructInitializeElements(v8::internal::Handle<v8::internal::JSArray>   array, v8::internal::Arguments * args) Line 4250
[Inline Frame]   node.exe!v8::internal::__RT_impl_Runtime_NewArray(v8::internal::Arguments)   Line 461
node.exe!v8::internal::Runtime_NewArray(int   args_length, v8::internal::Object * * args_object, v8::internal::Isolate *   isolate) Line 381
[External Code]
node.exe!v8::internal::Runtime_NewArray(int   args_length, v8::internal::Object * * args_object, v8::internal::Isolate *   isolate) Line 381
[External Code]

[bnoordhuis]

Hm, that's a rather obvious bug when you look at it but good job finding it. FWIW, it exists since v4.x.

This patch should fix it although I'm not 100% sure why it manifests with V8 6.3 and not with older versions (or maybe it does but only erratically.)

diff --git a/deps/v8/src/frames.cc b/deps/v8/src/frames.cc
index 37f5b4d9cf..dd2f6a3cb3 100644
--- a/deps/v8/src/frames.cc
+++ b/deps/v8/src/frames.cc
@@ -1938,9 +1938,11 @@ void JavaScriptFrame::Print(StringStream* accumulator,
   if (this->context() != NULL && this->context()->IsContext()) {
     context = Context::cast(this->context());
   }
-  while (context->IsWithContext()) {
-    context = context->previous();
-    DCHECK(context != NULL);
+  if (context != NULL) {
+    while (context->IsWithContext()) {
+      context = context->previous();
+      DCHECK(context != NULL);
+    }
   }
 
   // Print heap-allocated local variables.

The code further down does check for nullity, only here it wasn't.

[targos]

@bnoordhuis thank you. Do you think someone should create a CL with this patch? I can do it.

[bnoordhuis]

@targos Sorry, I was going to upstream it but I forgot. You are welcome to steal it, no attribution needed.

[targos]

Rebased and updated with a backport of the CL fixing the Windows issue.

CI: https://ci.nodejs.org/job/node-test-pull-request/11093/

Edit:
Reverted v8/v8@2d0a889 for now.
CI: https://ci.nodejs.org/job/node-test-pull-request/11095/

[benjamingr]

V8 6.3 ships async iterators cc @nodejs/streams

[targos]

Landed in b52c23b...3d43bce