Fix V8 5.4 for Node

[targos]

I don't want to pollute the node issue tracker that soon, so I'm opening a ticket here to track the issues that need to be solved on V8's side.

Here is a fresh CI run of targos/v8-5.4 as reference: https://ci.nodejs.org/job/node-test-commit/4668/

1. Compilation issue on BSD

Already fixed on my branch
See https://codereview.chromium.org/2251603004/

2. Compilation issue on SmartOS

See https://ci.nodejs.org/job/node-test-commit-smartos/3808/nodes=smartos14-32/console
I think it's due to the recent https://codereview.chromium.org/2248393002

3. Windows issue with mkpeephole

See https://ci.nodejs.org/job/node-compile-windows/3798/label=win-vcbt2015/console
@ofrobots: do you have any news about this?

4. FreeBSD issue with test-tick-processor

The process is probably crashing before the skip, at https://github.com/nodejs/node/blob/8ff3d61d8ba90fde01827643db3d87ee97f502e6/test/parallel/test-tick-processor.js#L21-L29
I don't understand what this test is doing so help would be much appreciated here.

Fixed with https://codereview.chromium.org/2268993002

Edit: new CI: https://ci.nodejs.org/job/node-test-commit/4686/

[matthewloring]

cc @nodejs/v8

edit: Hmmm, looks like you cannot cc a team from the parent repository?

[targos]

cc @fhinkel and @bnoordhuis in case you're interested

[bnoordhuis]

It's possible you need to add 'want_separate_host_toolset_mkpeephole%': 0 in common.gypi like we do for want_separate_host_toolset.

cc @misterdjules for the smartos build error.

[bnoordhuis]

1. FreeBSD issue with test-tick-processor

From looking at the stack trace, I suspect that the fix is this:

diff --git a/src/base/platform/semaphore.cc b/src/base/platform/semaphore.cc
index f6a518c..3713153 100644
--- a/src/base/platform/semaphore.cc
+++ b/src/base/platform/semaphore.cc
@@ -72,18 +72,18 @@ bool Semaphore::WaitFor(const TimeDelta& rel_time) {

 #elif V8_OS_POSIX

 Semaphore::Semaphore(int count) {
   // The sem_init() does not check for alignment of the native handle.
   // Unaligned native handle can later cause a failure in semaphore signal.
   // Check the alignment here to catch the failure earlier.
   // Context: crbug.com/605349.
-#if V8_OS_AIX
-  // On aix sem_t is of type int
+#if V8_OS_AIX || V8_OS_FREEBSD
+  // On AIX, sem_t is of type int. On FreeBSD, it's a struct of 32 bits fields.
   const uintptr_t kSemaphoreAlignmentMask = sizeof(int) - 1;
 #else
   const uintptr_t kSemaphoreAlignmentMask = sizeof(void*) - 1;
 #endif
   CHECK_EQ(
       0, reinterpret_cast<uintptr_t>(&native_handle_) &
       kSemaphoreAlignmentMask);
   DCHECK(count >= 0);

On freebsd, sem_t is a struct with a couple of uint32_t fields. It only needs dword alignment on 64 bits architectures, not the qword (pointer) alignment that the CHECK currently enforces.

[misterdjules]

Will look at this asap.

[matthewloring]

Based on @bnoordhuis' comment, it doesn't look like the tick processor failure is an issue in the code that the tick processor is testing but I'm happy to help out with any questions/work related to the test.

[misterdjules]

@targos

The following patch fixes the build issue on SmartOS:

diff --git a/deps/v8/src/base/debug/stack_trace_posix.cc b/deps/v8/src/base/debug/stack_trace_posix.cc
index cbd722d..45d1e8f 100644
--- a/deps/v8/src/base/debug/stack_trace_posix.cc
+++ b/deps/v8/src/base/debug/stack_trace_posix.cc
@@ -32,7 +32,9 @@
 #if V8_OS_MACOSX
 #include <AvailabilityMacros.h>
 #endif
-
+#if V8_OS_SOLARIS
+#include <execinfo.h>
+#endif
 #include "src/base/build_config.h"
 #include "src/base/free_deleter.h"
 #include "src/base/logging.h"

[ofrobots]

@targos sorry for the late reply, I was on vacation. From the new CI, it seems like that the common.gypi suggestion from @bnoordhuis didn't help. I will take a look at this today.

[targos]

@misterdjules thanks !

cc @mhart for the similar issue with https://ci.nodejs.org/job/node-test-commit-linux/4734/nodes=ubuntu1604_docker_alpine34-64/console
Do you know if there is a V8 macro applicable to this platform ?

[ofrobots]

@targos: I have submitted this CL for the mkpeephole issue on windows: https://codereview.chromium.org/2276733002.

[targos]

@ofrobots your fix seems to break the build on my local machine (Linux).

Running ./configure:

$ ./configure    
creating  ./icu_config.gypi
* Using ICU in deps/icu-small
creating  ./icu_config.gypi
{ 'target_defaults': { 'cflags': [],
                       'default_configuration': 'Release',
                       'defines': [],
                       'include_dirs': [],
                       'libraries': []},
  'variables': { 'asan': 0,
                 'debug_devtools': 'node',
                 'force_dynamic_crt': 0,
                 'gas_version': '2.26',
                 'host_arch': 'x64',
                 'icu_data_file': 'icudt57l.dat',
                 'icu_data_in': '../../deps/icu-small/source/data/in/icudt57l.dat',
                 'icu_endianness': 'l',
                 'icu_gyp_path': 'tools/icu/icu-generic.gyp',
                 'icu_locales': 'en,root',
                 'icu_path': 'deps/icu-small',
                 'icu_small': 'true',
                 'icu_ver_major': '57',
                 'node_byteorder': 'little',
                 'node_enable_d8': 'false',
                 'node_enable_v8_vtunejit': 'false',
                 'node_install_npm': 'true',
                 'node_module_version': 48,
                 'node_no_browser_globals': 'false',
                 'node_prefix': '/usr/local',
                 'node_release_urlbase': '',
                 'node_shared': 'false',
                 'node_shared_cares': 'false',
                 'node_shared_http_parser': 'false',
                 'node_shared_libuv': 'false',
                 'node_shared_openssl': 'false',
                 'node_shared_zlib': 'false',
                 'node_tag': '',
                 'node_use_bundled_v8': 'true',
                 'node_use_dtrace': 'false',
                 'node_use_etw': 'false',
                 'node_use_lttng': 'false',
                 'node_use_openssl': 'true',
                 'node_use_perfctr': 'false',
                 'node_use_v8_platform': 'true',
                 'openssl_fips': '',
                 'openssl_no_asm': 0,
                 'shlib_suffix': 'so.48',
                 'target_arch': 'x64',
                 'uv_parent_path': '/deps/uv/',
                 'uv_use_dtrace': 'false',
                 'v8_enable_gdbjit': 0,
                 'v8_enable_i18n_support': 1,
                 'v8_inspector': 'true',
                 'v8_no_strict_aliasing': 1,
                 'v8_optimized_debug': 0,
                 'v8_random_seed': 0,
                 'v8_use_snapshot': 'true',
                 'want_separate_host_toolset': 0}}
creating  ./config.gypi
creating  ./config.mk
gyp: Dependency '/home/mzasso/git/forks/node/deps/v8/src/v8.gyp:v8_libsampler#host' not found while trying to load target /home/mzasso/git/forks/node/deps/v8/src/v8.gyp:v8_base#host
Error running GYP

[ofrobots]

@targos: You still need want_separate_host_toolset_mkpeephole%': 0 in common.gypi as @bnoordhuis suggested above.

[targos]

@ofrobots I'm still seeing this error with 'want_separate_host_toolset_mkpeephole%': 0;

[ofrobots]

@targo: It seems that the variable definition in common.gypi is having no effect. Here's how I "fixed" it: ofrobots@b4ad793 on my branch. Today was my day off so I didn't have too much time to look into why common.gypi is not having an effect.

[targos]

OK thanks!

New CI: https://ci.nodejs.org/job/node-test-commit/4759/