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 15, 2015
2 parents af1c39c + 12706c9 commit f637fda
Show file tree
Hide file tree
Showing 11 changed files with 308 additions and 17 deletions.
29 changes: 29 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,35 @@ if [ -n "$CFG_ENABLE_DEBUG" ]; then
CFG_ENABLE_DEBUG_JEMALLOC=1
fi

at_most_one_opt() {
local ARG=$1
OP=$(echo $ARG | tr 'a-z-' 'A-Z_')
DISABLE_NAME=\$CFG_DISABLE_${OP}
ENABLE_NAME=\$CFG_ENABLE_${OP}
eval VD=$DISABLE_NAME
eval VE=$ENABLE_NAME
if [ -n "${VD}" ] && [ -n "${VE}" ]; then
msg "configure script bug: at most one of"
msg "$DISABLE_NAME and $ENABLE_NAME may be set;"
msg "you should report this as a bug, supplying your"
msg "CFG_CONFIGURE_ARGS=$CFG_CONFIGURE_ARGS"
err "configure script bug, at_most_one_opt $ARG"
fi
}

# At most one of each of the following enable/disable option pairs
# should be set. Check that. (i.e. if we are going to have some
# prioritization scheme, e.g. based on order of inputs to configure,
# then such prioritization should be explicitly applied before this
# point.)
at_most_one_opt optimize
at_most_one_opt optimize-cxx
at_most_one_opt optimize-llvm
at_most_one_opt llvm-assertions
at_most_one_opt debug-assertions
at_most_one_opt debuginfo
at_most_one_opt debug-jemalloc

# OK, now write the debugging options
if [ -n "$CFG_DISABLE_OPTIMIZE" ]; then putvar CFG_DISABLE_OPTIMIZE; fi
if [ -n "$CFG_DISABLE_OPTIMIZE_CXX" ]; then putvar CFG_DISABLE_OPTIMIZE_CXX; fi
Expand Down
3 changes: 3 additions & 0 deletions src/libcollections/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,6 @@ pub fn format(args: Arguments) -> string::String {
let _ = write!(&mut output, "{}", args);
output
}

#[unstable(feature = "ensure_not_fmt_string_literal")]
pub use ::core::fmt::ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE;
1 change: 1 addition & 0 deletions src/libcollections/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#![feature(slice_patterns)]
#![feature(debug_builders)]
#![feature(utf8_error)]
#![feature(ensure_not_fmt_string_literal)] // (referenced in expansions)
#![cfg_attr(test, feature(rand, rustc_private, test, hash, collections))]
#![cfg_attr(test, allow(deprecated))] // rand

Expand Down
11 changes: 11 additions & 0 deletions src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,5 +1084,16 @@ impl<'b, T: Debug> Debug for RefMut<'b, T> {
}
}

/// This constant is a trick to force `ensure_not_fmt_string_literal!`
/// to be treated as unstable whenever it occurs outside a macro
/// marked with `#[allow_internal_unstable]`.
///
/// This constant really should not ever be stabilized; if we ever
/// decide to stabilize the `ensure_not_fmt_string_literal!` macro
/// itself, then we should remove its use of this constant (and then
/// remove this constant).
#[unstable(feature = "ensure_not_fmt_string_literal")]
pub const ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE: () = ();

// If you expected tests to be here, look instead at the run-pass/ifmt.rs test,
// it's a lot easier than creating all of the rt::Piece structures here.
14 changes: 12 additions & 2 deletions src/libcore/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,23 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// SNAP 5520801
#[cfg(stage0)]
#[macro_export]
macro_rules! __unstable_rustc_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) =
(__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1,
file!(), line!());
::core::panicking::panic(&_MSG_FILE_LINE)
});
($fmt:expr, $($arg:tt)*) => ({
Expand Down Expand Up @@ -56,7 +65,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 { ... })")
}
}
21 changes: 15 additions & 6 deletions 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! __unstable_rustc_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,8 @@ macro_rules! panic {
panic!("explicit panic")
});
($msg:expr) => ({
$crate::rt::begin_unwind($msg, {
$crate::rt::begin_unwind(
__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, {
// static requires less code at runtime, more constant data
static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize);
&_FILE_LINE
Expand Down Expand Up @@ -90,11 +98,12 @@ macro_rules! panic {
panic!("explicit panic")
});
($msg:expr) => ({
$crate::rt::begin_unwind($msg, {
// static requires less code at runtime, more constant data
static _FILE_LINE: (&'static str, u32) = (file!(), line!());
&_FILE_LINE
})
$crate::rt::begin_unwind(
__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), {
// static requires less code at runtime, more constant data
static _FILE_LINE: (&'static str, u32) = (file!(), line!());
&_FILE_LINE
})
});
($fmt:expr, $($arg:tt)+) => ({
$crate::rt::begin_unwind_fmt(format_args!($fmt, $($arg)+), {
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("__unstable_rustc_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
105 changes: 105 additions & 0 deletions src/libsyntax/ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,111 @@ 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 warning = |cx: &ExtCtxt, c: char| {
cx.span_warn(sp, &format!("{} literal argument contains `{}`", name, c));
cx.span_note(sp, "Is it meant to be a `format!` string?");
cx.span_help(sp, "You can wrap the argument in parentheses \
to sidestep this warning");
};

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)) => {
for c in lit_str.chars() {
if c == '{' || c == '}' {
warning(cx, c);
break;
}
}
// we still return the expr itself, to allow catching of
// further errors in the input.
expr
}
};

let unstable_marker = cx.expr_path(cx.path_global(sp, vec![
cx.ident_of_std("core"),
cx.ident_of("fmt"),
cx.ident_of("ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE")]));

// Total hack: We do not (yet) have hygienic-marking of stabilty.
// Thus an unstable macro (like `ensure_not_fmt_string!`) can leak
// through another macro (like `panic!`), where the latter is just
// using the former as an implementation detail.
//
// The `#[allow_internal_unstable]` system does not suffice to
// address this; it explicitly (as described on Issue #22899)
// disallows the use of unstable functionality via a helper macro
// like `ensure_not_fmt_string!`, by design.
//
// So, the hack: the `ensure_not_fmt_string!` macro has just one
// stable client: `panic!`. So we give `panic!` a backdoor: we
// allow its name literal string to give it stable access. Any
// other argument that is passed in will cause us to emit the
// unstable-marker, which will then be checked against the enabled
// feature-set.
//
// This, combined with the awkward actual name of the unstable
// macro (hint: the real name is far more awkward than the one
// given in this comment) should suffice to ensure that people do
// not accidentally commit to using it.
let marker = if name == "unary `panic!`" {
cx.expr_tuple(sp, vec![]) // i.e. `()`
} else {
unstable_marker
};
let expr = cx.expr_tuple(sp, vec![marker, 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
25 changes: 25 additions & 0 deletions src/test/compile-fail/issue-22932-ensure-helper-unstable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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.

// This test is ensuring that the `ensure_not_fmt_string_literal!`
// macro cannot be invoked (at least not in code whose expansion ends
// up in the expanded output) without the appropriate feature.

pub fn f0() {
panic!("this should work");
}

pub fn main() {
__unstable_rustc_ensure_not_fmt_string_literal!(
"`main`", "this should work, but its unstable");
//~^^ ERROR use of unstable library feature 'ensure_not_fmt_string_literal'
//~| HELP add #![feature(ensure_not_fmt_string_literal)] to the crate attributes to enable
f0();
}
Loading

0 comments on commit f637fda

Please sign in to comment.