diff options
author | Florian Diebold <[email protected]> | 2020-03-13 12:57:44 +0000 |
---|---|---|
committer | Florian Diebold <[email protected]> | 2020-03-13 13:50:03 +0000 |
commit | 89eb9e8002466f3099de77bc42cdabbf67d5dc1b (patch) | |
tree | 210539acb34578b5ec4697e9995457700545af07 | |
parent | 93527b5f193ed214f6ae0f8112eaec2eebabd016 (diff) |
Protect against infinite macro expansion in def collector
There was a test for this, but it wasn't actually working because the first
recursive expansion failed. (The comma...)
Even with this limit, that test (when fixed) still takes some time to pass
because of the exponential growth of the expansions, so I disabled it and added
a different one without growth.
-rw-r--r-- | crates/ra_hir_def/src/nameres/collector.rs | 48 |
1 files changed, 39 insertions, 9 deletions
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index d0459d9b0..db9838cb5 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs | |||
@@ -102,6 +102,7 @@ struct MacroDirective { | |||
102 | module_id: LocalModuleId, | 102 | module_id: LocalModuleId, |
103 | ast_id: AstIdWithPath<ast::MacroCall>, | 103 | ast_id: AstIdWithPath<ast::MacroCall>, |
104 | legacy: Option<MacroCallId>, | 104 | legacy: Option<MacroCallId>, |
105 | depth: usize, | ||
105 | } | 106 | } |
106 | 107 | ||
107 | #[derive(Clone, Debug, Eq, PartialEq)] | 108 | #[derive(Clone, Debug, Eq, PartialEq)] |
@@ -134,6 +135,7 @@ where | |||
134 | self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id }; | 135 | self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id }; |
135 | ModCollector { | 136 | ModCollector { |
136 | def_collector: &mut *self, | 137 | def_collector: &mut *self, |
138 | macro_depth: 0, | ||
137 | module_id, | 139 | module_id, |
138 | file_id: file_id.into(), | 140 | file_id: file_id.into(), |
139 | raw_items: &raw_items, | 141 | raw_items: &raw_items, |
@@ -516,7 +518,7 @@ where | |||
516 | macros.retain(|directive| { | 518 | macros.retain(|directive| { |
517 | if let Some(call_id) = directive.legacy { | 519 | if let Some(call_id) = directive.legacy { |
518 | res = ReachedFixedPoint::No; | 520 | res = ReachedFixedPoint::No; |
519 | resolved.push((directive.module_id, call_id)); | 521 | resolved.push((directive.module_id, call_id, directive.depth)); |
520 | return false; | 522 | return false; |
521 | } | 523 | } |
522 | 524 | ||
@@ -530,7 +532,7 @@ where | |||
530 | ); | 532 | ); |
531 | resolved_res.resolved_def.take_macros() | 533 | resolved_res.resolved_def.take_macros() |
532 | }) { | 534 | }) { |
533 | resolved.push((directive.module_id, call_id)); | 535 | resolved.push((directive.module_id, call_id, directive.depth)); |
534 | res = ReachedFixedPoint::No; | 536 | res = ReachedFixedPoint::No; |
535 | return false; | 537 | return false; |
536 | } | 538 | } |
@@ -541,7 +543,7 @@ where | |||
541 | if let Some(call_id) = | 543 | if let Some(call_id) = |
542 | directive.ast_id.as_call_id(self.db, |path| self.resolve_attribute_macro(&path)) | 544 | directive.ast_id.as_call_id(self.db, |path| self.resolve_attribute_macro(&path)) |
543 | { | 545 | { |
544 | resolved.push((directive.module_id, call_id)); | 546 | resolved.push((directive.module_id, call_id, 0)); |
545 | res = ReachedFixedPoint::No; | 547 | res = ReachedFixedPoint::No; |
546 | return false; | 548 | return false; |
547 | } | 549 | } |
@@ -552,8 +554,12 @@ where | |||
552 | self.unexpanded_macros = macros; | 554 | self.unexpanded_macros = macros; |
553 | self.unexpanded_attribute_macros = attribute_macros; | 555 | self.unexpanded_attribute_macros = attribute_macros; |
554 | 556 | ||
555 | for (module_id, macro_call_id) in resolved { | 557 | for (module_id, macro_call_id, depth) in resolved { |
556 | self.collect_macro_expansion(module_id, macro_call_id); | 558 | if depth > 1024 { |
559 | log::debug!("Max macro expansion depth reached"); | ||
560 | continue; | ||
561 | } | ||
562 | self.collect_macro_expansion(module_id, macro_call_id, depth); | ||
557 | } | 563 | } |
558 | 564 | ||
559 | res | 565 | res |
@@ -573,12 +579,18 @@ where | |||
573 | None | 579 | None |
574 | } | 580 | } |
575 | 581 | ||
576 | fn collect_macro_expansion(&mut self, module_id: LocalModuleId, macro_call_id: MacroCallId) { | 582 | fn collect_macro_expansion( |
583 | &mut self, | ||
584 | module_id: LocalModuleId, | ||
585 | macro_call_id: MacroCallId, | ||
586 | depth: usize, | ||
587 | ) { | ||
577 | let file_id: HirFileId = macro_call_id.as_file(); | 588 | let file_id: HirFileId = macro_call_id.as_file(); |
578 | let raw_items = self.db.raw_items(file_id); | 589 | let raw_items = self.db.raw_items(file_id); |
579 | let mod_dir = self.mod_dirs[&module_id].clone(); | 590 | let mod_dir = self.mod_dirs[&module_id].clone(); |
580 | ModCollector { | 591 | ModCollector { |
581 | def_collector: &mut *self, | 592 | def_collector: &mut *self, |
593 | macro_depth: depth, | ||
582 | file_id, | 594 | file_id, |
583 | module_id, | 595 | module_id, |
584 | raw_items: &raw_items, | 596 | raw_items: &raw_items, |
@@ -595,6 +607,7 @@ where | |||
595 | /// Walks a single module, populating defs, imports and macros | 607 | /// Walks a single module, populating defs, imports and macros |
596 | struct ModCollector<'a, D> { | 608 | struct ModCollector<'a, D> { |
597 | def_collector: D, | 609 | def_collector: D, |
610 | macro_depth: usize, | ||
598 | module_id: LocalModuleId, | 611 | module_id: LocalModuleId, |
599 | file_id: HirFileId, | 612 | file_id: HirFileId, |
600 | raw_items: &'a raw::RawItems, | 613 | raw_items: &'a raw::RawItems, |
@@ -684,6 +697,7 @@ where | |||
684 | 697 | ||
685 | ModCollector { | 698 | ModCollector { |
686 | def_collector: &mut *self.def_collector, | 699 | def_collector: &mut *self.def_collector, |
700 | macro_depth: self.macro_depth, | ||
687 | module_id, | 701 | module_id, |
688 | file_id: self.file_id, | 702 | file_id: self.file_id, |
689 | raw_items: self.raw_items, | 703 | raw_items: self.raw_items, |
@@ -713,6 +727,7 @@ where | |||
713 | let raw_items = self.def_collector.db.raw_items(file_id.into()); | 727 | let raw_items = self.def_collector.db.raw_items(file_id.into()); |
714 | ModCollector { | 728 | ModCollector { |
715 | def_collector: &mut *self.def_collector, | 729 | def_collector: &mut *self.def_collector, |
730 | macro_depth: self.macro_depth, | ||
716 | module_id, | 731 | module_id, |
717 | file_id: file_id.into(), | 732 | file_id: file_id.into(), |
718 | raw_items: &raw_items, | 733 | raw_items: &raw_items, |
@@ -887,6 +902,7 @@ where | |||
887 | module_id: self.module_id, | 902 | module_id: self.module_id, |
888 | ast_id, | 903 | ast_id, |
889 | legacy: Some(macro_call_id), | 904 | legacy: Some(macro_call_id), |
905 | depth: self.macro_depth + 1, | ||
890 | }); | 906 | }); |
891 | 907 | ||
892 | return; | 908 | return; |
@@ -902,6 +918,7 @@ where | |||
902 | module_id: self.module_id, | 918 | module_id: self.module_id, |
903 | ast_id, | 919 | ast_id, |
904 | legacy: None, | 920 | legacy: None, |
921 | depth: self.macro_depth + 1, | ||
905 | }); | 922 | }); |
906 | } | 923 | } |
907 | 924 | ||
@@ -971,13 +988,26 @@ mod tests { | |||
971 | } | 988 | } |
972 | 989 | ||
973 | #[test] | 990 | #[test] |
974 | fn test_macro_expand_will_stop() { | 991 | fn test_macro_expand_will_stop_1() { |
992 | do_resolve( | ||
993 | r#" | ||
994 | macro_rules! foo { | ||
995 | ($($ty:ty)*) => { foo!($($ty)*); } | ||
996 | } | ||
997 | foo!(KABOOM); | ||
998 | "#, | ||
999 | ); | ||
1000 | } | ||
1001 | |||
1002 | #[ignore] // this test does succeed, but takes quite a while :/ | ||
1003 | #[test] | ||
1004 | fn test_macro_expand_will_stop_2() { | ||
975 | do_resolve( | 1005 | do_resolve( |
976 | r#" | 1006 | r#" |
977 | macro_rules! foo { | 1007 | macro_rules! foo { |
978 | ($($ty:ty)*) => { foo!($($ty)*, $($ty)*); } | 1008 | ($($ty:ty)*) => { foo!($($ty)* $($ty)*); } |
979 | } | 1009 | } |
980 | foo!(KABOOM); | 1010 | foo!(KABOOM); |
981 | "#, | 1011 | "#, |
982 | ); | 1012 | ); |
983 | } | 1013 | } |