Auto merge of #45198 - oli-obk:fmt_args, r=sfackler

Prevent fmt::Arguments from being shared across threads

Fixes #45197

This is a **breaking change**! Without doing this it's very easy to create race conditions.

There's probably a way to do this without breaking valid use cases, but it would require quite an overhaul of the formatting machinery.

3 files changed, 62 insertions, 0 deletions

diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs
index cc324b585bc..897222747f5 100644
--- a/src/libcore/fmt/mod.rs
+++ b/src/libcore/fmt/mod.rs
@@ -261,6 +261,14 @@ pub struct Formatter<'a> {
 
 struct Void {
     _priv: (),
+    /// Erases all oibits, because `Void` erases the type of the object that
+    /// will be used to produce formatted output. Since we do not know what
+    /// oibits the real types have (and they can have any or none), we need to
+    /// take the most conservative approach and forbid all oibits.
+    ///
+    /// It was added after #45197 showed that one could share a `!Sync`
+    /// object across threads by passing it into `format_args!`.
+    _oibit_remover: PhantomData<*mut Fn()>,
 }
 
 /// This struct represents the generic "argument" which is taken by the Xprintf
diff --git a/src/test/ui/fmt/send-sync.rs b/src/test/ui/fmt/send-sync.rs
new file mode 100644
index 00000000000..bb4f9dfffc7
--- /dev/null
+++ b/src/test/ui/fmt/send-sync.rs
@@ -0,0 +1,20 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn send<T: Send>(_: T) {}
+fn sync<T: Sync>(_: T) {}
+
+fn main() {
+    // `Cell` is not `Sync`, so `&Cell` is neither `Sync` nor `Send`,
+    // `std::fmt::Arguments` used to forget this...
+    let c = std::cell::Cell::new(42);
+    send(format_args!("{:?}", c));
+    sync(format_args!("{:?}", c));
+}
diff --git a/src/test/ui/fmt/send-sync.stderr b/src/test/ui/fmt/send-sync.stderr
new file mode 100644
index 00000000000..1ec53d220e9
--- /dev/null
+++ b/src/test/ui/fmt/send-sync.stderr
@@ -0,0 +1,34 @@
+error[E0277]: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `[std::fmt::ArgumentV1<'_>]`
+  --> $DIR/send-sync.rs:18:5
+   |
+18 |     send(format_args!("{:?}", c));
+   |     ^^^^ `*mut std::ops::Fn() + 'static` cannot be shared between threads safely
+   |
+   = help: within `[std::fmt::ArgumentV1<'_>]`, the trait `std::marker::Sync` is not implemented for `*mut std::ops::Fn() + 'static`
+   = note: required because it appears within the type `std::marker::PhantomData<*mut std::ops::Fn() + 'static>`
+   = note: required because it appears within the type `core::fmt::Void`
+   = note: required because it appears within the type `&core::fmt::Void`
+   = note: required because it appears within the type `std::fmt::ArgumentV1<'_>`
+   = note: required because it appears within the type `[std::fmt::ArgumentV1<'_>]`
+   = note: required because of the requirements on the impl of `std::marker::Send` for `&[std::fmt::ArgumentV1<'_>]`
+   = note: required because it appears within the type `std::fmt::Arguments<'_>`
+   = note: required by `send`
+
+error[E0277]: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `std::fmt::Arguments<'_>`
+  --> $DIR/send-sync.rs:19:5
+   |
+19 |     sync(format_args!("{:?}", c));
+   |     ^^^^ `*mut std::ops::Fn() + 'static` cannot be shared between threads safely
+   |
+   = help: within `std::fmt::Arguments<'_>`, the trait `std::marker::Sync` is not implemented for `*mut std::ops::Fn() + 'static`
+   = note: required because it appears within the type `std::marker::PhantomData<*mut std::ops::Fn() + 'static>`
+   = note: required because it appears within the type `core::fmt::Void`
+   = note: required because it appears within the type `&core::fmt::Void`
+   = note: required because it appears within the type `std::fmt::ArgumentV1<'_>`
+   = note: required because it appears within the type `[std::fmt::ArgumentV1<'_>]`
+   = note: required because it appears within the type `&[std::fmt::ArgumentV1<'_>]`
+   = note: required because it appears within the type `std::fmt::Arguments<'_>`
+   = note: required by `sync`
+
+error: aborting due to 2 previous errors
+