Skip to content

Commit

Permalink
Auto merge of #24370 - pnkfelix:fsk-unary-panic, r=<try>
Browse files Browse the repository at this point in the history
Ensure a sole string-literal passed to `panic!` is not a fmt string.

To accomplish this, adds `ensure_not_fmt_string_literal!` macro that will fail the compile if its expression argument is a fmt string literal.

Since this is making a certain kind of use of `panic!` illegal, it is a:

[breaking-change]

In particular, a panic like this:

```rust
panic!("Is it stringified code: { or is it a ill-formed fmt arg? }");
```

must be rewritten; one easy rewrite is to add parentheses:

```rust
panic!(("Is it stringified code: { or is it a ill-formed fmt arg? }"));
```

----

Fix #22932.
  • Loading branch information
bors committed Apr 13, 2015
2 parents f55e66a + 08079d5 commit 0ec30ad
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 12 deletions.
13 changes: 11 additions & 2 deletions src/libcore/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,22 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// SNAP 5520801
#[cfg(stage0)]
#[macro_export]
macro_rules! ensure_not_fmt_string_literal {
($name:expr, $e:expr) => { $e }
}

/// Entry point of task panic, for details, see std::macros
#[macro_export]
macro_rules! panic {
() => (
panic!("explicit panic")
);
($msg:expr) => ({
static _MSG_FILE_LINE: (&'static str, &'static str, u32) = ($msg, file!(), line!());
static _MSG_FILE_LINE: (&'static str, &'static str, u32) =
(ensure_not_fmt_string_literal!("panic!", $msg), file!(), line!());
::core::panicking::panic(&_MSG_FILE_LINE)
});
($fmt:expr, $($arg:tt)*) => ({
Expand Down Expand Up @@ -56,7 +64,8 @@ macro_rules! panic {
macro_rules! assert {
($cond:expr) => (
if !$cond {
panic!(concat!("assertion failed: ", stringify!($cond)))
const MSG: &'static str = concat!("assertion failed: ", stringify!($cond));
panic!(MSG)
}
);
($cond:expr, $($arg:tt)+) => (
Expand Down
2 changes: 1 addition & 1 deletion src/libcoretest/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,6 @@ fn test_match_option_string() {
let five = "Five".to_string();
match Some(five) {
Some(s) => assert_eq!(s, "Five"),
None => panic!("unexpected None while matching on Some(String { ... })")
None => panic!("{}", "unexpected None while matching on Some(String { ... })")
}
}
9 changes: 8 additions & 1 deletion src/libstd/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@
#![unstable(feature = "std_misc")]

// SNAP 5520801
#[cfg(stage0)]
#[macro_export]
macro_rules! ensure_not_fmt_string_literal {
($name:expr, $e:expr) => { $e }
}

/// The entry point for panic of Rust tasks.
///
/// This macro is used to inject panic into a Rust task, causing the task to
Expand Down Expand Up @@ -44,7 +51,7 @@ macro_rules! panic {
panic!("explicit panic")
});
($msg:expr) => ({
$crate::rt::begin_unwind($msg, {
$crate::rt::begin_unwind(ensure_not_fmt_string_literal!("panic!", $msg), {
// static requires less code at runtime, more constant data
static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize);
&_FILE_LINE
Expand Down
38 changes: 30 additions & 8 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>)
syntax_expanders.insert(intern("format_args"),
// format_args uses `unstable` things internally.
NormalTT(Box::new(ext::format::expand_format_args), None, true));
syntax_expanders.insert(intern("ensure_not_fmt_string_literal"),
builtin_normal_expander(
ext::format::ensure_not_fmt_string_literal));
syntax_expanders.insert(intern("env"),
builtin_normal_expander(
ext::env::expand_env));
Expand Down Expand Up @@ -748,19 +751,38 @@ impl<'a> ExtCtxt<'a> {
}
}

pub type ExprStringLitResult =
Result<(P<ast::Expr>, InternedString, ast::StrStyle), P<ast::Expr>>;

/// Extract a string literal from macro expanded version of `expr`.
///
/// if `expr` is not string literal, then Err with span for expanded
/// input, but does not emit any messages nor stop compilation.
pub fn expr_string_lit(cx: &mut ExtCtxt, expr: P<ast::Expr>) -> ExprStringLitResult
{
// we want to be able to handle e.g. concat("foo", "bar")
let expr = cx.expander().fold_expr(expr);
let lit = match expr.node {
ast::ExprLit(ref l) => match l.node {
ast::LitStr(ref s, style) => Some(((*s).clone(), style)),
_ => None
},
_ => None
};
match lit {
Some(lit) => Ok((expr, lit.0, lit.1)),
None => Err(expr),
}
}

/// Extract a string literal from the macro expanded version of `expr`,
/// emitting `err_msg` if `expr` is not a string literal. This does not stop
/// compilation on error, merely emits a non-fatal error and returns None.
pub fn expr_to_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
-> Option<(InternedString, ast::StrStyle)> {
// we want to be able to handle e.g. concat("foo", "bar")
let expr = cx.expander().fold_expr(expr);
match expr.node {
ast::ExprLit(ref l) => match l.node {
ast::LitStr(ref s, style) => return Some(((*s).clone(), style)),
_ => cx.span_err(l.span, err_msg)
},
_ => cx.span_err(expr.span, err_msg)
match expr_string_lit(cx, expr) {
Ok((_, s, style)) => return Some((s, style)),
Err(e) => cx.span_err(e.span, err_msg),
}
None
}
Expand Down
65 changes: 65 additions & 0 deletions src/libsyntax/ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,71 @@ impl<'a, 'b> Context<'a, 'b> {
}
}

/// Expands `ensure_not_fmt_string_literal!(<where-literal>, <expr>)`
/// into `<expr>`, but ensures that if `<expr>` is a string-literal,
/// then it is not a potential format string literal.
pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
sp: Span,
tts: &[ast::TokenTree])
-> Box<base::MacResult+'cx> {
use fold::Folder;
let takes_two_args = |cx: &ExtCtxt, rest| {
cx.span_err(sp, &format!("`ensure_not_fmt_string_literal!` \
takes 2 arguments, {}", rest));
DummyResult::expr(sp)
};
let mut p = cx.new_parser_from_tts(tts);
if p.token == token::Eof { return takes_two_args(cx, "given 0"); }
let arg1 = cx.expander().fold_expr(p.parse_expr());
if p.token == token::Eof { return takes_two_args(cx, "given 1"); }
if !panictry!(p.eat(&token::Comma)) { return takes_two_args(cx, "comma-separated"); }
if p.token == token::Eof { return takes_two_args(cx, "given 1"); }
let arg2 = cx.expander().fold_expr(p.parse_expr());
if p.token != token::Eof {
takes_two_args(cx, "given too many");
// (but do not return; handle two provided, nonetheless)
}

// First argument is name of where this was invoked (for error messages).
let lit_str_with_extended_lifetime;
let name: &str = match expr_string_lit(cx, arg1) {
Ok((_, lit_str, _)) => {
lit_str_with_extended_lifetime = lit_str;
&lit_str_with_extended_lifetime
}
Err(expr) => {
let msg = "first argument to `ensure_not_fmt_string_literal!` \
must be string literal";
cx.span_err(expr.span, msg);
// Compile proceeds using "ensure_not_fmt_string_literal"
// as the name.
"ensure_not_fmt_string_literal!"
}
};

// Second argument is the expression we are checking.
let expr = match expr_string_lit(cx, arg2) {
Err(expr) => {
// input was not a string literal; just ignore it.
expr
}
Ok((expr, lit_str, _style)) => {
let m = format!("{} input cannot be format string literal", name);
for c in lit_str.chars() {
if c == '{' || c == '}' {
cx.span_err(sp, &m);
break;
}
}
// we still return the expr itself, to allow catching of
// further errors in the input.
expr
}
};

MacEager::expr(expr)
}

pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt, sp: Span,
tts: &[ast::TokenTree])
-> Box<base::MacResult+'cx> {
Expand Down
51 changes: 51 additions & 0 deletions src/test/compile-fail/issue-22932.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2015 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.

// Issue 22932: `panic!("{}");` should not compile.

pub fn f1() { panic!("this does not work {}");
//~^ ERROR panic! input cannot be format string literal
}

pub fn workaround_1() {
panic!(("This *does* works {}"));
}

pub fn workaround_2() {
const MSG: &'static str = "This *does* work {}";
panic!(MSG);
}

pub fn f2() { panic!("this does not work {");
//~^ ERROR panic! input cannot be format string literal
}

pub fn f3() { panic!("nor this }");
//~^ ERROR panic! input cannot be format string literal
}

pub fn f4() { panic!("nor this {{");
//~^ ERROR panic! input cannot be format string literal
}

pub fn f5() { panic!("nor this }}");
//~^ ERROR panic! input cannot be format string literal
}

pub fn f0_a() {
ensure_not_fmt_string_literal!("`f0_a`", "this does not work {}");
//~^ ERROR `f0_a` input cannot be format string literal
}

pub fn f0_b() {
println!(ensure_not_fmt_string_literal!("`f0_b`", "this does work"));
}

pub fn main() {}

0 comments on commit 0ec30ad

Please sign in to comment.