[wasm] Fix code specialization for empty memory buffer

From asm.js code we might get an empty ArrayBuffer as heap memory. In
this case, both the old memory start and the new memory start will be
nullptr. The size however has to be patched from default_size to 0.

This CL changes code specialization to be able to either patch memory
references, or patch memory sizes or both.

R=titzer@chromium.org, ahaas@chromium.org
BUG=chromium:698587

Change-Id: I4d9d811d75cb83842f23df317e8e7fc02aeb5146
Reviewed-on: https://chromium-review.googlesource.com/450257
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43613}

src/assembler.cc

@@ -308,25 +308,25 @@ const int kCodeWithIdTag = 0;
 const int kDeoptReasonTag = 1;
 
 void RelocInfo::update_wasm_memory_reference(
-    Address old_base, Address new_base, uint32_t old_size, uint32_t new_size,
-    ICacheFlushMode icache_flush_mode) {
-  DCHECK(IsWasmMemoryReference(rmode_) || IsWasmMemorySizeReference(rmode_));
-  if (IsWasmMemoryReference(rmode_)) {
-    Address updated_reference;
-    DCHECK_GE(wasm_memory_reference(), old_base);
-    updated_reference = new_base + (wasm_memory_reference() - old_base);
-    // The reference is not checked here but at runtime. Validity of references
-    // may change over time.
-    unchecked_update_wasm_memory_reference(updated_reference,
-                                           icache_flush_mode);
-  } else if (IsWasmMemorySizeReference(rmode_)) {
-    uint32_t current_size_reference = wasm_memory_size_reference();
-    uint32_t updated_size_reference =
-        new_size + (current_size_reference - old_size);
-    unchecked_update_wasm_size(updated_size_reference, icache_flush_mode);
-  } else {
-    UNREACHABLE();
+    Address old_base, Address new_base, ICacheFlushMode icache_flush_mode) {
+  DCHECK(IsWasmMemoryReference(rmode_));
+  DCHECK_GE(wasm_memory_reference(), old_base);
+  Address updated_reference = new_base + (wasm_memory_reference() - old_base);
+  // The reference is not checked here but at runtime. Validity of references
+  // may change over time.
+  unchecked_update_wasm_memory_reference(updated_reference, icache_flush_mode);
+  if (icache_flush_mode != SKIP_ICACHE_FLUSH) {
+    Assembler::FlushICache(isolate_, pc_, sizeof(int64_t));
   }
+}
+
+void RelocInfo::update_wasm_memory_size(uint32_t old_size, uint32_t new_size,
+                                        ICacheFlushMode icache_flush_mode) {
+  DCHECK(IsWasmMemorySizeReference(rmode_));
+  uint32_t current_size_reference = wasm_memory_size_reference();
+  uint32_t updated_size_reference =
+      new_size + (current_size_reference - old_size);
+  unchecked_update_wasm_size(updated_size_reference, icache_flush_mode);
   if (icache_flush_mode != SKIP_ICACHE_FLUSH) {
     Assembler::FlushICache(isolate_, pc_, sizeof(int64_t));
   }

src/assembler.h

@@ -506,7 +506,10 @@ class RelocInfo {
   uint32_t wasm_function_table_size_reference();
   uint32_t wasm_memory_size_reference();
   void update_wasm_memory_reference(
-      Address old_base, Address new_base, uint32_t old_size, uint32_t new_size,
+      Address old_base, Address new_base,
+      ICacheFlushMode icache_flush_mode = FLUSH_ICACHE_IF_NEEDED);
+  void update_wasm_memory_size(
+      uint32_t old_size, uint32_t new_size,
       ICacheFlushMode icache_flush_mode = FLUSH_ICACHE_IF_NEEDED);
   void update_wasm_global_reference(
       Address old_base, Address new_base,

src/wasm/wasm-code-specialization.cc

@@ -75,8 +75,9 @@ void CodeSpecialization::RelocateMemoryReferences(Address old_start,
                                                   uint32_t old_size,
                                                   Address new_start,
                                                   uint32_t new_size) {
-  DCHECK(old_mem_start == 0 && new_mem_start == 0);
-  DCHECK(old_start != 0 || new_start != 0);
+  DCHECK(old_mem_start == nullptr && old_mem_size == 0 &&
+         new_mem_start == nullptr && new_mem_size == 0);
+  DCHECK(old_start != new_start || old_size != new_size);
   old_mem_start = old_start;
   old_mem_size = old_size;
   new_mem_start = new_start;
@@ -164,7 +165,8 @@ bool CodeSpecialization::ApplyToWasmCode(Code* code,
   DisallowHeapAllocation no_gc;
   DCHECK_EQ(Code::WASM_FUNCTION, code->kind());
 
-  bool reloc_mem = old_mem_start || new_mem_start;
+  bool reloc_mem_addr = old_mem_start != new_mem_start;
+  bool reloc_mem_size = old_mem_size != new_mem_size;
   bool reloc_globals = old_globals_start || new_globals_start;
   bool patch_table_size = old_function_table_size || new_function_table_size;
   bool reloc_direct_calls = !relocate_direct_calls_instance.is_null();
@@ -174,8 +176,8 @@ bool CodeSpecialization::ApplyToWasmCode(Code* code,
   auto add_mode = [&reloc_mode](bool cond, RelocInfo::Mode mode) {
     if (cond) reloc_mode |= RelocInfo::ModeMask(mode);
   };
-  add_mode(reloc_mem, RelocInfo::WASM_MEMORY_REFERENCE);
-  add_mode(reloc_mem, RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
+  add_mode(reloc_mem_addr, RelocInfo::WASM_MEMORY_REFERENCE);
+  add_mode(reloc_mem_size, RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   add_mode(reloc_globals, RelocInfo::WASM_GLOBAL_REFERENCE);
   add_mode(patch_table_size, RelocInfo::WASM_FUNCTION_TABLE_SIZE_REFERENCE);
   add_mode(reloc_direct_calls, RelocInfo::CODE_TARGET);
@@ -188,13 +190,17 @@ bool CodeSpecialization::ApplyToWasmCode(Code* code,
     RelocInfo::Mode mode = it.rinfo()->rmode();
     switch (mode) {
       case RelocInfo::WASM_MEMORY_REFERENCE:
-      case RelocInfo::WASM_MEMORY_SIZE_REFERENCE:
-        DCHECK(reloc_mem);
+        DCHECK(reloc_mem_addr);
         it.rinfo()->update_wasm_memory_reference(old_mem_start, new_mem_start,
-                                                 old_mem_size, new_mem_size,
                                                  icache_flush_mode);
         changed = true;
         break;
+      case RelocInfo::WASM_MEMORY_SIZE_REFERENCE:
+        DCHECK(reloc_mem_size);
+        it.rinfo()->update_wasm_memory_size(old_mem_size, new_mem_size,
+                                            icache_flush_mode);
+        changed = true;
+        break;
       case RelocInfo::WASM_GLOBAL_REFERENCE:
         DCHECK(reloc_globals);
         it.rinfo()->update_wasm_global_reference(

src/wasm/wasm-module.cc

@@ -1175,8 +1175,12 @@ class InstantiationHelper {
               ? static_cast<Address>(
                     compiled_module_->memory()->backing_store())
               : nullptr;
-      code_specialization.RelocateMemoryReferences(old_mem_start, old_mem_size,
-                                                   mem_start, mem_size);
+      // We might get instantiated again with the same memory. No patching
+      // needed in this case.
+      if (old_mem_start != mem_start || old_mem_size != mem_size) {
+        code_specialization.RelocateMemoryReferences(
+            old_mem_start, old_mem_size, mem_start, mem_size);
+      }
       compiled_module_->set_memory(memory_);
     }
 

test/cctest/compiler/test-run-wasm-machops.cc

@@ -27,13 +27,13 @@ static void UpdateMemoryReferences(Handle<Code> code, Address old_base,
                   RelocInfo::ModeMask(RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
     RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode) ||
-        RelocInfo::IsWasmMemorySizeReference(mode)) {
-      // Patch addresses with change in memory start address
-      it.rinfo()->update_wasm_memory_reference(old_base, new_base, old_size,
-                                               new_size);
-      modified = true;
+    if (RelocInfo::IsWasmMemoryReference(mode)) {
+      it.rinfo()->update_wasm_memory_reference(old_base, new_base);
+    } else {
+      DCHECK(RelocInfo::IsWasmMemorySizeReference(mode));
+      it.rinfo()->update_wasm_memory_size(old_size, new_size);
     }
+    modified = true;
   }
   if (modified) {
     Assembler::FlushICache(isolate, code->instruction_start(),

test/cctest/test-run-wasm-relocation-arm.cc

@@ -55,15 +55,10 @@ TEST(WasmRelocationArmMemoryReference) {
   // Relocating references by offset
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode)) {
-      // Dummy values of size used here as the objective of the test is to
-      // verify that the immediate is patched correctly
-      it.rinfo()->update_wasm_memory_reference(
-          it.rinfo()->wasm_memory_reference(),
-          it.rinfo()->wasm_memory_reference() + offset, 1, 2,
-          SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemoryReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_reference(
+        it.rinfo()->wasm_memory_reference(),
+        it.rinfo()->wasm_memory_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   // Call into relocated code object
@@ -114,13 +109,10 @@ TEST(WasmRelocationArmMemorySizeReference) {
 
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemorySizeReference(mode)) {
-      it.rinfo()->update_wasm_memory_reference(
-          reinterpret_cast<Address>(1234), reinterpret_cast<Address>(1234),
-          it.rinfo()->wasm_memory_size_reference(),
-          it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemorySizeReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_size(
+        it.rinfo()->wasm_memory_size_reference(),
+        it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
   }
 
   ret_value = runnable.Call();

test/cctest/test-run-wasm-relocation-arm64.cc

@@ -57,15 +57,10 @@ TEST(WasmRelocationArm64MemoryReference) {
   // Relocating reference by offset
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode)) {
-      // Dummy values of size used here as the objective of the test is to
-      // verify that the immediate is patched correctly
-      it.rinfo()->update_wasm_memory_reference(
-          it.rinfo()->wasm_memory_reference(),
-          it.rinfo()->wasm_memory_reference() + offset, 1, 2,
-          SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemoryReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_reference(
+        it.rinfo()->wasm_memory_reference(),
+        it.rinfo()->wasm_memory_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   // Call into relocated code object
@@ -117,13 +112,10 @@ TEST(WasmRelocationArm64MemorySizeReference) {
 
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemorySizeReference(mode)) {
-      it.rinfo()->update_wasm_memory_reference(
-          reinterpret_cast<Address>(0x1234), reinterpret_cast<Address>(0x1234),
-          it.rinfo()->wasm_memory_size_reference(),
-          it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemorySizeReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_size(
+        it.rinfo()->wasm_memory_size_reference(),
+        it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
   }
 
   ret_value = runnable.Call();

test/cctest/test-run-wasm-relocation-ia32.cc

@@ -61,15 +61,10 @@ TEST(WasmRelocationIa32MemoryReference) {
   // Relocating references by offset
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode)) {
-      // Dummy values of size used here as the objective of the test is to
-      // verify that the immediate is patched correctly
-      it.rinfo()->update_wasm_memory_reference(
-          it.rinfo()->wasm_memory_reference(),
-          it.rinfo()->wasm_memory_reference() + offset, 1, 2,
-          SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemoryReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_reference(
+        it.rinfo()->wasm_memory_reference(),
+        it.rinfo()->wasm_memory_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   // Check if immediate is updated correctly
@@ -128,13 +123,10 @@ TEST(WasmRelocationIa32MemorySizeReference) {
 
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemorySizeReference(mode)) {
-      it.rinfo()->update_wasm_memory_reference(
-          reinterpret_cast<Address>(1234), reinterpret_cast<Address>(1234),
-          it.rinfo()->wasm_memory_size_reference(),
-          it.rinfo()->wasm_memory_size_reference() + offset, SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemorySizeReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_size(
+        it.rinfo()->wasm_memory_size_reference(),
+        it.rinfo()->wasm_memory_size_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   ret_value = runnable.Call();

test/cctest/test-run-wasm-relocation-x64.cc

@@ -59,15 +59,10 @@ TEST(WasmRelocationX64MemoryReference) {
   // Relocating references by offset
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode)) {
-      // Dummy values of size used here as the objective of the test is to
-      // verify that the immediate is patched correctly
-      it.rinfo()->update_wasm_memory_reference(
-          it.rinfo()->wasm_memory_reference(),
-          it.rinfo()->wasm_memory_reference() + offset, 1, 2,
-          SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemoryReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_reference(
+        it.rinfo()->wasm_memory_reference(),
+        it.rinfo()->wasm_memory_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   // Check if immediate is updated correctly
@@ -122,13 +117,10 @@ TEST(WasmRelocationX64WasmMemorySizeReference) {
 
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemorySizeReference(mode)) {
-      it.rinfo()->update_wasm_memory_reference(
-          reinterpret_cast<Address>(1234), reinterpret_cast<Address>(1234),
-          it.rinfo()->wasm_memory_size_reference(),
-          it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemorySizeReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_size(
+        it.rinfo()->wasm_memory_size_reference(),
+        it.rinfo()->wasm_memory_size_reference() + diff, SKIP_ICACHE_FLUSH);
   }
 
   ret_value = runnable.Call();

test/cctest/test-run-wasm-relocation-x87.cc

@@ -61,15 +61,10 @@ TEST(WasmRelocationX87MemoryReference) {
   // Relocating references by offset
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
-    RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemoryReference(mode)) {
-      // Dummy values of size used here as the objective of the test is to
-      // verify that the immediate is patched correctly
-      it.rinfo()->update_wasm_memory_reference(
-          it.rinfo()->wasm_memory_reference(),
-          it.rinfo()->wasm_memory_reference() + offset, 1, 2,
-          SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemoryReference(it.rinfo()->rmode()));
+    it.rinfo()->update_wasm_memory_reference(
+        it.rinfo()->wasm_memory_reference(),
+        it.rinfo()->wasm_memory_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   // Check if immediate is updated correctly
@@ -129,12 +124,10 @@ TEST(WasmRelocationX87MemorySizeReference) {
   int mode_mask = (1 << RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
   for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
     RelocInfo::Mode mode = it.rinfo()->rmode();
-    if (RelocInfo::IsWasmMemorySizeReference(mode)) {
-      it.rinfo()->update_wasm_memory_reference(
-          reinterpret_cast<Address>(1234), reinterpret_cast<Address>(1234),
-          it.rinfo()->wasm_memory_size_reference(),
-          it.rinfo()->wasm_memory_size_reference() + offset, SKIP_ICACHE_FLUSH);
-    }
+    DCHECK(RelocInfo::IsWasmMemorySizeReference(mode));
+    it.rinfo()->update_wasm_memory_size(
+        it.rinfo()->wasm_memory_size_reference(),
+        it.rinfo()->wasm_memory_size_reference() + offset, SKIP_ICACHE_FLUSH);
   }
 
   ret_value = runnable.Call();

test/mjsunit/regress/wasm/regression-698587.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var heap = new ArrayBuffer();
+function asm(stdlib, ffi, heap) {
+  "use asm";
+  return {};
+}
+asm({}, {}, heap);