From 7fd5f7fdd9982e5583dd695551c315b058021d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Thu, 26 Sep 2024 17:01:54 +0200 Subject: [PATCH 1/3] Binary parser: lift the limit on the number of locals This raises the number of locals accepted by the binary parser to the absolute limit in the spec. A warning is now printed when writing a binary file if the Web limit of 50,000 locals is exceeded. This fixes #6968. --- src/wasm/wasm-binary.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index cb9ea3731ff..cd2d584ac71 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -472,6 +472,10 @@ void WasmBinaryWriter::writeFunctions() { std::cerr << "Some VMs may not accept this binary because it has a large " << "number of parameters in function " << func->name << ".\n"; } + if (func->getNumLocals() > WebLimitations::MaxFunctionLocals) { + std::cerr << "Some VMs may not accept this binary because it has a large " + << "number of locals in function " << func->name << ".\n"; + } }); finishSection(sectionStart); } @@ -2722,16 +2726,18 @@ void WasmBinaryReader::readFunctions() { void WasmBinaryReader::readVars() { uint32_t totalVars = 0; size_t numLocalTypes = getU32LEB(); + std::vector> decodedVars; + decodedVars.reserve(numLocalTypes); for (size_t t = 0; t < numLocalTypes; t++) { auto num = getU32LEB(); - // The core spec allows up to 2^32 locals, but to avoid allocation failures, - // we additionally impose a much smaller limit, matching the JS embedding. - if (std::ckd_add(&totalVars, totalVars, num) || - totalVars > WebLimitations::MaxFunctionLocals) { - throwError("too many locals"); + if (std::ckd_add(&totalVars, totalVars, num)) { + throwError("unaddressable number of locals"); } auto type = getConcreteType(); - + decodedVars.emplace_back(num, type); + } + currFunction->vars.reserve(totalVars); + for (auto [num, type] : decodedVars) { while (num > 0) { currFunction->vars.push_back(type); num--; From 7718da744681833af6a455dc0576feb5f9e6d056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Fri, 27 Sep 2024 10:40:27 +0200 Subject: [PATCH 2/3] Use small vector --- src/wasm/wasm-binary.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index cd2d584ac71..71cfd3a4e8d 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2726,7 +2726,9 @@ void WasmBinaryReader::readFunctions() { void WasmBinaryReader::readVars() { uint32_t totalVars = 0; size_t numLocalTypes = getU32LEB(); - std::vector> decodedVars; + // Use a SmallVector as in the common (MVP) case there are only 4 possible + // types. + SmallVector, 4> decodedVars; decodedVars.reserve(numLocalTypes); for (size_t t = 0; t < numLocalTypes; t++) { auto num = getU32LEB(); From 7ddff909b71da5a6bb0bf27aada9952c1d758187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Fri, 27 Sep 2024 10:40:44 +0200 Subject: [PATCH 3/3] Unit test for the warning --- test/unit/test_web_limitations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/test_web_limitations.py b/test/unit/test_web_limitations.py index 6359390f9a0..921e02e95bd 100644 --- a/test/unit/test_web_limitations.py +++ b/test/unit/test_web_limitations.py @@ -20,3 +20,19 @@ def test_many_params(self): input=module, capture_output=True) self.assertIn('Some VMs may not accept this binary because it has a large number of parameters in function foo.', p.stderr) + + def test_many_locals(self): + """Test that we warn on large numbers of locals, which Web VMs + disallow.""" + + params = '(local i32) ' * 50_001 + module = ''' + (module + (func $foo %s + ) + ) + ''' % params + p = shared.run_process(shared.WASM_OPT + ['-o', os.devnull], + input=module, capture_output=True) + self.assertIn('Some VMs may not accept this binary because it has a large number of locals in function foo.', + p.stderr)