Skip to content

Commit 89eb9e8

Browse files
committed
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.
1 parent 93527b5 commit 89eb9e8

File tree

1 file changed

+39
-9
lines changed

1 file changed

+39
-9
lines changed

crates/ra_hir_def/src/nameres/collector.rs

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ struct MacroDirective {
102102
module_id: LocalModuleId,
103103
ast_id: AstIdWithPath<ast::MacroCall>,
104104
legacy: Option<MacroCallId>,
105+
depth: usize,
105106
}
106107

107108
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -134,6 +135,7 @@ where
134135
self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id };
135136
ModCollector {
136137
def_collector: &mut *self,
138+
macro_depth: 0,
137139
module_id,
138140
file_id: file_id.into(),
139141
raw_items: &raw_items,
@@ -516,7 +518,7 @@ where
516518
macros.retain(|directive| {
517519
if let Some(call_id) = directive.legacy {
518520
res = ReachedFixedPoint::No;
519-
resolved.push((directive.module_id, call_id));
521+
resolved.push((directive.module_id, call_id, directive.depth));
520522
return false;
521523
}
522524

@@ -530,7 +532,7 @@ where
530532
);
531533
resolved_res.resolved_def.take_macros()
532534
}) {
533-
resolved.push((directive.module_id, call_id));
535+
resolved.push((directive.module_id, call_id, directive.depth));
534536
res = ReachedFixedPoint::No;
535537
return false;
536538
}
@@ -541,7 +543,7 @@ where
541543
if let Some(call_id) =
542544
directive.ast_id.as_call_id(self.db, |path| self.resolve_attribute_macro(&path))
543545
{
544-
resolved.push((directive.module_id, call_id));
546+
resolved.push((directive.module_id, call_id, 0));
545547
res = ReachedFixedPoint::No;
546548
return false;
547549
}
@@ -552,8 +554,12 @@ where
552554
self.unexpanded_macros = macros;
553555
self.unexpanded_attribute_macros = attribute_macros;
554556

555-
for (module_id, macro_call_id) in resolved {
556-
self.collect_macro_expansion(module_id, macro_call_id);
557+
for (module_id, macro_call_id, depth) in resolved {
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);
557563
}
558564

559565
res
@@ -573,12 +579,18 @@ where
573579
None
574580
}
575581

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+
) {
577588
let file_id: HirFileId = macro_call_id.as_file();
578589
let raw_items = self.db.raw_items(file_id);
579590
let mod_dir = self.mod_dirs[&module_id].clone();
580591
ModCollector {
581592
def_collector: &mut *self,
593+
macro_depth: depth,
582594
file_id,
583595
module_id,
584596
raw_items: &raw_items,
@@ -595,6 +607,7 @@ where
595607
/// Walks a single module, populating defs, imports and macros
596608
struct ModCollector<'a, D> {
597609
def_collector: D,
610+
macro_depth: usize,
598611
module_id: LocalModuleId,
599612
file_id: HirFileId,
600613
raw_items: &'a raw::RawItems,
@@ -684,6 +697,7 @@ where
684697

685698
ModCollector {
686699
def_collector: &mut *self.def_collector,
700+
macro_depth: self.macro_depth,
687701
module_id,
688702
file_id: self.file_id,
689703
raw_items: self.raw_items,
@@ -713,6 +727,7 @@ where
713727
let raw_items = self.def_collector.db.raw_items(file_id.into());
714728
ModCollector {
715729
def_collector: &mut *self.def_collector,
730+
macro_depth: self.macro_depth,
716731
module_id,
717732
file_id: file_id.into(),
718733
raw_items: &raw_items,
@@ -887,6 +902,7 @@ where
887902
module_id: self.module_id,
888903
ast_id,
889904
legacy: Some(macro_call_id),
905+
depth: self.macro_depth + 1,
890906
});
891907

892908
return;
@@ -902,6 +918,7 @@ where
902918
module_id: self.module_id,
903919
ast_id,
904920
legacy: None,
921+
depth: self.macro_depth + 1,
905922
});
906923
}
907924

@@ -971,13 +988,26 @@ mod tests {
971988
}
972989

973990
#[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() {
9751005
do_resolve(
9761006
r#"
9771007
macro_rules! foo {
978-
($($ty:ty)*) => { foo!($($ty)*, $($ty)*); }
1008+
($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
9791009
}
980-
foo!(KABOOM);
1010+
foo!(KABOOM);
9811011
"#,
9821012
);
9831013
}

0 commit comments

Comments
 (0)