Skip to content

Commit 9049932

Browse files
committed
C++: Implement the new predicate.
1 parent 064f68f commit 9049932

File tree

2 files changed

+52
-0
lines changed

2 files changed

+52
-0
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,6 @@ module CppDataFlow implements InputSig {
2020
Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
2121

2222
predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
23+
24+
predicate flowThroughStepAllowed = Private::flowThroughStepAllowed/2;
2325
}

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,3 +1149,53 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
11491149
)
11501150
)
11511151
}
1152+
1153+
/**
1154+
* Holds if the dataflow step from `node1` to `node2` should not be used when
1155+
* computing flow-through summaries because the dataflow step copies the value
1156+
* of `node1` to `node2` in a way that does not preserve the identity of the
1157+
* value. For example the assignment to `x` that reads the value of `*p` in:
1158+
* ```cpp
1159+
* int* p = ...
1160+
* int x = *p;
1161+
* ```
1162+
*/
1163+
bindingset[node1, node2]
1164+
pragma[inline_late]
1165+
predicate flowThroughStepAllowed(Node node1, Node node2) {
1166+
// When flow-through summaries are computed we track which parameters flow to out-going parameters.
1167+
// In an example such as:
1168+
// ```
1169+
// modify(int* px) { *px = source(); }
1170+
// void modify_copy(int* p) {
1171+
// int x = *p;
1172+
// modify(&x);
1173+
// }
1174+
// ```
1175+
// since dataflow tracks each indirection as a separate SSA variable dataflow
1176+
// sees the above roughly as
1177+
// ```
1178+
// modify(int* px, int deref_px) { deref_px = source(); }
1179+
// void modify_copy(int* p, int deref_p) {
1180+
// int x = deref_p;
1181+
// modify(&x, x);
1182+
// }
1183+
// ```
1184+
// and when dataflow computes flow from a parameter to a post-update node to
1185+
// conclude which parameters are "updated" by the call to `modify_copy` it
1186+
// finds flow from `x [post update]` to `deref_p [post update]`.
1187+
// To prevent this we exclude steps that don't preserve identity. We do this
1188+
// by excluding flow from the right-hand side of `StoreInstruction`s to the
1189+
// `StoreInstruction`. This is sufficient because, for flow-through summaries,
1190+
// we're only interested in indirect parameters such as `deref_p` in the
1191+
// exampe above (i.e., the parameters with a non-zero indirection index), and
1192+
// if that ever flows to the right-hand side of a `StoreInstruction` then
1193+
// there must have been a dereference to reduce its indirection index down to
1194+
// 0.
1195+
not exists(Operand operand |
1196+
node1.asOperand() = operand and
1197+
node2.asInstruction().(StoreInstruction).getSourceValueOperand() = operand
1198+
)
1199+
// TODO: Also block flow through models that don't preserve identity such
1200+
// as `strdup`.
1201+
}

0 commit comments

Comments
 (0)