Skip to content

Commit 032b47b

Browse files
committed
unified: Tolerate missing post-updates in unreachable code
1 parent ba0dc89 commit 032b47b

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

‎unified/ql/consistency-queries/DataFlowConsistency.ql‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ private import unified
22
private import codeql.unified.internal.dataflow.AllDataFlow
33
private import codeql.dataflow.internal.DataFlowImplConsistency
44

5-
module ConsistencyInput implements InputSig<Location, DataFlowInput> { }
5+
module ConsistencyInput implements InputSig<Location, DataFlowInput> {
6+
predicate argHasPostUpdateExclude(DataFlowInput::ArgumentNode n) {
7+
not exists(n.getBasicBlock()) // ignore unreachable data flow nodes
8+
}
9+
}
610

711
module ConsistencyOutput =
812
MakeConsistency<Location, DataFlowInput, TaintTrackingInput, ConsistencyInput>;

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,55 @@ class Node extends TDataFlowNode {
266266

267267
/** Gets the callable containing this data flow node. */
268268
Callable getEnclosingCallable() { result = this.getEnclosingCallableEx().asSourceCallable() }
269+
270+
/**
271+
* Holds if this data flow node is associated with the `i`'th index in the given basic block.
272+
*
273+
* Note that some data flow nodes may have an index appearing before the first or after the
274+
* last `ControlFlowNode` node in the basic block. Multiple data flow nodes may share the same control flow position.
275+
*
276+
* Also note that some data flow nodes have no associated control flow position, either because they are
277+
* in unreachable code, or belong to a synthesized callable that has no control flow graph.
278+
*/
279+
predicate hasControlFlowPosition(BasicBlock bb, int i) {
280+
exists(ControlFlowNode cfgNode | cfgNode = bb.getNode(i) |
281+
exists(Expr expr |
282+
this.isResultValue(expr) and cfgNode.asExpr() = expr
283+
or
284+
this.isIncomingValue(expr) and hasIncomingValueAtCfgNode(expr, cfgNode)
285+
or
286+
this.isPostUpdate(expr) and hasPostUpdate(expr, cfgNode)
287+
)
288+
or
289+
exists(AstNode repr, LocalVariable var, VariableRefKind kind |
290+
this.isLocalVariableRef(repr, var, kind) and
291+
performsVariableAccess(repr, var, kind, cfgNode)
292+
)
293+
or
294+
exists(DataFlowCallable callable |
295+
this.isReceiverParameterEx(callable) and
296+
cfgNode.(ControlFlow::EntryNode).getEnclosingCallable() = callable.asSourceCallable()
297+
)
298+
or
299+
exists(DataFlowCall call, CallExpr sourceCall |
300+
call.asExplicitCall() = sourceCall and
301+
(
302+
this.isReceiverArgumentEx(call) and cfgNode.injects(sourceCall)
303+
or
304+
this.isReceiverPostUpdateEx(call) and cfgNode.isAfter(sourceCall)
305+
)
306+
)
307+
)
308+
or
309+
exists(LocalSsaDataFlowOutput::SsaNode node |
310+
this = TLocalSsaNode(node) and
311+
bb = node.getBasicBlock() and
312+
i = node.getIndex() // TODO: why is this marked as internal in the SSA library?
313+
)
314+
}
315+
316+
/** Gets the basic block associated with this data flow node, if any. */
317+
BasicBlock getBasicBlock() { this.hasControlFlowPosition(result, _) }
269318
}
270319

271320
Node getPostUpdateNode(Node pre) {

0 commit comments

Comments
 (0)