Skip to content

Commit a8b19d2

Browse files
aschackmullaschackmull
authored
Merge pull request #19147 from aschackmull/ssa/writedef-source-refactor
Ssa: Refactor data flow integration to make the input signature simpler
2 parents 1c93e53 + 0d1ac77 commit a8b19d2

File tree

11 files changed

+54
-112
lines changed

11 files changed

+54
-112
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

+1-9
Original file line numberDiff line numberDiff line change
@@ -956,8 +956,6 @@ class GlobalDef extends Definition {
956956
private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
957957

958958
private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationInputSig {
959-
private import codeql.util.Void
960-
961959
class Expr extends Instruction {
962960
Expr() {
963961
exists(IRBlock bb, int i |
@@ -977,13 +975,7 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
977975
)
978976
}
979977

980-
predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) { none() }
981-
982-
class Parameter extends Void {
983-
Location getLocation() { none() }
984-
}
985-
986-
predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, Parameter p) { none() }
978+
predicate ssaDefHasSource(SsaImpl::WriteDefinition def) { none() }
987979

988980
predicate allowFlowIntoUncertainDef(SsaImpl::UncertainWriteDefinition def) { any() }
989981

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ module SsaFlow {
506506
result.(Impl::ExprPostUpdateNode).getExpr() =
507507
n.(PostUpdateNode).getPreUpdateNode().(ExprNode).getControlFlowNode()
508508
or
509-
result.(Impl::ParameterNode).getParameter() = n.(ExplicitParameterNode).getSsaDefinition()
509+
result.(Impl::WriteDefSourceNode).getDefinition() = n.(ExplicitParameterNode).getSsaDefinition()
510510
}
511511

512512
predicate localFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll

+3-7
Original file line numberDiff line numberDiff line change
@@ -1023,16 +1023,12 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
10231023

10241024
Expr getARead(Definition def) { exists(getAReadAtNode(def, result)) }
10251025

1026-
predicate ssaDefAssigns(WriteDefinition def, Expr value) {
1026+
predicate ssaDefHasSource(WriteDefinition def) {
10271027
// exclude flow directly from RHS to SSA definition, as we instead want to
1028-
// go from RHS to matching assingnable definition, and from there to SSA definition
1029-
none()
1028+
// go from RHS to matching assignable definition, and from there to SSA definition
1029+
def instanceof Ssa::ImplicitParameterDefinition
10301030
}
10311031

1032-
class Parameter = Ssa::ImplicitParameterDefinition;
1033-
1034-
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { def = p }
1035-
10361032
/**
10371033
* Allows for flow into uncertain defintions that are not call definitions,
10381034
* as we, conservatively, consider such definitions to be certain.

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

+14-1
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,27 @@ private predicate deadcode(Expr e) {
2626
module SsaFlow {
2727
module Impl = SsaImpl::DataFlowIntegration;
2828

29+
private predicate ssaDefAssigns(SsaExplicitUpdate def, Expr value) {
30+
exists(VariableUpdate upd | upd = def.getDefiningExpr() |
31+
value = upd.(VariableAssign).getSource() or
32+
value = upd.(AssignOp) or
33+
value = upd.(RecordBindingVariableExpr)
34+
)
35+
}
36+
2937
Impl::Node asNode(Node n) {
3038
n = TSsaNode(result)
3139
or
3240
result.(Impl::ExprNode).getExpr() = n.asExpr()
3341
or
3442
result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
3543
or
36-
TExplicitParameterNode(result.(Impl::ParameterNode).getParameter()) = n
44+
exists(Parameter p |
45+
n = TExplicitParameterNode(p) and
46+
result.(Impl::WriteDefSourceNode).getDefinition().(SsaImplicitInit).isParameterDefinition(p)
47+
)
48+
or
49+
ssaDefAssigns(result.(Impl::WriteDefSourceNode).getDefinition(), n.asExpr())
3750
}
3851

3952
predicate localFlowStep(SsaSourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {

‎java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

+2-16
Original file line numberDiff line numberDiff line change
@@ -647,22 +647,8 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
647647

648648
Expr getARead(Definition def) { result = getAUse(def) }
649649

650-
class Parameter = J::Parameter;
651-
652-
predicate ssaDefAssigns(Impl::WriteDefinition def, Expr value) {
653-
exists(VariableUpdate upd | upd = def.(SsaExplicitUpdate).getDefiningExpr() |
654-
value = upd.(VariableAssign).getSource() or
655-
value = upd.(AssignOp) or
656-
value = upd.(RecordBindingVariableExpr)
657-
)
658-
}
659-
660-
predicate ssaDefInitializesParam(Impl::WriteDefinition def, Parameter p) {
661-
def.(SsaImplicitInit).getSourceVariable() =
662-
any(SsaSourceVariable v |
663-
v.getVariable() = p and
664-
v.getEnclosingCallable() = p.getCallable()
665-
)
650+
predicate ssaDefHasSource(WriteDefinition def) {
651+
def instanceof SsaExplicitUpdate or def.(SsaImplicitInit).isParameterDefinition(_)
666652
}
667653

668654
predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) {

‎javascript/ql/lib/semmle/javascript/dataflow/internal/sharedlib/Ssa.qll

+1-8
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,7 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig {
5656
predicate hasCfgNode(js::BasicBlock bb, int i) { this = bb.getNode(i) }
5757
}
5858

59-
predicate ssaDefAssigns(WriteDefinition def, Expr value) {
60-
// This library only handles use-use flow after a post-update, there are no definitions, only uses.
61-
none()
62-
}
63-
64-
class Parameter = js::Parameter;
65-
66-
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
59+
predicate ssaDefHasSource(WriteDefinition def) {
6760
// This library only handles use-use flow after a post-update, there are no definitions, only uses.
6861
none()
6962
}

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

+6-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ module SsaFlow {
108108
or
109109
result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
110110
or
111-
n = toParameterNode(result.(Impl::ParameterNode).getParameter())
111+
exists(SsaImpl::ParameterExt p |
112+
n = toParameterNode(p) and
113+
p.isInitializedBy(result.(Impl::WriteDefSourceNode).getDefinition())
114+
)
115+
or
116+
result.(Impl::WriteDefSourceNode).getDefinition().(Ssa::WriteDefinition).assigns(n.asExpr())
112117
}
113118

114119
predicate localFlowStep(

‎ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll

+2-6
Original file line numberDiff line numberDiff line change
@@ -473,20 +473,16 @@ class ParameterExt extends TParameterExt {
473473
private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
474474
private import codeql.ruby.controlflow.internal.Guards as Guards
475475

476-
class Parameter = ParameterExt;
477-
478476
class Expr extends Cfg::CfgNodes::ExprCfgNode {
479477
predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) }
480478
}
481479

482480
Expr getARead(Definition def) { result = Cached::getARead(def) }
483481

484-
predicate ssaDefAssigns(WriteDefinition def, Expr value) {
485-
def.(Ssa::WriteDefinition).assigns(value)
482+
predicate ssaDefHasSource(WriteDefinition def) {
483+
any(ParameterExt p).isInitializedBy(def) or def.(Ssa::WriteDefinition).assigns(_)
486484
}
487485

488-
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { p.isInitializedBy(def) }
489-
490486
class Guard extends Cfg::CfgNodes::AstCfgNode {
491487
/**
492488
* Holds if the control flow branching from `bb1` is dependent on this guard,

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

-6
Original file line numberDiff line numberDiff line change
@@ -172,19 +172,13 @@ predicate isArgumentForCall(ExprCfgNode arg, CallExprBaseCfgNode call, Parameter
172172
module SsaFlow {
173173
private module SsaFlow = SsaImpl::DataFlowIntegration;
174174

175-
private ParameterNode toParameterNode(ParamCfgNode p) {
176-
result.(SourceParameterNode).getParameter() = p
177-
}
178-
179175
/** Converts a control flow node into an SSA control flow node. */
180176
SsaFlow::Node asNode(Node n) {
181177
n = TSsaNode(result)
182178
or
183179
result.(SsaFlow::ExprNode).getExpr() = n.asExpr()
184180
or
185181
result.(SsaFlow::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
186-
or
187-
n = toParameterNode(result.(SsaFlow::ParameterNode).getParameter())
188182
}
189183

190184
predicate localFlowStep(

‎rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

+1-11
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
340340

341341
Expr getARead(Definition def) { result = Cached::getARead(def) }
342342

343-
/** Holds if SSA definition `def` assigns `value` to the underlying variable. */
344-
predicate ssaDefAssigns(WriteDefinition def, Expr value) {
345-
none() // handled in `DataFlowImpl.qll` instead
346-
}
343+
predicate ssaDefHasSource(WriteDefinition def) { none() } // handled in `DataFlowImpl.qll` instead
347344

348345
private predicate isArg(CfgNodes::CallExprBaseCfgNode call, CfgNodes::ExprCfgNode e) {
349346
call.getArgument(_) = e
@@ -364,13 +361,6 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
364361
)
365362
}
366363

367-
class Parameter = CfgNodes::ParamBaseCfgNode;
368-
369-
/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
370-
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
371-
none() // handled in `DataFlowImpl.qll` instead
372-
}
373-
374364
class Guard extends CfgNodes::AstCfgNode {
375365
/**
376366
* Holds if the control flow branching from `bb1` is dependent on this guard,

‎shared/ssa/codeql/ssa/Ssa.qll

+23-46
Original file line numberDiff line numberDiff line change
@@ -1459,20 +1459,14 @@ module Make<LocationSig Location, InputSig<Location> Input> {
14591459
)
14601460
}
14611461

1462-
/** Holds if SSA definition `def` assigns `value` to the underlying variable. */
1463-
predicate ssaDefAssigns(WriteDefinition def, Expr value);
1464-
1465-
/** A parameter. */
1466-
class Parameter {
1467-
/** Gets a textual representation of this parameter. */
1468-
string toString();
1469-
1470-
/** Gets the location of this parameter. */
1471-
Location getLocation();
1472-
}
1473-
1474-
/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
1475-
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p);
1462+
/**
1463+
* Holds if `def` has some form of input flow. For example, the right-hand
1464+
* side of an assignment or a parameter of an SSA entry definition.
1465+
*
1466+
* For such definitions, a flow step is added from a synthetic node
1467+
* representing the source to the definition.
1468+
*/
1469+
default predicate ssaDefHasSource(WriteDefinition def) { any() }
14761470

14771471
/**
14781472
* Holds if flow should be allowed into uncertain SSA definition `def` from
@@ -1665,17 +1659,8 @@ module Make<LocationSig Location, InputSig<Location> Input> {
16651659

16661660
cached
16671661
private newtype TNode =
1668-
TParamNode(DfInput::Parameter p) {
1669-
exists(WriteDefinition def | DfInput::ssaDefInitializesParam(def, p))
1670-
} or
1671-
TExprNode(DfInput::Expr e, Boolean isPost) {
1672-
e = DfInput::getARead(_)
1673-
or
1674-
exists(DefinitionExt def |
1675-
DfInput::ssaDefAssigns(def, e) and
1676-
isPost = false
1677-
)
1678-
} or
1662+
TWriteDefSource(WriteDefinition def) { DfInput::ssaDefHasSource(def) } or
1663+
TExprNode(DfInput::Expr e, Boolean isPost) { e = DfInput::getARead(_) } or
16791664
TSsaDefinitionNode(DefinitionExt def) { not phiHasUniqNextNode(def) } or
16801665
TSsaInputNode(SsaPhiExt phi, BasicBlock input) { relevantPhiInputNode(phi, input) }
16811666

@@ -1696,21 +1681,21 @@ module Make<LocationSig Location, InputSig<Location> Input> {
16961681

16971682
final class Node = NodeImpl;
16981683

1699-
/** A parameter node. */
1700-
private class ParameterNodeImpl extends NodeImpl, TParamNode {
1701-
private DfInput::Parameter p;
1684+
/** A source of a write definition. */
1685+
private class WriteDefSourceNodeImpl extends NodeImpl, TWriteDefSource {
1686+
private WriteDefinition def;
17021687

1703-
ParameterNodeImpl() { this = TParamNode(p) }
1688+
WriteDefSourceNodeImpl() { this = TWriteDefSource(def) }
17041689

1705-
/** Gets the underlying parameter. */
1706-
DfInput::Parameter getParameter() { result = p }
1690+
/** Gets the underlying definition. */
1691+
WriteDefinition getDefinition() { result = def }
17071692

1708-
override string toString() { result = p.toString() }
1693+
override string toString() { result = "[source] " + def.toString() }
17091694

1710-
override Location getLocation() { result = p.getLocation() }
1695+
override Location getLocation() { result = def.getLocation() }
17111696
}
17121697

1713-
final class ParameterNode = ParameterNodeImpl;
1698+
final class WriteDefSourceNode = WriteDefSourceNodeImpl;
17141699

17151700
/** A (post-update) expression node. */
17161701
abstract private class ExprNodePreOrPostImpl extends NodeImpl, TExprNode {
@@ -1976,12 +1961,8 @@ module Make<LocationSig Location, InputSig<Location> Input> {
19761961
*/
19771962
predicate localFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
19781963
exists(Definition def |
1979-
// Flow from assignment into SSA definition
1980-
DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr())
1981-
or
1982-
// Flow from parameter into entry definition
1983-
DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter())
1984-
|
1964+
// Flow from write definition source into SSA definition
1965+
nodeFrom = TWriteDefSource(def) and
19851966
isUseStep = false and
19861967
if DfInput::includeWriteDefsInFlowStep()
19871968
then
@@ -2012,12 +1993,8 @@ module Make<LocationSig Location, InputSig<Location> Input> {
20121993
/** Holds if the value of `nodeTo` is given by `nodeFrom`. */
20131994
predicate localMustFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo) {
20141995
exists(Definition def |
2015-
// Flow from assignment into SSA definition
2016-
DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr())
2017-
or
2018-
// Flow from parameter into entry definition
2019-
DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter())
2020-
|
1996+
// Flow from write definition source into SSA definition
1997+
nodeFrom = TWriteDefSource(def) and
20211998
v = def.getSourceVariable() and
20221999
if DfInput::includeWriteDefsInFlowStep()
20232000
then nodeTo.(SsaDefinitionNode).getDefinition() = def

0 commit comments

Comments
 (0)