ref-fvm
ref-fvm copied to clipboard
filecoin-project
EVM Runtime: Runtime stack will panic from malformed asm
EVM stack will panic with negative overflow if bytecode doesnt have all the values it expects to pop off. This will happen with malformed bytecode but we shouldn't panic from it. this can be done easily enough in asm with the return opcode by itself:
let evm_bytecode = "
{constructor bytecode omitted}
return # runtime will panic
";
In this case, we should probably check stack heights from the EVM dispatch code (we record the stack height requirements). That should make error handling simpler.
This is likely to be prohibitively expensive.
Ideally we'd have guard pages around the stack and a proper segfault handler and.... oh wait, we are writing in this in rust. We should let the thing panic if it can.
We should let the thing panic if it can.
This makes DELEGATECALL a bit tricker.
- We can handle a panic (and, e.g., exit with the correct code).
- We cannot recover from a panic (in rust).
This is likely to be prohibitively expensive.
if we do it right, it'll actually be faster. At the moment, every stack pop checks to make sure we have enough elements on the stack. If we check up-front (and convince the compiler to inline the instructions), we can elide all the individual checks.
wait, why does it panic if it checks already?
Or is that check causing the panic?
In that case yes, we are already paying the prohibitive cost and doing the full stack check upfront will be faster if we can elide the subsequent checks.
Or is that check causing the panic?
This. Rust is a memory-safe language.
something or other people call it a fast language? :)
Joking aside, there has to be a way to do a single check upfront somehow.
Yes. If you do a check up-front, rust will elide the other checks.
However, if you do this in two separate functions and rust doesn't inline the called function, it can't elide the checks.
One workaround is to pass in a fixed-sized "stack". Given that EVM operations specify how many elements they read/push, we can define Stack<N> object that's internally backed by a &[U256; N] array. If we pass that, the rust compiler should be able to get rid of the checks.
hasnt been merged yet and i want to do the refactoring we discussed first, but yes, it will be fixed.