Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,36 @@ class ExitCfgNode = CfgImpl::ExitNode;

class AnnotatedExitCfgNode = CfgImpl::AnnotatedExitNode;

/** A variable access. */
final class VariableAccessCfgNode extends PathExprBaseCfgNode {
private VariableAccess a;

VariableAccessCfgNode() { a = this.getAstNode() }

/** Gets the underlying `VariableAccess`. */
VariableAccess getAccess() { result = a }
}

/** A variable write. */
final class VariableWriteAccessCfgNode extends VariableAccessCfgNode {
private VariableWriteAccess a;

VariableWriteAccessCfgNode() { a = this.getAstNode() }

/** Gets the underlying `VariableWriteAccess`. */
VariableWriteAccess getAccess() { result = a }
}

/** A variable read. */
final class VariableReadAccessCfgNode extends VariableAccessCfgNode {
private VariableReadAccess a;

VariableReadAccessCfgNode() { a = this.getAstNode() }

/** Gets the underlying `VariableReadAccess`. */
VariableReadAccess getAccess() { result = a }
}

/**
* An assignment expression, for example
*
Expand All @@ -24,12 +54,42 @@ class AnnotatedExitCfgNode = CfgImpl::AnnotatedExitNode;
* ```
*/
final class AssignmentExprCfgNode extends BinaryExprCfgNode {
AssignmentExpr a;
AssignmentExprChildMapping a;

AssignmentExprCfgNode() { a = this.getBinaryExpr() }

/** Gets the underlying `AssignmentExpr`. */
AssignmentExpr getAssignmentExpr() { result = a }

/**
* Gets a write access that occurs in the left-hand side of this assignment expression.
*/
VariableWriteAccessCfgNode getAWriteAccess() {
a = result.getAccess().getAssignmentExpr() and
any(ChildMapping mapping).hasCfgChild(a, result.getAccess(), this, result)
}
}

/**
* A compound assignment expression, for example:
* ```rust
* x += y;
* ```
*
* Note that compound assignment expressions are syntatic sugar for
* trait invocations, i.e., the above actually means
*
* ```rust
* (&mut x).add_assign(y);
* ```
*/
final class CompoundAssignmentExprCfgNode extends BinaryExprCfgNode {
CompoundAssignmentExpr a;

CompoundAssignmentExprCfgNode() { a = this.getBinaryExpr() }

/** Gets the underlying `CompoundAssignmentExpr`. */
CompoundAssignmentExpr getCompoundAssignmentExpr() { result = a }
}

/**
Expand Down
6 changes: 6 additions & 0 deletions rust/ql/lib/codeql/rust/controlflow/internal/CfgNodes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ class FormatArgsExprChildMapping extends ParentAstNode, CfgImpl::ExprTrees::Form
override predicate relevantChild(AstNode child) { child = this.getChildNode(_) }
}

class AssignmentExprChildMapping extends ParentAstNode, AssignmentExpr {
override predicate relevantChild(AstNode child) {
child.(VariableWriteAccess).getAssignmentExpr() = this
}
}

private class ChildMappingImpl extends ChildMapping {
/** Gets a CFG node for `child`, where `child` is a relevant child node of `parent`. */
private CfgNode getRelevantChildCfgNode(AstNode parent, AstNode child) {
Expand Down
15 changes: 13 additions & 2 deletions rust/ql/lib/codeql/rust/dataflow/Ssa.qll
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,20 @@ module Ssa {
private CfgNode write;

WriteDefinition() {
exists(BasicBlock bb, int i, Variable v |
exists(BasicBlock bb, int i, Variable v, CfgNode n |
this.definesAt(v, bb, i) and
SsaImpl::variableWriteActual(bb, i, v, write)
SsaImpl::variableWriteActual(bb, i, v, n)
|
write.(VariableAccessCfgNode).getAccess().getVariable() = v and
(
write = n.(AssignmentExprCfgNode).getAWriteAccess()
or
write = n.(CompoundAssignmentExprCfgNode).getLhs()
)
or
not n instanceof AssignmentExprCfgNode and
not n instanceof CompoundAssignmentExprCfgNode and
write = n
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ module LocalFlow {
or
// An edge from a pattern/expression to its corresponding SSA definition.
nodeFrom.(AstCfgFlowNode).getCfgNode() =
nodeTo.(SsaNode).asDefinition().(Ssa::WriteDefinition).getControlFlowNode()
nodeTo.(SsaNode).asDefinition().(Ssa::WriteDefinition).getWriteAccess()
or
nodeFrom.(SourceParameterNode).getParameter().(ParamCfgNode).getPat() = nodeTo.asPat()
or
Expand Down
17 changes: 8 additions & 9 deletions rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,22 @@ private predicate isInUninitializedLet(Name name) {
)
}

/** Holds if `write` writes to variable `v`. */
predicate variableWrite(AstNode write, Variable v) {
/** Holds if `write` writes to variable `v` via `access`. */
predicate variableWrite(AstNode write, AstNode access, Variable v) {
exists(Name name |
name = write and
access = write and
name = v.getName() and
not isInUninitializedLet(name)
)
or
exists(VariableAccess access |
access = write and
access.getVariable() = v
|
access instanceof VariableWriteAccess
v = access.(VariableAccess).getVariable() and
(
write = access.(VariableWriteAccess).getAssignmentExpr()
or
// Although compound assignments, like `x += y`, may in fact not update `x`,
// it makes sense to treat them as such
access = any(CompoundAssignmentExpr cae).getLhs()
access = write.(CompoundAssignmentExpr).getLhs()
)
}

Expand Down Expand Up @@ -226,7 +225,7 @@ private module Cached {
cached
predicate variableWriteActual(BasicBlock bb, int i, Variable v, CfgNode write) {
bb.getNode(i) = write and
variableWrite(write.getAstNode(), v)
variableWrite(write.getAstNode(), _, v)
}

cached
Expand Down
13 changes: 9 additions & 4 deletions rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -679,11 +679,11 @@ module Impl {
}

/** Holds if `e` occurs in the LHS of an assignment or compound assignment. */
private predicate assignmentExprDescendant(Expr e) {
e = any(AssignmentExpr ae).getLhs()
private predicate assignmentExprDescendant(AssignmentExpr ae, Expr e) {
e = ae.getLhs()
or
exists(Expr mid |
assignmentExprDescendant(mid) and
assignmentExprDescendant(ae, mid) and
getImmediateParentAdj(e) = mid and
not mid instanceof DerefExpr and
not mid instanceof FieldExpr and
Expand All @@ -693,11 +693,16 @@ module Impl {

/** A variable write. */
class VariableWriteAccess extends VariableAccess {
private AssignmentExpr ae;

cached
VariableWriteAccess() {
Cached::ref() and
assignmentExprDescendant(this)
assignmentExprDescendant(ae, this)
}

/** Gets the assignment expression that has this write access in the left-hand side. */
AssignmentExpr getAssignmentExpr() { result = ae }
}

/** A variable read. */
Expand Down
2 changes: 1 addition & 1 deletion rust/ql/src/queries/unusedentities/UnusedValue.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import UnusedVariable

from AstNode write, Ssa::Variable v
where
variableWrite(write, v) and
variableWrite(_, write, v) and
not v instanceof DiscardVariable and
not write.isInMacroExpansion() and
not isAllowableUnused(v) and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
multipleCallTargets
| main.rs:89:19:89:40 | ...::from(...) |
| main.rs:111:19:111:40 | ...::from(...) |
| main.rs:91:19:91:40 | ...::from(...) |
| main.rs:113:19:113:40 | ...::from(...) |
Loading