Fix array.{get*,set} validation (#31)

The `array.get*` and `array.set` instructions take an `i32` index on the stack, which were not previously being accounted for.
WebAssembly · Nov 19, 2020 · 45f56c0 · 45f56c0
1 parent 5242b44
commit 45f56c0
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/src/valid/validate_instruction.cc b/src/valid/validate_instruction.cc
@@ -1692,6 +1692,7 @@ bool ArrayNewDefaultWithRtt(Context& context,
 }
 
 bool ArrayGet(Context& context, Location loc, const At<Index>& immediate) {
+  bool valid = PopType(context, loc, StackType::I32());
   auto [stack_type, array_type] = PopArrayReference(context, loc, immediate);
   if (!array_type) {
     return false;
@@ -1703,12 +1704,13 @@ bool ArrayGet(Context& context, Location loc, const At<Index>& immediate) {
   }
 
   PushType(context, StackType{*value_type});
-  return true;
+  return valid;
 }
 
 bool ArrayGetPacked(Context& context,
                     Location loc,
                     const At<Index>& immediate) {
+  bool valid = PopType(context, loc, StackType::I32());
   auto [stack_type, array_type] = PopArrayReference(context, loc, immediate);
   if (!array_type) {
     return false;
@@ -1720,7 +1722,7 @@ bool ArrayGetPacked(Context& context,
   }
 
   PushType(context, StackType::I32());
-  return true;
+  return valid;
 }
 
 bool ArraySet(Context& context, Location loc, const At<Index>& immediate) {
@@ -1736,9 +1738,12 @@ bool ArraySet(Context& context, Location loc, const At<Index>& immediate) {
     valid = false;
   }
 
-  StackTypeList stack_types{StackType{ValueType{ReferenceType{
-                                RefType{HeapType{immediate}, Null::Yes}}}},
-                            ToStackType(array_type->field->type)};
+  StackTypeList stack_types{
+      StackType{
+          ValueType{ReferenceType{RefType{HeapType{immediate}, Null::Yes}}}},
+      StackType::I32(),
+      ToStackType(array_type->field->type),
+  };
   return AllTrue(valid, PopTypes(context, loc, stack_types));
 }
 

diff --git a/test/valid/validate_instruction_test.cc b/test/valid/validate_instruction_test.cc
@@ -3382,16 +3382,16 @@ TEST_F(ValidateInstructionTest, ArrayGet) {
   ValueType VT_RefNull_index = MakeValueTypeRef(index, Null::Yes);
   ValueType VT_Ref_index = MakeValueTypeRef(index, Null::No);
 
-  TestSignature(I{O::ArrayGet, index}, {VT_RefNull_index}, {VT_F32});
-  TestSignature(I{O::ArrayGet, index}, {VT_Ref_index}, {VT_F32});
+  TestSignature(I{O::ArrayGet, index}, {VT_RefNull_index, VT_I32}, {VT_F32});
+  TestSignature(I{O::ArrayGet, index}, {VT_Ref_index, VT_I32}, {VT_F32});
 }
 
 TEST_F(ValidateInstructionTest, ArrayGet_FailPacked) {
   auto index = AddArrayType(
       ArrayType{FieldType{StorageType{PackedType::I8}, Mutability::Const}});
   ValueType VT_RefNull_index = MakeValueTypeRef(index, Null::Yes);
 
-  FailWithTypeStack(I{O::ArrayGet, index}, {VT_RefNull_index});
+  FailWithTypeStack(I{O::ArrayGet, index}, {VT_RefNull_index, VT_I32});
 }
 
 TEST_F(ValidateInstructionTest, ArrayGetPacked) {
@@ -3401,21 +3401,21 @@ TEST_F(ValidateInstructionTest, ArrayGetPacked) {
   ValueType VT_Ref_index = MakeValueTypeRef(index, Null::No);
 
   // ArrayGetS
-  TestSignature(I{O::ArrayGetS, index}, {VT_RefNull_index}, {VT_I32});
-  TestSignature(I{O::ArrayGetS, index}, {VT_Ref_index}, {VT_I32});
+  TestSignature(I{O::ArrayGetS, index}, {VT_RefNull_index, VT_I32}, {VT_I32});
+  TestSignature(I{O::ArrayGetS, index}, {VT_Ref_index, VT_I32}, {VT_I32});
 
   // ArrayGetU
-  TestSignature(I{O::ArrayGetU, index}, {VT_RefNull_index}, {VT_I32});
-  TestSignature(I{O::ArrayGetU, index}, {VT_Ref_index}, {VT_I32});
+  TestSignature(I{O::ArrayGetU, index}, {VT_RefNull_index, VT_I32}, {VT_I32});
+  TestSignature(I{O::ArrayGetU, index}, {VT_Ref_index, VT_I32}, {VT_I32});
 }
 
 TEST_F(ValidateInstructionTest, ArrayGetPacked_FailUnpacked) {
   auto index = AddArrayType(
       ArrayType{FieldType{StorageType{VT_F32}, Mutability::Const}});
   ValueType VT_RefNull_index = MakeValueTypeRef(index, Null::Yes);
 
-  FailWithTypeStack(I{O::ArrayGetS, index}, {VT_RefNull_index});
-  FailWithTypeStack(I{O::ArrayGetU, index}, {VT_RefNull_index});
+  FailWithTypeStack(I{O::ArrayGetS, index}, {VT_RefNull_index, VT_I32});
+  FailWithTypeStack(I{O::ArrayGetU, index}, {VT_RefNull_index, VT_I32});
 }
 
 TEST_F(ValidateInstructionTest, ArraySet) {
@@ -3424,8 +3424,8 @@ TEST_F(ValidateInstructionTest, ArraySet) {
   ValueType VT_RefNull_index = MakeValueTypeRef(index, Null::Yes);
   ValueType VT_Ref_index = MakeValueTypeRef(index, Null::No);
 
-  TestSignature(I{O::ArraySet, index}, {VT_RefNull_index, VT_F32}, {});
-  TestSignature(I{O::ArraySet, index}, {VT_Ref_index, VT_F32}, {});
+  TestSignature(I{O::ArraySet, index}, {VT_RefNull_index, VT_I32, VT_F32}, {});
+  TestSignature(I{O::ArraySet, index}, {VT_Ref_index, VT_I32, VT_F32}, {});
 }
 
 TEST_F(ValidateInstructionTest, ArrayLen) {