Fix sdk assertion macros

[tchataigner]

While working on the High level Rust SDK I found out that low level macro were not working as expected. I started to fix their behavior but also found a problem in how things were handled.

At the time when we developed the macros, it seemed relevant to @raulk and myself to use catch_unwind to prevent a panic on an assert. But it seems that catch_unwind doesn't work in in WASM as it's behavior is panic=abort (https://github.com/rust-lang/rust/issues/58874) which will throw an error before we have time to call the abort syscall.

I do not really see an easy way of solving this, opening a PR to share it with the team and (maybe) get some start of a solution.

[codecov-commenter]

Codecov Report

Merging #660 (402af40) into master (e9657a7) will decrease coverage by 0.01%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #660      +/-   ##
==========================================
- Coverage   51.06%   51.05%   -0.02%     
==========================================
  Files         120      120              
  Lines        9694     9694              
==========================================
- Hits         4950     4949       -1     
- Misses       4744     4745       +1     

Impacted Files	Coverage Δ
ipld/amt/src/node.rs	86.56% <0.00%> (-0.26%)	:arrow_down:

[Stebalien]

We solved this in the builtin actors with https://github.com/filecoin-project/builtin-actors/blob/344ea72ed5b297bfb1291ee6fe8383cb6b587ea4/runtime/src/runtime/fvm.rs#L552-L554.

[Stebalien]

Basically:

1. Set panic=unwind (on build).
2. Add a panic hook that aborts.

[Stebalien]

Is there any reason we need to assert than catch it? Can't we just re-implement assert and abort directly? The implementation should be pretty simple.

Alternatively, just drop the wrapping entirely. I.e.:

1. Let the user call the builtin assert.
2. Register a panic hook in invoke to turn all panics into aborts.

[tchataigner]

Thanks for the tips @Stebalien ! Used them to fix it 👍

[raulk]

Conflicts developed :-(

[Stebalien]

I think there's some confusion here. We shouldn't be setting a panic hook than immediately calling assert.

• We should, ideally, not be overriding assert at all, instead setting a panic hook on invoke.
• Or... we could override the assert macro to immediately call vm::abort.

But the former would be better.

However, setting a hook a hook, then immediately asserting, is a very round-about way of going about this. Does that make sense?

[Kubuxu]

Basically: Set panic=unwind (on build). Add a panic hook that aborts.

Does that mean we carry unwind information within actor bundles?

[Stebalien]

Does that mean we carry unwind information within actor bundles?

Not the deployed ones, no. However, asserts will still give you nice errors even without actual backtraces.

[Stebalien]

See #896.