Build failure on AIX

[targos]

https://ci.nodejs.org/job/node-test-commit-aix/9727/nodes=aix61-ppc64/console

Error:

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++1y -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-config.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-config.o ../deps/v8/src/libplatform/tracing/trace-config.cc
../deps/v8/src/base/platform/platform-posix.cc: In static member function 'static bool v8::base::OS::CommitRegion(void*, size_t, bool)':
../deps/v8/src/base/platform/platform-posix.cc:253:20: error: expected primary-expression before ',' token
   if (mprotect(base, size, prot) == -1) return false;
                    ^
../deps/v8/src/base/platform/platform-posix.cc: In static member function 'static bool v8::base::OS::UncommitRegion(void*, size_t)':
../deps/v8/src/base/platform/platform-posix.cc:271:23: error: expected primary-expression before ',' token
   return mprotect(base, size, PROT_NONE) != -1;
                       ^
../deps/v8/src/base/platform/platform-posix.cc:273:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

I'm trying this patch:

diff --git a/src/base/platform/platform-posix.cc b/src/base/platform/platform-posix.cc
index 481795e08f..68819fd41f 100644
--- a/src/base/platform/platform-posix.cc
+++ b/src/base/platform/platform-posix.cc
@@ -250,7 +250,7 @@ bool OS::CommitRegion(void* address, size_t size, bool is_executable) {
     return false;
   }
 #else
-  if (mprotect(base, size, prot) == -1) return false;
+  if (mprotect(address, size, prot) == -1) return false;
 #endif  // !V8_OS_AIX
   return true;
 }
@@ -268,7 +268,7 @@ bool OS::UncommitRegion(void* address, size_t size) {
   return mmap(address, size, PROT_NONE, map_flags, kMmapFd, kMmapFdOffset) !=
          MAP_FAILED;
 #else   // V8_OS_AIX
-  return mprotect(base, size, PROT_NONE) != -1;
+  return mprotect(address, size, PROT_NONE) != -1;
 #endif  // V8_OS_AIX
 }
 

/cc @nodejs/platform-aix

CI: https://ci.nodejs.org/job/node-test-commit-aix/9732/

Edit: trying to fix the other error: https://ci.nodejs.org/job/node-test-commit-aix/9768/

[gireeshpunathil]

@targos - curious to know whether you were able to move this? Please let me know if I should pitch-in.

[targos]

I was blocked on this error: https://ci.nodejs.org/job/node-test-commit-aix/9768/nodes=aix61-ppc64/console

I'd be happy if you could help me with that, thanks!

[gireeshpunathil]

sure, will start look into it.

[gireeshpunathil]

@targos - this should fix it:

diff --git a/deps/v8/src/utils.h b/deps/v8/src/utils.h
index ab8d228..185d36e 100644
--- a/deps/v8/src/utils.h
+++ b/deps/v8/src/utils.h
@@ -11,6 +11,7 @@
 #include <limits.h>
 #include <stdlib.h>
 #include <string.h>
+#include <fenv.h>
 #include <cmath>
 #include <type_traits>

I compiled it fully locally and saw no more errors. Hope you will take it from here.

[gireeshpunathil]

ah! wait:

   +8  #if defined(V8_OS_AIX)
   +9  #include <fenv.h>  // NOLINT(build/c++11)
   +10  #endif

that line seem to be there already!
So don't know what is going on here. Please wait while I dig it further.

[gireeshpunathil]

ok - V8_OS_AIX is defined in v8config.h which comes to life through v8.h and that is included further down the line.

This patch breaks the readability a bit, but to conform to the platform definitions and to build, I see no other way.

diff --git a/deps/v8/src/utils.h b/deps/v8/src/utils.h
index ab8d228..1834eda 100644
--- a/deps/v8/src/utils.h
+++ b/deps/v8/src/utils.h
@@ -5,9 +5,6 @@
 #ifndef V8_UTILS_H_
 #define V8_UTILS_H_
 
-#if defined(V8_OS_AIX)
-#include <fenv.h>  // NOLINT(build/c++11)
-#endif
 #include <limits.h>
 #include <stdlib.h>
 #include <string.h>
@@ -25,6 +22,10 @@
 #include "src/vector.h"
 #include "src/zone/zone.h"
 
+#if defined (V8_OS_AIX)
+#include <fenv.h> // NOLINT(build/c++11)
+#endif
+
 namespace v8 {
 namespace internal {

@targos - looks fair to you?

[targos]

That seems fair to me. Thank you for investigating!

CI: https://ci.nodejs.org/job/node-test-commit-aix/9838/

@gireeshpunathil Do you mind if I upstream both fixes myself in the same CL?

[gireeshpunathil]

sure, please!

[targos]

CL landed today: https://chromium-review.googlesource.com/c/v8/v8/+/743941

Thanks again @gireeshpunathil !