Skip to content

Commit

Permalink
Rust: Implement UnusedValue.ql
Browse files Browse the repository at this point in the history
  • Loading branch information
hvitved committed Oct 10, 2024
1 parent 5215568 commit d50d1b0
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 73 deletions.
15 changes: 12 additions & 3 deletions rust/ql/src/queries/unusedentities/UnusedValue.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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."
7 changes: 2 additions & 5 deletions rust/ql/src/queries/unusedentities/UnusedVariable.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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."
14 changes: 14 additions & 0 deletions rust/ql/src/queries/unusedentities/UnusedVariable.qll
Original file line number Diff line number Diff line change
@@ -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
}
29 changes: 29 additions & 0 deletions rust/ql/test/query-tests/unusedentities/UnusedValue.expected
Original file line number Diff line number Diff line change
@@ -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:232:13:232:17 | total | Variable is assigned a value that is never used. |
| main.rs:301:9:301:9 | x | Variable is assigned a value that is never used. |
| main.rs:309:17:309:17 | x | Variable is assigned a value that is never used. |
20 changes: 10 additions & 10 deletions rust/ql/test/query-tests/unusedentities/UnusedVariable.expected
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
| 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:240:22:240: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:272:12:272:12 | j | 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:224:14:224:16 | val | Variable is not used. |
| main.rs:239:22:239:24 | val | Variable is not used. |
| main.rs:246:24:246:26 | val | Variable is not used. |
| main.rs:254:13:254:15 | num | Variable is not used. |
| main.rs:269:12:269:12 | j | Variable is not used. |
| main.rs:289:25:289:25 | y | Variable is not used. |
| main.rs:292:28:292:28 | a | Variable is not used. |
| main.rs:295:9:295:9 | p | Variable is not used. |
| main.rs:302:13:302:13 | y | Variable is not used. |
| main.rs:310:21:310:21 | y | Variable is not used. |
103 changes: 48 additions & 55 deletions rust/ql/test/query-tests/unusedentities/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 ---
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand All @@ -222,25 +223,22 @@ fn if_lets_matches() {
match c {
Some(val) => { // BAD: unused variable
}
None => {
}
None => {}
}

let d = Some(70);
match d {
Some(val) => {
total += val;
}
None => {
total += val; // BAD: unused variable
}
None => {}
}

let e = Option::Some(80);
match e {
Option::Some(val) => { // BAD: unused variable
}
Option::None => {
}
Option::None => {}
}

let f = MyOption::Some(90);
Expand All @@ -250,10 +248,9 @@ fn if_lets_matches() {
MyOption::None => {}
}

let g : Result<i64, i64> = Ok(100);
let g: Result<i64, i64> = Ok(100);
match g {
Ok(_) => {
}
Ok(_) => {}
Err(num) => {} // BAD: unused variable
}

Expand All @@ -279,8 +276,7 @@ fn if_lets_matches() {
}

let l = Yes;
if let Yes = l {
}
if let Yes = l {}

match 1 {
1 => {}
Expand All @@ -289,31 +285,28 @@ 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

{
let x = 2;
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
}

Expand All @@ -329,14 +322,14 @@ fn main() {
structs();
arrays();
statics();
println!("lets use result {}", parameters(1, 2, 3));
println!("lets use result {}", parameters(1, 2, 3));
loops();
if_lets_matches();
shadowing();

unreachable_if();
unreachable_panic();
unreachable_match();
unreachable_loop();
unreachable_paren();
unreachable_if();
unreachable_panic();
unreachable_match();
unreachable_loop();
unreachable_paren();
}

0 comments on commit d50d1b0

Please sign in to comment.