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

Try to ensure usize marker does not get merged #69651

Merged
merged 1 commit into from
Mar 8, 2020
Merged
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
12 changes: 11 additions & 1 deletion src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,18 @@ pub struct ArgumentV1<'a> {
// could have been miscompiled. In practice, we never call as_usize on non-usize
// containing data (as a matter of static generation of the formatting
// arguments), so this is merely an additional check.
//
// We primarily want to ensure that the function pointer at `USIZE_MARKER` has
// an address corresponding *only* to functions that also take `&usize` as their
// first argument. The read_volatile here ensures that we can safely ready out a
// usize from the passed reference and that this address does not point at a
// non-usize taking function.
#[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")]
static USIZE_MARKER: fn(&usize, &mut Formatter<'_>) -> Result = |_, _| loop {};
static USIZE_MARKER: fn(&usize, &mut Formatter<'_>) -> Result = |ptr, _| {
// SAFETY: ptr is a reference
let _v: usize = unsafe { crate::ptr::read_volatile(ptr) };
loop {}
Copy link
Member

Choose a reason for hiding this comment

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

If you make this return Ok(()), you could actually call it before reading the usize from Argument, ensuring that what this closure does is guaranteed to happen, giving more confidence that if there was UB, it was invoked by calling the fn pointer which should be safe to call.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit annoying to call the formatter as we don't have a &mut Formatter<'_> around and I'd rather not manufacture one just for this. I think we could likely thread it through but I think that hurts readability and such so I'd rather not.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right, that method doesn't take the formatter, fair enough.

};

impl<'a> ArgumentV1<'a> {
#[doc(hidden)]
Expand Down