Merge pull request #17773 from geoffw0/unusedval2

Rust: Implement UnusedValue.ql (2)
github · Oct 17, 2024 · 6e197b5 · 6e197b5
2 parents b07c788 + 7e2542b
commit 6e197b5
Show file tree

Hide file tree

Showing 8 changed files with 246 additions and 78 deletions.
diff --git 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
@@ -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
@@ -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/CONSISTENCY/CfgConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/CfgConsistency.expected
@@ -1,11 +1,17 @@
 multipleSuccessors
-| main.rs:428:17:428:17 | 0 | successor | main.rs:428:9:428:10 | a2 |
-| main.rs:428:17:428:17 | 0 | successor | main.rs:428:20:428:62 | ClosureExpr |
-| main.rs:431:17:431:17 | 0 | successor | main.rs:431:9:431:10 | a3 |
-| main.rs:431:17:431:17 | 0 | successor | main.rs:431:20:431:41 | ClosureExpr |
-| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a4 |
-| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:35 | ClosureExpr |
-| main.rs:437:17:437:17 | 0 | successor | main.rs:437:9:437:10 | a5 |
-| main.rs:437:17:437:17 | 0 | successor | main.rs:437:20:437:35 | ClosureExpr |
-| main.rs:441:17:441:17 | 0 | successor | main.rs:441:9:441:10 | a6 |
-| main.rs:441:17:441:17 | 0 | successor | main.rs:441:20:441:46 | ClosureExpr |
+| main.rs:421:17:421:17 | 0 | successor | main.rs:421:9:421:10 | a2 |
+| main.rs:421:17:421:17 | 0 | successor | main.rs:421:20:421:62 | ClosureExpr |
+| main.rs:424:17:424:17 | 0 | successor | main.rs:424:9:424:10 | a3 |
+| main.rs:424:17:424:17 | 0 | successor | main.rs:424:20:424:41 | ClosureExpr |
+| main.rs:427:17:427:17 | 0 | successor | main.rs:427:9:427:10 | a4 |
+| main.rs:427:17:427:17 | 0 | successor | main.rs:427:20:427:35 | ClosureExpr |
+| main.rs:430:17:430:17 | 0 | successor | main.rs:430:9:430:10 | a5 |
+| main.rs:430:17:430:17 | 0 | successor | main.rs:430:20:430:35 | ClosureExpr |
+| main.rs:434:17:434:17 | 0 | successor | main.rs:434:9:434:10 | a6 |
+| main.rs:434:17:434:17 | 0 | successor | main.rs:434:20:434:46 | ClosureExpr |
+| more.rs:32:17:32:17 | a | successor | more.rs:32:5:32:5 | i |
+| more.rs:32:17:32:17 | a | successor | more.rs:32:20:32:20 | b |
+deadEnd
+| more.rs:9:9:9:19 | Param |
+| more.rs:38:23:38:28 | Param |
+| more.rs:42:19:42:24 | Param |
diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
@@ -0,0 +1,30 @@
+| main.rs:6:9:6:9 | a | 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: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:40:5:40:5 | c | 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:50:5:50:5 | 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:87:9:87:9 | a | 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:133:13:133:17 | total | Variable is assigned a value that is never used. |
+| main.rs:203:13:203:31 | res | Variable is assigned a value that is never used. |
+| main.rs:218:9:218:24 | kind | Variable is assigned a value that is never used. |
+| main.rs:223:9:223:32 | kind | Variable is assigned a value that is never used. |
+| main.rs:280:13:280:17 | total | Variable is assigned a value that is never used. |
+| main.rs:348:5:348:39 | kind | Variable is assigned a value that is never used. |
+| main.rs:370:9:370:9 | x | Variable is assigned a value that is never used. |
+| main.rs:378:17:378:17 | x | Variable is assigned a value that is never used. |
+| main.rs:432:9:432:10 | i6 | Variable is assigned a value that is never used. |
+| more.rs:8:9:8:13 | times | Variable is assigned a value that is never used. |
+| more.rs:9:9:9:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:21:9:21:14 | unused | Variable is assigned a value that is never used. |
+| more.rs:38:23:38:25 | val | Variable is assigned a value that is never used. |
+| more.rs:42:19:42:21 | val | Variable is assigned a value that is never used. |
+| more.rs:58:9:58:11 | val | Variable is assigned a value that is never used. |
+| more.rs:80:9:80:14 | a_ptr4 | Variable is assigned a value that is never used. |
+| more.rs:95:9:95:13 | d_ptr | Variable is assigned a value that is never used. |
+| more.rs:101:9:101:17 | f_ptr | 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
@@ -6,17 +6,17 @@
 | main.rs:201:9:201:9 | x | Variable is not used. |
 | main.rs:250:17:250:17 | a | Variable is not used. |
 | main.rs:258:20:258:22 | val | Variable is not used. |
-| main.rs:271:14:271:16 | val | Variable is not used. |
-| main.rs:288:22:288:24 | val | Variable is not used. |
-| main.rs:296:24:296:26 | val | Variable is not used. |
-| main.rs:305:13:305:15 | num | Variable is not used. |
-| main.rs:320:12:320:12 | j | Variable is not used. |
-| main.rs:342:25:342:25 | y | Variable is not used. |
-| main.rs:346:28:346:28 | a | Variable is not used. |
-| main.rs:350:9:350:9 | p | Variable is not used. |
-| main.rs:365:9:365:13 | right | Variable is not used. |
-| main.rs:371:9:371:14 | right2 | Variable is not used. |
-| main.rs:378:13:378:13 | y | Variable is not used. |
-| main.rs:386:21:386:21 | y | Variable is not used. |
-| main.rs:434:27:434:29 | val | Variable is not used. |
-| main.rs:437:22:437:24 | acc | Variable is not used. |
+| main.rs:272:14:272:16 | val | Variable is not used. |
+| main.rs:287:22:287:24 | val | Variable is not used. |
+| main.rs:294:24:294:26 | val | Variable is not used. |
+| main.rs:302:13:302:15 | num | Variable is not used. |
+| main.rs:317:12:317:12 | j | Variable is not used. |
+| main.rs:337:25:337:25 | y | Variable is not used. |
+| main.rs:340:28:340:28 | a | Variable is not used. |
+| main.rs:343:9:343:9 | p | Variable is not used. |
+| main.rs:358:9:358:13 | right | Variable is not used. |
+| main.rs:364:9:364:14 | right2 | Variable is not used. |
+| main.rs:371:13:371:13 | y | Variable is not used. |
+| main.rs:379:21:379:21 | y | Variable is not used. |
+| main.rs:427:27:427:29 | val | Variable is not used. |
+| main.rs:430:22:430:24 | acc | Variable is not used. |
diff --git a/rust/ql/test/query-tests/unusedentities/main.rs b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -3,10 +3,10 @@
 // --- locals ---
 
 fn locals_1() {
-    let a = 1; // BAD: unused value [NOT DETECTED]
+    let a = 1; // BAD: unused value
     let b = 1;
     let c = 1;
-    let d = String::from("a"); // BAD: unused value [NOT DETECTED]
+    let d = String::from("a"); // BAD: unused value
     let e = String::from("b");
     let f = 1;
     let _ = 1; // (deliberately unused)
@@ -32,22 +32,22 @@ 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 = 1; // BAD: unused value
     c = 2;
     println!("use {}", c);
-    c = 3; // BAD: unused value [NOT DETECTED]
+    c = 3; // BAD: unused value
 
     d = 1;
     if cond() {
-        d = 2; // BAD: unused value [NOT DETECTED]
+        d = 2; // BAD: unused value
         d = 3;
     } else {
     }
     println!("use {}", d);
 
-    e = 1; // BAD: unused value [NOT DETECTED]
+    e = 1; // BAD: unused value
     if cond() {
         e = 2;
     } else {
@@ -58,16 +58,16 @@ fn locals_2() {
     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 };
     println!("use {}", i);
 
-    _ = 1; // (deliberately unused) [NOT DETECTED]
+    _ = 1; // GOOD (deliberately unused)
 }
 
 // --- structs ---
@@ -84,7 +84,7 @@ impl MyStruct {
 }
 
 fn structs() {
-    let a = MyStruct { val: 1 }; // BAD: unused value [NOT DETECTED]
+    let a = MyStruct { val: 1 }; // BAD: unused value
     let b = MyStruct { val: 2 };
     let c = MyStruct { val: 3 };
     let mut d: MyStruct; // BAD: unused variable
@@ -105,7 +105,7 @@ fn structs() {
 // --- arrays ---
 
 fn arrays() {
-    let is = [1, 2, 3]; // BAD: unused values (x3) [NOT DETECTED]
+    let is = [1, 2, 3]; // BAD: unused value
     let js = [1, 2, 3];
     let ks = [1, 2, 3];
 
@@ -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
     }
 }
 
@@ -200,7 +200,7 @@ fn loops() {
 
     for x // SPURIOUS: unused variable
     in 1..10 {
-        _ = format!("x is {x}");
+        _ = format!("x is {x}"); // SPURIOUS: unused value `res`
     }
 
     for x
@@ -215,12 +215,12 @@ fn loops() {
 
     for x
     in 1..10 {
-        assert_eq!(x, 1);
+        assert_eq!(x, 1); // SPURIOUS: unused value `kind`
     }
 
     for x
     in 1..10 {
-        assert_eq!(id(x), id(1));
+        assert_eq!(id(x), id(1)); // SPURIOUS: unused value `kind`
     }
 
 }
@@ -255,7 +255,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;
     }
@@ -270,25 +271,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 value
         }
+        None => {}
     }
 
     let e = Option::Some(80);
     match e {
         Option::Some(val) => { // BAD: unused variable
         }
-        Option::None => {
-        }
+        Option::None => {}
     }
 
     let f = MyOption::Some(90);
@@ -298,10 +296,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
     }
 
@@ -327,8 +324,7 @@ fn if_lets_matches() {
     }
 
     let l = Yes;
-    if let Yes = l {
-    }
+    if let Yes = l {}
 
     match 1 {
         1 => {}
@@ -337,22 +333,19 @@ 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
         }
     }
 
     let duration1 = std::time::Duration::new(10, 0); // ten seconds
-    assert_eq!(duration1.as_secs(), 10);
+    assert_eq!(duration1.as_secs(), 10); // SPURIOUS: unused value `kind`
 
     let duration2:Result<std::time::Duration, String> =
         Ok(std::time::Duration::new(10, 0));
@@ -374,15 +367,15 @@ fn if_lets_matches() {
 }
 
 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
         }
 
@@ -436,7 +429,7 @@ fn folds_and_closures() {
     let a5 = 1..10;
     _ = a5.fold(0, | acc, val | val); // BAD: unused variable
 
-    let i6 = 1;
+    let i6 = 1; // SPURIOUS: unused value
     let a6 = 1..10;
     _ = a6.fold(0, | acc, val | acc + val + i6);
 }
@@ -449,16 +442,16 @@ 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();
     func_ptrs();
     folds_and_closures();
 
-	unreachable_if();
-	unreachable_panic();
-	unreachable_match();
-	unreachable_loop();
-	unreachable_paren();
+    unreachable_if();
+    unreachable_panic();
+    unreachable_match();
+    unreachable_loop();
+    unreachable_paren();
 }