Merge #9823

9823: fix: avoid pathological macro expansions  r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Cargo.lock

@@ -508,6 +508,7 @@ version = "0.0.0"
 dependencies = [
  "base_db",
  "cfg",
+ "cov-mark",
  "either",
  "expect-test",
  "la-arena",

crates/cfg/src/tests.rs

@@ -1,5 +1,5 @@
 use expect_test::{expect, Expect};
-use mbe::ast_to_token_tree;
+use mbe::syntax_node_to_token_tree;
 use syntax::{ast, AstNode};
 
 use crate::{CfgAtom, CfgExpr, CfgOptions, DnfExpr};
@@ -8,7 +8,7 @@ fn assert_parse_result(input: &str, expected: CfgExpr) {
     let (tt, _) = {
         let source_file = ast::SourceFile::parse(input).ok().unwrap();
         let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-        ast_to_token_tree(&tt)
+        syntax_node_to_token_tree(tt.syntax())
     };
     let cfg = CfgExpr::parse(&tt);
     assert_eq!(cfg, expected);
@@ -18,7 +18,7 @@ fn check_dnf(input: &str, expect: Expect) {
     let (tt, _) = {
         let source_file = ast::SourceFile::parse(input).ok().unwrap();
         let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-        ast_to_token_tree(&tt)
+        syntax_node_to_token_tree(tt.syntax())
     };
     let cfg = CfgExpr::parse(&tt);
     let actual = format!("#![cfg({})]", DnfExpr::new(cfg));
@@ -29,7 +29,7 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
     let (tt, _) = {
         let source_file = ast::SourceFile::parse(input).ok().unwrap();
         let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-        ast_to_token_tree(&tt)
+        syntax_node_to_token_tree(tt.syntax())
     };
     let cfg = CfgExpr::parse(&tt);
     let dnf = DnfExpr::new(cfg);
@@ -42,7 +42,7 @@ fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
     let (tt, _) = {
         let source_file = ast::SourceFile::parse(input).ok().unwrap();
         let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-        ast_to_token_tree(&tt)
+        syntax_node_to_token_tree(tt.syntax())
     };
     let cfg = CfgExpr::parse(&tt);
     let dnf = DnfExpr::new(cfg);

crates/hir_def/src/attr.rs

@@ -12,7 +12,7 @@ use either::Either;
 use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
 use itertools::Itertools;
 use la_arena::ArenaMap;
-use mbe::{ast_to_token_tree, DelimiterKind};
+use mbe::{syntax_node_to_token_tree, DelimiterKind};
 use smallvec::{smallvec, SmallVec};
 use syntax::{
     ast::{self, AstNode, AttrsOwner},
@@ -679,7 +679,7 @@ impl Attr {
             };
             Some(Interned::new(AttrInput::Literal(value)))
         } else if let Some(tt) = ast.token_tree() {
-            Some(Interned::new(AttrInput::TokenTree(ast_to_token_tree(&tt).0)))
+            Some(Interned::new(AttrInput::TokenTree(syntax_node_to_token_tree(tt.syntax()).0)))
         } else {
             None
         };

crates/hir_def/src/nameres/tests/macros.rs

@@ -1,4 +1,5 @@
 use super::*;
+
 use crate::nameres::proc_macro::{ProcMacroDef, ProcMacroKind};
 
 #[test]
@@ -1021,3 +1022,20 @@ pub mod prelude {
         "#]],
     )
 }
+
+#[test]
+fn issue9358_bad_macro_stack_overflow() {
+    cov_mark::check!(issue9358_bad_macro_stack_overflow);
+    check(
+        r#"
+macro_rules! m {
+  ($cond:expr) => { m!($cond, stringify!($cond)) };
+  ($cond:expr, $($arg:tt)*) => { $cond };
+}
+m!(
+"#,
+        expect![[r#"
+            crate
+        "#]],
+    )
+}

crates/hir_expand/Cargo.toml

@@ -9,6 +9,7 @@ edition = "2018"
 doctest = false
 
 [dependencies]
+cov-mark = "2.0.0-pre.1"
 log = "0.4.8"
 either = "1.5.3"
 rustc-hash = "1.0.0"

crates/hir_expand/src/builtin_macro.rs

@@ -610,7 +610,7 @@ mod tests {
 
                 let fragment = crate::to_fragment_kind(&macro_call);
                 let args = macro_call.token_tree().unwrap();
-                let parsed_args = mbe::ast_to_token_tree(&args).0;
+                let parsed_args = mbe::syntax_node_to_token_tree(args.syntax()).0;
                 let call_id = AstId::new(file_id.into(), ast_id_map.ast_id(&macro_call));
 
                 let arg_id = db.intern_macro(MacroCallLoc {

crates/hir_expand/src/db.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 use base_db::{salsa, SourceDatabase};
 use limit::Limit;
 use mbe::{ExpandError, ExpandResult};
-use parser::FragmentKind;
+use parser::{FragmentKind, T};
 use syntax::{
     algo::diff,
     ast::{self, NameOwner},
@@ -273,6 +273,23 @@ fn macro_arg_text(db: &dyn AstDatabase, id: MacroCallId) -> Option<GreenNode> {
     let loc = db.lookup_intern_macro(id);
     let arg = loc.kind.arg(db)?;
     let arg = process_macro_input(db, arg, id);
+    if matches!(loc.kind, MacroCallKind::FnLike { .. }) {
+        let first = arg.first_child_or_token().map_or(T![.], |it| it.kind());
+        let last = arg.last_child_or_token().map_or(T![.], |it| it.kind());
+        let well_formed_tt =
+            matches!((first, last), (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']));
+        if !well_formed_tt {
+            // Don't expand malformed (unbalanced) macro invocations. This is
+            // less than ideal, but trying to expand unbalanced  macro calls
+            // sometimes produces pathological, deeply nested code which breaks
+            // all kinds of things.
+            //
+            // Some day, we'll have explicit recursion counters for all
+            // recursive things, at which point this code might be removed.
+            cov_mark::hit!(issue9358_bad_macro_stack_overflow);
+            return None;
+        }
+    }
     Some(arg.green().into())
 }
 
@@ -281,7 +298,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option<Arc<TokenExpander>>
         MacroDefKind::Declarative(ast_id) => match ast_id.to_node(db) {
             ast::Macro::MacroRules(macro_rules) => {
                 let arg = macro_rules.token_tree()?;
-                let (tt, def_site_token_map) = mbe::ast_to_token_tree(&arg);
+                let (tt, def_site_token_map) = mbe::syntax_node_to_token_tree(arg.syntax());
                 let mac = match mbe::MacroRules::parse(&tt) {
                     Ok(it) => it,
                     Err(err) => {
@@ -294,7 +311,7 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option<Arc<TokenExpander>>
             }
             ast::Macro::MacroDef(macro_def) => {
                 let arg = macro_def.body()?;
-                let (tt, def_site_token_map) = mbe::ast_to_token_tree(&arg);
+                let (tt, def_site_token_map) = mbe::syntax_node_to_token_tree(arg.syntax());
                 let mac = match mbe::MacroDef::parse(&tt) {
                     Ok(it) => it,
                     Err(err) => {

crates/hir_expand/src/eager.rs

@@ -107,7 +107,7 @@ pub fn expand_eager_macro(
     mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
 ) -> Result<MacroCallId, ErrorEmitted> {
     let parsed_args = diagnostic_sink.option_with(
-        || Some(mbe::ast_to_token_tree(&macro_call.value.token_tree()?).0),
+        || Some(mbe::syntax_node_to_token_tree(&macro_call.value.token_tree()?.syntax()).0),
         || err("malformed macro invocation"),
     )?;
 

crates/ide_completion/src/tests/expression.rs

@@ -308,27 +308,7 @@ fn quux(x: i32) {
     m!(x$0
 }
 "#,
-        expect![[r#"
-            kw unsafe
-            kw match
-            kw while
-            kw while let
-            kw loop
-            kw if
-            kw if let
-            kw for
-            kw true
-            kw false
-            kw return
-            kw self
-            kw super
-            kw crate
-            lc y         i32
-            bt u32
-            lc x         i32
-            fn quux(…)   fn(i32)
-            ma m!(…)     macro_rules! m
-        "#]],
+        expect![[r#""#]],
     );
 }
 

crates/ide_ssr/src/tests.rs

@@ -921,13 +921,13 @@ fn preserves_whitespace_within_macro_expansion() {
             macro_rules! macro1 {
                 ($a:expr) => {$a}
             }
-            fn f() {macro1!(1   *   2 + 3 + 4}
+            fn f() {macro1!(1   *   2 + 3 + 4)}
             "#,
         expect![[r#"
             macro_rules! macro1 {
                 ($a:expr) => {$a}
             }
-            fn f() {macro1!(4 - (3 - 1   *   2)}
+            fn f() {macro1!(4 - (3 - 1   *   2))}
             "#]],
     )
 }

crates/mbe/src/benchmark.rs

@@ -8,9 +8,8 @@ use syntax::{
 use test_utils::{bench, bench_fixture, skip_slow_tests};
 
 use crate::{
-    ast_to_token_tree,
     parser::{Op, RepeatKind, Separator},
-    MacroRules,
+    syntax_node_to_token_tree, MacroRules,
 };
 
 #[test]
@@ -65,7 +64,7 @@ fn macro_rules_fixtures_tt() -> FxHashMap<String, tt::Subtree> {
         .filter_map(ast::MacroRules::cast)
         .map(|rule| {
             let id = rule.name().unwrap().to_string();
-            let (def_tt, _) = ast_to_token_tree(&rule.token_tree().unwrap());
+            let (def_tt, _) = syntax_node_to_token_tree(rule.token_tree().unwrap().syntax());
             (id, def_tt)
         })
         .collect()

crates/mbe/src/expander.rs

@@ -120,7 +120,7 @@ mod tests {
     use syntax::{ast, AstNode};
 
     use super::*;
-    use crate::ast_to_token_tree;
+    use crate::syntax_node_to_token_tree;
 
     #[test]
     fn test_expand_rule() {
@@ -159,7 +159,8 @@ mod tests {
         let macro_definition =
             source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap();
 
-        let (definition_tt, _) = ast_to_token_tree(&macro_definition.token_tree().unwrap());
+        let (definition_tt, _) =
+            syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax());
         crate::MacroRules::parse(&definition_tt).unwrap()
     }
 
@@ -168,7 +169,8 @@ mod tests {
         let macro_invocation =
             source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
 
-        let (invocation_tt, _) = ast_to_token_tree(&macro_invocation.token_tree().unwrap());
+        let (invocation_tt, _) =
+            syntax_node_to_token_tree(macro_invocation.token_tree().unwrap().syntax());
 
         expand_rules(&rules.rules, &invocation_tt)
     }

crates/mbe/src/lib.rs

@@ -66,7 +66,7 @@ impl fmt::Display for ExpandError {
 
 pub use crate::{
     syntax_bridge::{
-        ast_to_token_tree, parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree,
+        parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree,
         token_tree_to_syntax_node,
     },
     token_map::TokenMap,

crates/mbe/src/syntax_bridge.rs

@@ -13,12 +13,6 @@ use tt::buffer::{Cursor, TokenBuffer};
 use crate::{subtree_source::SubtreeTokenSource, tt_iter::TtIter};
 use crate::{ExpandError, TokenMap};
 
-/// Convert the syntax tree (what user has written) to a `TokenTree` (what macro
-/// will consume).
-pub fn ast_to_token_tree(ast: &impl ast::AstNode) -> (tt::Subtree, TokenMap) {
-    syntax_node_to_token_tree(ast.syntax())
-}
-
 /// Convert the syntax node to a `TokenTree` (what macro
 /// will consume).
 pub fn syntax_node_to_token_tree(node: &SyntaxNode) -> (tt::Subtree, TokenMap) {
@@ -812,7 +806,7 @@ mod tests {
         // - T!['}']
         // - WHITE_SPACE
         let token_tree = ast::TokenTree::cast(token_tree).unwrap();
-        let tt = ast_to_token_tree(&token_tree).0;
+        let tt = syntax_node_to_token_tree(token_tree.syntax()).0;
 
         assert_eq!(tt.delimiter_kind(), Some(tt::DelimiterKind::Brace));
     }
@@ -821,7 +815,7 @@ mod tests {
     fn test_token_tree_multi_char_punct() {
         let source_file = ast::SourceFile::parse("struct Foo { a: x::Y }").ok().unwrap();
         let struct_def = source_file.syntax().descendants().find_map(ast::Struct::cast).unwrap();
-        let tt = ast_to_token_tree(&struct_def).0;
+        let tt = syntax_node_to_token_tree(struct_def.syntax()).0;
         token_tree_to_syntax_node(&tt, FragmentKind::Item).unwrap();
     }
 
@@ -829,7 +823,7 @@ mod tests {
     fn test_missing_closing_delim() {
         let source_file = ast::SourceFile::parse("m!(x").tree();
         let node = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-        let tt = ast_to_token_tree(&node).0.to_string();
+        let tt = syntax_node_to_token_tree(node.syntax()).0.to_string();
         assert_eq_text!(&*tt, "( x");
     }
 }

crates/mbe/src/tests.rs

@@ -29,7 +29,8 @@ macro_rules! impl_fixture {
                 let macro_invocation =
                     source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap();
 
-                let (invocation_tt, _) = ast_to_token_tree(&macro_invocation.token_tree().unwrap());
+                let (invocation_tt, _) =
+                    syntax_node_to_token_tree(macro_invocation.token_tree().unwrap().syntax());
 
                 self.rules.expand(&invocation_tt).result()
             }
@@ -100,7 +101,7 @@ macro_rules! impl_fixture {
                         .descendants()
                         .find_map(ast::TokenTree::cast)
                         .unwrap();
-                    let mut wrapped = ast_to_token_tree(&wrapped).0;
+                    let mut wrapped = syntax_node_to_token_tree(wrapped.syntax()).0;
                     wrapped.delimiter = None;
                     wrapped
                 };
@@ -163,7 +164,8 @@ fn parse_macro_rules_to_tt(ra_fixture: &str) -> tt::Subtree {
     let macro_definition =
         source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap();
 
-    let (definition_tt, _) = ast_to_token_tree(&macro_definition.token_tree().unwrap());
+    let (definition_tt, _) =
+        syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax());
 
     let parsed = parse_to_token_tree(
         &ra_fixture[macro_definition.token_tree().unwrap().syntax().text_range()],
@@ -180,7 +182,7 @@ fn parse_macro_def_to_tt(ra_fixture: &str) -> tt::Subtree {
     let macro_definition =
         source_file.syntax().descendants().find_map(ast::MacroDef::cast).unwrap();
 
-    let (definition_tt, _) = ast_to_token_tree(&macro_definition.body().unwrap());
+    let (definition_tt, _) = syntax_node_to_token_tree(macro_definition.body().unwrap().syntax());
 
     let parsed =
         parse_to_token_tree(&ra_fixture[macro_definition.body().unwrap().syntax().text_range()])

crates/mbe/src/tests/rule.rs

@@ -1,7 +1,5 @@
 use syntax::{ast, AstNode};
 
-use crate::ast_to_token_tree;
-
 use super::*;
 
 #[test]
@@ -44,6 +42,7 @@ fn parse_macro_arm(arm_definition: &str) -> Result<crate::MacroRules, ParseError
     let macro_definition =
         source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap();
 
-    let (definition_tt, _) = ast_to_token_tree(&macro_definition.token_tree().unwrap());
+    let (definition_tt, _) =
+        syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax());
     crate::MacroRules::parse(&definition_tt)
 }

crates/rust-analyzer/src/cargo_target_spec.rs

@@ -191,7 +191,7 @@ mod tests {
     use super::*;
 
     use cfg::CfgExpr;
-    use mbe::ast_to_token_tree;
+    use mbe::syntax_node_to_token_tree;
     use syntax::{
         ast::{self, AstNode},
         SmolStr,
@@ -201,7 +201,7 @@ mod tests {
         let cfg_expr = {
             let source_file = ast::SourceFile::parse(cfg).ok().unwrap();
             let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
-            let (tt, _) = ast_to_token_tree(&tt);
+            let (tt, _) = syntax_node_to_token_tree(tt.syntax());
             CfgExpr::parse(&tt)
         };