shims: improve Linux statx by bourumir-wyngs · Pull Request #4913 · rust-lang/miri

bourumir-wyngs · 2026-03-21T17:35:55Z

shims/unix: improve Linux statx

- Track more fields in `FileMetadata`.
- Populate these new fields in the `statx` shim and update the returned mask.
- Implement `linux_decode_dev` helper to split `dev_t` using glibc / musl layout (initially, now removed)
- Add an integration test in `tests/pass-dep/libc/libc-fs.rs` and remove
  the FIXME in `foreign_items.rs`.

rustbot · 2026-03-21T17:36:01Z

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

Sorry for the slow review!

I left some comments. Let's narrow the scope of the PR a bit and avoid those weird device number hacks. The rest looks like with a bit of cleanup it should work nicely. :)

@rustbot author

View changes since this review

RalfJung · 2026-04-26T11:43:55Z

+            unix => {
+                mask |= this.eval_libc_u32("STATX_INO")
+                    | this.eval_libc_u32("STATX_NLINK")
+                    | this.eval_libc_u32("STATX_UID")


It seems you are making a salient assumption here that every FileMetadata constructed on a unix host has all these fields. First of all, this took me some time to figure out since you didn't didn't document this assumption anywhere -- please ensure to leave more comments in the code to explain your reasoning.

Secondly, I don't think this is the right approach. Instead it'd be better if the FileMetadata type knew whether all these fields are present. That avoids non-local assumptions that could be easily broken in the future. In fact there's a concurrent PR that's about to land (#4812) that would have caused the assumption to be broken.

Working on this. Thanks for hearing from you!

RalfJung · 2026-04-26T11:44:32Z

+    fn linux_decode_dev(&self, dev: u64) -> (u32, u32) {
+        // Linux glibc and musl encode major and minor in a non-contiguous layout.
+        // In hex nybbles, the layout is: MMMM Mmmm mmmM MMmm
+
+        let major = ((dev & 0x0000_0000_000f_ff00) >> 8) | ((dev & 0xffff_f000_0000_0000) >> 32);
+        let minor = (dev & 0x0000_0000_0000_00ff) | ((dev & 0x0000_0fff_fff0_0000) >> 12);
+
+        (u32::try_from(major).unwrap(), u32::try_from(minor).unwrap())
+    }
+
+    #[cfg(not(target_os = "linux"))]
+    fn linux_decode_dev(&self, _dev: u64) -> (u32, u32) {
+        (0, 0)
+    }


Wow, what a mess...

Unless there's a concrete need, I'd prefer to skip handling device numbers in this PR and focus on things that don't need hacks like this.

The source code I used can be found here for glibc and here for musl. Surely we can remove from this PR for now and think about this later. Working on this.

rustbot · 2026-04-26T11:50:36Z

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

bourumir-wyngs · 2026-04-26T18:31:04Z

Has conflicts, but no worries, I work on resolving them

rustbot · 2026-04-26T20:52:22Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

bourumir-wyngs · 2026-04-26T22:06:37Z

@rustbot ready

bourumir-wyngs · 2026-04-26T22:07:17Z

Let's continue on #4983 that I did on the tip of the recent master branch. This looks for me safer, I messed up too much to undo reliably.

RalfJung · 2026-04-27T05:58:36Z

Continuing in the same PR would make it much easier for me to track what changed since I first looked at the PR. If you make a new PR I have to basically redo the entire review from scratch.

So I'd prefer if you could figure out how to properly rebase this PR. The bot posted some instructions above and the internet has tons of git tutorials as well. Please note that you have to reopen this PR before pushing to the old branch or else github won't let you reopen.

- Track more fields in `FileMetadata` (`ino`, `nlink`, `rdev`, `blksize`, `blocks`). - Populate these new fields in the `statx` shim and update the returned mask. - Implement `unix_dev_major`/`unix_dev_minor` helpers to split `dev_t` using glibc layout. - Add an integration test in `tests/pass-dep/libc/libc-fs.rs` and remove the FIXME in `foreign_items.rs`.

Address PR feedback by dynamically checking the presence of Unix-specific metadata fields instead of making unconditional platform assumptions. - Update `FileMetadata` to wrap fields like `ino`, `nlink`, `uid`, `gid`, `rdev`, `blksize`, and `blocks` in `Option`. - Only advertise `STATX_*` flags in the `statx` syscall if the corresponding data is actually present in the `FileMetadata`. - Provide zero fallbacks for unavailable fields when writing to `stat` and `statx` buffers.

…this PR.

bourumir-wyngs · 2026-04-27T13:14:17Z

A risk to address is not to touch recently merged change that modifies overlapping things. I added an extra regression test.

rustbot · 2026-04-27T14:31:24Z

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.

bourumir-wyngs · 2026-04-27T14:34:28Z

@rustbot ready

RalfJung · 2026-04-27T14:49:38Z

Thanks for figuring out how to rescue your old PR. :)

rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Mar 21, 2026

This comment has been minimized.

Sign in to view

RalfJung reviewed Apr 26, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Apr 26, 2026

This comment has been minimized.

Sign in to view

bourumir-wyngs marked this pull request as draft April 26, 2026 20:58

bourumir-wyngs mentioned this pull request Apr 26, 2026

shims: improve Linux statx (reapply #4913) #4983

Closed

rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Apr 26, 2026

bourumir-wyngs closed this Apr 26, 2026

rustbot removed the S-waiting-on-review Status: Waiting for a review to complete label Apr 26, 2026

bourumir-wyngs reopened this Apr 27, 2026

rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Apr 27, 2026

audrius and others added 9 commits April 27, 2026 14:19

Further work on the statx shim and its test

95f06c9

Further work on the statx shim and its test (rename, extra check)

1636007

Small grammar fix

23e7661

Rustfmt fix

3b46643

Using .unwrap() instead of as 32 as per Clippy requirement.

11c38a7

Another formatting fix

ec36c26

Remove linux_decode_dev as per request (returning to default 0 values)

dcdad7c

bourumir-wyngs added 2 commits April 27, 2026 14:20

Remove unnecessary test assertion

bda62b9

Document more on FileMetaData and remove rev field not in scope of …

91b38d3

…this PR.

bourumir-wyngs force-pushed the bw/statx branch from 85cd052 to d968320 Compare April 27, 2026 12:29

Last time adjusting comments

6732643

bourumir-wyngs force-pushed the bw/statx branch from d968320 to 6732643 Compare April 27, 2026 12:34

audrius added 4 commits April 27, 2026 14:43

Remove unimplemented unix_dev handling to match previous behavior

f347864

Format tests/pass-dep/libc/libc-fs.rs

b98f6e5

Fix statx mask fields and libc-fs assertions

88fe109

Grammar fix in comment.

7e522e8

audrius added 4 commits April 27, 2026 15:33

Revert STATX_MODE inclusion in statx_mask

98f579f

Initialize non-Unix FileMetadata fields with None

c53b739

Remove rdev from FileMetadata to reduce PR scope

3925b65

Add documentation to FileMetadata about optional field availability

aa81ab6

bourumir-wyngs marked this pull request as ready for review April 27, 2026 14:31

rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Apr 27, 2026

Conversation

bourumir-wyngs commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 21, 2026

Uh oh!

This comment has been minimized.

RalfJung left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

bourumir-wyngs Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

bourumir-wyngs Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 26, 2026

Uh oh!

bourumir-wyngs commented Apr 26, 2026

Uh oh!

rustbot commented Apr 26, 2026

Uh oh!

This comment has been minimized.

bourumir-wyngs commented Apr 26, 2026

Uh oh!

bourumir-wyngs commented Apr 26, 2026

Uh oh!

RalfJung commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bourumir-wyngs commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 27, 2026

Uh oh!

bourumir-wyngs commented Apr 27, 2026

Uh oh!

RalfJung commented Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bourumir-wyngs commented Mar 21, 2026 •

edited

Loading

RalfJung left a comment •

edited by rustbot

Loading

RalfJung commented Apr 27, 2026 •

edited

Loading

bourumir-wyngs commented Apr 27, 2026 •

edited

Loading