dtolnay / watt

Runtime for executing procedural macros as WebAssembly
Apache License 2.0
1.29k stars 28 forks source link

Eliminate unsafe memory pointer in sym.rs #13

Closed dtolnay closed 5 years ago

dtolnay commented 5 years ago

As of #11 the WASM memory is passed in to host functions as *mut [u8]. Is there any way to pass &mut [u8] to be safer? Does it have to be &mut [MaybeUninit<u8>]?

mystor commented 5 years ago

I would imagine that the runtime is probably initializing all of the memory it is giving to wasm, so I don't see a particular reason why the host shouldn't be able to see &mut [u8]. Giving guest code access to uninitialized memory would surprise me, as it could leak information, and would be much more unsafe than I expected.

alexcrichton commented 5 years ago

A weird reason I used *mut [u8] instead of &mut [u8] was that it made the WasmArg trait implementations easier, but other than that I think &mut [u8] should be fine since the JIT support is where this comes from and it's already basically unsafely asserting that all the pointers and such are valid anyway.

dtolnay commented 5 years ago

To avoid needing an unsafe WasmArg impl for memory, how about being explicit about what the signatures in import.rs are? Something like:

pub fn string_new(memory: *mut [u8], ptr: u32, len: u32) -> u32;
pub fn string_len(string: u32) -> u32;
-     "string_new" => sym::string_new.into_host_func(store),
+     "string_new" => MemFunc2(sym::string_new, store),

-     "string_len" => sym::string_len.into_host_func(store),
+     "string_len" => Func1(sym::string_len, store),

where e.g. MemFunc2 has an impl that works for F: FnMut(&mut [u8], A, B) -> C.

alexcrichton commented 5 years ago

Sounds like a good way forward to me!