Skip to content

Commit 82d7c0e

Browse files
rust-timerrust-timer
authored
Unrolled build for #126956
Rollup merge of #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](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](#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```
2 parents 42add88 + 7e7d0a9 commit 82d7c0e

File tree

5 files changed

+47
-55
lines changed

5 files changed

+47
-55
lines changed

‎library/core/src/fmt/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,12 @@ impl<'a> Arguments<'a> {
459459
}
460460
}
461461

462+
// Manually implementing these results in better error messages.
463+
#[stable(feature = "rust1", since = "1.0.0")]
464+
impl !Send for Arguments<'_> {}
465+
#[stable(feature = "rust1", since = "1.0.0")]
466+
impl !Sync for Arguments<'_> {}
467+
462468
#[stable(feature = "rust1", since = "1.0.0")]
463469
impl Debug for Arguments<'_> {
464470
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result {

‎library/core/src/fmt/rt.rs

+25-21
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use super::*;
77
use crate::hint::unreachable_unchecked;
8+
use crate::ptr::NonNull;
89

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

6768
#[derive(Copy, Clone)]
6869
enum ArgumentType<'a> {
69-
Placeholder { value: &'a Opaque, formatter: fn(&Opaque, &mut Formatter<'_>) -> Result },
70+
Placeholder {
71+
// INVARIANT: `formatter` has type `fn(&T, _) -> _` for some `T`, and `value`
72+
// was derived from a `&'a T`.
73+
value: NonNull<()>,
74+
formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result,
75+
_lifetime: PhantomData<&'a ()>,
76+
},
7077
Count(usize),
7178
}
7279

@@ -90,21 +97,15 @@ pub struct Argument<'a> {
9097
impl<'a> Argument<'a> {
9198
#[inline(always)]
9299
fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'b> {
93-
// SAFETY: `mem::transmute(x)` is safe because
94-
// 1. `&'b T` keeps the lifetime it originated with `'b`
95-
// (so as to not have an unbounded lifetime)
96-
// 2. `&'b T` and `&'b Opaque` have the same memory layout
97-
// (when `T` is `Sized`, as it is here)
98-
// `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result`
99-
// and `fn(&Opaque, &mut Formatter<'_>) -> Result` have the same ABI
100-
// (as long as `T` is `Sized`)
101-
unsafe {
102-
Argument {
103-
ty: ArgumentType::Placeholder {
104-
formatter: mem::transmute(f),
105-
value: mem::transmute(x),
106-
},
107-
}
100+
Argument {
101+
// INVARIANT: this creates an `ArgumentType<'b>` from a `&'b T` and
102+
// a `fn(&T, ...)`, so the invariant is maintained.
103+
ty: ArgumentType::Placeholder {
104+
value: NonNull::from(x).cast(),
105+
// SAFETY: function pointers always have the same layout.
106+
formatter: unsafe { mem::transmute(f) },
107+
_lifetime: PhantomData,
108+
},
108109
}
109110
}
110111

@@ -162,7 +163,14 @@ impl<'a> Argument<'a> {
162163
#[inline(always)]
163164
pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {
164165
match self.ty {
165-
ArgumentType::Placeholder { formatter, value } => formatter(value, f),
166+
// SAFETY:
167+
// Because of the invariant that if `formatter` had the type
168+
// `fn(&T, _) -> _` then `value` has type `&'b T` where `'b` is
169+
// the lifetime of the `ArgumentType`, and because references
170+
// and `NonNull` are ABI-compatible, this is completely equivalent
171+
// to calling the original function passed to `new` with the
172+
// original reference, which is sound.
173+
ArgumentType::Placeholder { formatter, value, .. } => unsafe { formatter(value, f) },
166174
// SAFETY: the caller promised this.
167175
ArgumentType::Count(_) => unsafe { unreachable_unchecked() },
168176
}
@@ -208,7 +216,3 @@ impl UnsafeArg {
208216
Self { _private: () }
209217
}
210218
}
211-
212-
extern "C" {
213-
type Opaque;
214-
}

‎tests/coverage/issue-83601.cov-map

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
Function name: issue_83601::main
2-
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]
2+
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]
33
Number of files: 1
44
- file 0 => global file 1
55
Number of expressions: 1
6-
- expression 0 operands: lhs = Counter(1), rhs = Zero
6+
- expression 0 operands: lhs = Counter(1), rhs = Counter(2)
77
Number of file 0 mappings: 3
88
- Code(Counter(0)) at (prev + 6, 1) to (start + 2, 28)
99
- Code(Counter(1)) at (prev + 3, 9) to (start + 1, 28)
1010
- Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 3, 2)
11-
= (c1 - Zero)
11+
= (c1 - c2)
1212

‎tests/coverage/issue-84561.cov-map

+7-7
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ Number of file 0 mappings: 1
5454
- Code(Counter(0)) at (prev + 167, 9) to (start + 2, 10)
5555

5656
Function name: issue_84561::test3
57-
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]
57+
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]
5858
Number of files: 1
5959
- file 0 => global file 1
6060
Number of expressions: 49
61-
- expression 0 operands: lhs = Counter(1), rhs = Zero
61+
- expression 0 operands: lhs = Counter(1), rhs = Counter(2)
6262
- expression 1 operands: lhs = Counter(3), rhs = Zero
63-
- expression 2 operands: lhs = Counter(5), rhs = Zero
63+
- expression 2 operands: lhs = Counter(5), rhs = Counter(6)
6464
- expression 3 operands: lhs = Expression(4, Sub), rhs = Zero
65-
- expression 4 operands: lhs = Counter(5), rhs = Zero
65+
- expression 4 operands: lhs = Counter(5), rhs = Counter(6)
6666
- expression 5 operands: lhs = Counter(8), rhs = Zero
6767
- expression 6 operands: lhs = Expression(7, Sub), rhs = Zero
6868
- expression 7 operands: lhs = Counter(8), rhs = Zero
@@ -111,15 +111,15 @@ Number of file 0 mappings: 51
111111
- Code(Counter(0)) at (prev + 8, 1) to (start + 3, 28)
112112
- Code(Counter(1)) at (prev + 4, 9) to (start + 1, 28)
113113
- Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 4, 31)
114-
= (c1 - Zero)
114+
= (c1 - c2)
115115
- Code(Counter(3)) at (prev + 5, 5) to (start + 0, 31)
116116
- Code(Expression(1, Sub)) at (prev + 1, 5) to (start + 0, 31)
117117
= (c3 - Zero)
118118
- Code(Counter(5)) at (prev + 1, 9) to (start + 1, 28)
119119
- Code(Expression(4, Sub)) at (prev + 2, 5) to (start + 0, 31)
120-
= (c5 - Zero)
120+
= (c5 - c6)
121121
- Code(Expression(3, Sub)) at (prev + 1, 5) to (start + 0, 15)
122-
= ((c5 - Zero) - Zero)
122+
= ((c5 - c6) - Zero)
123123
- Code(Zero) at (prev + 0, 32) to (start + 0, 48)
124124
- Code(Counter(8)) at (prev + 1, 5) to (start + 3, 15)
125125
- Code(Zero) at (prev + 3, 32) to (start + 0, 48)

‎tests/ui/fmt/send-sync.stderr

+6-24
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,27 @@
1-
error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely
1+
error[E0277]: `Arguments<'_>` cannot be sent between threads safely
22
--> $DIR/send-sync.rs:8:10
33
|
44
LL | send(format_args!("{:?}", c));
5-
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely
5+
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be sent between threads safely
66
| |
77
| required by a bound introduced by this call
88
|
9-
= help: within `[core::fmt::rt::Argument<'_>]`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Send`
10-
= note: required because it appears within the type `&core::fmt::rt::Opaque`
11-
note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>`
12-
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
13-
note: required because it appears within the type `core::fmt::rt::Argument<'_>`
14-
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
15-
= note: required because it appears within the type `[core::fmt::rt::Argument<'_>]`
16-
= note: required for `&[core::fmt::rt::Argument<'_>]` to implement `Send`
17-
note: required because it appears within the type `Arguments<'_>`
18-
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
9+
= help: the trait `Send` is not implemented for `Arguments<'_>`
1910
note: required by a bound in `send`
2011
--> $DIR/send-sync.rs:1:12
2112
|
2213
LL | fn send<T: Send>(_: T) {}
2314
| ^^^^ required by this bound in `send`
2415

25-
error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely
16+
error[E0277]: `Arguments<'_>` cannot be shared between threads safely
2617
--> $DIR/send-sync.rs:9:10
2718
|
2819
LL | sync(format_args!("{:?}", c));
29-
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely
20+
| ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be shared between threads safely
3021
| |
3122
| required by a bound introduced by this call
3223
|
33-
= help: within `Arguments<'_>`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Sync`
34-
= note: required because it appears within the type `&core::fmt::rt::Opaque`
35-
note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>`
36-
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
37-
note: required because it appears within the type `core::fmt::rt::Argument<'_>`
38-
--> $SRC_DIR/core/src/fmt/rt.rs:LL:COL
39-
= note: required because it appears within the type `[core::fmt::rt::Argument<'_>]`
40-
= note: required because it appears within the type `&[core::fmt::rt::Argument<'_>]`
41-
note: required because it appears within the type `Arguments<'_>`
42-
--> $SRC_DIR/core/src/fmt/mod.rs:LL:COL
24+
= help: the trait `Sync` is not implemented for `Arguments<'_>`
4325
note: required by a bound in `sync`
4426
--> $DIR/send-sync.rs:2:12
4527
|

0 commit comments

Comments
 (0)