binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Implement `elem.drop`

Open osa1 opened this issue 5 months ago • 2 comments

Fixes #7209.

osa1 avatar Jun 16 '25 13:06 osa1

@tlively I think this is ready, but it changes ids of some existing instructions (see CI failure). Is this a problem? I can't find where/how the ids are defined. Could you point me to the right code if we need to revert the id changes?

osa1 avatar Jun 16 '25 14:06 osa1

The IDs are just the Expression::ID values from wasm.h, so it is expected that they have changed. You can update the binaryen.js tests by running emcmake cmake -G Ninja . && ninja binaryen_js && ./scripts/auto_update_tests.py binaryenjs.

tlively avatar Jun 16 '25 14:06 tlively

The fuzzer errors on this:

(module          
 (type $0 (func))
 (rec               
  (type $1 (sub (struct (field i64) (field i8) (field f32) (field (mut (ref $6))))))
  (type $2 (array (mut i64)))
  (type $3 (sub (array (ref $10))))
  (type $4 (array (mut (ref null $2))))
  (type $5 (sub (array (mut v128))))
  (type $6 (struct (field v128) (field i16)))
  (type $7 (sub (func (param f32 i64))))
  (type $8 (func (param (ref null $3))))
  (type $9 (sub (func (result (ref null $6))))) 
  (type $10 (sub (struct (field (mut i64)) (field (mut (ref struct))))))
 )              
 (type $11 (func (param i32)))
 (type $12 (func (param f32)))
 (rec                                                                                          
  (type $13 (sub (array (mut v128))))
  (type $14 (sub (array (mut anyref))))
  (type $15 (sub $9 (func (result (ref $6)))))
  (type $16 (sub (struct)))
  (type $17 (sub $14 (array (mut anyref))))
  (type $18 (sub final $9 (func (result (ref $6)))))
  (type $19 (sub $7 (func (param f32 i64))))
  (type $20 (sub (func (param (ref $17)) (result nullexternref))))
  (type $21 (sub final $9 (func (result (ref $6)))))
  (type $22 (sub $14 (array (mut anyref))))
 )
 (type $23 (func (param f32) (result (ref $21))))
 (import "fuzzing-support" "log-f32" (func $fimport$0 (type $12) (param f32)))
 (import "fuzzing-support" "wasmtag" (tag $eimport$0 (type $11) (param i32)))
 (table $0 68 funcref)
 (elem $0 (i32.const 0) $0)
 (export "drop_active_invoker" (func $2))
 (export "func_47" (func $1))
 (func $0 (type $0)
 )
 (func $1 (type $23) (param $0 f32) (result (ref $21))
  (try
   (do
    (call_indirect $0 (type $0)
     (i32.const 0)
    )
    (call $fimport$0
     (f32.const 0)
    )
    (unreachable)
   )
   (catch $eimport$0
    (drop
     (pop i32)
    )
   )
   (catch_all
   )
  )
  (unreachable)
 )
 (func $2 (type $0)
  (elem.drop $0)
 )
)
$ bin/wasm-opt a.wat -all --fuzz-exec
[fuzz-exec] calling drop_active_invoker
[fuzz-exec] calling func_47
[LoggingExternalInterface logging 0]
[trap unreachable]
warning: no passes specified, not doing any work
[fuzz-exec] calling drop_active_invoker
[fuzz-exec] calling func_47
[trap uninitialized table element]
[fuzz-exec] comparing drop_active_invoker
[fuzz-exec] comparing func_47
logging counts not identical!
[fuzz-exec] optimization passes changed results

--fuzz-exec without any optimizations means that we are just executing the wasm twice, and it runs differently the second time. Is the implementation of elem.drop setting some global state somehow that affects the instance later?

kripken avatar Jun 18 '25 17:06 kripken

(btw @osa1 you can run the fuzzer locally, see docs at the top here: https://github.com/WebAssembly/binaryen/blob/main/scripts/fuzz_opt.py )

kripken avatar Jun 18 '25 17:06 kripken

Thanks for the ping. I'll have a look tomorrow.

(Sorry I forgot to add this to the unfuzzable tests as @tlively suggested, but I'll see if it's easy to implement. If not I'll add it to the list.)

osa1 avatar Jun 18 '25 17:06 osa1