Skip to content

Commit 92c8ce9

Browse files
aschackmullaschackmull
authored
Merge pull request #23051 from aschackmull/ssa/barrierguard-refactor
SSA: Refactor internal SSA barrier guards api
2 parents 021e538 + bebbdb9 commit 92c8ce9

9 files changed

Lines changed: 44 additions & 73 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1256,7 +1256,7 @@ module BarrierGuardWithIntParam<ParamSig P, IntWithParam<P>::guardChecksNodeSig/
12561256

12571257
Node getABarrierNode(int indirectionIndex, P p) {
12581258
fromDfNode(result) =
1259-
DataFlowIntegrationImpl::BarrierGuardDefWithState<ParamIntPair<P>::MkPair, guardChecksWithWrappers/4>::getABarrierNode(ParamIntPair<P>::MkPair(p,
1259+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<ParamIntPair<P>::MkPair, guardChecksWithWrappers/4>::getABarrierNode(ParamIntPair<P>::MkPair(p,
12601260
indirectionIndex))
12611261
}
12621262
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ private module Cached {
932932

933933
private Node getABarrierNodeImpl() {
934934
result =
935-
DataFlowIntegrationImpl::BarrierGuardDefWithState<Unit, guardChecksWithWrappers/4>::getABarrierNode(_)
935+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<Unit, guardChecksWithWrappers/4>::getABarrierNode(_)
936936
}
937937

938938
predicate getABarrierNode = getABarrierNodeImpl/0;
@@ -962,7 +962,7 @@ private module Cached {
962962

963963
private Node getABarrierNodeImpl(P param) {
964964
result =
965-
DataFlowIntegrationImpl::BarrierGuardDefWithState<P, guardChecksWithWrappers/4>::getABarrierNode(param)
965+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<P, guardChecksWithWrappers/4>::getABarrierNode(param)
966966
}
967967

968968
predicate getABarrierNode = getABarrierNodeImpl/1;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ private module Cached {
538538

539539
private Node getABarrierNodeImpl() {
540540
result =
541-
DataFlowIntegrationImpl::BarrierGuardDefWithState<Unit, guardChecksWithWrappers/4>::getABarrierNode(_)
541+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<Unit, guardChecksWithWrappers/4>::getABarrierNode(_)
542542
}
543543

544544
predicate getABarrierNode = getABarrierNodeImpl/0;
@@ -568,7 +568,7 @@ private module Cached {
568568

569569
private Node getABarrierNodeImpl(P param) {
570570
result =
571-
DataFlowIntegrationImpl::BarrierGuardDefWithState<P, guardChecksWithWrappers/4>::getABarrierNode(param)
571+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<P, guardChecksWithWrappers/4>::getABarrierNode(param)
572572
}
573573

574574
predicate getABarrierNode = getABarrierNodeImpl/1;

‎javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,16 +332,15 @@ module MakeStateBarrierGuard<
332332
}
333333

334334
private predicate ssa2GuardChecks(
335-
Ssa2::SsaDataflowInput::Guard guard, Ssa2::SsaDataflowInput::Expr test, Boolean branch,
336-
FlowState state
335+
Ssa2::SsaDataflowInput::Guard guard, Ssa2::Definition def, Boolean branch, FlowState state
337336
) {
338337
exists(BarrierGuard g |
339338
g.asExpr() = guard and
340-
g.blocksExpr(branch, test, state)
339+
g.blocksExpr(branch, Ssa2::getARead(def), state)
341340
)
342341
}
343342

344-
private module Ssa2Barrier = Ssa2::BarrierGuardWithState<FlowState, ssa2GuardChecks/4>;
343+
private module Ssa2Barrier = Ssa2::SsaBarrierGuardWithState<FlowState, ssa2GuardChecks/4>;
345344

346345
private predicate ssa2BlocksNode(DataFlow::Node node, FlowState state) {
347346
node = DataFlowPrivate::getNodeFromSsa2(Ssa2Barrier::getABarrierNode(state))

‎javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,12 @@ predicate defaultAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2,
3737
}
3838

3939
private predicate guardChecksFalsy(
40-
Ssa2::SsaDataflowInput::Guard g, Ssa2::SsaDataflowInput::Expr e, Boolean outcome
40+
Ssa2::SsaDataflowInput::Guard g, Ssa2::Definition def, Boolean outcome
4141
) {
4242
exists(ConditionGuardNode guard |
4343
guard.getTest() = g and
4444
guard.getOutcome() = outcome and
45-
e = g and
46-
e instanceof VarAccess and
45+
g = Ssa2::getARead(def) and
4746
outcome = false
4847
)
4948
}
@@ -68,7 +67,7 @@ private predicate guardChecksFalsy(
6867
* ```
6968
*/
7069
private predicate varAccessBarrier(DataFlow::Node node) {
71-
getNodeFromSsa2(Ssa2::BarrierGuard<guardChecksFalsy/3>::getABarrierNode()) = node
70+
getNodeFromSsa2(Ssa2::SsaBarrierGuard<guardChecksFalsy/3>::getABarrierNode()) = node
7271
}
7372

7473
/**

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ module SsaConfig implements InputSig<js::Location, js::Cfg::BasicBlock> {
4444

4545
import Make<js::Location, js::Cfg, SsaConfig>
4646

47+
pragma[nomagic]
48+
js::ControlFlowNode getARead(Definition def) {
49+
exists(SsaConfig::SourceVariable v, js::Cfg::BasicBlock bb, int i |
50+
ssaDefReachesRead(v, def, bb, i) and
51+
SsaConfig::variableRead(bb, i, v, true) and
52+
bb.getNode(i) = result
53+
)
54+
}
55+
4756
module SsaDataflowInput implements DataFlowIntegrationInputSig {
4857
private import codeql.util.Boolean
4958

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,14 @@ private module Cached {
329329
cached // nothing is actually cached
330330
module BarrierGuard<guardChecksSig/3 guardChecks> {
331331
private predicate guardChecksAdjTypes(
332-
DataFlowIntegrationInput::Guard g, DataFlowIntegrationInput::Expr e,
332+
DataFlowIntegrationInput::Guard g, Definition def,
333333
DataFlowIntegrationInput::GuardValue branch
334334
) {
335-
guardChecks(g, e, branch)
335+
guardChecks(g, Cached::getARead(def), branch)
336336
}
337337

338338
private Node getABarrierNodeImpl() {
339-
result = DataFlowIntegrationImpl::BarrierGuard<guardChecksAdjTypes/3>::getABarrierNode()
339+
result = DataFlowIntegrationImpl::SsaBarrierGuard<guardChecksAdjTypes/3>::getABarrierNode()
340340
}
341341

342342
predicate getABarrierNode = getABarrierNodeImpl/0;
@@ -355,15 +355,15 @@ private module Cached {
355355
cached // nothing is actually cached
356356
module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guardChecks> {
357357
private predicate guardChecksAdjTypes(
358-
DataFlowIntegrationInput::Guard g, DataFlowIntegrationInput::Expr e,
358+
DataFlowIntegrationInput::Guard g, Definition def,
359359
DataFlowIntegrationInput::GuardValue branch, P param
360360
) {
361-
guardChecks(g, e, branch, param)
361+
guardChecks(g, Cached::getARead(def), branch, param)
362362
}
363363

364364
private Node getABarrierNodeImpl(P param) {
365365
result =
366-
DataFlowIntegrationImpl::BarrierGuardWithState<P, guardChecksAdjTypes/4>::getABarrierNode(param)
366+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<P, guardChecksAdjTypes/4>::getABarrierNode(param)
367367
}
368368

369369
predicate getABarrierNode = getABarrierNodeImpl/1;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,14 @@ private module Cached {
285285
cached // nothing is actually cached
286286
module BarrierGuard<guardChecksSig/3 guardChecks> {
287287
private predicate guardChecksAdjTypes(
288-
DataFlowIntegrationInput::Guard g, DataFlowIntegrationInput::Expr e,
288+
DataFlowIntegrationInput::Guard g, Definition def,
289289
DataFlowIntegrationInput::GuardValue branch
290290
) {
291-
guardChecks(g, e, branch)
291+
guardChecks(g, Cached::getARead(def), branch)
292292
}
293293

294294
private Node getABarrierNodeImpl() {
295-
result = DataFlowIntegrationImpl::BarrierGuard<guardChecksAdjTypes/3>::getABarrierNode()
295+
result = DataFlowIntegrationImpl::SsaBarrierGuard<guardChecksAdjTypes/3>::getABarrierNode()
296296
}
297297

298298
predicate getABarrierNode = getABarrierNodeImpl/0;
@@ -309,15 +309,15 @@ private module Cached {
309309
cached // nothing is actually cached
310310
module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guardChecks> {
311311
private predicate guardChecksAdjTypes(
312-
DataFlowIntegrationInput::Guard g, DataFlowIntegrationInput::Expr e,
312+
DataFlowIntegrationInput::Guard g, Definition def,
313313
DataFlowIntegrationInput::GuardValue branch, P param
314314
) {
315-
guardChecks(g, e, branch, param)
315+
guardChecks(g, Cached::getARead(def), branch, param)
316316
}
317317

318318
private Node getABarrierNodeImpl(P param) {
319319
result =
320-
DataFlowIntegrationImpl::BarrierGuardWithState<P, guardChecksAdjTypes/4>::getABarrierNode(param)
320+
DataFlowIntegrationImpl::SsaBarrierGuardWithState<P, guardChecksAdjTypes/4>::getABarrierNode(param)
321321
}
322322

323323
predicate getABarrierNode = getABarrierNodeImpl/1;

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

Lines changed: 12 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2146,13 +2146,10 @@ module Make<
21462146
}
21472147

21482148
/**
2149-
* Holds if the guard `g` validates the expression `e` upon evaluating to `val`.
2150-
*
2151-
* The expression `e` is expected to be a syntactic part of the guard `g`.
2152-
* For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
2153-
* the argument `x`.
2149+
* Holds if the guard `g` validates the SSA definition `def` upon
2150+
* evaluating to `val`.
21542151
*/
2155-
signature predicate guardChecksSig(DfInput::Guard g, DfInput::Expr e, DfInput::GuardValue val);
2152+
signature predicate guardChecksSig(DfInput::Guard g, Definition def, DfInput::GuardValue val);
21562153

21572154
pragma[nomagic]
21582155
private Definition getAPhiInputDef(SsaInputNodeImpl n) {
@@ -2166,80 +2163,47 @@ module Make<
21662163
signature class StateSig;
21672164

21682165
private module WithState<StateSig State> {
2169-
/**
2170-
* Holds if the guard `g` validates the expression `e` upon evaluating to `val`, blocking
2171-
* flow in the given `state`.
2172-
*
2173-
* The expression `e` is expected to be a syntactic part of the guard `g`.
2174-
* For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
2175-
* the argument `x`.
2176-
*/
2177-
signature predicate guardChecksSig(
2178-
DfInput::Guard g, DfInput::Expr e, DfInput::GuardValue val, State state
2179-
);
2180-
21812166
/**
21822167
* Holds if the guard `g` validates the SSA definition `def` upon
21832168
* evaluating to `val`, blocking flow in the given `state`.
21842169
*/
2185-
signature predicate guardChecksDefSig(
2170+
signature predicate guardChecksSig(
21862171
DfInput::Guard g, Definition def, DfInput::GuardValue val, State state
21872172
);
21882173
}
21892174

21902175
/**
2191-
* Provides a set of barrier nodes for a guard that validates an expression.
2176+
* Provides a set of barrier nodes for a guard that validates an SSA definition.
21922177
*
21932178
* This is expected to be used in `isBarrier`/`isSanitizer` definitions
21942179
* in data flow and taint tracking.
21952180
*/
2196-
module BarrierGuard<guardChecksSig/3 guardChecks> {
2181+
module SsaBarrierGuard<guardChecksSig/3 guardChecks> {
21972182
private predicate guardChecksWithState(
2198-
DfInput::Guard g, DfInput::Expr e, DfInput::GuardValue val, Unit state
2183+
DfInput::Guard g, Definition def, DfInput::GuardValue val, Unit state
21992184
) {
2200-
guardChecks(g, e, val) and exists(state)
2185+
guardChecks(g, def, val) and exists(state)
22012186
}
22022187

2203-
private module StatefulBarrier = BarrierGuardWithState<Unit, guardChecksWithState/4>;
2188+
private module StatefulBarrier = SsaBarrierGuardWithState<Unit, guardChecksWithState/4>;
22042189

22052190
/** Gets a node that is safely guarded by the given guard check. */
22062191
pragma[nomagic]
22072192
Node getABarrierNode() { result = StatefulBarrier::getABarrierNode(_) }
22082193
}
22092194

22102195
/**
2211-
* Provides a set of barrier nodes for a guard that validates an expression.
2212-
*
2213-
* This is expected to be used in `isBarrier`/`isSanitizer` definitions
2214-
* in data flow and taint tracking.
2215-
*/
2216-
module BarrierGuardWithState<StateSig State, WithState<State>::guardChecksSig/4 guardChecks> {
2217-
pragma[nomagic]
2218-
private predicate guardChecksSsaDef(
2219-
DfInput::Guard g, Definition def, DfInput::GuardValue val, State state
2220-
) {
2221-
guardChecks(g, getARead(def), val, state)
2222-
}
2223-
2224-
private module Barrier = BarrierGuardDefWithState<State, guardChecksSsaDef/4>;
2225-
2226-
predicate getABarrierNode = Barrier::getABarrierNode/1;
2227-
}
2228-
2229-
/**
2230-
* Provides a set of barrier nodes for a guard that validates an expression.
2196+
* Provides a set of barrier nodes for a guard that validates an SSA definition.
22312197
*
22322198
* This is expected to be used in `isBarrier`/`isSanitizer` definitions
22332199
* in data flow and taint tracking.
22342200
*/
2235-
module BarrierGuardDefWithState<
2236-
StateSig State, WithState<State>::guardChecksDefSig/4 guardChecksSsaDef>
2237-
{
2201+
module SsaBarrierGuardWithState<StateSig State, WithState<State>::guardChecksSig/4 guardChecks> {
22382202
/** Gets a node that is safely guarded by the given guard check. */
22392203
pragma[nomagic]
22402204
Node getABarrierNode(State state) {
22412205
exists(DfInput::Guard g, DfInput::GuardValue val, Definition def, BasicBlock bb |
2242-
guardChecksSsaDef(g, def, val, state)
2206+
guardChecks(g, def, val, state)
22432207
|
22442208
// guard controls a read
22452209
exists(DfInput::Expr e |

0 commit comments

Comments
 (0)