At a quick glance, the following functions are not yet provided by nix, but would be useful to have in nix for use by wasi-common: mkdirat, unlinkat, renameat, linkat, clock_getres, clock_gettime, posix_fallocate, posix_fadvise.
Currently code in wasi-common and lucet-wasi is calling libc interfaces for these functions directly, which requires unsafe code. Having nix provide these will mean the unsafe code is shifted to nix instead. This may not seem like a win on a theoretical level, but the more we can push unsafe code into dedicated utilities that do simple unsafe things with safe, testable, interfaces, the easier it is to audit everything.
I've marked this as "help wanted" -- this is not an urgent project, but it is a nicely scoped, straightforward, and subdividable project, and it's a good chance to see some of the low-level parts of the software stack that wasi-common is built on. And mentoring is available :-).
Additional resources:
nix, though the functions above are more complex and will need testcases :-).Hi .. I was hoping to be able to help on the wasi-common request and this might be a good place to start. I have an initial patch here for unlinkat: https://github.com/nix-rust/nix/compare/master...jlb6740:implement_unlinkat?expand=1. I am new to rust so could use some pointers on testing this. Once this initial one is tested I'll submit a pull request and see about implementing some of the others.
Hi! Thanks for starting this! The patch looks good. For testing unlinkat, take a look at test_fchmodat in test/test_stat.rs. The basic setup it does is essentially what you want here: it creates a temp directory and a file inside that directory, obtains a directory file descriptor, and then does operations using that file descriptor plus a relative path. For your test, the operation should be to do unlinkat to remove the file. For completeness, you can also create a subdirectory inside the temp directory with mkdir and then remove it with unlinkat with RemoveDir.
Also, sorry, I accidentally hit the wrong button while looking at the link above and opened a PR. I've now closed it. We can open a new PR once the testcase is ready.
Hi .. I revisited this patch and I think it is ready. Please have a look when you have time .. I do have a couple of questions more related to how github works. The pull request ID still needs updating but not sure the workflow here. I'll begin work on some others assuming this will be ready soon. https://github.com/jlb6740/nix/commit/890de051f778cd4279769abe5da2bb66bd0494da
This looks good to go! I commented on the commit -- I think you can just submit the PR with "TBD" and fix it up once the PR number is assigned.
Hi! Was interested in helping out with WASI and figured this would be a good place to start. I gave mkdirat a shot. Let me know if you have any suggestions!
Hey, that's awesome, thanks! It'd be great if we could get all missing host function calls added to nix and especially those that we rely on internally in wasi-common. Would you be willing to scan the repo, find any calls that rely directly on libc, and try adding them to nix? For example, we're still missing posix_fadvise.
Feel free to ask any questions, and I'll be happy to help out as much as I can, and @sunfishcode can mentor and straighten me out in case I'm saying something stupid ;-)
Definitely up to help out however I can...I remember confirming that the list @sunfishcode had at the top of this issue covered everything but I will double-check that tomorrow. Regardless, I'll try to pick off things bit by bit.
Continuing to chip away at this, also did a bit of grepping here for direct references to libc functions. I ended up with:
clock_getres (https://github.com/nix-rust/nix/pull/1100/)clock_gettime (https://github.com/nix-rust/nix/pull/1100/)fdopendir (@jlb6740 working on this)futimens (this is actually done already)linkat( @jlb6740 working on this )memcpy (think this is more refactoring on our end, see dec_slice_of vs enc_slice_of--other call should disappear if we use nix for directory reading) mkdirat (https://github.com/nix-rust/nix/pull/1084)posix_fadvise, (https://github.com/nix-rust/nix/pull/1089)posix_fallocate ( nix-rust/nix#1105 )readdir_r ( assessing using nix::dir for this )renameat (worked on indepently of this issue, https://github.com/nix-rust/nix/pull/1097)sched_yield (https://github.com/nix-rust/nix/pull/1050, https://github.com/nix-rust/nix/pull/1090)seekdir ( assessing using nix::dir for this )symlinkat ( @jlb6740 working on this )unlinkat, (https://github.com/nix-rust/nix/pull/1058)telldir ( assessing using nix::dir for this )utimensat (this is actually done already)I need to look more into whether or not nix::dir as it is can replace the directory reading/iterating calls listed here. I think that's the case.
Any input @sunfishcode @kubkon ? I'll keep track of things here to the best of knowledge.
memcpy should be able to be replaced with std::ptr::copy_nonoverlapping
@kevinwern That list looks good! As a minor update, sched_yield is now in upstream nix here so we just need to update wasi-common to use it.
@kevinwern That list looks good! As a minor update,
sched_yieldis now in upstream nix here so we just need to update wasi-common to use it.
That's a good catch. One caveat here though. It is currently quite unfortunate that sched_yield landed in sched module in nix crate which is compiled only for linux-like hosts. This precludes its use on macos for example. I have now submitted a PR to remedy this nix-rust/nix#1090, and I guess when it lands, we can safely move away from using libc::sched_yield directly.
It might also be useful to explore CraneStation/wasmtime#519 a little bit more, i.e., see whether a fix is needed on our side or nixs.
I think unlinkat is close to merging. I will give linkat and symlinkat a go if no one else has started these.
@jlb6740 noted that on the list. Go ahead!
Looks like symlinkat was done (correct me if there was more here?) unlinkat is in review, though likely needs another update or two. Will start on fdopendir and probably fdreaddir if not taken.
Not sure how we are going to use posix_fallocate, but here is the work related to it: nix-rust/nix#1105
Sorry was away for a bit...updated the list to reflect updates from @jlb6740 and @dingxiangfei2009 .
@dingxiangfei2009 posix_fallocate is desirable for implementing WASI's fd_allocate, because it changes the file size with a single system call, rather than needing to do an fstat to check the size followed by an ftruncate (which is what Rust's File's set_len does).
I was trying to build wasmtime on darwin and I am hitting linker errors related to _utimensat: https://github.com/NixOS/nixpkgs/pull/71316. Is that related to this issue here?
Hey @nomeata, thanks for your comment! Hmm, that's interesting. I assume that by darwin you don't necessarily mean MacOS, right? Anyhow, it might as well be related to the issue you've linked to. Could you submit full logs here so that I could have a closer look please?
(moved to https://github.com/CraneStation/wasi-common/issues/132)
Actually @nomeata, would you mind submitting a new issue for the problem you've experienced? I have an idea for a fix, and would love to have it properly tracked. Also, would you mind working through it with me?
Given that we've effectively decided to abandon using nix in wasi-common, and use our own in-house thin, low-level wrapper crate for used Unix syscalls (aka yanix, see #649), I'm closing this issue.
Most helpful comment
I think unlinkat is close to merging. I will give linkat and symlinkat a go if no one else has started these.