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

use offset_of! to calculate dirent64 field offsets #114519

Merged
merged 1 commit into from
Aug 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@
#![feature(maybe_uninit_slice)]
#![feature(maybe_uninit_uninit_array)]
#![feature(maybe_uninit_write_slice)]
#![feature(offset_of)]
#![feature(panic_can_unwind)]
#![feature(panic_info_message)]
#![feature(panic_internals)]
Expand Down
27 changes: 2 additions & 25 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,6 @@ use crate::ffi::{CStr, OsStr, OsString};
use crate::fmt;
use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
use crate::mem;
#[cfg(any(
target_os = "android",
target_os = "linux",
target_os = "solaris",
target_os = "fuchsia",
target_os = "redox",
target_os = "illumos",
target_os = "nto",
target_os = "vita",
))]
use crate::mem::MaybeUninit;
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
use crate::path::{Path, PathBuf};
use crate::ptr;
Expand Down Expand Up @@ -712,22 +701,10 @@ impl Iterator for ReadDir {
// requires the full extent of *entry_ptr to be in bounds of the same
// allocation, which is not necessarily the case here.
//
// Absent any other way to obtain a pointer to `(*entry_ptr).d_name`
// legally in Rust analogously to how it would be done in C, we instead
// need to make our own non-libc allocation that conforms to the weird
// imaginary definition of dirent64, and use that for a field offset
// computation.
// Instead we must access fields individually through their offsets.
macro_rules! offset_ptr {
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, if rust-lang/reference#1387 gets accepted then all this can be removed and a direct addr_of!((*entry_ptr).d_name) should work.

($entry_ptr:expr, $field:ident) => {{
const OFFSET: isize = {
let delusion = MaybeUninit::<dirent64>::uninit();
let entry_ptr = delusion.as_ptr();
unsafe {
ptr::addr_of!((*entry_ptr).$field)
.cast::<u8>()
.offset_from(entry_ptr.cast::<u8>())
}
};
const OFFSET: isize = mem::offset_of!(dirent64, $field) as isize;
if true {
// Cast to the same type determined by the else branch.
$entry_ptr.byte_offset(OFFSET).cast::<_>()
Copy link
Member

Choose a reason for hiding this comment

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

curiosity: I see above it talks about "in bounds of the same allocation, which is not necessarily the case here", but then it uses byte_offset, which also has the "inside the same allocation" problem.

Should it perhaps be wrapping_byte_offset here? Or am I misunderstanding the restrictions under which this is operating?

Copy link
Member

@dtolnay dtolnay Aug 5, 2023

Choose a reason for hiding this comment

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

I think both the code and comment are correct as written. byte_offset(OFFSET) requires the start of $field to be in bounds, which it is. addr_of!((*ptr).$field) requires the start and end of $field (and every other field) to be in bounds which they are not.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks! I hadn't considered the whole width of the field problem.

Expand Down
Loading