Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing host functions to return an error instead of panicing #258

Closed
codefromthecrypt opened this issue Feb 18, 2022 · 11 comments
Closed

Comments

@codefromthecrypt
Copy link
Contributor

Right now, host functions that reach an abnormal end, or have to exit are required to panic out.

Ex. intentional return

	panic(wasi.ExitCode(exitCode))

Ex. abend (ex on I/O)

		n, err := writer.Write(b)
		if err != nil {
			panic(err)
		}

This is an alternative to allowing signatures that return an error, then propagating out anyway. The reasons this isn't done yet are:

  • Slight confusion about error because Wasm doesn't define them. We'd have to document the behaviour.
  • Panic is easier internally as it is already used for global exits (exit from any recursive or reentrant calls)
  • Less internal code (possibly more efficient to panic vs otherwise)

I believe this is confusing from a developer point-of-view, and can result in unnecessary error wrapping. An alternative may be to retain using panics internally for global exit, but use a different call site for functions with an error signature. These functions could internally panic on error to retain the same behavior.

@mathetake mathetake changed the title Alowing host functions to return an error instead of panicing Allowing host functions to return an error instead of panicing Mar 2, 2022
@codefromthecrypt
Copy link
Contributor Author

This is important for porting some existing gasm code, which return a second value which is an error. We need a clean path to migrate folks and saying "panic" looks like a step backwards.

@codefromthecrypt
Copy link
Contributor Author

actually I looked closer at the code and it was returning an errno (uint32) cast as an error. Let's start with #433 and also documenting what panics are used for now.

We have to analyze https://github.com/WebAssembly/exception-handling before proceeding with any typed errors I think

@clarkmcc
Copy link

clarkmcc commented Jun 1, 2022

What is the current approach for error handling within the host function? Right now, I am running a Rust-based wasm module and if my Go host function panics, it's not something I can catch on the Rust side. What is the defacto method for communicating host function errors to the wasm module calling the host function?

@codefromthecrypt
Copy link
Contributor Author

hi @clarkmcc WebAssembly doesn't have a way in 1.0 or 2.0 to catch exceptions, though it may be the case in the future.

So, the way errors work are somewhat old school meanwhile. The first return val can be an errno for a recoverable function. This is how WASI works, for example. Please reply if you need more guidance!

@clarkmcc
Copy link

clarkmcc commented Jun 1, 2022

@codefromthecrypt Okay, can you clarify for me what I should do about my existing return value? Right now, I'm returning a "fat pointer" I guess which is a u64 that contains both the pointer and the length of the bytes.

extern "C" {
    pub fn get_value(ptr: u32, len: u32) -> u64;
}

The host function get_value allocates some amount of memory that is unknown at runtime to the guest, puts a value into it, and returns the pointer and length to it. I would need some way for the host to convey to the guest the pointer, length, and errno. Any ideas?

@codefromthecrypt
Copy link
Contributor Author

I think what you'll want to look at are..

hope these breadcrumbs help!

@codefromthecrypt
Copy link
Contributor Author

I don't think rust supports multiple values, so your signature might look like this. Note you probably don't need to declare a i64 for the errorno a smaller i32 is likely fine

pub type Size = usize;
pub type Errno = u16;

#[link(wasm_import_module = "your_module")]
extern "C" {
    fn get_value(ptr: *mut u8, len: Size) -> Errno;
}

ported from wasi
https://github.com/bolucat/Firefox/blob/f9a6a21c8748b6d79a238c0285d9d24aad991767/third_party/rust/wasi/src/lib_generated.rs

@clarkmcc
Copy link

clarkmcc commented Jun 2, 2022

@codefromthecrypt let me clarify, in the get_value function, the guest will call the host and pass in a pointer and a length to the input parameters, and the host function needs to pass a pointer and a length back to the guest which will contain the result. I'm able to do the first half, I'm not sure how to pass a pointer and length back to the guest if my return value is reserved for an error.

@codefromthecrypt
Copy link
Contributor Author

@clarkmcc gotcha. here are two ideas:

  • If you are using a null-terminated string (Cstring) you can pack results together (hi bits are the message offset, low bits are the errno). then the caller needs to scan for the 0 similar to our go api Memory.IndexByte
    • the main advantage of this is message+errno are a single result and so they don't affect the params
  • If you are using offset/len you need to add a parameter to your function of where to write offset/len.
    • ex. ... (param $result.message_offset_len i32) and then write 8 little endian bytes to that location, which the caller reads on error.
    • main advantage of this is the result is a plain errno

does this help?

@clarkmcc
Copy link

clarkmcc commented Jun 2, 2022

@codefromthecrypt In the long term, I'd like to switch to flatbuffers instead of serialized JSON strings which is what I'm doing now so that I can get by bearings first. Given that, I'm guessing the null-terminated string approach is not an option.

Can you clarify what your second option would look like, I'm not sure that I follow completely. Last night I took a stab at this type of process

  • Host function signature looks like func(ptr uint32, len uint32, resultPtr uint32, resultLenPtr uint32) Errno
  • Guest calls host function and sets the value at resultPtr and resultLenPtr to 0, then passes a pointer to their memory locations to the host function.
  • Host function does it's work, allocates sufficient memory for the result, and then copies the data.
  • Host function then sets the value at the resultPtr address to the memory offset of the previously allocated result and sets the value at the resultLenPtr to the length of the result.
  • Guest function reads the updated values at resultPtr and resultLenPtr.

It almost worked but there was some slight data corruption and then out-of-bounds errors when the strings were really long so I'm sure I did something wrong. Then I realized this seems like quite a bit of work for what I think is a pretty common use case (arbitrary input bytes and arbitrary output bytes). Am I overthinking this?

Edit: Solved it within a few minutes this morning, it's amazing how that works. The problem was memory corruption by the wee allocator. Addresses read from Rust looked good, the same address read from Go through the guest's shared memory was corrupted between bytes 0-8. No idea what the problem actually is but switching to the default allocator seemed to solve the problem. I still would like to know if there's a better way to do this.

@codefromthecrypt
Copy link
Contributor Author

@clarkmcc I think we still owe (many things including) an error handling example, as using only numbers and memory is indeed a bit lacking. The code you accumulate to pass back an error message is an example of that. Also, the allocator in rust overwrites the first bytes on dealloc iirc. I ran into this in the allocation example, which is different than tinygo which lazy collects. the lack of a coherent GC approach is basically another issue when using "grey box wasm" eg functions you collaborate with vs what's easier (main functions that only use wasm and wasi) There are some things I could suggest to move forward with next, now that you've managed something working 🥂

  • check out the work @inkeliz has karmem wrt choice of moving to flat buffers etc. there's a lot of work even in choices like this in pre-component WebAssembly.
  • Open a small repo so I can watch it and help as I can. We've not decided a chat channel yet (gophers, discord etc) and long running issues on a main repo aren't the best way. A more focused repo could allow topical chats without adding too much weight to this issue, even if it is relevant background.

Either way, good luck and hope to see you around. The bruised knuckles are sadly a part of working on things right now, but we do want to help where we can. Thinking through what you asked above has helped.

@mathetake mathetake closed this as not planned Won't fix, can't repro, duplicate, stale Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants