Skip to content

Commit

Permalink
Rust: Handle ref patterns in data flow
Browse files Browse the repository at this point in the history
To do this we:
* Let SSA writes target the name inside identifier patterns instead of
  the pattern itself
* Include relevant names in the data flow graph
* Add a store step from a identifier patterns with `ref` into the
  contained name. So we have an edge `ref a` -> `a` that stores in the
  reference content type.
  • Loading branch information
paldepind committed Feb 12, 2025
1 parent 5da1425 commit 341c62f
Show file tree
Hide file tree
Showing 17 changed files with 454 additions and 161 deletions.
2 changes: 1 addition & 1 deletion rust/ql/.generated.list

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ module PatternTrees {
}

override predicate succ(AstNode pred, AstNode succ, Completion c) {
// Edge from succesful subpattern to name
// Edge from successful subpattern to name
super.succ(pred, succ, c) and c.(MatchCompletion).succeeded()
or
// Edge from name to the identifier pattern itself
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rust/ql/lib/codeql/rust/dataflow/Ssa.qll
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ module Ssa {
)
or
exists(LetStmtCfgNode ls |
ls.getPat() = write and
ls.getPat().(IdentPatCfgNode).getName() = write and
ls.getInitializer() = value
)
}
Expand Down
30 changes: 30 additions & 0 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,15 @@ module Node {
override PatCfgNode asPat() { result = n }
}

/** A data flow node that corresponds to a name node in the CFG. */
final class NameNode extends AstCfgFlowNode, TNameNode {
override NameCfgNode n;

NameNode() { this = TNameNode(n) }

NameCfgNode asName() { result = n }
}

/**
* The value of a parameter at function entry, viewed as a node in a data
* flow graph.
Expand Down Expand Up @@ -584,11 +593,23 @@ module LocalFlow {
predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
nodeFrom.getCfgNode() = getALastEvalNode(nodeTo.getCfgNode())
or
// An edge from the right-hand side of a let statement to the left-hand side.
exists(LetStmtCfgNode s |
nodeFrom.getCfgNode() = s.getInitializer() and
nodeTo.getCfgNode() = s.getPat()
)
or
exists(IdentPatCfgNode p |
not p.isRef() and
nodeFrom.getCfgNode() = p and
nodeTo.getCfgNode() = p.getName()
)
or
exists(SelfParamCfgNode self |
nodeFrom.getCfgNode() = self and
nodeTo.getCfgNode() = self.getName()
)
or
// An edge from a pattern/expression to its corresponding SSA definition.
nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()
Expand Down Expand Up @@ -1266,6 +1287,14 @@ module RustDataFlow implements InputSig<Location> {
node2.asExpr().(ArrayListExprCfgNode).getAnExpr()
]
or
// Store from a `ref` identifier pattern into the contained name.
exists(IdentPatCfgNode p |
c instanceof ReferenceContent and
p.isRef() and
node1.asPat() = p and
node2.(Node::NameNode).asName() = p.getName()
)
or
fieldAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
or
referenceAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
Expand Down Expand Up @@ -1560,6 +1589,7 @@ private module Cached {
TExprNode(ExprCfgNode n) { Stages::DataFlowStage::ref() } or
TSourceParameterNode(ParamBaseCfgNode p) or
TPatNode(PatCfgNode p) or
TNameNode(NameCfgNode n) { n.getName() = any(Variable v).getName() } or
TExprPostUpdateNode(ExprCfgNode e) {
isArgumentForCall(e, _, _) or
lambdaCallExpr(_, _, e) or
Expand Down
20 changes: 10 additions & 10 deletions rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ private import Cfg
private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
private import codeql.ssa.Ssa as SsaImplCommon

/** Holds if `v` is introduced like `let v : i64;`. */
private predicate isUnitializedLet(IdentPat pat, Variable v) {
pat = v.getPat() and
/**
* Holds if `name` occurs in the left-hand side of an uninitialized let
* statement such as in `let name : i64;`.
*/
private predicate isInUninitializedLet(Name name) {
exists(LetStmt let |
let = v.getLetStmt() and
let.getPat().(IdentPat).getName() = name and
not let.hasInitializer()
)
}

/** Holds if `write` writes to variable `v`. */
predicate variableWrite(AstNode write, Variable v) {
exists(IdentPat pat |
pat = write and
pat = v.getPat() and
not isUnitializedLet(pat, v)
exists(Name name |
name = write and
name = v.getName() and
not isInUninitializedLet(name)
)
or
exists(SelfParam self | self = write and self = v.getSelfParam())
or
exists(VariableAccess access |
access = write and
access.getVariable() = v
Expand Down
52 changes: 32 additions & 20 deletions rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -74,58 +74,66 @@ module Impl {
* where `definingNode` is the entire `Either::Left(x) | Either::Right(x)`
* pattern.
*/
private predicate variableDecl(AstNode definingNode, AstNode p, string name) {
p =
any(SelfParam sp |
definingNode = sp.getName() and
name = sp.getName().getText() and
private predicate variableDecl(AstNode definingNode, Name name, string text) {
text = name.getText() and
(
exists(SelfParam sp |
name = sp.getName() and
definingNode = name and
// exclude self parameters from functions without a body as these are
// trait method declarations without implementations
not exists(Function f | not f.hasBody() and f.getParamList().getSelfParam() = sp)
)
or
p =
any(IdentPat pat |
or
exists(IdentPat pat |
name = pat.getName() and
(
definingNode = getOutermostEnclosingOrPat(pat)
or
not exists(getOutermostEnclosingOrPat(pat)) and definingNode = pat.getName()
not exists(getOutermostEnclosingOrPat(pat)) and definingNode = name
) and
name = pat.getName().getText() and
// exclude for now anything starting with an uppercase character, which may be a reference to
// an enum constant (e.g. `None`). This excludes static and constant variables (UPPERCASE),
// which we don't appear to recognize yet anyway. This also assumes programmers follow the
// naming guidelines, which they generally do, but they're not enforced.
not name.charAt(0).isUppercase() and
not text.charAt(0).isUppercase() and
// exclude parameters from functions without a body as these are trait method declarations
// without implementations
not exists(Function f | not f.hasBody() and f.getParamList().getAParam().getPat() = pat) and
// exclude parameters from function pointer types (e.g. `x` in `fn(x: i32) -> i32`)
not exists(FnPtrTypeRepr fp | fp.getParamList().getParam(_).getPat() = pat)
)
)
}

/** A variable. */
class Variable extends MkVariable {
private AstNode definingNode;
private string name;
private string text;

Variable() { this = MkVariable(definingNode, name) }
Variable() { this = MkVariable(definingNode, text) }

/** Gets the name of this variable. */
string getName() { result = name }
/** Gets the name of this variable as a string. */
string getText() { result = text }

/** Gets the location of this variable. */
Location getLocation() { result = definingNode.getLocation() }

/** Gets a textual representation of this variable. */
string toString() { result = this.getName() }
string toString() { result = this.getText() }

/** Gets an access to this variable. */
VariableAccess getAnAccess() { result.getVariable() = this }

/** Gets the `self` parameter that declares this variable, if one exists. */
SelfParam getSelfParam() { variableDecl(definingNode, result, name) }
/**
* Get the name of this variable.
*
* Normally, the name is unique, except when introduced in an or pattern.
*/
Name getName() { variableDecl(definingNode, result, text) }

/** Gets the `self` parameter that declares this variable, if any. */
SelfParam getSelfParam() { result.getName() = this.getName() }

/**
* Gets the pattern that declares this variable, if any.
Expand All @@ -138,7 +146,7 @@ module Impl {
* }
* ```
*/
IdentPat getPat() { variableDecl(definingNode, result, name) }
IdentPat getPat() { result.getName() = this.getName() }

/** Gets the enclosing CFG scope for this variable declaration. */
CfgScope getEnclosingCfgScope() { result = definingNode.getEnclosingCfgScope() }
Expand Down Expand Up @@ -204,6 +212,10 @@ module Impl {
/** Gets the immediately enclosing variable scope of `n`. */
private VariableScope getEnclosingScope(AstNode n) { result = getAnAncestorInVariableScope(n) }

/**
* Get all the pattern ancestors of this variable up to an including the
* root of the pattern.
*/
private Pat getAVariablePatAncestor(Variable v) {
result = v.getPat()
or
Expand Down Expand Up @@ -322,7 +334,7 @@ module Impl {
* all nodes nester under `scope`, is `ord`.
*/
private predicate variableDeclInScope(Variable v, VariableScope scope, string name, int ord) {
name = v.getName() and
name = v.getText() and
(
parameterDeclInScope(v, scope) and
ord = getPreOrderNumbering(scope, scope)
Expand Down
Loading

0 comments on commit 341c62f

Please sign in to comment.