From 975bc18481879b69603674266a5239ecb579f928 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Thu, 5 Aug 2021 11:58:52 -0500 Subject: [PATCH] Make Arguments constructors unsafe --- compiler/rustc_builtin_macros/src/format.rs | 30 +++++++--- library/core/src/fmt/mod.rs | 31 ++++++++++ library/core/src/lib.rs | 1 + library/core/src/macros/mod.rs | 1 + library/core/src/panicking.rs | 10 +++- src/test/pretty/dollar-crate.pp | 10 ++-- src/test/pretty/issue-4264.pp | 56 +++++++++++-------- .../ui/attributes/key-value-expansion.stderr | 16 ++++-- 8 files changed, 114 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 00f2f37146d..1dbf7728421 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -3,8 +3,8 @@ use rustc_ast as ast; use rustc_ast::ptr::P; -use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; +use rustc_ast::{token, BlockCheckMode, UnsafeSource}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, Applicability, DiagnosticBuilder}; use rustc_expand::base::{self, *}; @@ -838,12 +838,15 @@ fn into_expr(self) -> P { // // But the nested match expression is proved to perform not as well // as series of let's; the first approach does. - let pat = self.ecx.pat_tuple(self.macsp, pats); - let arm = self.ecx.arm(self.macsp, pat, args_array); - let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads)); - let result = self.ecx.expr_match(self.macsp, head, vec![arm]); + let args_match = { + let pat = self.ecx.pat_tuple(self.macsp, pats); + let arm = self.ecx.arm(self.macsp, pat, args_array); + let head = self.ecx.expr(self.macsp, ast::ExprKind::Tup(heads)); + self.ecx.expr_match(self.macsp, head, vec![arm]) + }; - let args_slice = self.ecx.expr_addr_of(self.macsp, result); + let ident = Ident::from_str_and_span("args", self.macsp); + let args_slice = self.ecx.expr_ident(self.macsp, ident); // Now create the fmt::Arguments struct with all our locals we created. let (fn_name, fn_args) = if self.all_pieces_simple { @@ -857,7 +860,20 @@ fn into_expr(self) -> P { }; let path = self.ecx.std_path(&[sym::fmt, sym::Arguments, Symbol::intern(fn_name)]); - self.ecx.expr_call_global(self.macsp, path, fn_args) + let arguments = self.ecx.expr_call_global(self.macsp, path, fn_args); + let body = self.ecx.expr_block(P(ast::Block { + stmts: vec![self.ecx.stmt_expr(arguments)], + id: ast::DUMMY_NODE_ID, + rules: BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated), + span: self.macsp, + tokens: None, + })); + + let ident = Ident::from_str_and_span("args", self.macsp); + let binding_mode = ast::BindingMode::ByRef(ast::Mutability::Not); + let pat = self.ecx.pat_ident_binding_mode(self.macsp, ident, binding_mode); + let arm = self.ecx.arm(self.macsp, pat, body); + self.ecx.expr_match(self.macsp, args_match, vec![arm]) } fn format_arg( diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 6ad10990840..b32b1f86336 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -334,11 +334,29 @@ enum FlagV1 { impl<'a> Arguments<'a> { /// When using the format_args!() macro, this function is used to generate the /// Arguments structure. + #[cfg(not(bootstrap))] + #[doc(hidden)] + #[inline] + #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] + #[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")] + pub const unsafe fn new_v1( + pieces: &'a [&'static str], + args: &'a [ArgumentV1<'a>], + ) -> Arguments<'a> { + if pieces.len() < args.len() || pieces.len() > args.len() + 1 { + panic!("invalid args"); + } + Arguments { pieces, fmt: None, args } + } + #[cfg(bootstrap)] #[doc(hidden)] #[inline] #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] #[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")] pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> Arguments<'a> { + if pieces.len() < args.len() || pieces.len() > args.len() + 1 { + panic!("invalid args"); + } Arguments { pieces, fmt: None, args } } @@ -348,6 +366,19 @@ pub const fn new_v1(pieces: &'a [&'static str], args: &'a [ArgumentV1<'a>]) -> A /// `CountIsParam` or `CountIsNextParam` has to point to an argument /// created with `argumentusize`. However, failing to do so doesn't cause /// unsafety, but will ignore invalid . + #[cfg(not(bootstrap))] + #[doc(hidden)] + #[inline] + #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] + #[rustc_const_unstable(feature = "const_fmt_arguments_new", issue = "none")] + pub const unsafe fn new_v1_formatted( + pieces: &'a [&'static str], + args: &'a [ArgumentV1<'a>], + fmt: &'a [rt::v1::Argument], + ) -> Arguments<'a> { + Arguments { pieces, fmt: Some(fmt), args } + } + #[cfg(bootstrap)] #[doc(hidden)] #[inline] #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 37c3f8d4c16..229b05ee505 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -110,6 +110,7 @@ // // Language features: #![feature(abi_unadjusted)] +#![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] #![feature(asm)] #![feature(associated_type_bounds)] diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 476f37699ee..a3a074ac131 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -828,6 +828,7 @@ macro_rules! compile_error { /// assert_eq!(s, format!("hello {}", "world")); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[allow_internal_unsafe] #[allow_internal_unstable(fmt_internals)] #[rustc_builtin_macro] #[macro_export] diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 2ec6b4d15ff..85728551f53 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -47,7 +47,15 @@ pub fn panic(expr: &'static str) -> ! { // truncation and padding (even though none is used here). Using // Arguments::new_v1 may allow the compiler to omit Formatter::pad from the // output binary, saving up to a few kilobytes. - panic_fmt(fmt::Arguments::new_v1(&[expr], &[])); + panic_fmt( + #[cfg(bootstrap)] + fmt::Arguments::new_v1(&[expr], &[]), + #[cfg(not(bootstrap))] + // SAFETY: Arguments::new_v1 is safe with exactly one str and zero args + unsafe { + fmt::Arguments::new_v1(&[expr], &[]) + }, + ); } #[inline] diff --git a/src/test/pretty/dollar-crate.pp b/src/test/pretty/dollar-crate.pp index f4be3c1c63a..4eccba06b13 100644 --- a/src/test/pretty/dollar-crate.pp +++ b/src/test/pretty/dollar-crate.pp @@ -10,9 +10,11 @@ extern crate std; fn main() { { - ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], - &match () { - () => [], - })); + ::std::io::_print(match match () { () => [], } { + ref args => unsafe { + ::core::fmt::Arguments::new_v1(&["rust\n"], + args) + } + }); }; } diff --git a/src/test/pretty/issue-4264.pp b/src/test/pretty/issue-4264.pp index 199aee05622..a21ea520121 100644 --- a/src/test/pretty/issue-4264.pp +++ b/src/test/pretty/issue-4264.pp @@ -32,29 +32,39 @@ pub fn bar() ({ ({ let res = ((::alloc::fmt::format as - for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_v1 - as - fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test" - as - &str)] - as - [&str; 1]) - as - &[&str; 1]), - (&(match (() - as - ()) - { - () - => - ([] - as - [ArgumentV1; 0]), - } - as - [ArgumentV1; 0]) - as - &[ArgumentV1; 0])) + for<'r> fn(Arguments<'r>) -> String {format})((match (match (() + as + ()) + { + () + => + ([] + as + [ArgumentV1; 0]), + } + as + [ArgumentV1; 0]) + { + ref args + => + unsafe + { + ((::core::fmt::Arguments::new_v1 + as + unsafe fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test" + as + &str)] + as + [&str; 1]) + as + &[&str; 1]), + (args + as + &[ArgumentV1; 0])) + as + Arguments) + } + } as Arguments)) as String); diff --git a/src/test/ui/attributes/key-value-expansion.stderr b/src/test/ui/attributes/key-value-expansion.stderr index 31e93ef54f2..03ca515265c 100644 --- a/src/test/ui/attributes/key-value-expansion.stderr +++ b/src/test/ui/attributes/key-value-expansion.stderr @@ -17,12 +17,16 @@ LL | bug!(); error: unexpected token: `{ let res = - ::alloc::fmt::format(::core::fmt::Arguments::new_v1(&[""], - &match (&"u8",) { - (arg0,) => - [::core::fmt::ArgumentV1::new(arg0, - ::core::fmt::Display::fmt)], - })); + ::alloc::fmt::format(match match (&"u8",) { + (arg0,) => + [::core::fmt::ArgumentV1::new(arg0, + ::core::fmt::Display::fmt)], + } { + ref args => unsafe { + ::core::fmt::Arguments::new_v1(&[""], + args) + } + }); res }.as_str()` --> $DIR/key-value-expansion.rs:48:23 -- 2.44.0