From 82094c551c948798450c80683e4ec5b185f96da5 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 9 Oct 2024 20:34:34 +0200 Subject: [PATCH] Rust: Implement `UnusedValue.ql` --- .../src/queries/unusedentities/UnusedValue.ql | 15 +++- .../queries/unusedentities/UnusedVariable.ql | 7 +- .../queries/unusedentities/UnusedVariable.qll | 14 ++++ .../unusedentities/UnusedValue.expected | 29 +++++++ .../unusedentities/UnusedVariable.expected | 28 +++---- .../test/query-tests/unusedentities/main.rs | 84 +++++++++---------- 6 files changed, 111 insertions(+), 66 deletions(-) create mode 100644 rust/ql/src/queries/unusedentities/UnusedVariable.qll diff --git a/rust/ql/src/queries/unusedentities/UnusedValue.ql b/rust/ql/src/queries/unusedentities/UnusedValue.ql index a224136968223..6070927f640b3 100644 --- a/rust/ql/src/queries/unusedentities/UnusedValue.ql +++ b/rust/ql/src/queries/unusedentities/UnusedValue.ql @@ -9,7 +9,16 @@ */ import rust +import codeql.rust.dataflow.Ssa +import codeql.rust.dataflow.internal.SsaImpl +import UnusedVariable -from Locatable e -where none() // TODO: implement query -select e, "Variable is assigned a value that is never used." +from AstNode write, Ssa::Variable v +where + variableWrite(write, 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 +select write, "Variable is assigned a value that is never used." diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.ql b/rust/ql/src/queries/unusedentities/UnusedVariable.ql index f6be18b76e15b..0162d32ff8a3a 100644 --- a/rust/ql/src/queries/unusedentities/UnusedVariable.ql +++ b/rust/ql/src/queries/unusedentities/UnusedVariable.ql @@ -9,11 +9,8 @@ */ import rust +import UnusedVariable from Variable v -where - not exists(v.getAnAccess()) and - not exists(v.getInitializer()) and - not v.getName().charAt(0) = "_" and - exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results +where isUnused(v) select v, "Variable is not used." diff --git a/rust/ql/src/queries/unusedentities/UnusedVariable.qll b/rust/ql/src/queries/unusedentities/UnusedVariable.qll new file mode 100644 index 0000000000000..64cca6e237af9 --- /dev/null +++ b/rust/ql/src/queries/unusedentities/UnusedVariable.qll @@ -0,0 +1,14 @@ +import rust + +/** A deliberately unused variable. */ +class DiscardVariable extends Variable { + DiscardVariable() { this.getName().charAt(0) = "_" } +} + +/** Holds if variable `v` is unused. */ +predicate isUnused(Variable v) { + not exists(v.getAnAccess()) and + not exists(v.getInitializer()) and + not v instanceof DiscardVariable and + exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results +} diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected index e69de29bb2d1d..5781787693e5c 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected +++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected @@ -0,0 +1,29 @@ +| main.rs:6:9:6:9 | a | Variable is assigned a value that is never used. | +| main.rs:7:9:7:9 | b | Variable is assigned a value that is never used. | +| main.rs:8:9:8:9 | c | Variable is assigned a value that is never used. | +| main.rs:9:9:9:9 | d | Variable is assigned a value that is never used. | +| main.rs:10:9:10:9 | e | Variable is assigned a value that is never used. | +| main.rs:11:9:11:9 | f | Variable is assigned a value that is never used. | +| main.rs:35:5:35:5 | b | Variable is assigned a value that is never used. | +| main.rs:37:5:37:5 | c | Variable is assigned a value that is never used. | +| main.rs:38:5:38:5 | c | Variable is assigned a value that is never used. | +| main.rs:40:5:40:5 | c | Variable is assigned a value that is never used. | +| main.rs:42:5:42:5 | d | Variable is assigned a value that is never used. | +| main.rs:44:9:44:9 | d | Variable is assigned a value that is never used. | +| main.rs:45:9:45:9 | d | Variable is assigned a value that is never used. | +| main.rs:50:5:50:5 | e | Variable is assigned a value that is never used. | +| main.rs:52:9:52:9 | e | Variable is assigned a value that is never used. | +| main.rs:54:9:54:9 | e | Variable is assigned a value that is never used. | +| main.rs:61:5:61:5 | f | Variable is assigned a value that is never used. | +| main.rs:63:5:63:5 | f | Variable is assigned a value that is never used. | +| main.rs:65:5:65:5 | g | Variable is assigned a value that is never used. | +| main.rs:67:5:67:5 | i | Variable is assigned a value that is never used. | +| main.rs:87:9:87:9 | a | Variable is assigned a value that is never used. | +| main.rs:88:9:88:9 | b | Variable is assigned a value that is never used. | +| main.rs:89:9:89:9 | c | Variable is assigned a value that is never used. | +| main.rs:108:9:108:10 | is | Variable is assigned a value that is never used. | +| main.rs:109:9:109:10 | js | Variable is assigned a value that is never used. | +| main.rs:133:13:133:17 | total | Variable is assigned a value that is never used. | +| main.rs:233:13:233:17 | total | Variable is assigned a value that is never used. | +| main.rs:304:9:304:9 | x | Variable is assigned a value that is never used. | +| main.rs:312:17:312:17 | x | Variable is assigned a value that is never used. | diff --git a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected index 66b5ac84ab33f..026b41473baff 100644 --- a/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected +++ b/rust/ql/test/query-tests/unusedentities/UnusedVariable.expected @@ -7,18 +7,18 @@ | main.rs:174:9:174:9 | x | Variable is not used. | | main.rs:202:17:202:17 | a | Variable is not used. | | main.rs:210:20:210:22 | val | Variable is not used. | -| main.rs:223:14:223:16 | val | Variable is not used. | -| main.rs:225:9:225:12 | None | Variable is not used. | -| main.rs:234:9:234:12 | None | Variable is not used. | -| main.rs:240:22:240:24 | val | Variable is not used. | +| main.rs:224:14:224:16 | val | Variable is not used. | +| main.rs:226:9:226:12 | None | Variable is not used. | +| main.rs:235:9:235:12 | None | Variable is not used. | +| main.rs:241:22:241:24 | val | Variable is not used. | | main.rs:248:24:248:26 | val | Variable is not used. | -| main.rs:257:13:257:15 | num | Variable is not used. | -| main.rs:268:9:268:11 | Yes | Variable is not used. | -| main.rs:269:9:269:10 | No | Variable is not used. | -| main.rs:272:12:272:12 | j | Variable is not used. | -| main.rs:282:12:282:14 | Yes | Variable is not used. | -| main.rs:294:25:294:25 | y | Variable is not used. | -| main.rs:298:28:298:28 | a | Variable is not used. | -| main.rs:302:9:302:9 | p | Variable is not used. | -| main.rs:309:13:309:13 | y | Variable is not used. | -| main.rs:317:21:317:21 | y | Variable is not used. | +| main.rs:256:13:256:15 | num | Variable is not used. | +| main.rs:267:9:267:11 | Yes | Variable is not used. | +| main.rs:268:9:268:10 | No | Variable is not used. | +| main.rs:271:12:271:12 | j | Variable is not used. | +| main.rs:281:12:281:14 | Yes | Variable is not used. | +| main.rs:292:25:292:25 | y | Variable is not used. | +| main.rs:295:28:295:28 | a | Variable is not used. | +| main.rs:298:9:298:9 | p | Variable is not used. | +| main.rs:305:13:305:13 | y | Variable is not used. | +| main.rs:313:21:313:21 | y | Variable is not used. | diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs index d7c08f05b62a4..4d4d10428411b 100644 --- a/rust/ql/test/query-tests/unusedentities/main.rs +++ b/rust/ql/test/query-tests/unusedentities/main.rs @@ -3,12 +3,12 @@ // --- locals --- fn locals_1() { - let a = 1; // BAD: unused value [NOT DETECTED] - let b = 1; - let c = 1; - let d = String::from("a"); // BAD: unused value [NOT DETECTED] - let e = String::from("b"); - let f = 1; + let a = 1; // BAD: unused value + let b = 1; // SPURIOUS: unused value [macros not yet supported] + let c = 1; // SPURIOUS: unused value [macros not yet supported] + let d = String::from("a"); // BAD: unused value + let e = String::from("b"); // SPURIOUS: unused value [macros not yet supported] + let f = 1; // SPURIOUS: unused value [macros not yet supported] let _ = 1; // (deliberately unused) println!("use {}", b); @@ -32,42 +32,42 @@ fn locals_2() { let h: i32; let i: i32; - b = 1; // BAD: unused value [NOT DETECTED] + b = 1; // BAD: unused value - c = 1; // BAD: unused value [NOT DETECTED] - c = 2; + c = 1; // BAD: unused value + c = 2; // SPURIOUS: unused value [macros not yet supported] println!("use {}", c); - c = 3; // BAD: unused value [NOT DETECTED] + c = 3; // BAD: unused value - d = 1; + d = 1; // SPURIOUS: unused value [macros not yet supported] if cond() { - d = 2; // BAD: unused value [NOT DETECTED] - d = 3; + d = 2; // BAD: unused value + d = 3; // SPURIOUS: unused value [macros not yet supported] } else { } println!("use {}", d); - e = 1; // BAD: unused value [NOT DETECTED] + e = 1; // BAD: unused value if cond() { - e = 2; + e = 2; // SPURIOUS: unused value [macros not yet supported] } else { - e = 3; + e = 3; // SPURIOUS: unused value [macros not yet supported] } println!("use {}", e); f = 1; f += 1; println!("use {}", f); - f += 1; // BAD: unused value [NOT DETECTED] + f += 1; // BAD: unused value f = 1; - f += 1; // BAD: unused value [NOT DETECTED] + f += 1; // BAD: unused value - g = if cond() { 1 } else { 2 }; // BAD: unused value (x2) [NOT DETECTED] + g = if cond() { 1 } else { 2 }; // BAD: unused value h = if cond() { 3 } else { 4 }; - i = if cond() { h } else { 5 }; + i = if cond() { h } else { 5 }; // SPURIOUS: unused value [macros not yet supported] println!("use {}", i); - _ = 1; // (deliberately unused) [NOT DETECTED] + _ = 1; // GOOD (deliberately unused) } // --- structs --- @@ -84,9 +84,9 @@ impl MyStruct { } fn structs() { - let a = MyStruct { val: 1 }; // BAD: unused value [NOT DETECTED] - let b = MyStruct { val: 2 }; - let c = MyStruct { val: 3 }; + let a = MyStruct { val: 1 }; // BAD: unused value + let b = MyStruct { val: 2 }; // SPURIOUS: unused value [macros not yet supported] + let c = MyStruct { val: 3 }; // SPURIOUS: unused value [macros not yet supported] let mut d: MyStruct; // BAD: unused variable let mut e: MyStruct; let mut f: MyStruct; @@ -105,8 +105,8 @@ fn structs() { // --- arrays --- fn arrays() { - let is = [1, 2, 3]; // BAD: unused values (x3) [NOT DETECTED] - let js = [1, 2, 3]; + let is = [1, 2, 3]; // BAD: unused values (x3) + let js = [1, 2, 3]; // SPURIOUS: unused value [macros not yet supported] let ks = [1, 2, 3]; println!("lets use {:?}", js); @@ -130,7 +130,7 @@ fn statics() { static mut STAT4: i32 = 0; // BAD: unused value [NOT DETECTED] unsafe { - let total = CON1 + STAT1 + STAT3; + let total = CON1 + STAT1 + STAT3; // BAD: unused value } } @@ -189,7 +189,7 @@ enum YesOrNo { No, } -use YesOrNo::{Yes, No}; // allows `Yes`, `No` to be accessed without qualifiers. +use YesOrNo::{No, Yes}; // allows `Yes`, `No` to be accessed without qualifiers. struct MyPoint { x: i64, @@ -207,7 +207,8 @@ fn if_lets_matches() { } let mut next = Some(30); - while let Some(val) = next // BAD: unused variable + while let Some(val) = // BAD: unused variable + next { next = None; } @@ -229,7 +230,7 @@ fn if_lets_matches() { let d = Some(70); match d { Some(val) => { - total += val; + total += val; // BAD: unused variable } None => { // SPURIOUS: unused variable 'None' } @@ -239,8 +240,7 @@ fn if_lets_matches() { match e { Option::Some(val) => { // BAD: unused variable } - Option::None => { - } + Option::None => {} } let f = MyOption::Some(90); @@ -250,10 +250,9 @@ fn if_lets_matches() { MyOption::None => {} } - let g : Result = Ok(100); + let g: Result = Ok(100); match g { - Ok(_) => { - } + Ok(_) => {} Err(num) => {} // BAD: unused variable } @@ -266,7 +265,7 @@ fn if_lets_matches() { let i = Yes; match i { Yes => {} // SPURIOUS: unused variable 'Yes' - No => {} // SPURIOUS: unused variable 'No' + No => {} // SPURIOUS: unused variable 'No' } if let j = Yes { // BAD: unused variable @@ -289,23 +288,20 @@ fn if_lets_matches() { let p1 = MyPoint { x: 1, y: 2 }; match p1 { - MyPoint { x: 0, y: 0 } => { - } + MyPoint { x: 0, y: 0 } => {} MyPoint { x: 1, y } => { // BAD: unused variable } - MyPoint { x: 2, y: _ } => { - } + MyPoint { x: 2, y: _ } => {} MyPoint { x: 3, y: a } => { // BAD: unused variable } - MyPoint { x: 4, .. } => { - } + MyPoint { x: 4, .. } => {} p => { // BAD: unused variable } } } fn shadowing() -> i32 { - let x = 1; // BAD: unused value [NOT DETECTED] + let x = 1; // BAD: unused value let mut y: i32; // BAD: unused variable { @@ -313,7 +309,7 @@ fn shadowing() -> i32 { let mut y: i32; { - let x = 3; // BAD: unused value [NOT DETECTED] + let x = 3; // BAD: unused value let mut y: i32; // BAD: unused variable }