diff --git a/rust/ql/src/queries/unusedentities/UnusedValue.ql b/rust/ql/src/queries/unusedentities/UnusedValue.ql index 67c0857b413e..1b0a498afe5c 100644 --- a/rust/ql/src/queries/unusedentities/UnusedValue.ql +++ b/rust/ql/src/queries/unusedentities/UnusedValue.ql @@ -16,10 +16,11 @@ import UnusedVariable from AstNode write, Ssa::Variable v where variableWrite(write, v) and + not v instanceof DiscardVariable and + not write.isInMacroExpansion() and + not isAllowableUnused(v) and // SSA definitions are only created for live writes not write = any(Ssa::WriteDefinition def).getWriteAccess().getAstNode() and // avoid overlap with the unused variable query - not isUnused(v) and - not v instanceof DiscardVariable and - not write.isInMacroExpansion() + not isUnused(v) select write, "Variable $@ is assigned a value that is never used.", v, v.getText() diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.qll b/rust/ql/src/queries/unusedentities/UnusedVariable.qll index db4a22b6c7b9..2750ca1c42a5 100644 --- a/rust/ql/src/queries/unusedentities/UnusedVariable.qll +++ b/rust/ql/src/queries/unusedentities/UnusedVariable.qll @@ -16,6 +16,20 @@ predicate isUnused(Variable v) { not exists(v.getInitializer()) } +/** + * A callable for which we have incomplete information, for example because an unexpanded + * macro call is present. These callables are prone to false positive results from unused + * entities queries, unless they are excluded from results. + */ +class IncompleteCallable extends Callable { + IncompleteCallable() { + exists(MacroExpr me | + me.getEnclosingCallable() = this and + not me.getMacroCall().hasExpanded() + ) + } +} + /** * Holds if variable `v` is in a context where we may not find a use for it, * but that's expected and should not be considered a problem. @@ -24,6 +38,9 @@ predicate isAllowableUnused(Variable v) { // in a macro expansion v.getPat().isInMacroExpansion() or + // declared in an incomplete callable + v.getEnclosingCfgScope() instanceof IncompleteCallable + or // a 'self' variable v.getText() = "self" } diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/ExtractionConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/ExtractionConsistency.expected new file mode 100644 index 000000000000..f3834c238893 --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/ExtractionConsistency.expected @@ -0,0 +1,2 @@ +extractionWarning +| undefined_macros/main.rs:1:1:1:1 | semantic analyzer unavailable (not included as a module) | diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected index 07439e3e0712..4ef8ae416250 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected +++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected @@ -14,8 +14,8 @@ | main.rs:284:13:284:17 | total | Variable $@ is assigned a value that is never used. | main.rs:252:13:252:17 | total | total | | main.rs:377:9:377:9 | x | Variable $@ is assigned a value that is never used. | main.rs:377:9:377:9 | x | x | | main.rs:385:17:385:17 | x | Variable $@ is assigned a value that is never used. | main.rs:385:17:385:17 | x | x | -| main.rs:486:9:486:9 | c | Variable $@ is assigned a value that is never used. | main.rs:486:9:486:9 | c | c | -| main.rs:519:9:519:20 | var_in_macro | Variable $@ is assigned a value that is never used. | main.rs:519:9:519:20 | var_in_macro | var_in_macro | +| main.rs:531:9:531:20 | var_in_macro | Variable $@ is assigned a value that is never used. | main.rs:531:9:531:20 | var_in_macro | var_in_macro | +| main.rs:540:9:540:9 | c | Variable $@ is assigned a value that is never used. | main.rs:540:9:540:9 | c | c | | more.rs:44:9:44:14 | a_ptr4 | Variable $@ is assigned a value that is never used. | more.rs:44:9:44:14 | a_ptr4 | a_ptr4 | | more.rs:59:9:59:13 | d_ptr | Variable $@ is assigned a value that is never used. | more.rs:59:9:59:13 | d_ptr | d_ptr | | more.rs:65:13:65:17 | f_ptr | Variable $@ is assigned a value that is never used. | more.rs:65:13:65:17 | f_ptr | f_ptr | diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs index ccd63f1cd53a..6da7b55fe5cd 100644 --- a/rust/ql/test/query-tests/unusedentities/main.rs +++ b/rust/ql/test/query-tests/unusedentities/main.rs @@ -468,7 +468,45 @@ fn traits() { // --- macros --- -fn macros() { +macro_rules! set_value { + ($x:expr,$y:expr) => { + $x = $y + }; +} + +macro_rules! use_value { + ($x:expr) => { + println!("{}", $x) + }; +} + +fn macros1() { + let a: u16; + let b: u16 = 2; + set_value!(a, 1); + use_value!(b); + + match std::env::args().nth(1).unwrap().parse::() { + Ok(n) => { + use_value!(n); + } + _ => {} + } +} + +fn macros2() { + let a: u16 = 3; + println!("{}", a); + + match std::env::args().nth(1).unwrap().parse::() { + Ok(n) => { + println!("{}", n); + } + _ => {} + } +} + +fn macros3() { let x; println!( "The value of x is {}", @@ -476,8 +514,24 @@ fn macros() { x = 10; // $ MISSING: Alert[rust/unused-value] 10 }) - ) + ); +} + +macro_rules! let_in_macro { + ($e:expr) => {{ + let var_in_macro = 0; + $e + }}; } + +// Our analysis does not currently respect the hygiene rules of Rust macros +// (https://veykril.github.io/tlborm/decl-macros/minutiae/hygiene.html), because +// all we have access to is the expanded AST +fn hygiene_mismatch() { + let var_in_macro = 0; // $ SPURIOUS: Alert[rust/unused-value] + let_in_macro!(var_in_macro); +} + // --- references --- fn references() { @@ -505,21 +559,6 @@ trait MyTrait { fn my_func2(&self, x: i32) -> i32; } -macro_rules! let_in_macro { - ($e:expr) => {{ - let var_in_macro = 0; - $e - }}; -} - -// Our analysis does not currently respect the hygiene rules of Rust macros -// (https://veykril.github.io/tlborm/decl-macros/minutiae/hygiene.html), because -// all we have access to is the expanded AST -fn hygiene_mismatch() { - let var_in_macro = 0; // $ SPURIOUS: Alert[rust/unused-value] - let_in_macro!(var_in_macro); -} - // --- main --- fn main() { @@ -534,7 +573,10 @@ fn main() { shadowing(); func_ptrs(); folds_and_closures(); - macros(); + macros1(); + macros2(); + macros3(); + hygiene_mismatch(); references(); generics(); diff --git a/rust/ql/test/query-tests/unusedentities/undefined_macros/main.rs b/rust/ql/test/query-tests/unusedentities/undefined_macros/main.rs new file mode 100644 index 000000000000..5e345677254d --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/undefined_macros/main.rs @@ -0,0 +1,33 @@ + +// --- undefined macro calls --- + +fn undefined_macros1() { + let a: u16; + + undefined_macro_call!(d); +} + +fn undefined_macros2() { + { + let a: u16 = 1; // $ MISSING: Alert[rust/unused-value] + } + + undefined_macro_call!(5); + + let b: u16; // $ MISSING: Alert[rust/unused-variable] +} + +fn undefined_macros3() { + match std::env::args().nth(1).unwrap().parse::() { + Ok(n) => { + undefined_macro_call!(n); + } + _ => {} + } +} + +fn main() { + undefined_macros1(); + undefined_macros2(); + undefined_macros3(); +} diff --git a/rust/ql/test/query-tests/unusedentities/undefined_macros/options.yml b/rust/ql/test/query-tests/unusedentities/undefined_macros/options.yml new file mode 100644 index 000000000000..cf148dd35f86 --- /dev/null +++ b/rust/ql/test/query-tests/unusedentities/undefined_macros/options.yml @@ -0,0 +1 @@ +qltest_cargo_check: false