Skip to content

Handle file descriptors as usize#66

Open
dierbei wants to merge 1 commit intomewz-project:mainfrom
dierbei:update-filedescriptor-type
Open

Handle file descriptors as usize#66
dierbei wants to merge 1 commit intomewz-project:mainfrom
dierbei:update-filedescriptor-type

Conversation

@dierbei
Copy link
Contributor

@dierbei dierbei commented Apr 30, 2024

#3
There are a lot of modifications, so please help me out.

Signed-off-by: dierbei <1628652790@qq.com>
Copy link

@gkgoat1 gkgoat1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a refactor is needed in this area, as the current implementation is incompatible with the WASIp1 ABI. See my comments for specific issues.


// return opened fd
const return_ptr = @as(*i32, @ptrFromInt(@as(usize, @intCast(opened_fd_addr)) + linear_memory_offset));
const return_ptr = @as(*usize, @ptrFromInt(@as(usize, @intCast(opened_fd_addr)) + linear_memory_offset));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WASIp1 ABI requires that fds be u32 in WASM; this may cause WASM-side memory corruption

const port = 1234;
var ip_iovec_ptr = @as(*IoVec, @ptrFromInt(12 + linear_memory_offset));
ip_iovec_ptr.buf = 8;
var ip_iovec_ptr = @as(*IoVec, @ptrFromInt(20 + linear_memory_offset));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WASIp1 ABI requires that fds be u32 in WASM; this is ABI-incompatible

// fd_write
var buf_iovec = @as([*]IoVec, @ptrFromInt(12 + linear_memory_offset))[0..2];
var buf = @as([*]u8, @ptrFromInt(28 + linear_memory_offset));
var buf_iovec = @as([*]IoVec, @ptrFromInt(324 + linear_memory_offset))[0..2];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blatant ABI violation

}

pub export fn fd_write(fd: i32, buf_iovec_addr: i32, vec_len: i32, size_addr: i32) callconv(.C) WasiError {
pub export fn fd_write(fd: usize, buf_iovec_addr: i32, vec_len: i32, size_addr: i32) callconv(.C) WasiError {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WASIp1 ABI requires that fds be u32 in WASM; the high bits of fd may be undefined depending on Wasker's behavior

@ainozaki
Copy link
Contributor

ainozaki commented Jul 13, 2025

Hi @gkgoat1
Thank you for your comment.

The WASIp1 ABI requires that fds be u32 in WASM

Is there any documentation that states that the type of file descriptors is u32?

In wasi-libc, we see fd is defined as int32 like typedef int __wasi_fd_t.
Also, in wit, it's defined as typename $fd (handle), and handle is stated of type i32.

@gkgoat1
Copy link

gkgoat1 commented Jul 13, 2025

Hi @gkgoat1
Thank you for your comment.

The WASIp1 ABI requires that fds be u32 in WASM

Is there any documentation that states that the type of file descriptors is u32?

Yes; the two code snippets below (that you mentioned) are more than enough

In wasi-libc, we see fd is defined as int32 like typedef int __wasi_fd_t.
Also, in wit, it's defined as typename $fd (handle), and handle is stated of type i32.

@ainozaki
Copy link
Contributor

Mewz's code and the two examples I provided define fd as an i32. However, didn't you say in your comment that fd should be a u32?

@gkgoat1
Copy link

gkgoat1 commented Jul 14, 2025

Mewz's code and the two examples I provided define fd as an i32. However, didn't you say in your comment that fd should be a u32?

In WASM, u32 represents both signed and unsigned 32-bit integers. Therefore the examples would have a WASM type of u32 for fds. In contrast, 64-bit integers get the u64 type, which is not ABI-compatible

@saza-ku
Copy link
Contributor

saza-ku commented Jul 18, 2025

I understand usize is not acceptable because it's 64 bits. But i32 is signed integer, isn't it?

@gkgoat1
Copy link

gkgoat1 commented Jul 18, 2025

I understand usize is not acceptable because it's 64 bits. But i32 is signed integer, isn't it?

The WASM spec uses the same type, i32, for signed and unsigned integers

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

Successfully merging this pull request may close these issues.

4 participants