bytecodealliance / wasi-rs

Experimental WASI API bindings for Rust
Apache License 2.0
271 stars 48 forks source link

Is it safe to pass negative lengths to functions? #67

Closed newpavlov closed 2 years ago

newpavlov commented 2 years ago

random_get has the following signature:

pub fn random_get(arg0: i32, arg1: i32) -> i32;

Meanwhile in the witx files it's defined as:

(typename $size u32)

  (@interface func (export "random_get")
    ;;; The buffer to fill with random data.
    (param $buf (@witx pointer u8))
    (param $buf_len $size)
    (result $error (expected (error $errno)))
  )

Assuming buffer length is casted from usize to i32, would it work correctly? Note that according to this comment:

none of our 3 WASM environments (Emscripten, wasi-libc, wasm32-unknown-unknown) enforce isize::MAX right now

Is there a reason why all arguments and return values are defined use i32 instead of more descriptive types used in the witx files?

sunfishcode commented 2 years ago

No, the use of i32 there isn't essential; it's more of just a consequence of how the current witx IDL works. Witx tooling sometimes assumes that APIs are relatively like "system calls", being low-level APIs that most users call through something like libc rather than calling directly. Information like u32 vs. signed i32 isn't significant at this level because at the wasm level, they're all just signedness-independent i32 values that just need to just get passed through.

This is one of many things we're doing differently in wit, the successor to witx. In wit, there is a much greater focus on generating bindings that do expect to be directly called by users, so it preserves information like signed vs. unsigned.

sunfishcode commented 2 years ago

In particular, it's fine to cast arbitrary usize to i32 for the purpose of calling "random_get". The API here is just using the i32 as a way to pass 32 bits of information, and not as a way to interpret it as a signed quantity.

newpavlov commented 2 years ago

I think it could be worth to use the more descriptive types, but the main question was answered, so I will close this issue.