From 0a9816bff674df984e0f3b71109b2a5d7136e699 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 22 Feb 2023 16:46:45 -0800 Subject: [PATCH] [test] Fix invalid section ID tests Current `assert_malformed` invalid section ID tests are passing, meaning they fail to validate, but not because of invalid section IDs. https://github.com/WebAssembly/spec/blob/721dd6ce2cf53028133bd2ceeea9b0655570c641/test/core/binary.wast#L47-L52 They still fail to validate after replacing those invalid section IDs with valid ones, such as `01` or `02`. For example, Look at the (malformed) module in the first line, which says `0d` (13) is an invalid section ID: ```wast (module binary "\00asm" "\01\00\00\00" "\0d\00") ``` Actually now all major web browsers and Node have implemented EH and expose it by default (= without a flag). So that should be `0e` now, even before we merge the EH proposal. But anyway, this module fails to validate not because `0d` is an invalid ID. If you replace it with `01`, it still fails to validate, because even if it is an empty section, there should be the number `0` saying it has elements, and to emit that number `0`, the length of the payload should be at least 1. But in the example, the length of the payload, which is the next byte after the section ID (`0d`) is `00`, not leaving a space for the number `0`. All known sections, even if they are empty, need at least one byte of payload to say there is 0 elements. (Start section doesn't emit a number of elements, but it needs an index instead, so it needs at least a byte of payload as well.) So the line above should change to ```wast (module binary "\00asm" "\01\00\00\00" "\0e\01\00") ``` `0d` has changed to `0e`, and the next byte is the length of the payload of that section, and the next byte says the section has 0 elements, whatever that element is. This way, now we replace `0e` with a known section ID, such as `01`, the module validates. For these lines: ```wast (module binary "\00asm" "\01\00\00\00" "\80\00\01\00") (module binary "\00asm" "\01\00\00\00" "\81\00\01\00") (module binary "\00asm" "\01\00\00\00" "\ff\00\01\00") ``` I think the purpose of these tests are to have an invalid section with a valid section (`01`). In that case, we also need not only `\00` after the section ID but at least `\01\00` to properly emit an empty type section.) And this updates Node version to v19, because, for a reason I don't understand, Node v18 accepts section ID 0x0e(14)~0x13(19) as valid. I guess they are treated as reserved section number or something. So we cannot test `0x0e` as an invalid section ID with Node v18. Node v19 seems to have fixed it. --- .github/workflows/main.yml | 2 +- test/core/binary.wast | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b64d31bb34..a158de1ee1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,7 +22,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v1 with: - node-version: 18.x + node-version: 19.x - run: cd interpreter && opam exec make JS=node all ref-interpreter-js-library: diff --git a/test/core/binary.wast b/test/core/binary.wast index 1b6b471297..526e0a20e9 100644 --- a/test/core/binary.wast +++ b/test/core/binary.wast @@ -45,11 +45,11 @@ (assert_malformed (module binary "\00asm\00\00\00\01") "unknown binary version") ;; Invalid section id. -(assert_malformed (module binary "\00asm" "\01\00\00\00" "\0d\00") "malformed section id") -(assert_malformed (module binary "\00asm" "\01\00\00\00" "\7f\00") "malformed section id") -(assert_malformed (module binary "\00asm" "\01\00\00\00" "\80\00\01\00") "malformed section id") -(assert_malformed (module binary "\00asm" "\01\00\00\00" "\81\00\01\00") "malformed section id") -(assert_malformed (module binary "\00asm" "\01\00\00\00" "\ff\00\01\00") "malformed section id") +(assert_malformed (module binary "\00asm" "\01\00\00\00" "\0e\01\00") "malformed section id") +(assert_malformed (module binary "\00asm" "\01\00\00\00" "\7f\01\00") "malformed section id") +(assert_malformed (module binary "\00asm" "\01\00\00\00" "\80\01\00\01\01\00") "malformed section id") +(assert_malformed (module binary "\00asm" "\01\00\00\00" "\81\01\00\01\01\00") "malformed section id") +(assert_malformed (module binary "\00asm" "\01\00\00\00" "\ff\01\00\01\01\00") "malformed section id") ;; Unsigned LEB128 can have non-minimal length (module binary