Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,6 @@ class GlobalDef extends Definition {
private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;

private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationInputSig {
private import codeql.util.Void

class Expr extends Instruction {
Expr() {
exists(IRBlock bb, int i |
Expand All @@ -977,13 +975,7 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
)
}

predicate ssaDefAssigns(SsaImpl::WriteDefinition def, Expr value) { none() }

class Parameter extends Void {
Location getLocation() { none() }
}

predicate ssaDefInitializesParam(SsaImpl::WriteDefinition def, Parameter p) { none() }
predicate ssaDefHasSource(SsaImpl::WriteDefinition def) { none() }

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ module SsaFlow {
result.(Impl::ExprPostUpdateNode).getExpr() =
n.(PostUpdateNode).getPreUpdateNode().(ExprNode).getControlFlowNode()
or
result.(Impl::ParameterNode).getParameter() = n.(ExplicitParameterNode).getSsaDefinition()
result.(Impl::WriteDefSourceNode).getDefinition() = n.(ExplicitParameterNode).getSsaDefinition()
}

predicate localFlowStep(Ssa::SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
Expand Down
10 changes: 3 additions & 7 deletions csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1023,16 +1023,12 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu

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

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

class Parameter = Ssa::ImplicitParameterDefinition;

predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { def = p }

/**
* Allows for flow into uncertain defintions that are not call definitions,
* as we, conservatively, consider such definitions to be certain.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,27 @@ private predicate deadcode(Expr e) {
module SsaFlow {
module Impl = SsaImpl::DataFlowIntegration;

private predicate ssaDefAssigns(SsaExplicitUpdate def, Expr value) {
exists(VariableUpdate upd | upd = def.getDefiningExpr() |
value = upd.(VariableAssign).getSource() or
value = upd.(AssignOp) or
value = upd.(RecordBindingVariableExpr)
)
}

Impl::Node asNode(Node n) {
n = TSsaNode(result)
or
result.(Impl::ExprNode).getExpr() = n.asExpr()
or
result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
or
TExplicitParameterNode(result.(Impl::ParameterNode).getParameter()) = n
exists(Parameter p |
n = TExplicitParameterNode(p) and
result.(Impl::WriteDefSourceNode).getDefinition().(SsaImplicitInit).isParameterDefinition(p)
)
or
ssaDefAssigns(result.(Impl::WriteDefSourceNode).getDefinition(), n.asExpr())
}

predicate localFlowStep(SsaSourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
Expand Down
18 changes: 2 additions & 16 deletions java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -647,22 +647,8 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu

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

class Parameter = J::Parameter;

predicate ssaDefAssigns(Impl::WriteDefinition def, Expr value) {
exists(VariableUpdate upd | upd = def.(SsaExplicitUpdate).getDefiningExpr() |
value = upd.(VariableAssign).getSource() or
value = upd.(AssignOp) or
value = upd.(RecordBindingVariableExpr)
)
}

predicate ssaDefInitializesParam(Impl::WriteDefinition def, Parameter p) {
def.(SsaImplicitInit).getSourceVariable() =
any(SsaSourceVariable v |
v.getVariable() = p and
v.getEnclosingCallable() = p.getCallable()
)
predicate ssaDefHasSource(WriteDefinition def) {
def instanceof SsaExplicitUpdate or def.(SsaImplicitInit).isParameterDefinition(_)
}

predicate allowFlowIntoUncertainDef(UncertainWriteDefinition def) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,7 @@ module SsaDataflowInput implements DataFlowIntegrationInputSig {
predicate hasCfgNode(js::BasicBlock bb, int i) { this = bb.getNode(i) }
}

predicate ssaDefAssigns(WriteDefinition def, Expr value) {
// This library only handles use-use flow after a post-update, there are no definitions, only uses.
none()
}

class Parameter = js::Parameter;

predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
predicate ssaDefHasSource(WriteDefinition def) {
// This library only handles use-use flow after a post-update, there are no definitions, only uses.
none()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ module SsaFlow {
or
result.(Impl::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
or
n = toParameterNode(result.(Impl::ParameterNode).getParameter())
exists(SsaImpl::ParameterExt p |
n = toParameterNode(p) and
p.isInitializedBy(result.(Impl::WriteDefSourceNode).getDefinition())
)
or
result.(Impl::WriteDefSourceNode).getDefinition().(Ssa::WriteDefinition).assigns(n.asExpr())
}

predicate localFlowStep(
Expand Down
8 changes: 2 additions & 6 deletions ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -473,20 +473,16 @@ class ParameterExt extends TParameterExt {
private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInputSig {
private import codeql.ruby.controlflow.internal.Guards as Guards

class Parameter = ParameterExt;

class Expr extends Cfg::CfgNodes::ExprCfgNode {
predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { this = bb.getNode(i) }
}

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

predicate ssaDefAssigns(WriteDefinition def, Expr value) {
def.(Ssa::WriteDefinition).assigns(value)
predicate ssaDefHasSource(WriteDefinition def) {
any(ParameterExt p).isInitializedBy(def) or def.(Ssa::WriteDefinition).assigns(_)
}

predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) { p.isInitializedBy(def) }

Copy link
Contributor

Choose a reason for hiding this comment

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

Should ssaDefHasSource be implemented, restricting to any(Parameter p).isInitializedBy(def) or def.(Ssa::WriteDefinition).assigns(_)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Optionally, yes. It makes no semantic difference, it only causes a slight reduction in generated nodes (nodes that are of course unreachable by any step). I figured that such a restriction was likely most of the write definitions, which is why I didn't bother originally, but I just checked, and there is a noticeable count difference, so I'll add it.

class Guard extends Cfg::CfgNodes::AstCfgNode {
/**
* Holds if the control flow branching from `bb1` is dependent on this guard,
Expand Down
6 changes: 0 additions & 6 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,13 @@ predicate isArgumentForCall(ExprCfgNode arg, CallExprBaseCfgNode call, Parameter
module SsaFlow {
private module SsaFlow = SsaImpl::DataFlowIntegration;

private ParameterNode toParameterNode(ParamCfgNode p) {
result.(SourceParameterNode).getParameter() = p
}

/** Converts a control flow node into an SSA control flow node. */
SsaFlow::Node asNode(Node n) {
n = TSsaNode(result)
or
result.(SsaFlow::ExprNode).getExpr() = n.asExpr()
or
result.(SsaFlow::ExprPostUpdateNode).getExpr() = n.(PostUpdateNode).getPreUpdateNode().asExpr()
or
n = toParameterNode(result.(SsaFlow::ParameterNode).getParameter())
}

predicate localFlowStep(
Expand Down
12 changes: 1 addition & 11 deletions rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu

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

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

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

class Parameter = CfgNodes::ParamBaseCfgNode;

/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p) {
none() // handled in `DataFlowImpl.qll` instead
}

class Guard extends CfgNodes::AstCfgNode {
/**
* Holds if the control flow branching from `bb1` is dependent on this guard,
Expand Down
69 changes: 23 additions & 46 deletions shared/ssa/codeql/ssa/Ssa.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1459,20 +1459,14 @@ module Make<LocationSig Location, InputSig<Location> Input> {
)
}

/** Holds if SSA definition `def` assigns `value` to the underlying variable. */
predicate ssaDefAssigns(WriteDefinition def, Expr value);

/** A parameter. */
class Parameter {
/** Gets a textual representation of this parameter. */
string toString();

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

/** Holds if SSA definition `def` initializes parameter `p` at function entry. */
predicate ssaDefInitializesParam(WriteDefinition def, Parameter p);
/**
* Holds if `def` has some form of input flow. For example, the right-hand
* side of an assignment or a parameter of an SSA entry definition.
*
* For such definitions, a flow step is added from a synthetic node
* representing the source to the definition.
*/
default predicate ssaDefHasSource(WriteDefinition def) { any() }

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

cached
private newtype TNode =
TParamNode(DfInput::Parameter p) {
exists(WriteDefinition def | DfInput::ssaDefInitializesParam(def, p))
} or
TExprNode(DfInput::Expr e, Boolean isPost) {
e = DfInput::getARead(_)
or
exists(DefinitionExt def |
DfInput::ssaDefAssigns(def, e) and
isPost = false
)
} or
TWriteDefSource(WriteDefinition def) { DfInput::ssaDefHasSource(def) } or
TExprNode(DfInput::Expr e, Boolean isPost) { e = DfInput::getARead(_) } or
TSsaDefinitionNode(DefinitionExt def) { not phiHasUniqNextNode(def) } or
TSsaInputNode(SsaPhiExt phi, BasicBlock input) { relevantPhiInputNode(phi, input) }

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

final class Node = NodeImpl;

/** A parameter node. */
private class ParameterNodeImpl extends NodeImpl, TParamNode {
private DfInput::Parameter p;
/** A source of a write definition. */
private class WriteDefSourceNodeImpl extends NodeImpl, TWriteDefSource {
private WriteDefinition def;

ParameterNodeImpl() { this = TParamNode(p) }
WriteDefSourceNodeImpl() { this = TWriteDefSource(def) }

/** Gets the underlying parameter. */
DfInput::Parameter getParameter() { result = p }
/** Gets the underlying definition. */
WriteDefinition getDefinition() { result = def }

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

override Location getLocation() { result = p.getLocation() }
override Location getLocation() { result = def.getLocation() }
}

final class ParameterNode = ParameterNodeImpl;
final class WriteDefSourceNode = WriteDefSourceNodeImpl;

/** A (post-update) expression node. */
abstract private class ExprNodePreOrPostImpl extends NodeImpl, TExprNode {
Expand Down Expand Up @@ -1976,12 +1961,8 @@ module Make<LocationSig Location, InputSig<Location> Input> {
*/
predicate localFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) {
exists(Definition def |
// Flow from assignment into SSA definition
DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr())
or
// Flow from parameter into entry definition
DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter())
|
// Flow from write definition source into SSA definition
nodeFrom = TWriteDefSource(def) and
isUseStep = false and
if DfInput::includeWriteDefsInFlowStep()
then
Expand Down Expand Up @@ -2012,12 +1993,8 @@ module Make<LocationSig Location, InputSig<Location> Input> {
/** Holds if the value of `nodeTo` is given by `nodeFrom`. */
predicate localMustFlowStep(SourceVariable v, Node nodeFrom, Node nodeTo) {
exists(Definition def |
// Flow from assignment into SSA definition
DfInput::ssaDefAssigns(def, nodeFrom.(ExprNode).getExpr())
or
// Flow from parameter into entry definition
DfInput::ssaDefInitializesParam(def, nodeFrom.(ParameterNode).getParameter())
|
// Flow from write definition source into SSA definition
nodeFrom = TWriteDefSource(def) and
v = def.getSourceVariable() and
if DfInput::includeWriteDefsInFlowStep()
then nodeTo.(SsaDefinitionNode).getDefinition() = def
Expand Down
Loading