[AIX]: Testcase related to wasm memory failing

[jbajwa]

>out/ppc64.release/d8 --test --random-seed=-1656925804 --nohard-abort --nodead-code-elimination --nofold-constants test/mjsunit/mjsunit.js  test/mjsunit/regress/wasm/regression-680938.js

test/mjsunit/regress/wasm/regression-680938.js:7: RangeError: WebAssembly.Memory(): could not allocate memory
v39 = new WebAssembly.Memory(v32);
      ^
RangeError: WebAssembly.Memory(): could not allocate memory
    at test/mjsunit/regress/wasm/regression-680938.js:7:7

Bad CL: https://codereview.chromium.org/2649743002

>out/ppc64.release/cctest --random-seed=-1656925804 --ignition-staging test-wasm-trap-position/IllegalLoad --nohard-abort --nodead-code-elimination --nofold-constants

#
# Fatal error in .././test/cctest/wasm/wasm-run-utils.h, line 113
# Check failed: instance->mem_start.
#
IOT/Abort trap (core dumped)

>out/ppc64.release/cctest --random-seed=-30826564 test-run-wasm/RunWasmCompiled_Unreachable_Load --nohard-abort --nodead-code-elimination --nofold-constants

#
# Fatal error in .././test/cctest/wasm/wasm-run-utils.h, line 113
# Check failed: instance->mem_start.
#
IOT/Abort trap (core dumped

>out/ppc64.release/cctest --random-seed=-30826564 test-run-wasm/RunWasmInterpreted_Unreachable_Load --nohard-abort --nodead-code-elimination --nofold-constants

#
# Fatal error in .././test/cctest/wasm/wasm-run-utils.h, line 113
# Check failed: instance->mem_start.
#
IOT/Abort trap (core dumped)

Bad CL: https://codereview.chromium.org/2640453003/

[jbajwa]

Some more details:

• regression-680938.js
  Unable to allocate NewJSArrayBuffer, in file wasm-js.cc

527   size_t size = static_cast<size_t>(i::wasm::WasmModule::kPageSize) *
528                 static_cast<size_t>(initial);
529   i::Handle<i::JSArrayBuffer> buffer =
530       i::wasm::NewArrayBuffer(i_isolate, size, i::FLAG_wasm_guard_pages);
531   if (buffer.is_null()) {  <<-- buffer here is NULL

Also, with --wasm_guard_pages (adds guard page at the end of wasm memory) the testcase passes.
Following the code execution path, TryAllocateBackingStore in wasm-module.cc returns 0, specifically

 116   } else {
 117     void* memory = isolate->array_buffer_allocator()->Allocate(size); <<----
 118     return memory;

Where Allocate is platform dependent code, src/base/platform/platform-aix.cc

 67 void* OS::Allocate(const size_t requested, size_t* allocated, bool executable) {
 68   const size_t msize = RoundUp(requested, getpagesize());
 69   int prot = PROT_READ | PROT_WRITE | (executable ? PROT_EXEC : 0);
 70   void* mbase = mmapHelper(msize, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 71 
 72   if (mbase == MAP_FAILED) return NULL;
 73   *allocated = msize;
 74   return mbase;
 75 }

This might be affecting the other cctest tests where WebAssembly.AddMemory(0L); is causing the failure.

[jbajwa]

Also, test-run-wasm/RunWasmCompiled_Unreachable_Load/test-run-wasm/RunWasmInterpreted_Unreachable_Load are fixed in 5.8 as a side-effect of https://codereview.chromium.org/2670673002/

[jbajwa]

Finally got around to look at this issue.
Found the root cause and its, malloc(0) is returning 0, which seems to expected on AIX as per this.
The fix to this issue is to have _LINUX_SOURCE_COMPAT macro defined, which we already have in V8 build configs for AIX, here.
Seems that macro is not behaving correctly.

in src/d8.cc I added the following:

#ifndef _LINUX_SOURCE_COMPAT
  #define _LINUX_SOURCE_COMPAT
#endif
printf("malloc(length): %p , malloc(0): %p \n", malloc(length), malloc(0));
return malloc(length);  //<<-- length is 0 here

prints malloc(length): 0 , malloc(0): 0
(using printf because gdb is not the best on aix, doesn't break where I ask it to break)

I tested the macro _LINUX_SOURCE_COMPAT with a snippet returning malloc(0) and it works fine.
Looking at the preprocessed output malloc() gets converted to __linux_malloc() which doesn't happen when d8.cc is compiled, although __linux_malloc is defined there. If I use __linux_malloc in d8.cc then the testcase passes.
Is that a reasonable fix to use __linux_malloc guarded with #ifdef _LINUX_SOURCE_COMPAT?
@john-yan @jBarz @joransiu

[jBarz]

The issue seems to be in <cstdlib> which is included after stdlib.h

// Get rid of those macros defined in <stdlib.h> in lieu of real functions.
#undef malloc

Your suggestion looks good to me for now but we will keep running into this error in the future unless the cstdlib header is fixed.

[mhdawson]

We had run into what sounds like a similar problem on AIX for Node.js. In that case is was that when certain headers were pulled it it caused the non LINUX_SOURECE_COMPAT behaviour even though it was set.

For Node.js in the end malloc's were wrapped to do the right thing on the basis that the spec is not clear on the behavior so we should not depend on one way or the other. nodejs/node#7564

We had talked to David Edelsohn about the issue and he was looking at getting it fixed in a later version of the compiler but we've not tried that out.

[jbajwa]

I don't think a similar change would be accepted in V8. It might break things as in some places they assume malloc(0) to return a valid pointer.
For now only 3 testcases are affected by this, so I'm thinking of putting in the following fix:

diff --git a/src/d8.cc b/src/d8.cc
index 31dc895..53268e9 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -100,7 +100,11 @@ class ShellArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
 #if USE_VM
     if (RoundToPageSize(&length)) return VirtualMemoryAllocate(length);
 #endif
+#if V8_OS_AIX && _LINUX_SOURCE_COMPAT
+    return __linux_malloc(length);
+#else
     return malloc(length);
+#endif
   }
   virtual void Free(void* data, size_t length) {
 #if USE_VM
diff --git a/test/cctest/wasm/wasm-run-utils.h b/test/cctest/wasm/wasm-run-utils.h
index 0ddb6f5..06e1d57 100644
--- a/test/cctest/wasm/wasm-run-utils.h
+++ b/test/cctest/wasm/wasm-run-utils.h
@@ -109,7 +109,11 @@ class TestingModule : public ModuleEnv {
     CHECK_NULL(instance->mem_start);
     CHECK_EQ(0, instance->mem_size);
     module_.has_memory = true;
+#if V8_OS_AIX && _LINUX_SOURCE_COMPAT
+    instance->mem_start = reinterpret_cast<byte*>(__linux_malloc(size));
+#else
     instance->mem_start = reinterpret_cast<byte*>(malloc(size));
+#endif
     CHECK(instance->mem_start);
     memset(instance->mem_start, 0, size);
     instance->mem_size = size;

To add more info should I open a gcc bugzilla defect (unless there is one already opened, I searched but didn't find any) and mention it as comment? For eg:
// Known GCC issue on AIX, bug #. Work around until the issue is fixed

[jbajwa]

https://codereview.chromium.org/2732743002