Skip to content

Rust: Implement enclosing callable #17921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Rust: Implement enclosing callable #17921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dadc605
Rust: Implement enclosing callable
paldepind Nov 6, 2024
1a000ec
Rust: Make minor data flow changes based on PR feedback
paldepind Nov 7, 2024
22835c2
Rust: Make a return node the last node before function exit
paldepind Nov 7, 2024
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
8 changes: 8 additions & 0 deletions rust/ql/lib/codeql/rust/controlflow/CfgNodes.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

private import rust
private import ControlFlowGraph
private import internal.ControlFlowGraphImpl

/** A CFG node that corresponds to an element in the AST. */
class AstCfgNode extends CfgNode {
Expand All @@ -20,3 +21,10 @@ class ExprCfgNode extends AstCfgNode {
/** Gets the underlying expression. */
Expr getExpr() { result = node }
}

/** A CFG node that corresponds to a call in the AST. */
class CallCfgNode extends ExprCfgNode {
override CallExpr node;
}

final class ExitCfgNode = ExitNode;
132 changes: 112 additions & 20 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,65 @@ private import codeql.rust.controlflow.ControlFlowGraph
private import codeql.rust.controlflow.CfgNodes
private import codeql.rust.dataflow.Ssa

private newtype TReturnKind = TNormalReturnKind()

/**
* A return kind. A return kind describes how a value can be returned from a
* callable.
*
* The only return kind is a "normal" return from a `return` statement or an
* expression body.
*/
final class ReturnKind extends TNormalReturnKind {
string toString() { result = "return" }
}

/**
* A callable. This includes callables from source code, as well as callables
* defined in library code.
*/
final class DataFlowCallable extends TDataFlowCallable {
/**
* Gets the underlying CFG scope, if any.
*/
CfgScope asCfgScope() { this = TCfgScope(result) }

/** Gets a textual representation of this callable. */
string toString() { result = this.asCfgScope().toString() }

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

abstract class DataFlowCall extends TDataFlowCall {
/** Gets the enclosing callable. */
abstract DataFlowCallable getEnclosingCallable();

/** Gets the underlying source code call, if any. */
abstract CallCfgNode asCall();

/** Gets a textual representation of this call. */
abstract string toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add QL doc: Gets a textual representation of this call.


/** Gets the location of this call. */
abstract Location getLocation();
}

final class NormalCall extends DataFlowCall, TNormalCall {
private CallCfgNode c;

NormalCall() { this = TNormalCall(c) }

/** Gets the underlying call in the CFG, if any. */
override CallCfgNode asCall() { result = c }

override DataFlowCallable getEnclosingCallable() { none() }

override string toString() { result = c.toString() }

override Location getLocation() { result = c.getLocation() }
}

module Node {
/**
* An element, viewed as a node in a data flow graph. Either an expression
Expand All @@ -29,6 +88,12 @@ module Node {
*/
Expr asExpr() { none() }

/** Gets the enclosing callable. */
DataFlowCallable getEnclosingCallable() { result = TCfgScope(this.getCfgScope()) }

/** Do not call: use `getEnclosingCallable()` instead. */
abstract CfgScope getCfgScope();

/**
* Gets the control flow node that corresponds to this data flow node.
*/
Expand All @@ -49,6 +114,8 @@ module Node {
final class NaNode extends Node {
NaNode() { none() }

override CfgScope getCfgScope() { none() }

override string toString() { result = "N/A" }

override Location getLocation() { none() }
Expand All @@ -62,11 +129,13 @@ module Node {
* to multiple `ExprNode`s, just like it may correspond to multiple
* `ControlFlow::Node`s.
*/
final class ExprNode extends Node, TExprNode {
class ExprNode extends Node, TExprNode {
ExprCfgNode n;

ExprNode() { this = TExprNode(n) }

override CfgScope getCfgScope() { result = this.asExpr().getEnclosingCallable() }

override Location getLocation() { result = n.getExpr().getLocation() }

override string toString() { result = n.getExpr().toString() }
Expand All @@ -85,6 +154,8 @@ module Node {

ParameterNode() { this = TParameterNode(parameter) }

override CfgScope getCfgScope() { result = parameter.getEnclosingCallable() }

override Location getLocation() { result = parameter.getLocation() }

override string toString() { result = parameter.toString() }
Expand All @@ -105,6 +176,8 @@ module Node {
def = node.getDefinitionExt()
}

override CfgScope getCfgScope() { result = def.getBasicBlock().getScope() }

SsaImpl::DefinitionExt getDefinitionExt() { result = def }

/** Holds if this node should be hidden from path explanations. */
Expand All @@ -115,11 +188,25 @@ module Node {
override string toString() { result = node.toString() }
}

final class ReturnNode extends NaNode {
RustDataFlow::ReturnKind getKind() { none() }
/** A data flow node that represents a value returned by a callable. */
final class ReturnNode extends ExprNode {
ReturnNode() { this.getCfgNode().getASuccessor() instanceof ExitCfgNode }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should just be ReturnExprs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so we don't want to include implicit returns like fn foo() { 12 }?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, we need to include that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reinstated the old implementation again :). If we want to include implicit returns then I think just looking at edges into the exit node handles all cases.

  • return goes directly to the exit node
  • In the absence of explicit returns, the BlockExpr is last both in a function and in a closure and it lets the final expression flow through if there is no ; at the end.
  • For a lambda without braces |a| a + 1 the single expression has an edge to the exit node.


ReturnKind getKind() { any() }
}

/** A data-flow node that represents the output of a call. */
abstract class OutNode extends Node, ExprNode {
/** Gets the underlying call for this node. */
abstract DataFlowCall getCall();
}

final class OutNode = NaNode;
final private class ExprOutNode extends OutNode {
ExprOutNode() { this.asExpr() instanceof CallExpr }

/** Gets the underlying call CFG node that includes this out node. */
override DataFlowCall getCall() { result.(NormalCall).asCall() = this.getCfgNode() }
}

/**
* A node associated with an object after an operation that might have
Expand Down Expand Up @@ -198,6 +285,12 @@ module LocalFlow {
}
}

private class DataFlowCallableAlias = DataFlowCallable;

private class ReturnKindAlias = ReturnKind;

private class DataFlowCallAlias = DataFlowCall;

module RustDataFlow implements InputSig<Location> {
/**
* An element, viewed as a node in a data flow graph. Either an expression
Expand All @@ -221,7 +314,7 @@ module RustDataFlow implements InputSig<Location> {

predicate isArgumentNode(ArgumentNode n, DataFlowCall call, ArgumentPosition pos) { none() }

DataFlowCallable nodeGetEnclosingCallable(Node node) { none() }
DataFlowCallable nodeGetEnclosingCallable(Node node) { result = node.getEnclosingCallable() }

DataFlowType getNodeType(Node node) { any() }

Expand All @@ -232,26 +325,22 @@ module RustDataFlow implements InputSig<Location> {
/** Gets the node corresponding to `e`. */
Node exprNode(DataFlowExpr e) { result.getCfgNode() = e }

final class DataFlowCall extends TNormalCall {
private CallExpr c;

DataFlowCall() { this = TNormalCall(c) }

DataFlowCallable getEnclosingCallable() { none() }

string toString() { result = c.toString() }

Location getLocation() { result = c.getLocation() }
}
final class DataFlowCall = DataFlowCallAlias;

final class DataFlowCallable = CfgScope;
final class DataFlowCallable = DataFlowCallableAlias;

final class ReturnKind = Void;
final class ReturnKind = ReturnKindAlias;

/** Gets a viable implementation of the target of the given `Call`. */
DataFlowCallable viableCallable(DataFlowCall c) { none() }

OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) { none() }
/**
* Gets a node that can read the value returned from `call` with return kind
* `kind`.
*/
OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
call = result.getCall() and exists(kind)
}

// NOTE: For now we use the type `Unit` and do not benefit from type
// information in the data flow analysis.
Expand Down Expand Up @@ -400,7 +489,7 @@ private module Cached {
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node)

cached
newtype TDataFlowCall = TNormalCall(CallExpr c)
newtype TDataFlowCall = TNormalCall(CallCfgNode c)

cached
newtype TOptionalContentSet =
Expand All @@ -410,6 +499,9 @@ private module Cached {
cached
class TContentSet = TAnyElementContent or TAnyContent;

cached
newtype TDataFlowCallable = TCfgScope(CfgScope scope)

/** This is the local flow predicate that is exposed. */
cached
predicate localFlowStepImpl(Node::Node nodeFrom, Node::Node nodeTo) {
Expand Down
3 changes: 0 additions & 3 deletions rust/ql/test/extractor-tests/generated/BecomeExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
uniqueEnclosingCallable
| gen_become_expr.rs:4:11:4:16 | Param | Node should have one enclosing callable but has 0. |
| gen_become_expr.rs:4:19:4:24 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| gen_become_expr.rs:8:17:8:36 | CallExpr | Call should have one enclosing callable but has 0. |
7 changes: 0 additions & 7 deletions rust/ql/test/extractor-tests/generated/ClosureExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion rust/ql/test/extractor-tests/generated/ContinueExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
uniqueCallEnclosingCallable
| gen_continue_expr.rs:6:12:6:22 | CallExpr | Call should have one enclosing callable but has 0. |
| gen_continue_expr.rs:11:12:11:22 | CallExpr | Call should have one enclosing callable but has 0. |
2 changes: 0 additions & 2 deletions rust/ql/test/extractor-tests/generated/Function/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions rust/ql/test/extractor-tests/generated/LetExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
uniqueEnclosingCallable
| gen_let_expr.rs:3:18:3:43 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| gen_let_expr.rs:6:18:6:24 | CallExpr | Call should have one enclosing callable but has 0. |
1 change: 0 additions & 1 deletion rust/ql/test/extractor-tests/generated/LoopExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
uniqueCallEnclosingCallable
| gen_loop_expr.rs:6:18:6:40 | CallExpr | Call should have one enclosing callable but has 0. |
| gen_loop_expr.rs:9:18:9:39 | CallExpr | Call should have one enclosing callable but has 0. |
3 changes: 0 additions & 3 deletions rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
uniqueEnclosingCallable
| common_definitions.rs:3:15:3:25 | Param | Node should have one enclosing callable but has 0. |
| file://:0:0:0:0 | Param | Node should have one enclosing callable but has 0. |
uniqueNodeLocation
| file://:0:0:0:0 | BlockExpr | Node should have one location but has 0. |
| file://:0:0:0:0 | MethodCallExpr | Node should have one location but has 0. |
Expand Down
2 changes: 0 additions & 2 deletions rust/ql/test/extractor-tests/generated/MatchArm/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions rust/ql/test/extractor-tests/generated/MatchExpr/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop

This file was deleted.

47 changes: 0 additions & 47 deletions rust/ql/test/library-tests/controlflow/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,4 @@
uniqueEnclosingCallable
| test.rs:18:32:18:37 | Param | Node should have one enclosing callable but has 0. |
| test.rs:36:31:36:37 | Param | Node should have one enclosing callable but has 0. |
| test.rs:50:34:50:40 | Param | Node should have one enclosing callable but has 0. |
| test.rs:64:34:64:40 | Param | Node should have one enclosing callable but has 0. |
| test.rs:78:19:78:24 | Param | Node should have one enclosing callable but has 0. |
| test.rs:98:17:98:22 | Param | Node should have one enclosing callable but has 0. |
| test.rs:114:25:114:30 | Param | Node should have one enclosing callable but has 0. |
| test.rs:115:20:115:20 | Param | Node should have one enclosing callable but has 0. |
| test.rs:121:21:121:26 | Param | Node should have one enclosing callable but has 0. |
| test.rs:129:25:129:38 | Param | Node should have one enclosing callable but has 0. |
| test.rs:137:20:137:33 | Param | Node should have one enclosing callable but has 0. |
| test.rs:144:23:144:28 | Param | Node should have one enclosing callable but has 0. |
| test.rs:152:29:152:34 | Param | Node should have one enclosing callable but has 0. |
| test.rs:163:29:163:34 | Param | Node should have one enclosing callable but has 0. |
| test.rs:174:27:174:32 | Param | Node should have one enclosing callable but has 0. |
| test.rs:183:22:183:27 | Param | Node should have one enclosing callable but has 0. |
| test.rs:196:22:196:27 | Param | Node should have one enclosing callable but has 0. |
| test.rs:209:28:209:33 | Param | Node should have one enclosing callable but has 0. |
| test.rs:222:26:222:32 | Param | Node should have one enclosing callable but has 0. |
| test.rs:222:35:222:41 | Param | Node should have one enclosing callable but has 0. |
| test.rs:222:44:222:50 | Param | Node should have one enclosing callable but has 0. |
| test.rs:227:25:227:31 | Param | Node should have one enclosing callable but has 0. |
| test.rs:227:34:227:40 | Param | Node should have one enclosing callable but has 0. |
| test.rs:227:43:227:49 | Param | Node should have one enclosing callable but has 0. |
| test.rs:232:27:232:33 | Param | Node should have one enclosing callable but has 0. |
| test.rs:232:36:232:41 | Param | Node should have one enclosing callable but has 0. |
| test.rs:232:44:232:50 | Param | Node should have one enclosing callable but has 0. |
| test.rs:237:26:237:32 | Param | Node should have one enclosing callable but has 0. |
| test.rs:242:29:242:35 | Param | Node should have one enclosing callable but has 0. |
| test.rs:242:38:242:43 | Param | Node should have one enclosing callable but has 0. |
| test.rs:242:46:242:52 | Param | Node should have one enclosing callable but has 0. |
| test.rs:250:28:250:34 | Param | Node should have one enclosing callable but has 0. |
| test.rs:250:37:250:42 | Param | Node should have one enclosing callable but has 0. |
| test.rs:250:45:250:51 | Param | Node should have one enclosing callable but has 0. |
| test.rs:258:29:258:35 | Param | Node should have one enclosing callable but has 0. |
| test.rs:269:38:269:44 | Param | Node should have one enclosing callable but has 0. |
| test.rs:273:38:273:52 | Param | Node should have one enclosing callable but has 0. |
| test.rs:283:19:283:42 | Param | Node should have one enclosing callable but has 0. |
| test.rs:291:44:291:67 | Param | Node should have one enclosing callable but has 0. |
| test.rs:302:23:302:32 | Param | Node should have one enclosing callable but has 0. |
| test.rs:302:35:302:48 | Param | Node should have one enclosing callable but has 0. |
| test.rs:309:35:309:58 | Param | Node should have one enclosing callable but has 0. |
| test.rs:319:23:319:36 | Param | Node should have one enclosing callable but has 0. |
| test.rs:324:29:324:42 | Param | Node should have one enclosing callable but has 0. |
| test.rs:335:28:335:35 | Param | Node should have one enclosing callable but has 0. |
| test.rs:342:29:342:40 | Param | Node should have one enclosing callable but has 0. |
| test.rs:401:15:401:25 | Param | Node should have one enclosing callable but has 0. |
| test.rs:408:16:408:19 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| test.rs:6:9:6:44 | CallExpr | Call should have one enclosing callable but has 0. |
Expand Down
3 changes: 0 additions & 3 deletions rust/ql/test/library-tests/dataflow/barrier/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
uniqueEnclosingCallable
| main.rs:5:9:5:15 | Param | Node should have one enclosing callable but has 0. |
| main.rs:9:13:9:19 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| main.rs:6:14:6:20 | CallExpr | Call should have one enclosing callable but has 0. |
| main.rs:17:13:17:20 | CallExpr | Call should have one enclosing callable but has 0. |
Expand Down
5 changes: 0 additions & 5 deletions rust/ql/test/library-tests/dataflow/local/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
uniqueEnclosingCallable
| main.rs:6:18:6:27 | Param | Node should have one enclosing callable but has 0. |
| main.rs:31:21:31:26 | Param | Node should have one enclosing callable but has 0. |
| main.rs:31:29:31:34 | Param | Node should have one enclosing callable but has 0. |
| main.rs:31:37:31:50 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| main.rs:3:14:3:33 | CallExpr | Call should have one enclosing callable but has 0. |
| main.rs:39:5:39:14 | CallExpr | Call should have one enclosing callable but has 0. |
Expand Down
15 changes: 0 additions & 15 deletions rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
uniqueEnclosingCallable
| variables.rs:3:14:3:20 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:7:14:7:19 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:11:18:11:24 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:261:5:261:12 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:262:5:265:19 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:272:5:272:50 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:316:10:316:15 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:324:10:324:15 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:356:17:356:28 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:363:22:363:36 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:363:39:363:57 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:435:8:435:15 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:451:13:451:21 | Param | Node should have one enclosing callable but has 0. |
| variables.rs:451:24:451:32 | Param | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
| variables.rs:4:14:4:20 | CallExpr | Call should have one enclosing callable but has 0. |
| variables.rs:8:14:8:20 | CallExpr | Call should have one enclosing callable but has 0. |
Expand Down
Loading
Loading