Skip to content

JS: Compute barrier guard dominance after exploratory flow #3000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 10, 2020
Merged

JS: Compute barrier guard dominance after exploratory flow #3000

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eed4204
JS: Lift some internal members to private top-level
asgerf Feb 25, 2020
fd1a14d
JS: Add qldoc to a private predicate
asgerf Feb 25, 2020
2ef21ea
JS: Only evaluate relevant barrier guards
asgerf Feb 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 79 additions & 45 deletions javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ abstract class Configuration extends string {
predicate isBarrier(DataFlow::Node node) {
exists(BarrierGuardNode guard |
isBarrierGuardInternal(guard) and
guard.internalBlocks(node, "")
barrierGuardBlocksNode(guard, node, "")
)
}

Expand Down Expand Up @@ -183,7 +183,7 @@ abstract class Configuration extends string {
predicate isLabeledBarrier(DataFlow::Node node, FlowLabel lbl) {
exists(BarrierGuardNode guard |
isBarrierGuardInternal(guard) and
guard.internalBlocks(node, lbl)
barrierGuardBlocksNode(guard, node, lbl)
)
or
none() // relax type inference to account for overriding
Expand All @@ -199,6 +199,10 @@ abstract class Configuration extends string {
*/
predicate isBarrierGuard(BarrierGuardNode guard) { none() }

/**
* Holds if `guard` is a barrier guard for this configuration, added through
* `isBarrierGuard` or `AdditionalBarrierGuardNode`.
*/
private predicate isBarrierGuardInternal(BarrierGuardNode guard) {
isBarrierGuard(guard)
or
Expand Down Expand Up @@ -319,44 +323,6 @@ module FlowLabel {
* implementations of `blocks` will _both_ apply to any configuration that includes either of them.
*/
abstract class BarrierGuardNode extends DataFlow::Node {
/**
* Holds if data flow node `nd` acts as a barrier for data flow, possibly due to aliasing
* through an access path.
*
* `label` is bound to the blocked label, or the empty string if all labels should be blocked.
*
* INTERNAL: this predicate should only be used from within `blocks(boolean, Expr)`.
*/
predicate internalBlocks(DataFlow::Node nd, string label) {
// 1) `nd` is a use of a refinement node that blocks its input variable
exists(SsaRefinementNode ref, boolean outcome |
nd = DataFlow::ssaDefinitionNode(ref) and
outcome = ref.getGuard().(ConditionGuardNode).getOutcome() and
ssaRefinementBlocks(outcome, ref, label)
)
or
// 2) `nd` is an instance of an access path `p`, and dominated by a barrier for `p`
exists(AccessPath p, BasicBlock bb, ConditionGuardNode cond, boolean outcome |
nd = DataFlow::valueNode(p.getAnInstanceIn(bb)) and
getEnclosingExpr() = cond.getTest() and
outcome = cond.getOutcome() and
barrierGuardBlocksAccessPath(this, outcome, p, label) and
cond.dominates(bb)
)
}

/**
* Holds if there exists an input variable of `ref` that blocks the label `label`.
*
* This predicate is outlined to give the optimizer a hint about the join ordering.
*/
private predicate ssaRefinementBlocks(boolean outcome, SsaRefinementNode ref, string label) {
getEnclosingExpr() = ref.getGuard().getTest() and
forex(SsaVariable input | input = ref.getAnInput() |
barrierGuardBlocksExpr(this, outcome, input.getAUse(), label)
)
}

/**
* Holds if this node blocks expression `e` provided it evaluates to `outcome`.
*
Expand Down Expand Up @@ -387,6 +353,17 @@ private predicate barrierGuardBlocksExpr(
guard.(AdditionalBarrierGuardCall).internalBlocksLabel(outcome, test, label)
}

/**
* Holds if `guard` blocks the flow of a value reachable through exploratory flow.
*/
pragma[noinline]
private predicate barrierGuardIsRelevant(BarrierGuardNode guard) {
exists(Expr e |
barrierGuardBlocksExpr(guard, _, e, _) and
isRelevantForward(e.flow(), _)
)
}

/**
* Holds if data flow node `nd` acts as a barrier for data flow due to aliasing through
* an access path.
Expand All @@ -397,9 +374,50 @@ pragma[noinline]
private predicate barrierGuardBlocksAccessPath(
BarrierGuardNode guard, boolean outcome, AccessPath ap, string label
) {
barrierGuardIsRelevant(guard) and
barrierGuardBlocksExpr(guard, outcome, ap.getAnInstance(), label)
}

/**
* Holds if there exists an input variable of `ref` that blocks the label `label`.
*
* This predicate is outlined to give the optimizer a hint about the join ordering.
*/
private predicate barrierGuardBlocksSsaRefinement(
BarrierGuardNode guard, boolean outcome, SsaRefinementNode ref, string label
) {
barrierGuardIsRelevant(guard) and
guard.getEnclosingExpr() = ref.getGuard().getTest() and
forex(SsaVariable input | input = ref.getAnInput() |
barrierGuardBlocksExpr(guard, outcome, input.getAUse(), label)
)
}

/**
* Holds if data flow node `nd` acts as a barrier for data flow, possibly due to aliasing
* through an access path.
*
* `label` is bound to the blocked label, or the empty string if all labels should be blocked.
*/
private predicate barrierGuardBlocksNode(BarrierGuardNode guard, DataFlow::Node nd, string label) {
// 1) `nd` is a use of a refinement node that blocks its input variable
exists(SsaRefinementNode ref, boolean outcome |
nd = DataFlow::ssaDefinitionNode(ref) and
outcome = ref.getGuard().(ConditionGuardNode).getOutcome() and
barrierGuardBlocksSsaRefinement(guard, outcome, ref, label)
)
or
// 2) `nd` is an instance of an access path `p`, and dominated by a barrier for `p`
barrierGuardIsRelevant(guard) and
exists(AccessPath p, BasicBlock bb, ConditionGuardNode cond, boolean outcome |
nd = DataFlow::valueNode(p.getAnInstanceIn(bb)) and
guard.getEnclosingExpr() = cond.getTest() and
outcome = cond.getOutcome() and
barrierGuardBlocksAccessPath(guard, outcome, p, label) and
cond.dominates(bb)
)
}

/**
* Holds if `guard` should block flow along the edge `pred -> succ`.
*
Expand All @@ -408,6 +426,7 @@ private predicate barrierGuardBlocksAccessPath(
private predicate barrierGuardBlocksEdge(
BarrierGuardNode guard, DataFlow::Node pred, DataFlow::Node succ, string label
) {
barrierGuardIsRelevant(guard) and
exists(
SsaVariable input, SsaPhiNode phi, BasicBlock bb, ConditionGuardNode cond, boolean outcome
|
Expand Down Expand Up @@ -569,11 +588,12 @@ private class FlowStepThroughImport extends AdditionalFlowStep, DataFlow::ValueN

/**
* Holds if there is a flow step from `pred` to `succ` described by `summary`
* under configuration `cfg`.
* under configuration `cfg`, disregarding barriers.
*
* Summary steps through function calls are not taken into account.
*/
private predicate basicFlowStep(
pragma[inline]
private predicate basicFlowStepNoBarrier(
DataFlow::Node pred, DataFlow::Node succ, PathSummary summary, DataFlow::Configuration cfg
) {
isLive() and
Expand All @@ -582,8 +602,7 @@ private predicate basicFlowStep(
// Local flow
exists(FlowLabel predlbl, FlowLabel succlbl |
localFlowStep(pred, succ, cfg, predlbl, succlbl) and
not isLabeledBarrierEdge(cfg, pred, succ, predlbl) and
not isBarrierEdge(cfg, pred, succ) and
not cfg.isBarrierEdge(pred, succ) and
summary = MkPathSummary(false, false, predlbl, succlbl)
)
or
Expand All @@ -605,6 +624,20 @@ private predicate basicFlowStep(
)
}

/**
* Holds if there is a flow step from `pred` to `succ` described by `summary`
* under configuration `cfg`.
*
* Summary steps through function calls are not taken into account.
*/
private predicate basicFlowStep(
DataFlow::Node pred, DataFlow::Node succ, PathSummary summary, DataFlow::Configuration cfg
) {
basicFlowStepNoBarrier(pred, succ, summary, cfg) and
not isLabeledBarrierEdge(cfg, pred, succ, summary.getStartLabel()) and
not isBarrierEdge(cfg, pred, succ)
}

/**
* Holds if there is a flow step from `pred` to `succ` under configuration `cfg`,
* including both basic flow steps and steps into/out of properties.
Expand All @@ -615,7 +648,7 @@ private predicate basicFlowStep(
private predicate exploratoryFlowStep(
DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg
) {
basicFlowStep(pred, succ, _, cfg) or
basicFlowStepNoBarrier(pred, succ, _, cfg) or
basicStoreStep(pred, succ, _) or
basicLoadStep(pred, succ, _) or
isAdditionalStoreStep(pred, succ, _, cfg) or
Expand Down Expand Up @@ -1442,6 +1475,7 @@ private class BarrierGuardFunction extends Function {
string label;

BarrierGuardFunction() {
barrierGuardIsRelevant(guard) and
exists(Expr e |
exists(Expr returnExpr |
returnExpr = guard.asExpr()
Expand Down