Changes required to make the new Memory64 spec tests run (#1560)

These uncovered some things the previous tests didn't! Also required the switching of the location of the index as discussed in WebAssembly/memory64#5 Also one small .py change that ensures the new tests have consistent posix paths.
WebAssembly · Oct 26, 2020 · 8e42376 · 8e42376
1 parent 30af7af
commit 8e42376
Show file tree

Hide file tree

Showing 33 changed files with 932 additions and 65 deletions.
diff --git a/src/interp/interp-inl.h b/src/interp/interp-inl.h
@@ -633,7 +633,11 @@ inline Memory::Ptr Memory::New(interp::Store& store, MemoryType type) {
 }
 
 inline bool Memory::IsValidAccess(u64 offset, u64 addend, u64 size) const {
-  return offset + addend + size <= data_.size();
+  // FIXME: make this faster.
+  return offset <= data_.size() &&
+         addend <= data_.size() &&
+         size <= data_.size() &&
+         offset + addend + size <= data_.size();
 }
 
 inline bool Memory::IsValidAtomicAccess(u64 offset,

diff --git a/src/interp/interp.cc b/src/interp/interp.cc
@@ -1023,6 +1023,10 @@ Value Thread::Pop() {
   return value;
 }
 
+u64 Thread::PopPtr(const Memory::Ptr& memory) {
+  return memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+}
+
 template <typename T>
 void WABT_VECTORCALL Thread::Push(T value) {
   Push(Value::Make(value));
@@ -1187,15 +1191,15 @@ RunResult Thread::StepInternal(Trap::Ptr* out_trap) {
     case O::MemoryGrow: {
       Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32]};
       u64 old_size = memory->PageSize();
-      if (Failed(memory->Grow(Pop<u32>()))) {
-        if (memory->type().limits.is_64) {
+      if (memory->type().limits.is_64) {
+        if (Failed(memory->Grow(Pop<u64>()))) {
           Push<s64>(-1);
         } else {
-          Push<s32>(-1);
+          Push<u64>(old_size);
         }
       } else {
-        if (memory->type().limits.is_64) {
-          Push<u64>(old_size);
+        if (Failed(memory->Grow(Pop<u32>()))) {
+          Push<s32>(-1);
         } else {
           Push<u32>(old_size);
         }
@@ -1754,7 +1758,7 @@ RunResult Thread::DoCall(const Func::Ptr& func, Trap::Ptr* out_trap) {
 template <typename T>
 RunResult Thread::Load(Instr instr, T* out, Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32x2.fst]};
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   TRAP_IF(Failed(memory->Load(offset, instr.imm_u32x2.snd, out)),
           StringPrintf("out of bounds memory access: access at %" PRIu64
                        "+%" PRIzd " >= max value %" PRIu64,
@@ -1777,7 +1781,7 @@ template <typename T, typename V>
 RunResult Thread::DoStore(Instr instr, Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32x2.fst]};
   V val = static_cast<V>(Pop<T>());
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   TRAP_IF(Failed(memory->Store(offset, instr.imm_u32x2.snd, val)),
           StringPrintf("out of bounds memory access: access at %" PRIu64
                        "+%" PRIzd " >= max value %" PRIu64,
@@ -1843,7 +1847,7 @@ RunResult Thread::DoMemoryInit(Instr instr, Trap::Ptr* out_trap) {
   auto&& data = inst_->datas()[instr.imm_u32x2.snd];
   auto size = Pop<u32>();
   auto src = Pop<u32>();
-  auto dst = Pop<u32>();
+  auto dst = PopPtr(memory);
   TRAP_IF(Failed(memory->Init(dst, data, src, size)),
           "out of bounds memory access: memory.init out of bounds");
   return RunResult::Ok;
@@ -1857,9 +1861,9 @@ RunResult Thread::DoDataDrop(Instr instr) {
 RunResult Thread::DoMemoryCopy(Instr instr, Trap::Ptr* out_trap) {
   Memory::Ptr mem_dst{store_, inst_->memories()[instr.imm_u32x2.fst]};
   Memory::Ptr mem_src{store_, inst_->memories()[instr.imm_u32x2.snd]};
-  auto size = Pop<u32>();
-  auto src = Pop<u32>();
-  auto dst = Pop<u32>();
+  auto size = PopPtr(mem_src);
+  auto src = PopPtr(mem_src);
+  auto dst = PopPtr(mem_dst);
   // TODO: change to "out of bounds"
   TRAP_IF(Failed(Memory::Copy(*mem_dst, dst, *mem_src, src, size)),
           "out of bounds memory access: memory.copy out of bound");
@@ -1868,9 +1872,9 @@ RunResult Thread::DoMemoryCopy(Instr instr, Trap::Ptr* out_trap) {
 
 RunResult Thread::DoMemoryFill(Instr instr, Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32]};
-  auto size = Pop<u32>();
+  auto size = PopPtr(memory);
   auto value = Pop<u32>();
-  auto dst = Pop<u32>();
+  auto dst = PopPtr(memory);
   TRAP_IF(Failed(memory->Fill(dst, value, size)),
           "out of bounds memory access: memory.fill out of bounds");
   return RunResult::Ok;
@@ -2151,7 +2155,7 @@ RunResult Thread::DoSimdLoadExtend(Instr instr, Trap::Ptr* out_trap) {
 template <typename T, typename V>
 RunResult Thread::DoAtomicLoad(Instr instr, Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32x2.fst]};
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   V val;
   TRAP_IF(Failed(memory->AtomicLoad(offset, instr.imm_u32x2.snd, &val)),
           StringPrintf("invalid atomic access at %" PRIaddress "+%u", offset,
@@ -2164,7 +2168,7 @@ template <typename T, typename V>
 RunResult Thread::DoAtomicStore(Instr instr, Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32x2.fst]};
   V val = static_cast<V>(Pop<T>());
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   TRAP_IF(Failed(memory->AtomicStore(offset, instr.imm_u32x2.snd, val)),
           StringPrintf("invalid atomic access at %" PRIaddress "+%u", offset,
                        instr.imm_u32x2.snd));
@@ -2177,7 +2181,7 @@ RunResult Thread::DoAtomicRmw(BinopFunc<T, T> f,
                               Trap::Ptr* out_trap) {
   Memory::Ptr memory{store_, inst_->memories()[instr.imm_u32x2.fst]};
   T val = static_cast<T>(Pop<R>());
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   T old;
   TRAP_IF(Failed(memory->AtomicRmw(offset, instr.imm_u32x2.snd, val, f, &old)),
           StringPrintf("invalid atomic access at %" PRIaddress "+%u", offset,
@@ -2192,7 +2196,7 @@ RunResult Thread::DoAtomicRmwCmpxchg(Instr instr, Trap::Ptr* out_trap) {
   V replace = static_cast<V>(Pop<T>());
   V expect = static_cast<V>(Pop<T>());
   V old;
-  u64 offset = memory->type().limits.is_64 ? Pop<u64>() : Pop<u32>();
+  u64 offset = PopPtr(memory);
   TRAP_IF(Failed(memory->AtomicRmwCmpxchg(offset, instr.imm_u32x2.snd, expect,
                                           replace, &old)),
           StringPrintf("invalid atomic access at %" PRIaddress "+%u", offset,

diff --git a/src/interp/interp.h b/src/interp/interp.h
@@ -1065,6 +1065,7 @@ class Thread : public Object {
   template <typename T>
   T WABT_VECTORCALL Pop();
   Value Pop();
+  u64 PopPtr(const Memory::Ptr& memory);
 
   template <typename T>
   void WABT_VECTORCALL Push(T);

diff --git a/src/wast-parser.cc b/src/wast-parser.cc
@@ -939,6 +939,22 @@ bool WastParser::ParseAlignOpt(Address* out_align) {
   }
 }
 
+Result WastParser::ParseLimitsIndex(Limits* out_limits) {
+  WABT_TRACE(ParseLimitsIndex);
+
+  if (PeekMatch(TokenType::ValueType)) {
+    if (GetToken().type() == Type::I64) {
+      Consume();
+      out_limits->is_64 = true;
+    } else if (GetToken().type() == Type::I32) {
+      Consume();
+      out_limits->is_64 = false;
+    }
+  }
+
+  return Result::Ok;
+}
+
 Result WastParser::ParseLimits(Limits* out_limits) {
   WABT_TRACE(ParseLimits);
 
@@ -954,11 +970,6 @@ Result WastParser::ParseLimits(Limits* out_limits) {
     out_limits->is_shared = true;
   }
 
-  if (PeekMatch(TokenType::ValueType) && GetToken().type() == Type::I64) {
-    Consume();
-    out_limits->is_64 = true;
-  }
-
   return Result::Ok;
 }
 
@@ -1387,6 +1398,7 @@ Result WastParser::ParseImportModuleField(Module* module) {
       Consume();
       ParseBindVarOpt(&name);
       auto import = MakeUnique<MemoryImport>(name);
+      CHECK_RESULT(ParseLimitsIndex(&import->memory.page_limits));
       CHECK_RESULT(ParseLimits(&import->memory.page_limits));
       EXPECT(Rpar);
       field = MakeUnique<ImportModuleField>(std::move(import), loc);
@@ -1441,32 +1453,35 @@ Result WastParser::ParseMemoryModuleField(Module* module) {
     CheckImportOrdering(module);
     auto import = MakeUnique<MemoryImport>(name);
     CHECK_RESULT(ParseInlineImport(import.get()));
+    CHECK_RESULT(ParseLimitsIndex(&import->memory.page_limits));
     CHECK_RESULT(ParseLimits(&import->memory.page_limits));
     auto field =
         MakeUnique<ImportModuleField>(std::move(import), GetLocation());
     module->AppendField(std::move(field));
-  } else if (MatchLpar(TokenType::Data)) {
-    auto data_segment_field = MakeUnique<DataSegmentModuleField>(loc);
-    DataSegment& data_segment = data_segment_field->data_segment;
-    data_segment.memory_var = Var(module->memories.size());
-    data_segment.offset.push_back(MakeUnique<ConstExpr>(Const::I32(0)));
-    data_segment.offset.back().loc = loc;
-    ParseTextListOpt(&data_segment.data);
-    EXPECT(Rpar);
-
-    auto memory_field = MakeUnique<MemoryModuleField>(loc, name);
-    uint32_t byte_size = WABT_ALIGN_UP_TO_PAGE(data_segment.data.size());
-    uint32_t page_size = WABT_BYTES_TO_PAGES(byte_size);
-    memory_field->memory.page_limits.initial = page_size;
-    memory_field->memory.page_limits.max = page_size;
-    memory_field->memory.page_limits.has_max = true;
-
-    module->AppendField(std::move(memory_field));
-    module->AppendField(std::move(data_segment_field));
   } else {
     auto field = MakeUnique<MemoryModuleField>(loc, name);
-    CHECK_RESULT(ParseLimits(&field->memory.page_limits));
-    module->AppendField(std::move(field));
+    CHECK_RESULT(ParseLimitsIndex(&field->memory.page_limits));
+    if (MatchLpar(TokenType::Data)) {
+      auto data_segment_field = MakeUnique<DataSegmentModuleField>(loc);
+      DataSegment& data_segment = data_segment_field->data_segment;
+      data_segment.memory_var = Var(module->memories.size());
+      data_segment.offset.push_back(MakeUnique<ConstExpr>(Const::I32(0)));
+      data_segment.offset.back().loc = loc;
+      ParseTextListOpt(&data_segment.data);
+      EXPECT(Rpar);
+
+      uint32_t byte_size = WABT_ALIGN_UP_TO_PAGE(data_segment.data.size());
+      uint32_t page_size = WABT_BYTES_TO_PAGES(byte_size);
+      field->memory.page_limits.initial = page_size;
+      field->memory.page_limits.max = page_size;
+      field->memory.page_limits.has_max = true;
+
+      module->AppendField(std::move(field));
+      module->AppendField(std::move(data_segment_field));
+    } else {
+      CHECK_RESULT(ParseLimits(&field->memory.page_limits));
+      module->AppendField(std::move(field));
+    }
   }
 
   AppendInlineExportFields(module, &export_fields, module->memories.size() - 1);

diff --git a/src/wast-parser.h b/src/wast-parser.h
@@ -137,6 +137,7 @@ class WastParser {
   Result ParseQuotedText(std::string* text);
   bool ParseOffsetOpt(Address* offset);
   bool ParseAlignOpt(Address* align);
+  Result ParseLimitsIndex(Limits*);
   Result ParseLimits(Limits*);
   Result ParseNat(uint64_t*);
 

diff --git a/src/wat-writer.cc b/src/wat-writer.cc
@@ -1267,16 +1267,16 @@ void WatWriter::WriteEvent(const Event& event) {
 }
 
 void WatWriter::WriteLimits(const Limits& limits) {
+  if (limits.is_64) {
+    Writef("i64");
+  }
   Writef("%" PRIu64, limits.initial);
   if (limits.has_max) {
     Writef("%" PRIu64, limits.max);
   }
   if (limits.is_shared) {
     Writef("shared");
   }
-  if (limits.is_64) {
-    Writef("i64");
-  }
 }
 
 void WatWriter::WriteTable(const Table& table) {

diff --git a/test/dump/bulk-memory64.txt b/test/dump/bulk-memory64.txt
@@ -2,7 +2,7 @@
 ;;; ARGS0: --enable-bulk-memory
 
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (data "a")
   (func
     i64.const 0 i32.const 0 i32.const 0 memory.init 0

diff --git a/test/dump/load64.txt b/test/dump/load64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: run-objdump
 ;;; ARGS0: -v
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (func
     i64.const 0
     i32.load

diff --git a/test/dump/store64.txt b/test/dump/store64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: run-objdump
 ;;; ARGS0: -v
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (func
     i64.const 0
     i32.const 0

diff --git a/test/interp/load64.txt b/test/interp/load64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: run-interp
 ;;; ARGS: --enable-memory64
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (data (i32.const 0) "\ff\ff\ff\ff")
   (data (i32.const 4) "\00\00\ce\41")
   (data (i32.const 8) "\00\00\00\00\00\ff\8f\40")

diff --git a/test/interp/store64.txt b/test/interp/store64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: run-interp
 ;;; ARGS: --enable-memory64
 (module
-  (memory 1 i64)
+  (memory i64 1)
 
   (func (export "i32_store8") (result i32)
       i64.const 0

diff --git a/test/parse/expr/atomic64.txt b/test/parse/expr/atomic64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-threads --enable-memory64
 (module
-  (memory 1 1 shared i64)
+  (memory i64 1 1 shared)
   (func
     i64.const 0 i32.const 0 atomic.notify drop
     i64.const 0 i32.const 0 i64.const 0 i32.atomic.wait drop

diff --git a/test/parse/expr/bulk-memory-named64.txt b/test/parse/expr/bulk-memory-named64.txt
@@ -2,7 +2,7 @@
 ;;; ARGS: --enable-bulk-memory --enable-memory64
 
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (data $data "a")
   (func
     i64.const 0 i32.const 0 i32.const 0 memory.init $data

diff --git a/test/parse/expr/grow-memory64.txt b/test/parse/expr/grow-memory64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-memory64
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (func
     i64.const 100
     grow_memory

diff --git a/test/parse/expr/load64.txt b/test/parse/expr/load64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-memory64
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (func
     i64.const 0
     i32.load

diff --git a/test/parse/expr/memory-copy64.txt b/test/parse/expr/memory-copy64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-bulk-memory --enable-memory64
 (module
-  (memory 0 i64)
+  (memory i64 0)
 
   (func
     i64.const 0

diff --git a/test/parse/expr/memory-fill64.txt b/test/parse/expr/memory-fill64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-bulk-memory --enable-memory64
 (module
-  (memory 0 i64)
+  (memory i64 0)
 
   (func
     i64.const 0

diff --git a/test/parse/expr/memory-init64.txt b/test/parse/expr/memory-init64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-bulk-memory  --enable-memory64
 (module
-  (memory 0 i64)
+  (memory i64 0)
 
   (func
     i64.const 0

diff --git a/test/parse/expr/store64.txt b/test/parse/expr/store64.txt
@@ -1,7 +1,7 @@
 ;;; TOOL: wat2wasm
 ;;; ARGS: --enable-memory64
 (module
-  (memory 1 i64)
+  (memory i64 1)
   (func
     i64.const 0
     i32.const 0