[MERGE #2174 @Cellule] WASM - elem/data segment validation

Merge pull request #2174 from Cellule:wasm/elem Check if a table is present when reading an element section Validate the init_expr for element and data segments Disallow internal global in init_expr The issue WebAssembly/spec#367 made me realize we were doing the same thing the spec interpreter was which was not according to the design documents Check the type of init_expr for globals
chakra-core · Dec 12, 2016 · 302ec10 · 302ec10
2 parents 14aa392 + 5b6798b
commit 302ec10
Show file tree

Hide file tree

Showing 28 changed files with 112 additions and 88 deletions.
diff --git a/lib/Runtime/Library/WebAssemblyModule.cpp b/lib/Runtime/Library/WebAssemblyModule.cpp
@@ -553,29 +553,46 @@ WebAssemblyModule::AddTableImport(const char16* modName, uint32 modNameLen, cons
 uint
 WebAssemblyModule::GetOffsetFromInit(const Wasm::WasmNode& initExpr, const WebAssemblyEnvironment* env) const
 {
-    if (initExpr.op != Wasm::wbI32Const && initExpr.op != Wasm::wbGetGlobal)
+    try
+    {
+        ValidateInitExportForOffset(initExpr);
+    }
+    catch (Wasm::WasmCompilationException &e)
     {
-        throw Wasm::WasmCompilationException(_u("Invalid init_expr for element offset"));
+        // Should have been checked at compile time
+        Assert(UNREACHED);
+        throw e;
     }
+
     uint offset = 0;
     if (initExpr.op == Wasm::wbI32Const)
     {
         offset = initExpr.cnst.i32;
     }
     else if (initExpr.op == Wasm::wbGetGlobal)
     {
-        if (initExpr.var.num >= (uint)m_globals->Count())
-        {
-            throw Wasm::WasmCompilationException(_u("global %d doesn't exist"), initExpr.var.num);
-        }
-        Wasm::WasmGlobal* global = m_globals->Item(initExpr.var.num);
+        Wasm::WasmGlobal* global = GetGlobal(initExpr.var.num);
+        Assert(global->GetType() == Wasm::WasmTypes::I32);
+        offset = env->GetGlobalValue(global).i32;
+    }
+    return offset;
+}
+
+void
+WebAssemblyModule::ValidateInitExportForOffset(const Wasm::WasmNode& initExpr) const
+{
+    if (initExpr.op == Wasm::wbGetGlobal)
+    {
+        Wasm::WasmGlobal* global = GetGlobal(initExpr.var.num);
         if (global->GetType() != Wasm::WasmTypes::I32)
         {
-            throw Wasm::WasmCompilationException(_u("global %d must be i32"), initExpr.var.num);
+            throw Wasm::WasmCompilationException(_u("global %u must be i32 for init_expr"), initExpr.var.num);
         }
-        offset = env->GetGlobalValue(global).i32;
     }
-    return offset;
+    else if (initExpr.op != Wasm::wbI32Const)
+    {
+        throw Wasm::WasmCompilationException(_u("Invalid init_expr for offset"));
+    }
 }
 
 void

diff --git a/lib/Runtime/Library/WebAssemblyModule.h b/lib/Runtime/Library/WebAssemblyModule.h
@@ -95,7 +95,8 @@ class WebAssemblyModule : public DynamicObject
     Wasm::WasmImport * GetTableImport() const { return m_tableImport; }
     uint32 GetImportedFunctionCount() const { return m_importedFunctionCount; }
 
-    uint GetOffsetFromInit(const Wasm::WasmNode& initexpr, const class WebAssemblyEnvironment* env) const;
+    uint GetOffsetFromInit(const Wasm::WasmNode& initExpr, const class WebAssemblyEnvironment* env) const;
+    void ValidateInitExportForOffset(const Wasm::WasmNode& initExpr) const;
 
     void AddGlobal(Wasm::GlobalReferenceTypes::Type refType, Wasm::WasmTypes::WasmType type, bool isMutable, Wasm::WasmNode init);
     uint32 GetGlobalCount() const;

diff --git a/lib/WasmReader/WasmBinaryReader.cpp b/lib/WasmReader/WasmBinaryReader.cpp
@@ -846,12 +846,12 @@ WasmBinaryReader::ReadElementSection()
     for (uint32 i = 0; i < count; ++i)
     {
         uint32 index = LEB128(length); // Table id
-        if (index != 0)
+        if (index != 0 || !(m_module->HasTable() || m_module->HasTableImport()))
         {
-            ThrowDecodingError(_u("Invalid table index %d"), index); //MVP limitation
+            ThrowDecodingError(_u("Unknown table index %d"), index); //MVP limitation
         }
 
-        WasmNode initExpr = ReadInitExpr(); //Offset Init
+        WasmNode initExpr = ReadInitExpr(true);
         uint32 numElem = LEB128(length);
 
         WasmElementSegment *eSeg = Anew(m_alloc, WasmElementSegment, m_alloc, index, initExpr, numElem);
@@ -883,12 +883,12 @@ WasmBinaryReader::ReadDataSegments()
     for (uint32 i = 0; i < entries; ++i)
     {
         UINT32 index = LEB128(len);
-        if (index != 0)
+        if (index != 0 || !(m_module->HasMemory() || m_module->HasMemoryImport()))
         {
-            ThrowDecodingError(_u("Memory index out of bounds %d > 0"), index);
+            ThrowDecodingError(_u("Unknown memory index %u"), index);
         }
         TRACE_WASM_DECODER(_u("Data Segment #%u"), i);
-        WasmNode initExpr = ReadInitExpr();
+        WasmNode initExpr = ReadInitExpr(true);
 
         //UINT32 offset = initExpr.cnst.i32;
         UINT32 dataByteLen = LEB128(len);
@@ -934,23 +934,27 @@ WasmBinaryReader::ReadGlobalsSection()
         WasmTypes::WasmType type = ReadWasmType(len);
         bool isMutable = ReadConst<UINT8>() == 1;
         WasmNode globalNode = ReadInitExpr();
+        GlobalReferenceTypes::Type refType = GlobalReferenceTypes::Const;
+        WasmTypes::WasmType initType;
+
         switch (globalNode.op) {
-        case  wbI32Const:
-        case  wbF32Const:
-        case  wbF64Const:
-        case  wbI64Const:
-            m_module->AddGlobal(GlobalReferenceTypes::Const, type, isMutable, globalNode);
-            break;
+        case  wbI32Const: initType = WasmTypes::I32; break;
+        case  wbF32Const: initType = WasmTypes::F32; break;
+        case  wbF64Const: initType = WasmTypes::F64; break;
+        case  wbI64Const: initType = WasmTypes::I64; break;
         case  wbGetGlobal:
-            if (m_module->GetGlobal(globalNode.var.num)->GetReferenceType() != GlobalReferenceTypes::ImportedReference)
-            {
-                ThrowDecodingError(_u("Global can only be initialized with a const or an imported global"));
-            }
-            m_module->AddGlobal(GlobalReferenceTypes::LocalReference, type, isMutable, globalNode);
+            initType = m_module->GetGlobal(globalNode.var.num)->GetType();
+            refType = GlobalReferenceTypes::LocalReference;
             break;
         default:
             Assert(UNREACHED);
+            ThrowDecodingError(_u("Unknown global init_expr"));
+        }
+        if (type != initType)
+        {
+            ThrowDecodingError(_u("Type mismatch for global initialization"));
         }
+        m_module->AddGlobal(refType, type, isMutable, globalNode);
     }
 }
 
@@ -1133,7 +1137,7 @@ WasmBinaryReader::SLEB128(UINT &length)
 }
 
 WasmNode
-WasmBinaryReader::ReadInitExpr()
+WasmBinaryReader::ReadInitExpr(bool isOffset)
 {
     if (m_readerState != READER_STATE_MODULE)
     {
@@ -1155,6 +1159,10 @@ WasmBinaryReader::ReadInitExpr()
     {
         uint32 globalIndex = node.var.num;
         WasmGlobal* global = m_module->GetGlobal(globalIndex);
+        if (global->GetReferenceType() != GlobalReferenceTypes::ImportedReference)
+        {
+            ThrowDecodingError(_u("initializer expression can only use imported globals"));
+        }
         if (global->IsMutable())
         {
             ThrowDecodingError(_u("initializer expression cannot reference a mutable global"));
@@ -1169,6 +1177,10 @@ WasmBinaryReader::ReadInitExpr()
     {
         ThrowDecodingError(_u("Missing end opcode after init expr"));
     }
+    if (isOffset)
+    {
+        m_module->ValidateInitExportForOffset(node);
+    }
     return node;
 }
 

diff --git a/lib/WasmReader/WasmBinaryReader.h b/lib/WasmReader/WasmBinaryReader.h
@@ -86,7 +86,7 @@ namespace Wasm
         MaxAllowedType LEB128(UINT &length, bool sgn = false);
         template<typename MaxAllowedType = INT>
         MaxAllowedType SLEB128(UINT &length);
-        WasmNode ReadInitExpr();
+        WasmNode ReadInitExpr(bool isOffset = false);
 
         void CheckBytesLeft(UINT bytesNeeded);
         bool EndOfFunc();

diff --git a/test/WasmSpec/testsuite-bin/memory.10.wasm b/test/WasmSpec/testsuite-bin/memory.10.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.11.wasm b/test/WasmSpec/testsuite-bin/memory.11.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.12.wasm b/test/WasmSpec/testsuite-bin/memory.12.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.13.wasm b/test/WasmSpec/testsuite-bin/memory.13.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.14.wasm b/test/WasmSpec/testsuite-bin/memory.14.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.15.wasm b/test/WasmSpec/testsuite-bin/memory.15.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.16.wasm b/test/WasmSpec/testsuite-bin/memory.16.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.17.wasm b/test/WasmSpec/testsuite-bin/memory.17.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.18.wasm b/test/WasmSpec/testsuite-bin/memory.18.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.19.wasm b/test/WasmSpec/testsuite-bin/memory.19.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.20.wasm b/test/WasmSpec/testsuite-bin/memory.20.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.21.wasm b/test/WasmSpec/testsuite-bin/memory.21.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.22.wasm b/test/WasmSpec/testsuite-bin/memory.22.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.23.wasm b/test/WasmSpec/testsuite-bin/memory.23.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.24.wasm b/test/WasmSpec/testsuite-bin/memory.24.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.25.wasm b/test/WasmSpec/testsuite-bin/memory.25.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.9.wasm b/test/WasmSpec/testsuite-bin/memory.9.wasm
diff --git a/test/WasmSpec/testsuite-bin/memory.json b/test/WasmSpec/testsuite-bin/memory.json
@@ -34,34 +34,34 @@
     "commands": []
   }, {
     "filename": "memory.11.wasm",
-    "commands": []
-  }, {
-    "filename": "memory.12.wasm",
-    "commands": []
-  }, {
-    "filename": "memory.13.wasm",
     "commands": [{
       "type": "assert_return",
       "name": "$assert_return_0",
       "file": "memory.wast",
-      "line": 19
+      "line": 20
     }]
   }, {
-    "filename": "memory.14.wasm",
+    "filename": "memory.12.wasm",
     "commands": [{
       "type": "assert_return",
       "name": "$assert_return_0",
       "file": "memory.wast",
-      "line": 21
+      "line": 22
     }]
   }, {
-    "filename": "memory.15.wasm",
+    "filename": "memory.13.wasm",
     "commands": [{
       "type": "assert_return",
       "name": "$assert_return_0",
       "file": "memory.wast",
-      "line": 23
+      "line": 24
     }]
+  }, {
+    "filename": "memory.14.wasm",
+    "commands": []
+  }, {
+    "filename": "memory.15.wasm",
+    "commands": []
   }, {
     "filename": "memory.16.wasm",
     "commands": []
@@ -85,132 +85,126 @@
     "commands": []
   }, {
     "filename": "memory.23.wasm",
-    "commands": []
-  }, {
-    "filename": "memory.24.wasm",
-    "commands": []
-  }, {
-    "filename": "memory.25.wasm",
     "commands": [{
       "type": "assert_return",
       "name": "$assert_return_0",
       "file": "memory.wast",
-      "line": 281
+      "line": 283
     }, {
       "type": "assert_return",
       "name": "$assert_return_1",
       "file": "memory.wast",
-      "line": 282
+      "line": 284
     }, {
       "type": "assert_return",
       "name": "$assert_return_2",
       "file": "memory.wast",
-      "line": 283
+      "line": 285
     }, {
       "type": "assert_return",
       "name": "$assert_return_3",
       "file": "memory.wast",
-      "line": 284
+      "line": 286
     }, {
       "type": "assert_return",
       "name": "$assert_return_4",
       "file": "memory.wast",
-      "line": 286
+      "line": 288
     }, {
       "type": "assert_return",
       "name": "$assert_return_5",
       "file": "memory.wast",
-      "line": 287
+      "line": 289
     }, {
       "type": "assert_return",
       "name": "$assert_return_6",
       "file": "memory.wast",
-      "line": 288
+      "line": 290
     }, {
       "type": "assert_return",
       "name": "$assert_return_7",
       "file": "memory.wast",
-      "line": 289
+      "line": 291
     }, {
       "type": "assert_return",
       "name": "$assert_return_8",
       "file": "memory.wast",
-      "line": 291
+      "line": 293
     }, {
       "type": "assert_return",
       "name": "$assert_return_9",
       "file": "memory.wast",
-      "line": 292
+      "line": 294
     }, {
       "type": "assert_return",
       "name": "$assert_return_10",
       "file": "memory.wast",
-      "line": 293
+      "line": 295
     }, {
       "type": "assert_return",
       "name": "$assert_return_11",
       "file": "memory.wast",
-      "line": 294
+      "line": 296
     }, {
       "type": "assert_return",
       "name": "$assert_return_12",
       "file": "memory.wast",
-      "line": 296
+      "line": 298
     }, {
       "type": "assert_return",
       "name": "$assert_return_13",
       "file": "memory.wast",
-      "line": 297
+      "line": 299
     }, {
       "type": "assert_return",
       "name": "$assert_return_14",
       "file": "memory.wast",
-      "line": 298
+      "line": 300
     }, {
       "type": "assert_return",
       "name": "$assert_return_15",
       "file": "memory.wast",
-      "line": 299
+      "line": 301
     }, {
       "type": "assert_return",
       "name": "$assert_return_16",
       "file": "memory.wast",
-      "line": 300
+      "line": 302
     }, {
       "type": "assert_return",
       "name": "$assert_return_17",
       "file": "memory.wast",
-      "line": 301
+      "line": 303
     }, {
       "type": "assert_return",
       "name": "$assert_return_18",
       "file": "memory.wast",
-      "line": 303
+      "line": 305
     }, {
       "type": "assert_return",
       "name": "$assert_return_19",
       "file": "memory.wast",
-      "line": 304
+      "line": 306
     }, {
       "type": "assert_return",
       "name": "$assert_return_20",
       "file": "memory.wast",
-      "line": 305
+      "line": 307
     }, {
       "type": "assert_return",
       "name": "$assert_return_21",
       "file": "memory.wast",
-      "line": 306
+      "line": 308
     }, {
       "type": "assert_return",
       "name": "$assert_return_22",
       "file": "memory.wast",
-      "line": 307
+      "line": 309
     }, {
       "type": "assert_return",
       "name": "$assert_return_23",
       "file": "memory.wast",
-      "line": 308
+      "line": 310
     }]
   }]
 }