Skip to content

Commit 5ae8b02

Browse files
committed
C++: Clarify the docs on DataFlow::Node::asExpr
For IR data flow I also added a `definitionByReferenceNodeFromArgument` predicate to improve compatibility with AST data flow.
1 parent b1be367 commit 5ae8b02

File tree

2 files changed

+50
-13
lines changed

2 files changed

+50
-13
lines changed

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,25 @@ class Node extends TNode {
5050
/** Gets the type of this node. */
5151
Type getType() { none() } // overridden in subclasses
5252

53-
/** Gets the expression corresponding to this node, if any. */
53+
/**
54+
* Gets the expression corresponding to this node, if any. This predicate
55+
* only has a result on nodes that represent the value of evaluating the
56+
* expression. For data flowing _out of_ an expression, like when an
57+
* argument is passed by reference, use `asDefiningArgument` instead of
58+
* `asExpr`.
59+
*/
5460
Expr asExpr() { result = this.(ExprNode).getExpr() }
5561

5662
/** Gets the parameter corresponding to this node, if any. */
5763
Parameter asParameter() { result = this.(ExplicitParameterNode).getParameter() }
5864

59-
/** Gets the argument that defines this `DefinitionByReferenceNode`, if any. */
65+
/**
66+
* Gets the argument that defines this `DefinitionByReferenceNode`, if any.
67+
* This predicate should be used instead of `asExpr` when referring to the
68+
* value of a reference argument _after_ the call has returned. For example,
69+
* in `f(&x)`, this predicate will have `&x` as its result for the `Node`
70+
* that represents the new value of `x`.
71+
*/
6072
Expr asDefiningArgument() { result = this.(DefinitionByReferenceNode).getArgument() }
6173

6274
/**
@@ -383,7 +395,9 @@ class PreConstructorInitThis extends Node, TPreConstructorInitThis {
383395
}
384396

385397
/**
386-
* Gets the `Node` corresponding to `e`.
398+
* Gets the `Node` corresponding to the value of evaluating `e`. For data
399+
* flowing _out of_ an expression, like when an argument is passed by
400+
* reference, use `definitionByReferenceNodeFromArgument` instead.
387401
*/
388402
ExprNode exprNode(Expr e) { result.getExpr() = e }
389403

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,15 @@ class Node extends TIRDataFlowNode {
4242
Operand asOperand() { result = this.(OperandNode).getOperand() }
4343

4444
/**
45-
* Gets the non-conversion expression corresponding to this node, if any. If
46-
* this node strictly (in the sense of `asConvertedExpr`) corresponds to a
47-
* `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
48-
* expression.
45+
* Gets the non-conversion expression corresponding to this node, if any.
46+
* This predicate only has a result on nodes that represent the value of
47+
* evaluating the expression. For data flowing _out of_ an expression, like
48+
* when an argument is passed by reference, use `asDefiningArgument` instead
49+
* of `asExpr`.
50+
*
51+
* If this node strictly (in the sense of `asConvertedExpr`) corresponds to
52+
* a `Conversion`, then the result is that `Conversion`'s non-`Conversion`
53+
* base expression.
4954
*/
5055
Expr asExpr() { result = this.(ExprNode).getExpr() }
5156

@@ -55,7 +60,13 @@ class Node extends TIRDataFlowNode {
5560
*/
5661
Expr asConvertedExpr() { result = this.(ExprNode).getConvertedExpr() }
5762

58-
/** Gets the argument that defines this `DefinitionByReferenceNode`, if any. */
63+
/**
64+
* Gets the argument that defines this `DefinitionByReferenceNode`, if any.
65+
* This predicate should be used instead of `asExpr` when referring to the
66+
* value of a reference argument _after_ the call has returned. For example,
67+
* in `f(&x)`, this predicate will have `&x` as its result for the `Node`
68+
* that represents the new value of `x`.
69+
*/
5970
Expr asDefiningArgument() { result = this.(DefinitionByReferenceNode).getArgument() }
6071

6172
/** Gets the positional parameter corresponding to this node, if any. */
@@ -378,7 +389,7 @@ private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNod
378389
class DefinitionByReferenceNode extends InstructionNode {
379390
override WriteSideEffectInstruction instr;
380391

381-
/** Gets the argument corresponding to this node. */
392+
/** Gets the unconverted argument corresponding to this node. */
382393
Expr getArgument() {
383394
result =
384395
instr
@@ -468,14 +479,18 @@ InstructionNode instructionNode(Instruction instr) { result.getInstruction() = i
468479
DefinitionByReferenceNode definitionByReferenceNode(Expr e) { result.getArgument() = e }
469480

470481
/**
471-
* Gets a `Node` corresponding to `e` or any of its conversions. There is no
472-
* result if `e` is a `Conversion`.
482+
* Gets the `Node` corresponding to the value of evaluating `e` or any of its
483+
* conversions. There is no result if `e` is a `Conversion`. For data flowing
484+
* _out of_ an expression, like when an argument is passed by reference, use
485+
* `definitionByReferenceNodeFromArgument` instead.
473486
*/
474487
ExprNode exprNode(Expr e) { result.getExpr() = e }
475488

476489
/**
477-
* Gets the `Node` corresponding to `e`, if any. Here, `e` may be a
478-
* `Conversion`.
490+
* Gets the `Node` corresponding to the value of evaluating `e`. Here, `e` may
491+
* be a `Conversion`. For data flowing _out of_ an expression, like when an
492+
* argument is passed by reference, use
493+
* `definitionByReferenceNodeFromArgument` instead.
479494
*/
480495
ExprNode convertedExprNode(Expr e) { result.getConvertedExpr() = e }
481496

@@ -484,6 +499,14 @@ ExprNode convertedExprNode(Expr e) { result.getConvertedExpr() = e }
484499
*/
485500
ExplicitParameterNode parameterNode(Parameter p) { result.getParameter() = p }
486501

502+
/**
503+
* Gets the `Node` corresponding to a definition by reference of the variable
504+
* that is passed as unconverted `argument` of a call.
505+
*/
506+
DefinitionByReferenceNode definitionByReferenceNodeFromArgument(Expr argument) {
507+
result.getArgument() = argument
508+
}
509+
487510
/** Gets the `VariableNode` corresponding to the variable `v`. */
488511
VariableNode variableNode(Variable v) { result.getVariable() = v }
489512

0 commit comments

Comments
 (0)