Skip to content

core: make the public fmt API completely safe. #19506

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

Merged
merged 3 commits into from
Dec 8, 2014
Merged
Show file tree
Hide file tree
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
89 changes: 53 additions & 36 deletions src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ pub struct Formatter<'a> {
args: &'a [Argument<'a>],
}

// NB. Argument is essentially an optimized partially applied formatting function,
// equivalent to `exists T.(&T, fn(&T, &mut Formatter) -> Result`.

enum Void {}

/// This struct represents the generic "argument" which is taken by the Xprintf
Expand All @@ -100,37 +103,66 @@ enum Void {}
/// types, and then this struct is used to canonicalize arguments to one type.
#[experimental = "implementation detail of the `format_args!` macro"]
pub struct Argument<'a> {
formatter: extern "Rust" fn(&Void, &mut Formatter) -> Result,
value: &'a Void,
formatter: fn(&Void, &mut Formatter) -> Result,
}

impl<'a> Argument<'a> {
#[inline(never)]
fn show_uint(x: &uint, f: &mut Formatter) -> Result {
Show::fmt(x, f)
}

fn new<'a, T>(x: &'a T, f: fn(&T, &mut Formatter) -> Result) -> Argument<'a> {
unsafe {
Argument {
formatter: mem::transmute(f),
value: mem::transmute(x)
}
}
}

fn from_uint<'a>(x: &'a uint) -> Argument<'a> {
Argument::new(x, Argument::show_uint)
}

fn as_uint(&self) -> Option<uint> {
if self.formatter as uint == Argument::show_uint as uint {
Some(unsafe { *(self.value as *const _ as *const uint) })
} else {
None
}
}
}

impl<'a> Arguments<'a> {
/// When using the format_args!() macro, this function is used to generate the
/// Arguments structure. The compiler inserts an `unsafe` block to call this,
/// which is valid because the compiler performs all necessary validation to
/// ensure that the resulting call to format/write would be safe.
/// Arguments structure.
#[doc(hidden)] #[inline]
#[experimental = "implementation detail of the `format_args!` macro"]
pub unsafe fn new<'a>(pieces: &'static [&'static str],
args: &'a [Argument<'a>]) -> Arguments<'a> {
pub fn new<'a>(pieces: &'a [&'a str],
args: &'a [Argument<'a>]) -> Arguments<'a> {
Arguments {
pieces: mem::transmute(pieces),
pieces: pieces,
fmt: None,
args: args
}
}

/// This function is used to specify nonstandard formatting parameters.
/// The `pieces` array must be at least as long as `fmt` to construct
/// a valid Arguments structure.
/// a valid Arguments structure. Also, any `Count` within `fmt` that is
/// `CountIsParam` or `CountIsNextParam` has to point to an argument
/// created with `argumentuint`. However, failing to do so doesn't cause
/// unsafety, but will ignore invalid .
#[doc(hidden)] #[inline]
#[experimental = "implementation detail of the `format_args!` macro"]
pub unsafe fn with_placeholders<'a>(pieces: &'static [&'static str],
fmt: &'static [rt::Argument<'static>],
args: &'a [Argument<'a>]) -> Arguments<'a> {
pub fn with_placeholders<'a>(pieces: &'a [&'a str],
fmt: &'a [rt::Argument<'a>],
args: &'a [Argument<'a>]) -> Arguments<'a> {
Arguments {
pieces: mem::transmute(pieces),
fmt: Some(mem::transmute(fmt)),
pieces: pieces,
fmt: Some(fmt),
args: args
}
}
Expand Down Expand Up @@ -312,15 +344,13 @@ impl<'a> Formatter<'a> {

fn getcount(&mut self, cnt: &rt::Count) -> Option<uint> {
match *cnt {
rt::CountIs(n) => { Some(n) }
rt::CountImplied => { None }
rt::CountIs(n) => Some(n),
rt::CountImplied => None,
rt::CountIsParam(i) => {
let v = self.args[i].value;
unsafe { Some(*(v as *const _ as *const uint)) }
self.args[i].as_uint()
}
rt::CountIsNextParam => {
let v = self.curarg.next().unwrap().value;
unsafe { Some(*(v as *const _ as *const uint)) }
self.curarg.next().and_then(|arg| arg.as_uint())
}
}
}
Expand Down Expand Up @@ -533,30 +563,17 @@ impl Show for Error {
/// create the Argument structures that are passed into the `format` function.
#[doc(hidden)] #[inline]
#[experimental = "implementation detail of the `format_args!` macro"]
pub fn argument<'a, T>(f: extern "Rust" fn(&T, &mut Formatter) -> Result,
pub fn argument<'a, T>(f: fn(&T, &mut Formatter) -> Result,
t: &'a T) -> Argument<'a> {
unsafe {
Argument {
formatter: mem::transmute(f),
value: mem::transmute(t)
}
}
}

/// When the compiler determines that the type of an argument *must* be a string
/// (such as for select), then it invokes this method.
#[doc(hidden)] #[inline]
#[experimental = "implementation detail of the `format_args!` macro"]
pub fn argumentstr<'a>(s: &'a &str) -> Argument<'a> {
argument(Show::fmt, s)
Argument::new(t, f)
}

/// When the compiler determines that the type of an argument *must* be a uint
/// (such as for plural), then it invokes this method.
/// (such as for width and precision), then it invokes this method.
#[doc(hidden)] #[inline]
#[experimental = "implementation detail of the `format_args!` macro"]
pub fn argumentuint<'a>(s: &'a uint) -> Argument<'a> {
argument(Show::fmt, s)
Argument::from_uint(s)
}

// Implementations of the core formatting traits
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ pub use core::fmt::Error;
pub use core::fmt::{Argument, Arguments, write, radix, Radix, RadixFmt};

#[doc(hidden)]
pub use core::fmt::{argument, argumentstr, argumentuint};
pub use core::fmt::{argument, argumentuint};

/// The format function takes a precompiled format string and a list of
/// arguments, to return the resulting formatted string.
Expand Down
33 changes: 8 additions & 25 deletions src/libsyntax/ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,17 +577,11 @@ impl<'a, 'b> Context<'a, 'b> {
}

// Now create a vector containing all the arguments
let slicename = self.ecx.ident_of("__args_vec");
{
let args = names.into_iter().map(|a| a.unwrap());
let args = locals.into_iter().chain(args);
let args = self.ecx.expr_vec_slice(self.fmtsp, args.collect());
lets.push(self.ecx.stmt_let(self.fmtsp, false, slicename, args));
}
let args = locals.into_iter().chain(names.into_iter().map(|a| a.unwrap()));

// Now create the fmt::Arguments struct with all our locals we created.
let pieces = self.ecx.expr_ident(self.fmtsp, static_str_name);
let args_slice = self.ecx.expr_ident(self.fmtsp, slicename);
let args_slice = self.ecx.expr_vec_slice(self.fmtsp, args.collect());

let (fn_name, fn_args) = if self.all_pieces_simple {
("new", vec![pieces, args_slice])
Expand All @@ -602,29 +596,18 @@ impl<'a, 'b> Context<'a, 'b> {
self.ecx.ident_of("Arguments"),
self.ecx.ident_of(fn_name)), fn_args);

// We did all the work of making sure that the arguments
// structure is safe, so we can safely have an unsafe block.
let result = self.ecx.expr_block(P(ast::Block {
view_items: Vec::new(),
stmts: Vec::new(),
expr: Some(result),
id: ast::DUMMY_NODE_ID,
rules: ast::UnsafeBlock(ast::CompilerGenerated),
span: self.fmtsp,
}));
let resname = self.ecx.ident_of("__args");
lets.push(self.ecx.stmt_let(self.fmtsp, false, resname, result));
let res = self.ecx.expr_ident(self.fmtsp, resname);
let result = match invocation {
Call(e) => {
let span = e.span;
self.ecx.expr_call(span, e,
vec!(self.ecx.expr_addr_of(span, res)))
self.ecx.expr_call(span, e, vec![
self.ecx.expr_addr_of(span, result)
])
}
MethodCall(e, m) => {
let span = e.span;
self.ecx.expr_method_call(span, e, m,
vec!(self.ecx.expr_addr_of(span, res)))
self.ecx.expr_method_call(span, e, m, vec![
self.ecx.expr_addr_of(span, result)
])
}
};
let body = self.ecx.expr_block(self.ecx.block(self.fmtsp, lets,
Expand Down
25 changes: 10 additions & 15 deletions src/test/pretty/issue-4264.pp
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,6 @@
static __STATIC_FMTSTR: &'static [&'static str] =
(&([("test" as &'static str)] as [&'static str, ..1]) as
&'static [&'static str, ..1]);
let __args_vec =
(&([] as [core::fmt::Argument<'_>, ..0]) as
&[core::fmt::Argument<'_>, ..0]);
let __args =
(unsafe {
((::std::fmt::Arguments::new as
unsafe fn(&'static [&'static str], &'a [core::fmt::Argument<'a>]) -> core::fmt::Arguments<'a>)((__STATIC_FMTSTR
as
&'static [&'static str]),
(__args_vec
as
&[core::fmt::Argument<'_>, ..0]))
as core::fmt::Arguments<'_>)
} as core::fmt::Arguments<'_>);



Expand All @@ -64,7 +50,16 @@


((::std::fmt::format as
fn(&core::fmt::Arguments<'_>) -> collections::string::String)((&(__args
fn(&core::fmt::Arguments<'_>) -> collections::string::String)((&((::std::fmt::Arguments::new
as
fn(&'a [&'a str], &'a [core::fmt::Argument<'a>]) -> core::fmt::Arguments<'a>)((__STATIC_FMTSTR
as
&'static [&'static str]),
(&([]
as
[core::fmt::Argument<'_>, ..0])
as
&[core::fmt::Argument<'_>, ..0]))
as
core::fmt::Arguments<'_>)
as
Expand Down