Skip to content

Commit

Permalink
Unrolled build for rust-lang#126956
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#126956 - joboet:fmt_no_extern_ty, r=RalfJung

core: avoid `extern type`s in formatting infrastructure

```@RalfJung``` [said](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Use.20of.20.60extern.20type.60.20in.20formatting.20machinery/near/446552837):

>How attached are y'all to using `extern type` in the formatting machinery?
Seems like this was introduced a [long time ago](rust-lang@34ef8f5). However, it's also [not really compatible with Stacked Borrows](rust-lang/unsafe-code-guidelines#256), and only works currently because we effectively treat references-to-extern-type almost like raw pointers in Stacked Borrows -- which of course is unsound, it's not how LLVM works. I was planning to make Miri emit a warning when this happens to avoid cases like [this](rust-lang#126814 (comment)) where people use extern type specifically to silence Miri without realizing what happens. but with the formatting machinery using  extern type, this warning would just show up everywhere...
>
> The "proper" way to do this in Stacked Borrows is to use raw pointers (or `NonNull`).

This PR does just that.

r? ```@RalfJung```
  • Loading branch information
rust-timer authored Jun 28, 2024
2 parents 42add88 + 7e7d0a9 commit 82d7c0e
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 55 deletions.
6 changes: 6 additions & 0 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,12 @@ impl<'a> Arguments<'a> {
}
}

// Manually implementing these results in better error messages.
#[stable(feature = "rust1", since = "1.0.0")]
impl !Send for Arguments<'_> {}
#[stable(feature = "rust1", since = "1.0.0")]
impl !Sync for Arguments<'_> {}

#[stable(feature = "rust1", since = "1.0.0")]
impl Debug for Arguments<'_> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result {
Expand Down
46 changes: 25 additions & 21 deletions library/core/src/fmt/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use super::*;
use crate::hint::unreachable_unchecked;
use crate::ptr::NonNull;

#[lang = "format_placeholder"]
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -66,7 +67,13 @@ pub(super) enum Flag {

#[derive(Copy, Clone)]
enum ArgumentType<'a> {
Placeholder { value: &'a Opaque, formatter: fn(&Opaque, &mut Formatter<'_>) -> Result },
Placeholder {
// INVARIANT: `formatter` has type `fn(&T, _) -> _` for some `T`, and `value`
// was derived from a `&'a T`.
value: NonNull<()>,
formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result,
_lifetime: PhantomData<&'a ()>,
},
Count(usize),
}

Expand All @@ -90,21 +97,15 @@ pub struct Argument<'a> {
impl<'a> Argument<'a> {
#[inline(always)]
fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'b> {
// SAFETY: `mem::transmute(x)` is safe because
// 1. `&'b T` keeps the lifetime it originated with `'b`
// (so as to not have an unbounded lifetime)
// 2. `&'b T` and `&'b Opaque` have the same memory layout
// (when `T` is `Sized`, as it is here)
// `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result`
// and `fn(&Opaque, &mut Formatter<'_>) -> Result` have the same ABI
// (as long as `T` is `Sized`)
unsafe {
Argument {
ty: ArgumentType::Placeholder {
formatter: mem::transmute(f),
value: mem::transmute(x),
},
}
Argument {
// INVARIANT: this creates an `ArgumentType<'b>` from a `&'b T` and
// a `fn(&T, ...)`, so the invariant is maintained.
ty: ArgumentType::Placeholder {
value: NonNull::from(x).cast(),
// SAFETY: function pointers always have the same layout.
formatter: unsafe { mem::transmute(f) },
_lifetime: PhantomData,
},
}
}

Expand Down Expand Up @@ -162,7 +163,14 @@ impl<'a> Argument<'a> {
#[inline(always)]
pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {
match self.ty {
ArgumentType::Placeholder { formatter, value } => formatter(value, f),
// SAFETY:
// Because of the invariant that if `formatter` had the type
// `fn(&T, _) -> _` then `value` has type `&'b T` where `'b` is
// the lifetime of the `ArgumentType`, and because references
// and `NonNull` are ABI-compatible, this is completely equivalent
// to calling the original function passed to `new` with the
// original reference, which is sound.
ArgumentType::Placeholder { formatter, value, .. } => unsafe { formatter(value, f) },
// SAFETY: the caller promised this.
ArgumentType::Count(_) => unsafe { unreachable_unchecked() },
}
Expand Down Expand Up @@ -208,7 +216,3 @@ impl UnsafeArg {
Self { _private: () }
}
}

extern "C" {
type Opaque;
}
6 changes: 3 additions & 3 deletions tests/coverage/issue-83601.cov-map
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
Function name: issue_83601::main
Raw bytes (21): 0x[01, 01, 01, 05, 00, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02]
Raw bytes (21): 0x[01, 01, 01, 05, 09, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 1
- expression 0 operands: lhs = Counter(1), rhs = Zero
- expression 0 operands: lhs = Counter(1), rhs = Counter(2)
Number of file 0 mappings: 3
- Code(Counter(0)) at (prev + 6, 1) to (start + 2, 28)
- Code(Counter(1)) at (prev + 3, 9) to (start + 1, 28)
- Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 3, 2)
= (c1 - Zero)
= (c1 - c2)

14 changes: 7 additions & 7 deletions tests/coverage/issue-84561.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 167, 9) to (start + 2, 10)

Function name: issue_84561::test3
Raw bytes (375): 0x[01, 01, 31, 05, 00, 0d, 00, 15, 00, 12, 00, 15, 00, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02]
Raw bytes (375): 0x[01, 01, 31, 05, 09, 0d, 00, 15, 19, 12, 00, 15, 19, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 49
- expression 0 operands: lhs = Counter(1), rhs = Zero
- expression 0 operands: lhs = Counter(1), rhs = Counter(2)
- expression 1 operands: lhs = Counter(3), rhs = Zero
- expression 2 operands: lhs = Counter(5), rhs = Zero
- expression 2 operands: lhs = Counter(5), rhs = Counter(6)
- expression 3 operands: lhs = Expression(4, Sub), rhs = Zero
- expression 4 operands: lhs = Counter(5), rhs = Zero
- expression 4 operands: lhs = Counter(5), rhs = Counter(6)
- expression 5 operands: lhs = Counter(8), rhs = Zero
- expression 6 operands: lhs = Expression(7, Sub), rhs = Zero
- expression 7 operands: lhs = Counter(8), rhs = Zero
Expand Down Expand Up @@ -111,15 +111,15 @@ Number of file 0 mappings: 51
- Code(Counter(0)) at (prev + 8, 1) to (start + 3, 28)
- Code(Counter(1)) at (prev + 4, 9) to (start + 1, 28)
- Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 4, 31)
= (c1 - Zero)
= (c1 - c2)
- Code(Counter(3)) at (prev + 5, 5) to (start + 0, 31)
- Code(Expression(1, Sub)) at (prev + 1, 5) to (start + 0, 31)
= (c3 - Zero)
- Code(Counter(5)) at (prev + 1, 9) to (start + 1, 28)
- Code(Expression(4, Sub)) at (prev + 2, 5) to (start + 0, 31)
= (c5 - Zero)
= (c5 - c6)
- Code(Expression(3, Sub)) at (prev + 1, 5) to (start + 0, 15)
= ((c5 - Zero) - Zero)
= ((c5 - c6) - Zero)
- Code(Zero) at (prev + 0, 32) to (start + 0, 48)
- Code(Counter(8)) at (prev + 1, 5) to (start + 3, 15)
- Code(Zero) at (prev + 3, 32) to (start + 0, 48)
Expand Down
30 changes: 6 additions & 24 deletions tests/ui/fmt/send-sync.stderr
Original file line number Diff line number Diff line change
@@ -1,45 +1,27 @@
error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely
error[E0277]: `Arguments<'_>` cannot be sent between threads safely
--> $DIR/send-sync.rs:8:10
|
LL | send(format_args!("{:?}", c));
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be sent between threads safely
| |
| required by a bound introduced by this call
|
= help: within `[core::fmt::rt::Argument<'_>]`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Send`
= note: required because it appears within the type `&core::fmt::rt::Opaque`
note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>`
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
note: required because it appears within the type `core::fmt::rt::Argument<'_>`
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
= note: required because it appears within the type `[core::fmt::rt::Argument<'_>]`
= note: required for `&[core::fmt::rt::Argument<'_>]` to implement `Send`
note: required because it appears within the type `Arguments<'_>`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
= help: the trait `Send` is not implemented for `Arguments<'_>`
note: required by a bound in `send`
--> $DIR/send-sync.rs:1:12
|
LL | fn send<T: Send>(_: T) {}
| ^^^^ required by this bound in `send`

error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely
error[E0277]: `Arguments<'_>` cannot be shared between threads safely
--> $DIR/send-sync.rs:9:10
|
LL | sync(format_args!("{:?}", c));
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be shared between threads safely
| |
| required by a bound introduced by this call
|
= help: within `Arguments<'_>`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Sync`
= note: required because it appears within the type `&core::fmt::rt::Opaque`
note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>`
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
note: required because it appears within the type `core::fmt::rt::Argument<'_>`
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
= note: required because it appears within the type `[core::fmt::rt::Argument<'_>]`
= note: required because it appears within the type `&[core::fmt::rt::Argument<'_>]`
note: required because it appears within the type `Arguments<'_>`
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
= help: the trait `Sync` is not implemented for `Arguments<'_>`
note: required by a bound in `sync`
--> $DIR/send-sync.rs:2:12
|
Expand Down

0 comments on commit 82d7c0e

Please sign in to comment.