Skip to content

Commit 223f0c8

Browse files
redsun82redsun82
committed
Rust: fix macro expansion in library code
There was a mismatch between a `self.macro_context_level += 1` and the corresponding `self.macro_context_level -= 1`, which resulted in an `usize` underflow (panic in debug mode, wrong behaviour in release mode). This fixes it and adds a relevant assertion and test. In order to properly test library mode extraction, a special option enforcing that on source code as well is added.
1 parent b813010 commit 223f0c8

File tree

13 files changed

+264
-49
lines changed

13 files changed

+264
-49
lines changed

‎rust/extractor/src/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ pub struct Config {
7171
pub proc_macro_server: Option<PathBuf>,
7272
pub skip_path_resolution: bool,
7373
pub extract_dependencies_as_source: bool,
74+
pub force_library_mode: bool, // for testing purposes
7475
}
7576

7677
impl Config {

‎rust/extractor/src/main.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,11 @@ fn main() -> anyhow::Result<()> {
293293
} else {
294294
(SourceKind::Library, ResolvePaths::No)
295295
};
296+
let (source_mode, source_resolve_paths) = if cfg.force_library_mode {
297+
(library_mode, library_resolve_paths)
298+
} else {
299+
(SourceKind::Source, resolve_paths)
300+
};
296301
let mut processed_files: HashSet<PathBuf, RandomState> =
297302
HashSet::from_iter(files.iter().cloned());
298303
for (manifest, files) in map.values().filter(|(_, files)| !files.is_empty()) {
@@ -311,12 +316,10 @@ fn main() -> anyhow::Result<()> {
311316
file,
312317
&semantics,
313318
vfs,
314-
resolve_paths,
315-
SourceKind::Source,
319+
source_resolve_paths,
320+
source_mode,
316321
),
317-
Err(reason) => {
318-
extractor.extract_without_semantics(file, SourceKind::Source, &reason)
319-
}
322+
Err(reason) => extractor.extract_without_semantics(file, source_mode, &reason),
320323
};
321324
}
322325
for (file_id, file) in vfs.iter() {

‎rust/extractor/src/qltest.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn dump_lib() -> anyhow::Result<()> {
1919
.iter()
2020
.map(|p| p.file_stem().expect("results of glob must have a name"))
2121
.filter(|&p| !["main", "lib", "proc_macro"].map(OsStr::new).contains(&p))
22-
.map(|p| format!("mod {};", p.to_string_lossy()))
22+
.map(|p| format!("pub mod {};", p.to_string_lossy()))
2323
.join("\n");
2424
fs::write("lib.rs", lib).context("writing lib.rs")
2525
}

‎rust/extractor/src/translate/base.rs

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -24,33 +24,31 @@ use ra_ap_syntax::{
2424

2525
impl Emission<ast::Item> for Translator<'_> {
2626
fn pre_emit(&mut self, node: &ast::Item) -> Option<Label<generated::Item>> {
27-
self.prepare_item_expansion(node).map(Into::into)
27+
self.item_pre_emit(node).map(Into::into)
2828
}
2929

3030
fn post_emit(&mut self, node: &ast::Item, label: Label<generated::Item>) {
31-
self.emit_item_expansion(node, label);
31+
self.item_post_emit(node, label);
3232
}
3333
}
3434

3535
impl Emission<ast::AssocItem> for Translator<'_> {
3636
fn pre_emit(&mut self, node: &ast::AssocItem) -> Option<Label<generated::AssocItem>> {
37-
self.prepare_item_expansion(&node.clone().into())
38-
.map(Into::into)
37+
self.item_pre_emit(&node.clone().into()).map(Into::into)
3938
}
4039

4140
fn post_emit(&mut self, node: &ast::AssocItem, label: Label<generated::AssocItem>) {
42-
self.emit_item_expansion(&node.clone().into(), label.into());
41+
self.item_post_emit(&node.clone().into(), label.into());
4342
}
4443
}
4544

4645
impl Emission<ast::ExternItem> for Translator<'_> {
4746
fn pre_emit(&mut self, node: &ast::ExternItem) -> Option<Label<generated::ExternItem>> {
48-
self.prepare_item_expansion(&node.clone().into())
49-
.map(Into::into)
47+
self.item_pre_emit(&node.clone().into()).map(Into::into)
5048
}
5149

5250
fn post_emit(&mut self, node: &ast::ExternItem, label: Label<generated::ExternItem>) {
53-
self.emit_item_expansion(&node.clone().into(), label.into());
51+
self.item_post_emit(&node.clone().into(), label.into());
5452
}
5553
}
5654

@@ -849,35 +847,6 @@ impl<'a> Translator<'a> {
849847
})
850848
}
851849

852-
pub(crate) fn prepare_item_expansion(
853-
&mut self,
854-
node: &ast::Item,
855-
) -> Option<Label<generated::MacroCall>> {
856-
if self.source_kind == SourceKind::Library {
857-
// if the item expands via an attribute macro, we want to only emit the expansion
858-
if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
859-
// we wrap it in a dummy MacroCall to get a single Item label that can replace
860-
// the original Item
861-
let label = self.trap.emit(generated::MacroCall {
862-
id: TrapId::Star,
863-
attrs: vec![],
864-
path: None,
865-
token_tree: None,
866-
});
867-
generated::MacroCall::emit_macro_call_expansion(
868-
label,
869-
expanded.into(),
870-
&mut self.trap.writer,
871-
);
872-
return Some(label);
873-
}
874-
}
875-
if self.is_attribute_macro_target(node) {
876-
self.macro_context_depth += 1;
877-
}
878-
None
879-
}
880-
881850
fn process_item_macro_expansion(
882851
&mut self,
883852
node: &impl ast::AstNode,
@@ -915,10 +884,6 @@ impl<'a> Translator<'a> {
915884
&mut self,
916885
node: &ast::Item,
917886
) -> Option<Label<generated::MacroItems>> {
918-
if !self.is_attribute_macro_target(node) {
919-
return None;
920-
}
921-
self.macro_context_depth -= 1;
922887
if self.macro_context_depth > 0 {
923888
// only expand the outermost attribute macro
924889
return None;
@@ -927,7 +892,48 @@ impl<'a> Translator<'a> {
927892
self.process_item_macro_expansion(node, expansion.map(|x| x.value))
928893
}
929894

930-
pub(crate) fn emit_item_expansion(&mut self, node: &ast::Item, label: Label<generated::Item>) {
895+
pub(crate) fn item_pre_emit(
896+
&mut self,
897+
node: &ast::Item,
898+
) -> Option<Label<generated::MacroCall>> {
899+
if !self.is_attribute_macro_target(node) {
900+
return None;
901+
}
902+
if self.source_kind == SourceKind::Library {
903+
// if the item expands via an attribute macro, we want to only emit the expansion
904+
if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
905+
// we wrap it in a dummy MacroCall to get a single Item label that can replace
906+
// the original Item
907+
let label = self.trap.emit(generated::MacroCall {
908+
id: TrapId::Star,
909+
attrs: vec![],
910+
path: None,
911+
token_tree: None,
912+
});
913+
generated::MacroCall::emit_macro_call_expansion(
914+
label,
915+
expanded.into(),
916+
&mut self.trap.writer,
917+
);
918+
return Some(label);
919+
}
920+
}
921+
self.macro_context_depth += 1;
922+
None
923+
}
924+
925+
pub(crate) fn item_post_emit(&mut self, node: &ast::Item, label: Label<generated::Item>) {
926+
if !self.is_attribute_macro_target(node) {
927+
return;
928+
}
929+
// see `item_pre_emit`:
930+
// if self.is_attribute_macro_target(node), then we either exited early with `Some(label)`
931+
// and are not here, or we did self.macro_context_depth += 1
932+
assert!(
933+
self.macro_context_depth > 0,
934+
"macro_context_depth should be > 0 for an attribute macro target"
935+
);
936+
self.macro_context_depth -= 1;
931937
if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
932938
generated::Item::emit_attribute_macro_expansion(label, expanded, &mut self.trap.writer);
933939
}

‎rust/ql/test/TestUtils.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ predicate toBeTested(Element e) {
66
(
77
not e instanceof Locatable
88
or
9-
e.(Locatable).fromSource()
9+
exists(e.(Locatable).getFile().getRelativePath())
1010
)
1111
}
1212

‎rust/ql/test/extractor-tests/macro-in-library/Cargo.lock

Lines changed: 53 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rust/ql/test/extractor-tests/macro-in-library/PrintAst.expected

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
lib.rs:
2+
# 1| [SourceFile] SourceFile
3+
# 1| getItem(0): [Module] mod macro_in_library
4+
# 1| getName(): [Name] macro_in_library
5+
# 1| getVisibility(): [Visibility] Visibility
6+
macro_in_library.rs:
7+
# 1| [SourceFile] SourceFile
8+
# 4| getItem(1): [Function] fn bar
9+
# 4| getParamList(): [ParamList] ParamList
10+
# 4| getName(): [Name] bar
11+
# 4| getVisibility(): [Visibility] Visibility
12+
# 2| [MacroItems] MacroItems
13+
# 2| getItem(0): [Function] fn foo
14+
# 2| getParamList(): [ParamList] ParamList
15+
# 2| getName(): [Name] foo
16+
# 2| getVisibility(): [Visibility] Visibility
17+
# 2| getItem(1): [Function] fn foo_new
18+
# 2| getParamList(): [ParamList] ParamList
19+
# 2| getName(): [Name] foo_new
20+
# 2| getVisibility(): [Visibility] Visibility
21+
proc_macro.rs:
22+
# 1| [SourceFile] SourceFile
23+
# 1| getItem(0): [Use] use ...::TokenStream
24+
# 1| getUseTree(): [UseTree] ...::TokenStream
25+
# 1| getPath(): [Path] ...::TokenStream
26+
# 1| getQualifier(): [Path] proc_macro
27+
# 1| getSegment(): [PathSegment] proc_macro
28+
# 1| getIdentifier(): [NameRef] proc_macro
29+
# 1| getSegment(): [PathSegment] TokenStream
30+
# 1| getIdentifier(): [NameRef] TokenStream
31+
# 2| getItem(1): [Use] use ...::quote
32+
# 2| getUseTree(): [UseTree] ...::quote
33+
# 2| getPath(): [Path] ...::quote
34+
# 2| getQualifier(): [Path] quote
35+
# 2| getSegment(): [PathSegment] quote
36+
# 2| getIdentifier(): [NameRef] quote
37+
# 2| getSegment(): [PathSegment] quote
38+
# 2| getIdentifier(): [NameRef] quote
39+
# 4| getItem(2): [Function] fn add_one
40+
# 5| getParamList(): [ParamList] ParamList
41+
# 5| getParam(0): [Param] : TokenStream
42+
# 5| getTypeRepr(): [PathTypeRepr] TokenStream
43+
# 5| getPath(): [Path] TokenStream
44+
# 5| getSegment(): [PathSegment] TokenStream
45+
# 5| getIdentifier(): [NameRef] TokenStream
46+
# 5| getParam(1): [Param] : TokenStream
47+
# 5| getTypeRepr(): [PathTypeRepr] TokenStream
48+
# 5| getPath(): [Path] TokenStream
49+
# 5| getSegment(): [PathSegment] TokenStream
50+
# 5| getIdentifier(): [NameRef] TokenStream
51+
# 4| getAttr(0): [Attr] Attr
52+
# 4| getMeta(): [Meta] Meta
53+
# 4| getPath(): [Path] proc_macro_attribute
54+
# 4| getSegment(): [PathSegment] proc_macro_attribute
55+
# 4| getIdentifier(): [NameRef] proc_macro_attribute
56+
# 5| getName(): [Name] add_one
57+
# 5| getRetType(): [RetTypeRepr] RetTypeRepr
58+
# 5| getTypeRepr(): [PathTypeRepr] TokenStream
59+
# 5| getPath(): [Path] TokenStream
60+
# 5| getSegment(): [PathSegment] TokenStream
61+
# 5| getIdentifier(): [NameRef] TokenStream
62+
# 5| getVisibility(): [Visibility] Visibility
63+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/allocator-api2-0.2.21/src/lib.rs:
64+
# 1| [SourceFile] SourceFile
65+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cfg-if-1.0.0/src/lib.rs:
66+
# 1| [SourceFile] SourceFile
67+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/compiler_builtins-0.1.146/src/lib.rs:
68+
# 1| [SourceFile] SourceFile
69+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/getopts-0.2.21/src/lib.rs:
70+
# 1| [SourceFile] SourceFile
71+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/hashbrown-0.15.2/src/lib.rs:
72+
# 1| [SourceFile] SourceFile
73+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/libc-0.2.169/src/lib.rs:
74+
# 1| [SourceFile] SourceFile
75+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/proc-macro2-1.0.95/src/lib.rs:
76+
# 1| [SourceFile] SourceFile
77+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/quote-1.0.40/src/lib.rs:
78+
# 1| [SourceFile] SourceFile
79+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rand-0.9.0/src/lib.rs:
80+
# 1| [SourceFile] SourceFile
81+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rand_core-0.9.0/src/lib.rs:
82+
# 1| [SourceFile] SourceFile
83+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rand_xorshift-0.4.0/src/lib.rs:
84+
# 1| [SourceFile] SourceFile
85+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustc-demangle-0.1.24/src/lib.rs:
86+
# 1| [SourceFile] SourceFile
87+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/syn-2.0.104/src/lib.rs:
88+
# 1| [SourceFile] SourceFile
89+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/unicode-ident-1.0.18/src/lib.rs:
90+
# 1| [SourceFile] SourceFile
91+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/unicode-width-0.1.14/src/lib.rs:
92+
# 1| [SourceFile] SourceFile
93+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/zerocopy-0.8.17/src/lib.rs:
94+
# 1| [SourceFile] SourceFile
95+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/lib.rs:
96+
# 1| [SourceFile] SourceFile
97+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/lib.rs:
98+
# 1| [SourceFile] SourceFile
99+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/panic_abort/src/lib.rs:
100+
# 1| [SourceFile] SourceFile
101+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/panic_unwind/src/lib.rs:
102+
# 1| [SourceFile] SourceFile
103+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/proc_macro/src/lib.rs:
104+
# 1| [SourceFile] SourceFile
105+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/lib.rs:
106+
# 1| [SourceFile] SourceFile
107+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/stdarch/crates/std_detect/src/lib.rs:
108+
# 1| [SourceFile] SourceFile
109+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:
110+
# 1| [SourceFile] SourceFile
111+
resolved/macro-in-library.testproj/trap/rust/crates/home/redsun82/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/unwind/src/lib.rs:
112+
# 1| [SourceFile] SourceFile
113+
resolved/macro-in-library.testproj/trap/rust/crateslib.rs:
114+
# 1| [SourceFile] SourceFile
115+
resolved/macro-in-library.testproj/trap/rust/cratesproc_macro.rs:
116+
# 1| [SourceFile] SourceFile

‎rust/ql/test/extractor-tests/macro-in-library/PrintAst.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
utils/PrintAst.ql

‎rust/ql/test/extractor-tests/macro-in-library/macro_in_library.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#[proc_macro::add_one]
2+
pub fn foo() {}
3+
4+
pub fn bar() {
5+
foo_new();
6+
}

‎rust/ql/test/extractor-tests/macro-in-library/options.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
force_library_mode: true

0 commit comments

Comments
 (0)