From 4c8302272a6b26517959004fdd7c5ccb4365bac0 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Wed, 8 Jan 2025 13:57:57 +0100
Subject: [PATCH] Rust: Avoid location based variable analysis

---
 .../rust/elements/internal/VariableImpl.qll   | 155 ++++++++++++++----
 .../strings/inline-taint-flow.expected        |  30 ++++
 .../library-tests/dataflow/strings/main.rs    |   4 +-
 .../test/library-tests/variables/Ssa.expected |  14 +-
 rust/ql/test/library-tests/variables/main.rs  |   7 +-
 .../variables/variables.expected              |   6 +-
 .../test/library-tests/variables/variables.ql |  16 +-
 .../unusedentities/UnusedValue.expected       |   1 +
 .../test/query-tests/unusedentities/main.rs   |  18 +-
 9 files changed, 200 insertions(+), 51 deletions(-)

diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
index 77087fa06463f..5ebb3c5dcd87a 100644
--- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
+++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -165,7 +165,7 @@ module Impl {
 
   /**
    * A path expression that may access a local variable. These are paths that
-   * only consists of a simple name (i.e., without generic arguments,
+   * only consist of a simple name (i.e., without generic arguments,
    * qualifiers, etc.).
    */
   private class VariableAccessCand extends PathExprBase {
@@ -231,23 +231,115 @@ module Impl {
     )
   }
 
+  /** A subset of `Element`s for which we want to compute pre-order numbers. */
+  private class RelevantElement extends Element {
+    RelevantElement() {
+      this instanceof VariableScope or
+      this instanceof VariableAccessCand or
+      this instanceof LetStmt or
+      getImmediateChildAndAccessor(this, _, _) instanceof RelevantElement
+    }
+
+    pragma[nomagic]
+    private RelevantElement getChild(int index) {
+      result = getImmediateChildAndAccessor(this, index, _)
+    }
+
+    pragma[nomagic]
+    private RelevantElement getImmediateChildMin(int index) {
+      // A child may have multiple positions for different accessors,
+      // so always use the first
+      result = this.getChild(index) and
+      index = min(int i | result = this.getChild(i) | i)
+    }
+
+    pragma[nomagic]
+    RelevantElement getImmediateChild(int index) {
+      result =
+        rank[index + 1](Element res, int i | res = this.getImmediateChildMin(i) | res order by i)
+    }
+
+    pragma[nomagic]
+    RelevantElement getImmediateLastChild() {
+      exists(int last |
+        result = this.getImmediateChild(last) and
+        not exists(this.getImmediateChild(last + 1))
+      )
+    }
+  }
+
+  /**
+   * Gets the pre-order numbering of `n`, where the immediately enclosing
+   * variable scope of `n` is `scope`.
+   */
+  pragma[nomagic]
+  private int getPreOrderNumbering(VariableScope scope, RelevantElement n) {
+    n = scope and
+    result = 0
+    or
+    exists(RelevantElement parent |
+      not parent instanceof VariableScope
+      or
+      parent = scope
+    |
+      // first child of a previously numbered node
+      result = getPreOrderNumbering(scope, parent) + 1 and
+      n = parent.getImmediateChild(0)
+      or
+      // non-first child of a previously numbered node
+      exists(RelevantElement child, int i |
+        result = getLastPreOrderNumbering(scope, child) + 1 and
+        child = parent.getImmediateChild(i) and
+        n = parent.getImmediateChild(i + 1)
+      )
+    )
+  }
+
+  /**
+   * Gets the pre-order numbering of the _last_ node nested under `n`, where the
+   * immediately enclosing variable scope of `n` (and the last node) is `scope`.
+   */
+  pragma[nomagic]
+  private int getLastPreOrderNumbering(VariableScope scope, RelevantElement n) {
+    exists(RelevantElement leaf |
+      result = getPreOrderNumbering(scope, leaf) and
+      leaf != scope and
+      (
+        not exists(leaf.getImmediateChild(_))
+        or
+        leaf instanceof VariableScope
+      )
+    |
+      n = leaf
+      or
+      n.getImmediateLastChild() = leaf and
+      not n instanceof VariableScope
+    )
+    or
+    exists(RelevantElement mid |
+      mid = n.getImmediateLastChild() and
+      result = getLastPreOrderNumbering(scope, mid) and
+      not mid instanceof VariableScope and
+      not n instanceof VariableScope
+    )
+  }
+
   /**
    * Holds if `v` is named `name` and is declared inside variable scope
-   * `scope`, and `v` is bound starting from `(line, column)`.
+   * `scope`. The pre-order numbering of the binding site of `v`, amongst
+   * all nodes nester under `scope`, is `ord`.
    */
-  private predicate variableDeclInScope(
-    Variable v, VariableScope scope, string name, int line, int column
-  ) {
+  private predicate variableDeclInScope(Variable v, VariableScope scope, string name, int ord) {
     name = v.getName() and
     (
       parameterDeclInScope(v, scope) and
-      scope.getLocation().hasLocationFileInfo(_, line, column, _, _)
+      ord = getPreOrderNumbering(scope, scope)
       or
       exists(Pat pat | pat = getAVariablePatAncestor(v) |
         scope =
           any(MatchArmScope arm |
             arm.getPat() = pat and
-            arm.getLocation().hasLocationFileInfo(_, line, column, _, _)
+            ord = getPreOrderNumbering(scope, arm)
           )
         or
         exists(LetStmt let |
@@ -255,56 +347,53 @@ module Impl {
           scope = getEnclosingScope(let) and
           // for `let` statements, variables are bound _after_ the statement, i.e.
           // not in the RHS
-          let.getLocation().hasLocationFileInfo(_, _, _, line, column)
+          ord = getLastPreOrderNumbering(scope, let) + 1
         )
         or
         exists(IfExpr ie, LetExpr let |
           let.getPat() = pat and
           ie.getCondition() = let and
           scope = ie.getThen() and
-          scope.getLocation().hasLocationFileInfo(_, line, column, _, _)
+          ord = getPreOrderNumbering(scope, scope)
         )
         or
         exists(ForExpr fe |
           fe.getPat() = pat and
           scope = fe.getLoopBody() and
-          scope.getLocation().hasLocationFileInfo(_, line, column, _, _)
+          ord = getPreOrderNumbering(scope, scope)
         )
         or
         exists(WhileExpr we, LetExpr let |
           let.getPat() = pat and
           we.getCondition() = let and
           scope = we.getLoopBody() and
-          scope.getLocation().hasLocationFileInfo(_, line, column, _, _)
+          ord = getPreOrderNumbering(scope, scope)
         )
       )
     )
   }
 
   /**
-   * Holds if `cand` may access a variable named `name` at
-   * `(startline, startcolumn, endline, endcolumn)` in the variable scope
-   * `scope`.
+   * Holds if `cand` may access a variable named `name` at pre-order number `ord`
+   * in the variable scope `scope`.
    *
    * `nestLevel` is the number of nested scopes that need to be traversed
    * to reach `scope` from `cand`.
    */
   private predicate variableAccessCandInScope(
-    VariableAccessCand cand, VariableScope scope, string name, int nestLevel, int startline,
-    int startcolumn, int endline, int endcolumn
+    VariableAccessCand cand, VariableScope scope, string name, int nestLevel, int ord
   ) {
     name = cand.getName() and
     scope = [cand.(VariableScope), getEnclosingScope(cand)] and
-    cand.getLocation().hasLocationFileInfo(_, startline, startcolumn, endline, endcolumn) and
+    ord = getPreOrderNumbering(scope, cand) and
     nestLevel = 0
     or
     exists(VariableScope inner |
-      variableAccessCandInScope(cand, inner, name, nestLevel - 1, _, _, _, _) and
+      variableAccessCandInScope(cand, inner, name, nestLevel - 1, _) and
       scope = getEnclosingScope(inner) and
-      // Use the location of the inner scope as the location of the access, instead of the
-      // actual access location. This allows us to collapse multiple accesses in inner
-      // scopes to a single entity
-      inner.getLocation().hasLocationFileInfo(_, startline, startcolumn, endline, endcolumn)
+      // Use the pre-order number of the inner scope as the number of the access. This allows
+      // us to collapse multiple accesses in inner scopes to a single entity
+      ord = getPreOrderNumbering(scope, inner)
     )
   }
 
@@ -375,15 +464,12 @@ module Impl {
     }
 
     pragma[nomagic]
-    predicate rankBy(
-      string name, VariableScope scope, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      variableDeclInScope(this.asVariable(), scope, name, startline, startcolumn) and
-      endline = -1 and
-      endcolumn = -1
+    predicate rankBy(string name, VariableScope scope, int ord, int kind) {
+      variableDeclInScope(this.asVariable(), scope, name, ord) and
+      kind = 0
       or
-      variableAccessCandInScope(this.asVariableAccessCand(), scope, name, _, startline, startcolumn,
-        endline, endcolumn)
+      variableAccessCandInScope(this.asVariableAccessCand(), scope, name, _, ord) and
+      kind = 1
     }
   }
 
@@ -396,11 +482,10 @@ module Impl {
 
     int getRank(VariableScope scope, string name, VariableOrAccessCand v) {
       v =
-        rank[result](VariableOrAccessCand v0, int startline, int startcolumn, int endline,
-          int endcolumn |
-          v0.rankBy(name, scope, startline, startcolumn, endline, endcolumn)
+        rank[result](VariableOrAccessCand v0, int ord, int kind |
+          v0.rankBy(name, scope, ord, kind)
         |
-          v0 order by startline, startcolumn, endline, endcolumn
+          v0 order by ord, kind
         )
     }
   }
@@ -440,7 +525,7 @@ module Impl {
     exists(int rnk |
       variableReachesRank(scope, name, v, rnk) and
       rnk = rankVariableOrAccess(scope, name, TVariableOrAccessCandVariableAccessCand(cand)) and
-      variableAccessCandInScope(cand, scope, name, nestLevel, _, _, _, _)
+      variableAccessCandInScope(cand, scope, name, nestLevel, _)
     )
   }
 
diff --git a/rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected b/rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected
index 3349f43dcb3d4..0e6b9351ac7ef 100644
--- a/rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected
+++ b/rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected
@@ -1,6 +1,7 @@
 models
 | 1 | Summary: lang:alloc; <crate::string::String>::as_str; Argument[self]; ReturnValue; taint |
 | 2 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint |
+| 3 | Summary: lang:core; crate::hint::must_use; Argument[0]; ReturnValue; value |
 edges
 | main.rs:26:9:26:9 | s | main.rs:27:19:27:25 | s[...] | provenance |  |
 | main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | s | provenance |  |
@@ -27,6 +28,19 @@ edges
 | main.rs:77:9:77:18 | formatted3 | main.rs:78:10:78:19 | formatted3 | provenance |  |
 | main.rs:77:22:77:75 | ...::format(...) | main.rs:77:9:77:18 | formatted3 | provenance |  |
 | main.rs:77:34:77:74 | MacroExpr | main.rs:77:22:77:75 | ...::format(...) | provenance | MaD:2 |
+| main.rs:82:9:82:10 | s1 | main.rs:86:18:86:25 | MacroExpr | provenance |  |
+| main.rs:82:9:82:10 | s1 | main.rs:87:18:87:32 | MacroExpr | provenance |  |
+| main.rs:82:14:82:23 | source(...) | main.rs:82:9:82:10 | s1 | provenance |  |
+| main.rs:86:10:86:26 | res | main.rs:86:18:86:25 | { ... } | provenance |  |
+| main.rs:86:18:86:25 | ...::format(...) | main.rs:86:10:86:26 | res | provenance |  |
+| main.rs:86:18:86:25 | ...::must_use(...) | main.rs:86:10:86:26 | MacroExpr | provenance |  |
+| main.rs:86:18:86:25 | MacroExpr | main.rs:86:18:86:25 | ...::format(...) | provenance | MaD:2 |
+| main.rs:86:18:86:25 | { ... } | main.rs:86:18:86:25 | ...::must_use(...) | provenance | MaD:3 |
+| main.rs:87:10:87:33 | res | main.rs:87:18:87:32 | { ... } | provenance |  |
+| main.rs:87:18:87:32 | ...::format(...) | main.rs:87:10:87:33 | res | provenance |  |
+| main.rs:87:18:87:32 | ...::must_use(...) | main.rs:87:10:87:33 | MacroExpr | provenance |  |
+| main.rs:87:18:87:32 | MacroExpr | main.rs:87:18:87:32 | ...::format(...) | provenance | MaD:2 |
+| main.rs:87:18:87:32 | { ... } | main.rs:87:18:87:32 | ...::must_use(...) | provenance | MaD:3 |
 nodes
 | main.rs:26:9:26:9 | s | semmle.label | s |
 | main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |
@@ -58,6 +72,20 @@ nodes
 | main.rs:77:22:77:75 | ...::format(...) | semmle.label | ...::format(...) |
 | main.rs:77:34:77:74 | MacroExpr | semmle.label | MacroExpr |
 | main.rs:78:10:78:19 | formatted3 | semmle.label | formatted3 |
+| main.rs:82:9:82:10 | s1 | semmle.label | s1 |
+| main.rs:82:14:82:23 | source(...) | semmle.label | source(...) |
+| main.rs:86:10:86:26 | MacroExpr | semmle.label | MacroExpr |
+| main.rs:86:10:86:26 | res | semmle.label | res |
+| main.rs:86:18:86:25 | ...::format(...) | semmle.label | ...::format(...) |
+| main.rs:86:18:86:25 | ...::must_use(...) | semmle.label | ...::must_use(...) |
+| main.rs:86:18:86:25 | MacroExpr | semmle.label | MacroExpr |
+| main.rs:86:18:86:25 | { ... } | semmle.label | { ... } |
+| main.rs:87:10:87:33 | MacroExpr | semmle.label | MacroExpr |
+| main.rs:87:10:87:33 | res | semmle.label | res |
+| main.rs:87:18:87:32 | ...::format(...) | semmle.label | ...::format(...) |
+| main.rs:87:18:87:32 | ...::must_use(...) | semmle.label | ...::must_use(...) |
+| main.rs:87:18:87:32 | MacroExpr | semmle.label | MacroExpr |
+| main.rs:87:18:87:32 | { ... } | semmle.label | { ... } |
 subpaths
 testFailures
 #select
@@ -67,3 +95,5 @@ testFailures
 | main.rs:71:10:71:19 | formatted1 | main.rs:68:13:68:22 | source(...) | main.rs:71:10:71:19 | formatted1 | $@ | main.rs:68:13:68:22 | source(...) | source(...) |
 | main.rs:74:10:74:19 | formatted2 | main.rs:68:13:68:22 | source(...) | main.rs:74:10:74:19 | formatted2 | $@ | main.rs:68:13:68:22 | source(...) | source(...) |
 | main.rs:78:10:78:19 | formatted3 | main.rs:76:17:76:32 | source_usize(...) | main.rs:78:10:78:19 | formatted3 | $@ | main.rs:76:17:76:32 | source_usize(...) | source_usize(...) |
+| main.rs:86:10:86:26 | MacroExpr | main.rs:82:14:82:23 | source(...) | main.rs:86:10:86:26 | MacroExpr | $@ | main.rs:82:14:82:23 | source(...) | source(...) |
+| main.rs:87:10:87:33 | MacroExpr | main.rs:82:14:82:23 | source(...) | main.rs:87:10:87:33 | MacroExpr | $@ | main.rs:82:14:82:23 | source(...) | source(...) |
diff --git a/rust/ql/test/library-tests/dataflow/strings/main.rs b/rust/ql/test/library-tests/dataflow/strings/main.rs
index 0194546250b92..975fc99ce1300 100644
--- a/rust/ql/test/library-tests/dataflow/strings/main.rs
+++ b/rust/ql/test/library-tests/dataflow/strings/main.rs
@@ -83,8 +83,8 @@ fn format_macro() {
     let s2 = "2";
     let s3 = "3";
 
-    sink(format!("{}", s1)); // $ MISSING: hasTaintFlow=34
-    sink(format!("{s1} and {s3}")); // $ MISSING: hasTaintFlow=34
+    sink(format!("{}", s1)); // $ hasTaintFlow=34
+    sink(format!("{s1} and {s3}")); // $ hasTaintFlow=34
     sink(format!("{s2} and {s3}"));
 }
 
diff --git a/rust/ql/test/library-tests/variables/Ssa.expected b/rust/ql/test/library-tests/variables/Ssa.expected
index a82a66e72559e..d1b99a69082c5 100644
--- a/rust/ql/test/library-tests/variables/Ssa.expected
+++ b/rust/ql/test/library-tests/variables/Ssa.expected
@@ -146,7 +146,9 @@ definition
 | main.rs:523:9:523:9 | z | main.rs:523:9:523:9 | z |
 | main.rs:532:10:532:18 | SelfParam | main.rs:532:15:532:18 | self |
 | main.rs:563:9:563:22 | var_from_macro | main.rs:563:9:563:22 | var_from_macro |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
 | main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro |
+| main.rs:567:15:567:42 | var_in_macro | main.rs:567:15:567:42 | var_in_macro |
 read
 | main.rs:3:14:3:14 | s | main.rs:3:14:3:14 | s | main.rs:4:20:4:20 | s |
 | main.rs:7:14:7:14 | i | main.rs:7:14:7:14 | i | main.rs:8:20:8:20 | i |
@@ -285,8 +287,9 @@ read
 | main.rs:523:9:523:9 | z | main.rs:523:9:523:9 | z | main.rs:524:20:524:20 | z |
 | main.rs:532:10:532:18 | SelfParam | main.rs:532:15:532:18 | self | main.rs:533:6:533:9 | self |
 | main.rs:563:9:563:22 | var_from_macro | main.rs:563:9:563:22 | var_from_macro | main.rs:565:15:565:28 | var_from_macro |
-| main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:567:30:567:41 | var_in_macro |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
 | main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:568:15:568:26 | var_in_macro |
+| main.rs:567:15:567:42 | var_in_macro | main.rs:567:15:567:42 | var_in_macro | main.rs:567:30:567:41 | var_in_macro |
 firstRead
 | main.rs:3:14:3:14 | s | main.rs:3:14:3:14 | s | main.rs:4:20:4:20 | s |
 | main.rs:7:14:7:14 | i | main.rs:7:14:7:14 | i | main.rs:8:20:8:20 | i |
@@ -398,7 +401,9 @@ firstRead
 | main.rs:523:9:523:9 | z | main.rs:523:9:523:9 | z | main.rs:524:20:524:20 | z |
 | main.rs:532:10:532:18 | SelfParam | main.rs:532:15:532:18 | self | main.rs:533:6:533:9 | self |
 | main.rs:563:9:563:22 | var_from_macro | main.rs:563:9:563:22 | var_from_macro | main.rs:565:15:565:28 | var_from_macro |
-| main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:567:30:567:41 | var_in_macro |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
+| main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:568:15:568:26 | var_in_macro |
+| main.rs:567:15:567:42 | var_in_macro | main.rs:567:15:567:42 | var_in_macro | main.rs:567:30:567:41 | var_in_macro |
 lastRead
 | main.rs:3:14:3:14 | s | main.rs:3:14:3:14 | s | main.rs:4:20:4:20 | s |
 | main.rs:7:14:7:14 | i | main.rs:7:14:7:14 | i | main.rs:8:20:8:20 | i |
@@ -511,7 +516,9 @@ lastRead
 | main.rs:523:9:523:9 | z | main.rs:523:9:523:9 | z | main.rs:524:20:524:20 | z |
 | main.rs:532:10:532:18 | SelfParam | main.rs:532:15:532:18 | self | main.rs:533:6:533:9 | self |
 | main.rs:563:9:563:22 | var_from_macro | main.rs:563:9:563:22 | var_from_macro | main.rs:565:15:565:28 | var_from_macro |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
 | main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:568:15:568:26 | var_in_macro |
+| main.rs:567:15:567:42 | var_in_macro | main.rs:567:15:567:42 | var_in_macro | main.rs:567:30:567:41 | var_in_macro |
 adjacentReads
 | main.rs:35:9:35:10 | x3 | main.rs:35:9:35:10 | x3 | main.rs:36:15:36:16 | x3 | main.rs:38:9:38:10 | x3 |
 | main.rs:43:9:43:10 | x4 | main.rs:43:9:43:10 | x4 | main.rs:44:15:44:16 | x4 | main.rs:49:15:49:16 | x4 |
@@ -543,7 +550,6 @@ adjacentReads
 | main.rs:510:9:510:13 | a | main.rs:510:13:510:13 | a | main.rs:511:15:511:15 | a | main.rs:512:5:512:5 | a |
 | main.rs:510:9:510:13 | a | main.rs:510:13:510:13 | a | main.rs:512:5:512:5 | a | main.rs:513:15:513:15 | a |
 | main.rs:519:9:519:9 | x | main.rs:519:9:519:9 | x | main.rs:520:20:520:20 | x | main.rs:521:15:521:15 | x |
-| main.rs:566:9:566:20 | var_in_macro | main.rs:566:9:566:20 | var_in_macro | main.rs:567:30:567:41 | var_in_macro | main.rs:568:15:568:26 | var_in_macro |
 phi
 | main.rs:191:9:191:44 | [match(true)] phi | main.rs:191:9:191:44 | a3 | main.rs:191:22:191:23 | a3 |
 | main.rs:191:9:191:44 | [match(true)] phi | main.rs:191:9:191:44 | a3 | main.rs:191:42:191:43 | a3 |
@@ -665,4 +671,6 @@ assigns
 | main.rs:519:9:519:9 | x | main.rs:519:13:519:14 | 16 |
 | main.rs:523:9:523:9 | z | main.rs:523:13:523:14 | 17 |
 | main.rs:563:9:563:22 | var_from_macro | main.rs:564:9:564:25 | MacroExpr |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:23:564:24 | 37 |
 | main.rs:566:9:566:20 | var_in_macro | main.rs:566:24:566:25 | 33 |
+| main.rs:567:15:567:42 | var_in_macro | main.rs:567:15:567:42 | 0 |
diff --git a/rust/ql/test/library-tests/variables/main.rs b/rust/ql/test/library-tests/variables/main.rs
index 5b76d22f034cf..f3ab36c2a646b 100644
--- a/rust/ql/test/library-tests/variables/main.rs
+++ b/rust/ql/test/library-tests/variables/main.rs
@@ -561,10 +561,13 @@ macro_rules! let_in_macro2 {
 
 fn macro_invocation() {
     let var_from_macro = // var_from_macro1
-        let_in_macro!(37); // $ MISSING: read_access=var_in_macro
+        let_in_macro!(37); // $ read_access=var_in_macro
     print_i64(var_from_macro); // $ read_access=var_from_macro1
     let var_in_macro = 33; // var_in_macro1
-    print_i64(let_in_macro2!(var_in_macro)); // $ read_access=var_in_macro1
+    // 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
+    print_i64(let_in_macro2!(var_in_macro)); // $ MISSING: read_access=var_in_macro1 $ SPURIOUS: read_access=var_in_macro
     print_i64(var_in_macro); // $ read_access=var_in_macro1
 }
 
diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected
index cf1581387a165..d62c278c875cb 100644
--- a/rust/ql/test/library-tests/variables/variables.expected
+++ b/rust/ql/test/library-tests/variables/variables.expected
@@ -278,8 +278,9 @@ variableAccess
 | main.rs:533:6:533:9 | self | main.rs:532:15:532:18 | self |
 | main.rs:539:3:539:3 | a | main.rs:538:11:538:11 | a |
 | main.rs:541:13:541:13 | a | main.rs:538:11:538:11 | a |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
 | main.rs:565:15:565:28 | var_from_macro | main.rs:563:9:563:22 | var_from_macro |
-| main.rs:567:30:567:41 | var_in_macro | main.rs:566:9:566:20 | var_in_macro |
+| main.rs:567:30:567:41 | var_in_macro | main.rs:567:15:567:42 | var_in_macro |
 | main.rs:568:15:568:26 | var_in_macro | main.rs:566:9:566:20 | var_in_macro |
 variableWriteAccess
 | main.rs:23:5:23:6 | x2 | main.rs:21:13:21:14 | x2 |
@@ -440,8 +441,9 @@ variableReadAccess
 | main.rs:533:6:533:9 | self | main.rs:532:15:532:18 | self |
 | main.rs:539:3:539:3 | a | main.rs:538:11:538:11 | a |
 | main.rs:541:13:541:13 | a | main.rs:538:11:538:11 | a |
+| main.rs:564:9:564:25 | var_in_macro | main.rs:564:9:564:25 | var_in_macro |
 | main.rs:565:15:565:28 | var_from_macro | main.rs:563:9:563:22 | var_from_macro |
-| main.rs:567:30:567:41 | var_in_macro | main.rs:566:9:566:20 | var_in_macro |
+| main.rs:567:30:567:41 | var_in_macro | main.rs:567:15:567:42 | var_in_macro |
 | main.rs:568:15:568:26 | var_in_macro | main.rs:566:9:566:20 | var_in_macro |
 variableInitializer
 | main.rs:16:9:16:10 | x1 | main.rs:16:14:16:16 | "a" |
diff --git a/rust/ql/test/library-tests/variables/variables.ql b/rust/ql/test/library-tests/variables/variables.ql
index cbadfe53d3468..f3ecd6f068511 100644
--- a/rust/ql/test/library-tests/variables/variables.ql
+++ b/rust/ql/test/library-tests/variables/variables.ql
@@ -18,8 +18,9 @@ query predicate capturedAccess(VariableAccess va) { va.isCapture() }
 module VariableAccessTest implements TestSig {
   string getARelevantTag() { result = ["", "write_", "read_"] + "access" }
 
-  private predicate declAt(Variable v, string filepath, int line) {
-    v.getLocation().hasLocationInfo(filepath, _, _, line, _)
+  private predicate declAt(Variable v, string filepath, int line, boolean inMacro) {
+    v.getLocation().hasLocationInfo(filepath, _, _, line, _) and
+    if v.getPat().isInMacroExpansion() then inMacro = true else inMacro = false
   }
 
   private predicate commmentAt(string text, string filepath, int line) {
@@ -30,10 +31,15 @@ module VariableAccessTest implements TestSig {
   }
 
   private predicate decl(Variable v, string value) {
-    exists(string filepath, int line | declAt(v, filepath, line) |
-      commmentAt(value, filepath, line)
+    exists(string filepath, int line, boolean inMacro | declAt(v, filepath, line, inMacro) |
+      commmentAt(value, filepath, line) and
+      inMacro = false
       or
-      not commmentAt(_, filepath, line) and
+      (
+        not commmentAt(_, filepath, line)
+        or
+        inMacro = true
+      ) and
       value = v.getName()
     )
   }
diff --git a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
index d5b3854076479..bae487e716e11 100644
--- a/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
+++ b/rust/ql/test/query-tests/unusedentities/UnusedValue.expected
@@ -15,6 +15,7 @@
 | 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 |
 | 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:9: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 2f729690f92ec..ccd63f1cd53a9 100644
--- a/rust/ql/test/query-tests/unusedentities/main.rs
+++ b/rust/ql/test/query-tests/unusedentities/main.rs
@@ -496,8 +496,7 @@ fn references() {
 pub struct my_declaration {
     field1: fn(i32) -> i32,
     field2: fn(x: i32) -> i32,
-    field3: fn(y:
-        fn(z: i32) -> i32) -> i32,
+    field3: fn(y: fn(z: i32) -> i32) -> i32,
 }
 
 type MyType = fn(x: i32) -> i32;
@@ -506,6 +505,21 @@ 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() {