[Old EH] "pop's location is not valid"-error since Emscripten 3.1.73

[kleisauke]

Since Emscripten 3.1.73 (https://chromium.googlesource.com/emscripten-releases/+/b363a83) I encounter errors similar to those described in #6918 when attempting to print the target features section of an already built Wasm binary.

For example:

$ $EMSDK/upstream/bin/wasm-opt --mvp-features --print-features -o /dev/null vips.wasm | sed 's/^--enable-//' | paste -sd' '
...
[wasm-validator error in function 3998] unexpected false: , on 
(try
 (do
  (local.set $0
   (call $3
    (local.get $0)
   )
  )
 )
 (catch $eimport$1
  (drop
   (call $354
    (block (result i32)
     (local.set $2
      (pop i32)
     )
     (global.set $gimport$1
      (local.get $1)
     )
     (local.get $2)
    )
   )
  )
  (call $295)
  (local.set $0
   (i32.const 0)
  )
 )
)
catch's body (eimport$1)'s pop's location is not validFatal: error validating input

However, with Emscripten 3.1.72, these errors do not occur for the same Wasm binary:

$ $EMSDK/upstream/bin/wasm-opt --mvp-features --print-features -o /dev/null vips.wasm | sed 's/^--enable-//' | paste -sd' '
threads mutable-globals nontrapping-float-to-int simd bulk-memory sign-ext exception-handling reference-types multivalue

This indicates a possible(?) regression in this revision range:
69591de...b1c5a00

The Wasm binary in question is relatively large (~5.4 MiB) and is available for download here:
reproducer.zip

Alternatively, you can reproduce this issue using:

$ git clone https://github.com/kleisauke/wasm-vips.git
$ cd wasm-vips/
$ npm run build -- --enable-wasm-eh

[kleisauke]

Bisected to https://chromium.googlesource.com/emscripten-releases/+/95ab551 / 6f0f2e0...b1c5a00.

[kripken]

Confirmed, this binary can no longer be parsed as of #6963, so this is a regression from the new binary parsing that uses IRBuilder. @tlively

[kripken]

Actually, the issue is that you are specifying MVP features, @kleisauke , but the wasm files uses a feature, wasm EH. It looks like the old parser did not error on such input, but the new one does. The error is slightly confusing, though, but an error is expected there.

OTOH, enabling features and then doing --print-features is unhelpful, since it no longer detects the features... so actually I am not sure how --print-features is meant to work. @tlively ?

[kleisauke]

Ah, it seems to work as expected when I add the --enable-exception-handling flag. IIUC, wasm-opt should automatically enable this feature based on the target features specified in the binary.

$ strings vips.wasm | grep exception-handling
exception-handling+

(the binary was linked with the -sBINARYEN_EXTRA_PASSES=--emit-target-features flag for this reason)

I think the reason I used --mvp-features because, without it, the output would include features not present in the original binary. For example, using the --all-features flag lists every available Wasm feature.

[tlively]

OTOH, enabling features and then doing --print-features is unhelpful, since it no longer detects the features... so actually I am not sure how --print-features is meant to work. @tlively ?

The features from the target features section are applied while parsing the module, then --print-features prints them. The problem here is that the parser seemingly fails to fix up a pop correctly.