Skip to content

Commit 849e0b4

Browse files
authored
Merge pull request #248 from microsoft/fix-ps-performance
PS: Fix lots of performance problems
2 parents 6ab05cd + 52f5ac5 commit 849e0b4

File tree

10 files changed

+98
-35
lines changed

10 files changed

+98
-35
lines changed

powershell/ql/lib/semmle/code/powershell/ast/internal/Ast.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ private import AstImport
33
class Ast extends TAst {
44
string toString() { none() }
55

6+
pragma[nomagic]
67
final Ast getParent() { result.getChild(_) = this }
78

89
Location getLocation() {

powershell/ql/lib/semmle/code/powershell/ast/internal/CallExpr.qll

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,16 @@ class CallExpr extends Expr, TCallExpr {
2626
*/
2727
Expr getCallee() { none() }
2828

29-
/** Holds if an argument with name `name` is provided to this call. */
29+
/**
30+
* Holds if an argument with name `name` is provided to this call.
31+
* Note: `name` is normalized to lower case.
32+
*/
3033
final predicate hasNamedArgument(string name) { exists(this.getNamedArgument(name)) }
3134

32-
/** Gets the argument to this call with the name `name`. */
35+
/**
36+
* Gets the named argument with the given name.
37+
* Note: `name` is normalized to lower case.
38+
*/
3339
Expr getNamedArgument(string name) { none() }
3440

3541
/** Gets any argument to this call. */

powershell/ql/lib/semmle/code/powershell/ast/internal/Command.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
private import AstImport
22

33
class CmdCall extends CallExpr, TCmd {
4-
final override string getLowerCaseName() { result = getRawAst(this).(Raw::Cmd).getLowerCaseName() }
4+
final override string getLowerCaseName() {
5+
result = getRawAst(this).(Raw::Cmd).getLowerCaseName()
6+
}
57

68
final override Expr getArgument(int i) { synthChild(getRawAst(this), cmdArgument(i), result) }
79

powershell/ql/lib/semmle/code/powershell/ast/internal/Parameter.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ class PipelineByPropertyNameParameter extends Parameter instanceof PipelineByPro
6565
* Gets the iterator variable that is used to iterate over the elements in the pipeline.
6666
*/
6767
PipelineByPropertyNameIteratorVariable getIteratorVariable() { result.getParameter() = this }
68+
69+
ProcessBlock getProcessBlock() { result = this.getIteratorVariable().getProcessBlock() }
6870
}
6971

7072
/**

powershell/ql/lib/semmle/code/powershell/ast/internal/Raw/Ast.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ private import Scope
55
class Ast extends @ast {
66
final string toString() { none() }
77

8+
pragma[nomagic]
89
final Ast getParent() { result.getAChild() = this }
910

1011
Ast getChild(ChildIndex i) { none() }
1112

13+
pragma[nomagic]
1214
final Ast getAChild() { result = this.getChild(_) }
1315

1416
Location getLocation() { none() }

powershell/ql/lib/semmle/code/powershell/ast/internal/Synthesis.qll

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ newtype VarKind =
2626
PipelineIteratorKind() or
2727
PipelineByPropertyNameIteratorKind(string name) {
2828
exists(Raw::ProcessBlock pb |
29-
name = pb.getScriptBlock().getParamBlock().getAPipelineByPropertyNameParameter().getLowerCaseName()
29+
name =
30+
pb.getScriptBlock().getParamBlock().getAPipelineByPropertyNameParameter().getLowerCaseName()
3031
)
3132
}
3233

@@ -771,28 +772,35 @@ private module IteratorAccessSynth {
771772
// TODO: We could join on something other than the string if we wanted (i.e., the raw parameter).
772773
v.getPropertyName().toLowerCase() = result and
773774
result =
774-
pb.getScriptBlock()
775-
.getParamBlock()
776-
.getAPipelineByPropertyNameParameter()
777-
.getLowerCaseName()
775+
pb.getScriptBlock().getParamBlock().getAPipelineByPropertyNameParameter().getLowerCaseName()
778776
}
779777

778+
private Raw::Ast getParent(Raw::Ast a) { a.getParent() = result }
779+
780+
private predicate isVarAccess(Raw::VarAccess va) { any() }
781+
782+
private predicate isProcessBlock(Raw::ProcessBlock pb) { any() }
783+
784+
private Raw::ProcessBlock getProcessBlock(Raw::VarAccess va) =
785+
doublyBoundedFastTC(getParent/1, isVarAccess/1, isProcessBlock/1)(va, result)
786+
780787
private class IteratorAccessSynth extends Synthesis {
781788
final override predicate isRelevant(Raw::Ast a) {
782-
exists(Raw::ProcessBlock pb, Raw::VarAccess va |
783-
va = a and
784-
pb = va.getParent+()
785-
|
789+
exists(Raw::VarAccess va | va = a |
786790
va.getUserPath() = "_"
787791
or
788-
va.getUserPath().toLowerCase() =
789-
pb.getScriptBlock().getParamBlock().getPipelineParameter().getLowerCaseName()
790-
or
791-
va.getUserPath().toLowerCase() =
792-
pb.getScriptBlock()
793-
.getParamBlock()
794-
.getAPipelineByPropertyNameParameter()
795-
.getLowerCaseName()
792+
exists(Raw::ProcessBlock pb |
793+
pragma[only_bind_into](pb) = getProcessBlock(pragma[only_bind_into](va))
794+
|
795+
va.getUserPath().toLowerCase() =
796+
pb.getScriptBlock().getParamBlock().getPipelineParameter().getLowerCaseName()
797+
or
798+
va.getUserPath().toLowerCase() =
799+
pb.getScriptBlock()
800+
.getParamBlock()
801+
.getAPipelineByPropertyNameParameter()
802+
.getLowerCaseName()
803+
)
796804
)
797805
}
798806

@@ -810,7 +818,7 @@ private module IteratorAccessSynth {
810818

811819
private PipelineOrPipelineByPropertyNameIteratorVariable varAccess(Raw::VarAccess va) {
812820
exists(Raw::ProcessBlock pb |
813-
pb = va.getParent+() and
821+
pb = getProcessBlock(va) and
814822
result = TVariableSynth(pb, _) and
815823
va.getUserPath().toLowerCase() = getAPipelineIteratorName(pb, result)
816824
)
@@ -896,6 +904,7 @@ private module PipelineAccess {
896904
exists(PipelineByPropertyNameParameter pipelineVar, Raw::PipelineByPropertyNameParameter p |
897905
i = processBlockPipelineByPropertyNameVarReadAccess(p.getLowerCaseName()) and
898906
getResultAst(p) = pipelineVar and
907+
pipelineVar = TVariableSynth(pb.getScriptBlock(), _) and
899908
child = SynthChild(VarAccessSynthKind(pipelineVar))
900909
)
901910
)

powershell/ql/lib/semmle/code/powershell/ast/internal/Variable.qll

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,9 @@ module Private {
5959

6060
PipelineParameterImpl() {
6161
exists(int index |
62-
i = FunParam(index) and
63-
any(Synthesis s).pipelineParameterHasIndex(super.getDeclaringScopeImpl(), index)
62+
i = FunParam(pragma[only_bind_into](index)) and
63+
any(Synthesis s)
64+
.pipelineParameterHasIndex(super.getDeclaringScopeImpl(), pragma[only_bind_into](index))
6465
)
6566
}
6667

powershell/ql/lib/semmle/code/powershell/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,15 +427,17 @@ module Trees {
427427
override predicate last(AstNode last, Completion c) {
428428
// Exit the loop body when the condition is false
429429
last(this.getCondition(), last, c) and
430-
this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue().booleanNot())
430+
this.entersLoopWhenConditionIs(pragma[only_bind_into](c.(BooleanCompletion)
431+
.getValue()
432+
.booleanNot()))
431433
or
432434
super.last(last, c)
433435
}
434436

435437
override predicate succ(AstNode pred, AstNode succ, Completion c) {
436438
// Condition -> body
437439
last(this.getCondition(), pred, c) and
438-
this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue()) and
440+
this.entersLoopWhenConditionIs(pragma[only_bind_into](c.(BooleanCompletion).getValue())) and
439441
first(this.getBody(), succ)
440442
or
441443
// Body -> condition

powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -465,27 +465,47 @@ class NamedSet extends NamedSet0 {
465465
/** Gets the non-empty set of names, if any. */
466466
NamedSetModule::Set asNonEmpty() { this = TNonEmptyNamedSet(result) }
467467

468+
/** Gets the `i`'th name in this set according to some ordering. */
469+
private string getRankedName(int i) {
470+
result = rank[i + 1](string s | s = this.getALowerCaseName() | s)
471+
}
472+
468473
/** Holds if this is the empty set. */
469474
predicate isEmpty() { this = TEmptyNamedSet() }
470475

471-
/** Gets a name in this set. */
472-
string getAName() { this.asNonEmpty().contains(result) }
476+
int getSize() {
477+
result = strictcount(this.getALowerCaseName())
478+
or
479+
this.isEmpty() and
480+
result = 0
481+
}
482+
483+
/** Gets a lower-case name in this set. */
484+
string getALowerCaseName() { this.asNonEmpty().contains(result) }
473485

474486
/** Gets the textual representation of this set. */
475487
string toString() {
476-
result = "{" + strictconcat(this.getAName(), ", ") + "}"
488+
result = "{" + strictconcat(this.getALowerCaseName(), ", ") + "}"
477489
or
478490
this.isEmpty() and
479491
result = "{}"
480492
}
481493

494+
private CfgNodes::ExprNodes::CallExprCfgNode getABindingCallRec(int i) {
495+
exists(string name | name = this.getRankedName(i) and exists(result.getNamedArgument(name)) |
496+
i = 0
497+
or
498+
result = this.getABindingCallRec(i - 1)
499+
)
500+
}
501+
482502
/**
483503
* Gets a `CfgNodes::CallCfgNode` that provides a named parameter for every name in `this`.
484504
*
485505
* NOTE: The `CfgNodes::CallCfgNode` may also provide more names.
486506
*/
487507
CfgNodes::ExprNodes::CallExprCfgNode getABindingCall() {
488-
forex(string name | name = this.getAName() | exists(result.getNamedArgument(name)))
508+
result = this.getABindingCallRec(this.getSize() - 1)
489509
or
490510
this.isEmpty() and
491511
exists(result)
@@ -496,16 +516,28 @@ class NamedSet extends NamedSet0 {
496516
* this set.
497517
*/
498518
CfgNodes::ExprNodes::CallExprCfgNode getAnExactBindingCall() {
499-
forex(string name | name = this.getAName() | exists(result.getNamedArgument(name))) and
500-
forex(string name | exists(result.getNamedArgument(name)) | name = this.getAName())
519+
result = this.getABindingCallRec(this.getSize() - 1) and
520+
strictcount(string name | result.hasNamedArgument(name)) = this.getSize()
501521
or
502522
this.isEmpty() and
503523
not exists(result.getNamedArgument(_))
504524
}
505525

526+
pragma[nomagic]
527+
private Function getAFunctionRec(int i) {
528+
i = 0 and
529+
result.getAParameter().getLowerCaseName() = this.getRankedName(0)
530+
or
531+
exists(string name |
532+
pragma[only_bind_into](name) = this.getRankedName(i) and
533+
result.getAParameter().getLowerCaseName() = pragma[only_bind_into](name) and
534+
result = this.getAFunctionRec(i - 1)
535+
)
536+
}
537+
506538
/** Gets a function that has a parameter for each name in this set. */
507539
Function getAFunction() {
508-
forex(string name | name = this.getAName() | result.getAParameter().matchesName(name))
540+
result = this.getAFunctionRec(this.getSize() - 1)
509541
or
510542
this.isEmpty() and
511543
exists(result)
@@ -533,6 +565,12 @@ private module ParameterNodes {
533565
}
534566
}
535567

568+
bindingset[p]
569+
pragma[inline_late]
570+
private predicate namedSetHasParameter(NamedSet ns, Parameter p) {
571+
ns.getALowerCaseName() = p.getLowerCaseName()
572+
}
573+
536574
/**
537575
* The value of a normal parameter at function entry, viewed as a node in a data
538576
* flow graph.
@@ -566,13 +604,13 @@ private module ParameterNodes {
566604
f = parameter.getFunction() and
567605
f = ns.getAFunction() and
568606
name = parameter.getLowerCaseName() and
569-
not name = ns.getAName() and
607+
not name = ns.getALowerCaseName() and
570608
j =
571609
i -
572610
count(int k, Parameter p |
573611
k < i and
574612
p = getNormalParameter(f, k) and
575-
p.getLowerCaseName() = ns.getAName()
613+
namedSetHasParameter(ns, p)
576614
)
577615
)
578616
)

powershell/ql/lib/semmle/code/powershell/dataflow/internal/SsaImpl.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ private module DataFlowIntegrationInput implements Impl::DataFlowIntegrationInpu
278278
* guard to `branch`.
279279
*/
280280
predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
281-
hasBranchEdge(bb1, bb2, branch)
281+
this.hasBranchEdge(bb1, bb2, branch)
282282
}
283283
/**
284284
* Holds if the evaluation of this guard to `branch` corresponds to the edge

0 commit comments

Comments
 (0)