Repository navigation
wasm2c: report wide arithmetic instructions as unsupported instead of aborting - #2909
Conversation
… aborting The wide arithmetic opcodes are read without --enable-wide-arithmetic, but the C writer has no translation for them: i64.add128 and i64.sub128 reached UNIMPLEMENTED, which prints to buffered stdout and calls abort(), and i64.mul_wide_s and i64.mul_wide_u reached WABT_UNREACHABLE in the binary expression switch. wasm2c died with SIGABRT and no message. Handle the quaternary expressions in their own Write, as for the ternary ones, and report all four with WABT_FATAL, which names the instruction and exits with status 1.
| case ExprType::CallRef: | ||
| case ExprType::ReturnCallRef: | ||
| case ExprType::Quaternary: | ||
| UNIMPLEMENTED("..."); |
There was a problem hiding this comment.
Isn't this UNIMPLEMENTED message enough? Why is it enough for things like CallRef but not for Quaternary
There was a problem hiding this comment.
I think the post said that the unimplemented message stays in the stdout buffer since this path calls an abort instead of exit(1)
There was a problem hiding this comment.
In that case we should instead fix UNIMPLEMENTED to print to stderr I think
| expr.opcode.GetName()); | ||
|
|
||
| default: | ||
| WABT_UNREACHABLE; |
There was a problem hiding this comment.
Should we instead give a better error here, so that all unsupported instructions get reported nicely?
| case Opcode::I64MulWideS: | ||
| case Opcode::I64MulWideU: | ||
| WABT_FATAL("error: wasm2c does not support the %s instruction\n", | ||
| expr.opcode.GetName()); |
There was a problem hiding this comment.
I think this is maybe the wrong fix. Shouldn't we have improved the default: error handler instead of just handling these two instructions?
There was a problem hiding this comment.
Hmm... sorry, looks like i had automerge enabled on this. I'll look into cleaning this up better when I get a chance.
The wide arithmetic opcodes are read without
--enable-wide-arithmetic, but the Cwriter has no translation for them, so
wasm2cdied with SIGABRT and no message:i64.add128andi64.sub128(quaternary expressions) reachedUNIMPLEMENTED,which prints to buffered stdout and calls
abort(), so nothing was shown;i64.mul_wide_sandi64.mul_wide_ureachedWABT_UNREACHABLEin the switchover binary expressions.
The module is accepted by
wasm-validateandwasm2wat, so the abort comes onlyfrom the C writer.
This handles the quaternary expressions in their own
Write, as is done for theternary ones, and reports all four instructions with
WABT_FATAL, which names theinstruction and exits with status 1:
It does not add the translation of wide arithmetic; it only makes the failure a
diagnostic.
Testing:
test/wasm2c/wide-arithmetic-unsupported.txtrunswasm2con a module withi64.add128and expects the message and exit status 1. Before the change theprocess aborted with status 134.
i64.sub128,i64.mul_wide_sandi64.mul_wide_ugive the same message (checkedby hand, not in the test).
test/wasm2c/pass on Linux.Fixes #2895