Skip to content

Add basic getsockopt shim for reading SO_ERROR#5005

Merged
RalfJung merged 1 commit intorust-lang:masterfrom
WhySoBad:network-socket-getsockopt
May 8, 2026
Merged

Add basic getsockopt shim for reading SO_ERROR#5005
RalfJung merged 1 commit intorust-lang:masterfrom
WhySoBad:network-socket-getsockopt

Conversation

@WhySoBad
Copy link
Copy Markdown
Contributor

@WhySoBad WhySoBad commented May 6, 2026

Hi,

This pull request adds the getsockopt shim. Currently, it only supports reading the SO_ERROR option.

By this, Miri is now able to run programs which use tokio/mio sockets. Thus, I also added a simple tokio socket test which tests connecting/accepting and reading/writing. Let me know if I should test more things with the tokio sockets.
I've also ran the tokio TCP test suite and we pass everything except:

  • Some tests which are using syscalls which aren't shimmed (mainly socket options)
  • One test failing due to a Miri panic when the write end of a connecting socket is closed before it received a writable event
  • One test failing due to Miri not clearing the readiness after a short read/write on a linux target

For the latter two points I'll soon open pull requests here.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 6, 2026

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.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label May 6, 2026
@rustbot

This comment has been minimized.

@WhySoBad WhySoBad force-pushed the network-socket-getsockopt branch from c2fe5a4 to 8b09c4b Compare May 6, 2026 12:31
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 6, 2026

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.

Copy link
Copy Markdown
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.

Amazing how everything is coming together and now we can actually run that Tokio smoke test. :)

I still have some comments though, of course. ;)
@rustbot author

View changes since this review

Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs
Comment thread src/shims/unix/socket.rs
Comment thread src/shims/unix/socket.rs Outdated
Comment thread tests/pass-dep/libc/libc-socket-no-blocking-epoll.rs Outdated
Comment thread tests/pass-dep/libc/libc-socket-no-blocking-epoll.rs Outdated
@rustbot 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 May 7, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 7, 2026

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

@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented May 8, 2026

@rustbot ready

@rustbot 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 May 8, 2026
Copy link
Copy Markdown
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.

That truncation of the value is really annoying, we may have to iterate a bit until we find a good way to deal with that.
@rustbot author

View changes since this review

Comment thread tests/utils/libc.rs
Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs
@rustbot 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 May 8, 2026
Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs Outdated
Comment thread src/shims/unix/socket.rs
Comment thread src/shims/unix/socket.rs Outdated
Comment thread tests/pass-dep/libc/libc-socket-no-blocking-epoll.rs Outdated
@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented May 8, 2026

I think if you restart the failed CI job then everything should be fine.

@rustbot ready

@rustbot 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 May 8, 2026
Copy link
Copy Markdown
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.

Very nice. :)

Just one last question. Please also squash the commits.
@rustbot author

View changes since this review

Comment thread src/shims/unix/socket.rs Outdated
@rustbot 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 May 8, 2026
Comment thread src/shims/unix/socket.rs
@WhySoBad WhySoBad force-pushed the network-socket-getsockopt branch from fbefc74 to 378a643 Compare May 8, 2026 14:05
@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented May 8, 2026

@rustbot ready

@rustbot 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 May 8, 2026
@RalfJung RalfJung enabled auto-merge May 8, 2026 14:06
@RalfJung RalfJung added this pull request to the merge queue May 8, 2026
Merged via the queue into rust-lang:master with commit f16f680 May 8, 2026
13 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Waiting for a review to complete label May 8, 2026
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.

3 participants