feat: add required syscalls for remove_dir_all

[phraakture]

summary

implemented three syscalls- openat(), unlinkat() and fdopendir(), that the standard library's remove_dir_all uses. kept the fd type similar to windows's DirHandle. The only difference is unix one additionally overrides path(), which the Windows doesn't need as they are unix specific.

related issue #4982

Note: I haven't included any test yet. Any guidance on how these syscalls should be tested, or references to similar existing tests, would be really appreciated.

[rustbot]

Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two.
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

[RalfJung]

Thanks for giving this a try!

This week we Rust devs are all meeting at a conference so a backlog of PRs will pile up that'll take a bit to get through -- please be patient. However I can already tell you that the PR is missing tests. :) Specifically

• tests that invoke the std APIs
• and tests that directly invoke the libc functions

If you look around the test suite you can find examples for both, e.g. tests/pass/shims/fs.rs and tests/pass-dep/libc/libc-fs.rs. You can just add the new tests to those two files.

[RalfJung]

Looking at the implementation, it seems you are trying to implement these APIs based on top of paths represented as strings. I don't think we should do that. Lucky enough the standard library is gaining support for directory handles so the hope is that we can use those. :)

It's bad enough that we have this hack on Windows, but you can't actually do much with a DirHandle there so it's not too big of a problem.

@rustbot author

View changes since this review

[RalfJung]

Given that these APIs are meant specifically to avoid the risk of race conditions, I don't think we should implement them by just storing a path. Rather, a DirHandle should hold a fs::Dir so that we can be actually sure that this refers to a stable directory.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[RalfJung]

So my suggestion would to instead add a new FD type that wraps fs::Dir, and restrict this PR to only those operations that can be implemented on top of fs::Dir. Making this PR smaller (fewer new shims at once) is anyway a good idea.

That means it's not yet enough to remove_dir_all. That's fine. If you want to make progress towards that, you can help by implementing more operations on fs::Dir. :) (Note that there's work in progress for that, see rust-lang/rust#120426.)

[RalfJung]

Also, a note on terminology, since it also came up in another PR of course: the PR title talks about "sycalls" but that's not quite correct. What we are implementing here aren't syscalls, they are libc functions. These often but not always correspond to syscalls. If you say "syscall" in the context of Miri, then I will assume you are referring to supporting more cases in libc::syscall.

[rustbot]

☔ The latest upstream changes (possibly #5082) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

@phraakture do you still plan to get back to this PR? It's fine if not, we should just know about it so someone else can then pick up the work. :)
As mentioned above, it would be better to start smaller, with just a single libc shim. Also the implementation of the shim in Miri should use std's directory handles.

[phraakture]

@phraakture do you still plan to get back to this PR? It's fine if not, we should just know about it so someone else can then pick up the work. :) As mentioned above, it would be better to start smaller, with just a single libc shim. Also the implementation of the shim in Miri should use std's directory handles.

Sorry for the delay. I do still plan to work on this PR.
If it turns out I can't continue for some reason, I'll let you know so someone else can pick it up.

[RalfJung]

Okay, thanks for the update!

Note that DirHandle now exists in Miri. I would suggest the first PR to be very minimal and only add two things:

• "open" with O_DIRECTORY creates a DirHandle
• "openat" on a DirHandle can open a file

Ignore remove_dir_all for now and focus just on getting the basics of directory handles working. (Further directory handle functionality is anyway blocked on rust-lang/rust#150679.)