Repository navigation
Add shared non-blocking socket tests, implement ioctl using fcntl - #5497
Conversation
| fn ioctl<'tcx>( | ||
| &self, | ||
| op: Scalar, | ||
| args: Varargs<'tcx, '_>, | ||
| ecx: &mut MiriInterpCx<'tcx>, | ||
| ) -> InterpResult<'tcx, i32> { | ||
| assert!(ecx.machine.communicate(), "cannot have `UdpSocket` with isolation enabled!"); | ||
|
|
||
| let fionbio = ecx.eval_libc("FIONBIO"); | ||
|
|
||
| if op == fionbio { | ||
| // On these OSes, Rust uses the ioctl, so we trust that it is reasonable and controls | ||
| // the same internal flag as fcntl. | ||
| if !matches!(ecx.tcx.sess.target.os, Os::Linux | Os::Android | Os::MacOs | Os::FreeBsd) | ||
| { | ||
| // FIONBIO cannot be used to change the blocking mode of a socket on solarish targets: | ||
| // <https://github.com/rust-lang/rust/commit/dda5c97675b4f5b1f6fdab64606c8a1f21021b0a> | ||
| // Since there might be more targets which do weird things with this option, we use | ||
| // an allowlist instead of just denying solarish targets. | ||
| throw_unsup_format!( | ||
| "ioctl: setting FIONBIO on sockets is unsupported on target {}", | ||
| ecx.tcx.sess.target.os | ||
| ); | ||
| } | ||
|
|
||
| let ([value_ptr], _) = ecx.check_varargs(shim_varargs![*i32], args, "ioctl")?; | ||
| let value = ecx.deref_pointer(&value_ptr)?; | ||
| let non_block = ecx.read_scalar(&value)?.to_i32()? != 0; | ||
| self.is_non_block.set(non_block); | ||
| return interp_ok(0); | ||
| } | ||
|
|
||
| throw_unsup_format!("ioctl: unsupported operation {op:#x} on socket"); | ||
| } |
There was a problem hiding this comment.
This is an exact copy of the code for TCP sockets, right?
That seems unfortunate. Would a generic implementation in terms of get_flags / set_flags make sense?
There was a problem hiding this comment.
Yes this is a copy of the TCP socket implementation.
Would a generic implementation in terms of get_flags / set_flags make sense?
Do you mean having a set_non_blocking and get_non_blocking implementation? I think this could be confusing since we can also set/get the blocking mode through set_flags/get_flags.
However, as far as I can tell, all socket types have the same ioctl options available. Thus, we could instead have a helper in the socket.rs which handles ioctl for the different socket types (similar to how we did it with the socket options) such that we don't need to duplicate this implementation.
There was a problem hiding this comment.
What I meant is that here we could have a case for FIONBIO that calls get_flags + set_flags on the unix FD. Then we can remove UnixFileDescription::ioctl completely I think?
There was a problem hiding this comment.
Yes, this seems better. I'll change it to this.
75745d1 to
e93b5a5
Compare
This comment has been minimized.
This comment has been minimized.
WhySoBad
changed the title
ioctl support to UDP sockets, add shared non-blocking socket testsioctl using fcntl
|
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. |
|
I noticed that the virtual socket shim had a check that disallowed |
|
If it works naively then yeah it's fine for this to also work in Miri. :) |
This pull request adds
ioctl(udp_sockfd, FIONBIO, ...)support to UDP sockets such that the standard library'sUdpSocket::set_nonblockingmethod can be supported.Additionally, the TCP socket tests which test changing the socket's blocking behavior in various ways have been moved to a shared
socket-no-blocking.rsfile such that they can be reused for UDP sockets.