Skip to content

Commit 1ec0f53

Browse files
committed
PS: Don't perform store steps and (and index removal, and all the othe complex return business) when there is only a single returned expression.
1 parent b2225fe commit 1ec0f53

File tree

1 file changed

+41
-7
lines changed

1 file changed

+41
-7
lines changed

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

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ module LocalFlow {
113113
nodeFrom = TImplicitWrapNode(cfgNode, false) and
114114
nodeTo = TReturnNodeImpl(cfgNode.getScope())
115115
)
116+
or
117+
exists(CfgNode cfgNode |
118+
cfgNode = nodeFrom.(AstNode).getCfgNode() and
119+
isUniqueReturned(cfgNode) and
120+
nodeTo.(ReturnNodeImpl).getCfgScope() = cfgNode.getScope()
121+
)
116122
}
117123

118124
predicate localMustFlowStep(Node nodeFrom, Node nodeTo) {
@@ -148,8 +154,8 @@ private module Cached {
148154
or
149155
n = any(CfgNodes::ExprNodes::IndexCfgNode index).getBase()
150156
} or
151-
TPreReturnNodeImpl(CfgNodes::AstCfgNode n, Boolean isArray) { isReturned(n) } or
152-
TImplicitWrapNode(CfgNodes::AstCfgNode n, Boolean shouldWrap) { isReturned(n) } or
157+
TPreReturnNodeImpl(CfgNodes::AstCfgNode n, Boolean isArray) { isMultiReturned(n) } or
158+
TImplicitWrapNode(CfgNodes::AstCfgNode n, Boolean shouldWrap) { isMultiReturned(n) } or
153159
TReturnNodeImpl(CfgScope scope) or
154160
TProcessNode(ProcessBlock process)
155161

@@ -564,6 +570,14 @@ private module ReturnNodes {
564570
or
565571
result = this.getAChild().getAReturnedNode()
566572
}
573+
574+
/** Holds if `n` may be returned multiples times. */
575+
predicate mayBeMultiReturned(CfgNode n) {
576+
n = this.getANode() and
577+
n.getASuccessor+() = n
578+
or
579+
this.getAChild().mayBeMultiReturned(n)
580+
}
567581
}
568582

569583
class ScriptBlockReturnContainer extends ReturnContainer, ScriptBlock {
@@ -618,14 +632,34 @@ private module ReturnNodes {
618632
final override ReturnContainer getAChild() { none() }
619633
}
620634

621-
/** Holds if `n` is returned from the enclosing callable. */
622-
predicate isReturned(CfgNodes::AstCfgNode n) {
623-
exists(ReturnContainer container |
624-
container = n.getScope() and
625-
n = container.getAReturnedNode()
635+
private predicate isReturnedImpl(CfgNodes::AstCfgNode n, ReturnContainer container) {
636+
container = n.getScope() and
637+
n = container.getAReturnedNode()
638+
}
639+
640+
/**
641+
* Holds if `n` may be returned, and there are possibly
642+
* more than one return value from the function.
643+
*/
644+
predicate isMultiReturned(CfgNodes::AstCfgNode n) {
645+
exists(ReturnContainer container | isReturnedImpl(n, container) |
646+
strictcount(container.getAReturnedNode()) > 1
647+
or
648+
container.mayBeMultiReturned(n)
626649
)
627650
}
628651

652+
/**
653+
* Holds if `n` may be returned.
654+
*/
655+
predicate isReturned(CfgNodes::AstCfgNode n) { isReturnedImpl(n, _) }
656+
657+
/**
658+
* Holds if `n` may be returned, and this is the only value that may be
659+
* returned from the function.
660+
*/
661+
predicate isUniqueReturned(CfgNodes::AstCfgNode n) { isReturned(n) and not isMultiReturned(n) }
662+
629663
class NormalReturnNode extends ReturnNode instanceof ReturnNodeImpl {
630664
final override NormalReturnKind getKind() { any() }
631665
}

0 commit comments

Comments
 (0)