Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pread and pwrite shims #3743

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Add pread and pwrite shims #3743

merged 1 commit into from
Jul 22, 2024

Conversation

newpavlov
Copy link
Contributor

Requested in #2057.

@newpavlov
Copy link
Contributor Author

Any updates on this? Lack of these shims is the main blocker for testing our code with MIRI.

@RalfJung
Copy link
Member

RalfJung commented Jul 19, 2024

This is in my review queue, but unfortunately I currently don't have a lot of time for reviewing -- sorry.

In terms of the general approach, as Ben said this is quite the departure from how all file system code works in Miri so far -- which is via a a portable implementation on top of std, using platform-specific functions only when absolutely needed. In that sense I think I would strongly prefer exploring an approach that only needs std APIs that are available everywhere. Miri is entirely sequential, so something like "record old position in file, seek, read, seek back to old position" should be a correct emulation I think?

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I still have some comments/questions.

src/shims/unix/fd.rs Show resolved Hide resolved
src/shims/unix/fs.rs Outdated Show resolved Hide resolved
src/shims/unix/fs.rs Show resolved Hide resolved
tests/pass-dep/tempfile.rs Outdated Show resolved Hide resolved
tests/pass-dep/tempfile.rs Outdated Show resolved Hide resolved
tests/pass-dep/tempfile.rs Outdated Show resolved Hide resolved
@RalfJung RalfJung added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Jul 22, 2024
@newpavlov newpavlov force-pushed the pread_pwrite branch 3 times, most recently from 2c82694 to 0d29380 Compare July 22, 2024 13:30
@newpavlov newpavlov requested a review from RalfJung July 22, 2024 13:54
src/shims/unix/fd.rs Outdated Show resolved Hide resolved
src/shims/unix/fd.rs Outdated Show resolved Hide resolved
tests/pass/shims/fs.rs Show resolved Hide resolved
tests/pass/shims/fs.rs Show resolved Hide resolved
@newpavlov newpavlov requested a review from RalfJung July 22, 2024 15:43
@RalfJung
Copy link
Member

Looks good, thanks. :)

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 22, 2024

📌 Commit e472d33 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 22, 2024

⌛ Testing commit e472d33 with merge aaff0eb...

@bors
Copy link
Collaborator

bors commented Jul 22, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing aaff0eb to master...

@bors bors merged commit aaff0eb into rust-lang:master Jul 22, 2024
8 checks passed
@bors bors mentioned this pull request Jul 22, 2024
@newpavlov newpavlov deleted the pread_pwrite branch July 22, 2024 16:45
@newpavlov
Copy link
Contributor Author

@RalfJung
Hi! Do you know when approximately we can expect for these changes to land on the Nightly toolchain? I tried to run tests on miri 0.1.0 (7c2012d 2024-07-26), but they still fail with:

unsupported operation: can't call foreign function pread64 on OS linux

@RalfJung
Copy link
Member

RalfJung commented Jul 29, 2024

I usually do a sync every weekend but this weekend I had other things to do.

I started a sync in rust-lang/rust#128333.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: Waiting for the PR author to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants