From 59abdb6a7eef003b1a1b0711ceb9a1edb1d1b84c Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 27 Jul 2020 21:25:36 -0700 Subject: [PATCH 1/4] Mark `-1` as an available niche for file descriptors Based on discussion from https://internals.rust-lang.org/t/can-the-standard-library-shrink-option-file/12768, the file descriptor -1 is chosen based on the POSIX API designs that use it as a sentinel to report errors. A bigger niche could've been chosen, particularly on Linux, but would not necessarily be portable. This PR also adds a test case to ensure that the -1 niche (which is kind of hacky and has no obvious test case) works correctly. It requires the "upper" bound, which is actually -1, to be expressed in two's complement. --- library/std/src/sys/unix/fd.rs | 8 +++++++- src/test/ui/print_type_sizes/niche-filling.rs | 16 ++++++++++++---- .../ui/print_type_sizes/niche-filling.stdout | 6 ++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index d3a279a23553e..0eeaa68d55a7d 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -12,6 +12,11 @@ use crate::sys_common::AsInner; use libc::{c_int, c_void}; #[derive(Debug)] +#[rustc_layout_scalar_valid_range_start(0)] +// libstd/os/raw/mod.rs assures me that every libstd-supported platform has a +// 32-bit c_int. Below is -2, in two's complement, but that only works out +// because c_int is 32 bits. +#[rustc_layout_scalar_valid_range_end(0xFF_FF_FF_FE)] pub struct FileDesc { fd: c_int, } @@ -63,7 +68,8 @@ const fn max_iov() -> usize { impl FileDesc { pub fn new(fd: c_int) -> FileDesc { - FileDesc { fd } + assert_ne!(fd, -1); + unsafe { FileDesc { fd } } } pub fn raw(&self) -> c_int { diff --git a/src/test/ui/print_type_sizes/niche-filling.rs b/src/test/ui/print_type_sizes/niche-filling.rs index 37ac45f7e053c..0716cee21c662 100644 --- a/src/test/ui/print_type_sizes/niche-filling.rs +++ b/src/test/ui/print_type_sizes/niche-filling.rs @@ -15,12 +15,19 @@ // padding and overall computed sizes can be quite different. #![feature(start)] +#![feature(rustc_attrs)] #![allow(dead_code)] use std::num::NonZeroU32; pub enum MyOption { None, Some(T) } +#[rustc_layout_scalar_valid_range_start(0)] +#[rustc_layout_scalar_valid_range_end(0xFF_FF_FF_FE)] +pub struct MyNotNegativeOne { + _i: i32, +} + impl Default for MyOption { fn default() -> Self { MyOption::None } } @@ -77,17 +84,18 @@ fn start(_: isize, _: *const *const u8) -> isize { let _a: MyOption = Default::default(); let _b: MyOption = Default::default(); let _c: MyOption = Default::default(); - let _b: MyOption> = Default::default(); + let _d: MyOption> = Default::default(); let _e: Enum4<(), char, (), ()> = Enum4::One(()); let _f: Enum4<(), (), bool, ()> = Enum4::One(()); let _g: Enum4<(), (), (), MyOption> = Enum4::One(()); + let _h: MyOption = Default::default(); // Unions do not currently participate in niche filling. - let _h: MyOption> = Default::default(); + let _i: MyOption> = Default::default(); // ...even when theoretically possible. - let _i: MyOption> = Default::default(); - let _j: MyOption> = Default::default(); + let _j: MyOption> = Default::default(); + let _k: MyOption> = Default::default(); 0 } diff --git a/src/test/ui/print_type_sizes/niche-filling.stdout b/src/test/ui/print_type_sizes/niche-filling.stdout index 1894cd218ee34..d1753c26ca83b 100644 --- a/src/test/ui/print_type_sizes/niche-filling.stdout +++ b/src/test/ui/print_type_sizes/niche-filling.stdout @@ -43,6 +43,12 @@ print-type-size variant `Three`: 0 bytes print-type-size field `.0`: 0 bytes print-type-size variant `Four`: 0 bytes print-type-size field `.0`: 0 bytes +print-type-size type: `MyNotNegativeOne`: 4 bytes, alignment: 4 bytes +print-type-size field `._i`: 4 bytes +print-type-size type: `MyOption`: 4 bytes, alignment: 4 bytes +print-type-size variant `Some`: 4 bytes +print-type-size field `.0`: 4 bytes +print-type-size variant `None`: 0 bytes print-type-size type: `MyOption`: 4 bytes, alignment: 4 bytes print-type-size variant `Some`: 4 bytes print-type-size field `.0`: 4 bytes From a50811a214b50d4a835ba1ad8eac546f9c7af9fa Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 30 Jul 2020 12:11:30 -0700 Subject: [PATCH 2/4] Add safety note to library/std/src/sys/unix/fd.rs Co-authored-by: Elichai Turkel --- library/std/src/sys/unix/fd.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 0eeaa68d55a7d..08c63444e2bb6 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -69,6 +69,7 @@ const fn max_iov() -> usize { impl FileDesc { pub fn new(fd: c_int) -> FileDesc { assert_ne!(fd, -1); + // SAFETY: we just asserted that the value is in the valid range and isn't `-1` (the only value bigger than `0xFF_FF_FF_FE` unsigned) unsafe { FileDesc { fd } } } From 08b70eda2c2767951dbd8a421187d6922164afde Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 10 Dec 2020 15:05:22 -0700 Subject: [PATCH 3/4] Fix fd test case --- library/std/src/sys/unix/fd/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fd/tests.rs b/library/std/src/sys/unix/fd/tests.rs index a932043cbc62b..c9520485c3c7c 100644 --- a/library/std/src/sys/unix/fd/tests.rs +++ b/library/std/src/sys/unix/fd/tests.rs @@ -3,7 +3,7 @@ use core::mem::ManuallyDrop; #[test] fn limit_vector_count() { - let stdout = ManuallyDrop::new(FileDesc { fd: 1 }); + let stdout = ManuallyDrop::new(unsafe { FileDesc { fd: 1 } }); let bufs = (0..1500).map(|_| IoSlice::new(&[])).collect::>(); assert!(stdout.write_vectored(&bufs).is_ok()); } From 094b1da3a1beb23b47e58aabe8d2c8ce74bb4f7f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 20 Dec 2020 11:56:51 +0000 Subject: [PATCH 4/4] Check that c_int is i32 in FileDesc::new. --- library/std/src/sys/unix/fd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 08c63444e2bb6..821851a6c65b7 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -68,7 +68,7 @@ const fn max_iov() -> usize { impl FileDesc { pub fn new(fd: c_int) -> FileDesc { - assert_ne!(fd, -1); + assert_ne!(fd, -1i32); // SAFETY: we just asserted that the value is in the valid range and isn't `-1` (the only value bigger than `0xFF_FF_FF_FE` unsigned) unsafe { FileDesc { fd } } }