Optional argument to resume_execution allows value stack underflow.

[bddap]

Interperter assumes this invariant:

An interpreted function will not pop too many or too few values off the value stack.

Since pop is in a hot code path, the above invariant allows for optimizations.

If, however, we trusted the user to push a runtime value to the stack, then we popped it, the stack would be one shorter than expected. Near the end of execution, the value stack would underflow.

Enter func::resume_execution

func::resume_execution looks like this:

pub fn resume_execution<'externals, E: Externals + 'externals>(
    &mut self,
    return_val: Option<RuntimeValue>,
    externals: &'externals mut E,
) -> Result<Option<RuntimeValue>, ResumableError> { ... }

That return_val argument gets passed to Interpreter::resume_execution.
When return_val is None, Interpreter::resume_execution ignores it.
When return_val is Some, it gets pushed to the value stack.

• If the user passes Some(val) and the host doesn't pop, the value stack becomes too tall.
• Conversely, passing None when the host expects an argument 1) causes the host to pop some random value, and 2) makes the stack one shorter than expected.

Sugessted fix

Either get rid of the return_val argument, or make it mandatory.

[eira-fransham]

Neither of the suggested fixes work, I'm afraid. It's not an optional argument, really, it just has type Option. None doesn't mean that the user forgot to supply a value or didn't care, it means that the resumption should return void. I mentioned getting rid of it in my review of your PR, but I was wrong - the return_val is the return value of the yield, not the return type of the yield (which is what I thought it was). The fix for this is to have resume_execution check the (runtime) type of return_val against the expected type.