FIR CFA: add edges from maybe-throwing statements to catch/finally

This commit is contained in:
pyos
2023-01-16 12:15:52 +01:00
committed by teamcity
parent ef208d9eac
commit 8aa50e9446
17 changed files with 156 additions and 64 deletions
@@ -6076,6 +6076,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/scopeOfAnonymousInitializer.kt");
}
@Test
@TestMetadata("smartCastInCatch.kt")
public void testSmartCastInCatch() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/smartCastInCatch.kt");
}
@Test
@TestMetadata("throwInLambda.kt")
public void testThrowInLambda() throws Exception {
@@ -90,9 +90,12 @@ digraph initializationInTry_kt {
15 -> {16};
16 -> {17};
17 -> {18};
17 -> {24} [label="onUncaughtException"];
18 -> {19};
18 -> {24} [label="onUncaughtException"];
19 -> {20};
20 -> {21};
20 -> {24} [label="onUncaughtException"];
21 -> {22};
22 -> {23};
23 -> {24};
@@ -157,8 +160,10 @@ digraph initializationInTry_kt {
38 -> {39};
39 -> {40};
40 -> {41};
40 -> {46} [label="onUncaughtException"];
41 -> {42};
42 -> {43};
42 -> {46} [label="onUncaughtException"];
43 -> {44};
44 -> {45};
45 -> {46};
@@ -145,7 +145,7 @@ digraph complex_kt {
21 -> {22 27};
22 -> {23};
23 -> {24};
24 -> {25};
24 -> {25 27};
25 -> {26};
26 -> {27 36};
27 -> {28};
@@ -177,12 +177,12 @@ digraph inplaceLambdaInControlFlowExpressions_kt {
43 -> {44} [style=dashed];
44 -> {45};
45 -> {46};
46 -> {47};
46 -> {47 53};
47 -> {48};
48 -> {49};
49 -> {50} [color=green];
49 -> {59} [color=red];
50 -> {51};
50 -> {51 53};
51 -> {52};
52 -> {53 59};
53 -> {54};
@@ -324,7 +324,7 @@ digraph postponedLambdaInReturn_kt {
86 -> {87};
87 -> {88};
88 -> {89} [color=green];
89 -> {90};
89 -> {90 94};
90 -> {107};
90 -> {91} [style=dotted];
91 -> {92} [style=dotted];
@@ -221,11 +221,11 @@ digraph tryCatch_kt {
subgraph cluster_31 {
color=blue
84 [label="Catch enter"];
85 [label="Variable declaration: e: R|kotlin/Exception|"];
85 [label="Variable declaration: e: R|kotlin/RuntimeException|"];
subgraph cluster_32 {
color=blue
86 [label="Enter block"];
87 [label="Jump: continue@@@[Boolean(true)] "];
87 [label="Jump: break@@@[Boolean(true)] "];
88 [label="Stub" style="filled" fillcolor=gray];
89 [label="Exit block" style="filled" fillcolor=gray];
}
@@ -234,11 +234,11 @@ digraph tryCatch_kt {
subgraph cluster_33 {
color=blue
91 [label="Catch enter"];
92 [label="Variable declaration: e: R|kotlin/RuntimeException|"];
92 [label="Variable declaration: e: R|kotlin/Exception|"];
subgraph cluster_34 {
color=blue
93 [label="Enter block"];
94 [label="Jump: break@@@[Boolean(true)] "];
94 [label="Jump: continue@@@[Boolean(true)] "];
95 [label="Stub" style="filled" fillcolor=gray];
96 [label="Exit block" style="filled" fillcolor=gray];
}
@@ -290,7 +290,7 @@ digraph tryCatch_kt {
69 -> {70};
70 -> {71};
71 -> {72};
72 -> {73};
72 -> {73 84 91};
73 -> {74 75};
74 -> {81};
75 -> {76};
@@ -306,7 +306,7 @@ digraph tryCatch_kt {
84 -> {85};
85 -> {86};
86 -> {87};
87 -> {47} [color=green style=dashed];
87 -> {103};
87 -> {88} [style=dotted];
88 -> {89} [style=dotted];
89 -> {90} [style=dotted];
@@ -314,7 +314,7 @@ digraph tryCatch_kt {
91 -> {92};
92 -> {93};
93 -> {94};
94 -> {103};
94 -> {47} [color=green style=dashed];
94 -> {95} [style=dotted];
95 -> {96} [style=dotted];
96 -> {97} [style=dotted];
@@ -63,6 +63,7 @@ digraph variableInitializedInTryBlock_kt {
4 -> {5};
5 -> {6};
6 -> {7};
6 -> {21} [label="onUncaughtException"];
7 -> {8 11};
8 -> {9};
9 -> {21} [label="return@/test"];
@@ -72,9 +73,12 @@ digraph variableInitializedInTryBlock_kt {
12 -> {13};
13 -> {14};
14 -> {15};
14 -> {21} [label="onUncaughtException"];
15 -> {16};
15 -> {21} [label="onUncaughtException"];
16 -> {17};
17 -> {18};
17 -> {21} [label="onUncaughtException"];
18 -> {19};
19 -> {20};
20 -> {21};
@@ -115,6 +115,7 @@ finally {
23 -> {24};
23 -> {17} [color=green style=dashed];
24 -> {25};
24 -> {27} [label="onUncaughtException"];
25 -> {26};
26 -> {27};
27 -> {28};
@@ -202,6 +203,7 @@ finally {
46 -> {47};
48 -> {49};
49 -> {50};
49 -> {52} [label="onUncaughtException"];
50 -> {51};
51 -> {52};
52 -> {53};
@@ -6082,6 +6082,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/scopeOfAnonymousInitializer.kt");
}
@Test
@TestMetadata("smartCastInCatch.kt")
public void testSmartCastInCatch() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/smartCastInCatch.kt");
}
@Test
@TestMetadata("throwInLambda.kt")
public void testThrowInLambda() throws Exception {
@@ -6076,6 +6076,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/scopeOfAnonymousInitializer.kt");
}
@Test
@TestMetadata("smartCastInCatch.kt")
public void testSmartCastInCatch() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/smartCastInCatch.kt");
}
@Test
@TestMetadata("throwInLambda.kt")
public void testThrowInLambda() throws Exception {
@@ -101,8 +101,8 @@ class ControlFlowGraphBuilder {
val edge = exitNode.edgeFrom(it)
// * NormalPath: last expression = return value
// * UncaughtExceptionPath: last expression = whatever threw, *not* a return value
// * ReturnPath(this lambda): these go from `finally` blocks, so that's not the return value;
// look in `nonDirectJumps` instead.
// * Other labels can only originate from finally block exits - look in nonDirectJumps to find the last node
// before the block was entered
it.takeIf { edge.kind.usedInCfa && edge.label == NormalPath }?.returnExpression()
}
return nonDirectJumps[exitNode].mapNotNullTo(returnValues) { it.returnExpression() }
@@ -620,7 +620,7 @@ class ControlFlowGraphBuilder {
fun exitJump(jump: FirJump<*>): JumpNode {
val node = createJumpNode(jump)
addNonTerminatingNode(node)
addNonSuccessfullyTerminatingNode(node)
if (jump is FirReturnExpression && jump.target.labeledElement is FirAnonymousFunction) {
// TODO: these should be DFA-only edges; they should be pointed into the postponed function exit node?
@@ -637,7 +637,7 @@ class ControlFlowGraphBuilder {
} ?: return node
val nextFinally = finallyEnterNodes.topOrNull()?.takeIf { it.level > nextNode.level }
if (nextFinally != null) {
addEdge(node, nextFinally, propagateDeadness = false, label = nextNode.returnPathLabel)
addEdge(node, nextFinally, propagateDeadness = false, label = nextNode)
nonDirectJumps.put(nextNode, node)
} else if (nextNode.returnPathIsBackwards) {
addBackEdge(node, nextNode)
@@ -647,14 +647,6 @@ class ControlFlowGraphBuilder {
return node
}
private val CFGNode<*>.returnPathLabel: EdgeLabel
get() = when (this) {
is FunctionExitNode -> ReturnPath(fir.symbol)
is LoopConditionEnterNode -> LoopContinuePath(loop)
is LoopExitNode -> LoopBreakPath(fir)
else -> throw IllegalStateException("not a labeled jump target: $this")
}
// while (x) { continue }
// ^------------/ back
// do { continue } while (x)
@@ -867,9 +859,8 @@ class ControlFlowGraphBuilder {
addEdge(node, nextNode, propagateDeadness = false)
for (catchEnterNode in catchNodes.pop().asReversed()) {
catchBlocksInProgress.push(catchEnterNode)
// At least merge the data flow from enter + exit...but this doesn't really help,
// see the comment for `addExceptionEdgesFrom`. Better than nothing, though.
// Like `finally`, `catch` nodes are only dead if the entire try-catch is dead.
// TODO: figure out if this edge is correct.
// try { x = /* something non-throwing like variable read */ } catch (...) { /* can assume assignment didn't happen? */ }
addEdge(node, catchEnterNode, propagateDeadness = false)
}
return node
@@ -942,15 +933,15 @@ class ControlFlowGraphBuilder {
return exitNode
}
private fun CFGNode<*>.addReturnEdges(nodes: Iterable<CFGNode<*>>, minLevel: Int) {
private fun <T> CFGNode<*>.addReturnEdges(nodes: Iterable<T>, minLevel: Int) where T : CFGNode<*>, T : EdgeLabel {
for (node in nodes) {
when {
// TODO: this check is imprecise and can add redundant edges:
// x@{ try { return@x } finally {}; try {} finally { /* return@x target is in nonDirectJumps */ }
node.level < minLevel || node !in nonDirectJumps -> continue
// TODO: if the input to finally with that label is dead, then so should be the exit probably
node.returnPathIsBackwards -> addBackEdge(this, node, label = node.returnPathLabel)
else -> addEdge(this, node, propagateDeadness = false, label = node.returnPathLabel)
node.returnPathIsBackwards -> addBackEdge(this, node, label = node)
else -> addEdge(this, node, propagateDeadness = false, label = node)
}
}
}
@@ -964,20 +955,11 @@ class ControlFlowGraphBuilder {
return node
}
// TODO: these edges are true for literally any node in the graph. Their existence for *some* nodes might lead
// to a false sense of security, but things are broken. This should be some sort of implicit knowledge instead
// of requiring a ton of edges? (Some nodes never throw, but calls are never safe, and most useful stuff is calls.)
// var x: Any?
// x = ""
// try {
// x = null
// listOf(1, 2, 3).single()
// x = ""
// } catch (e: Throwable) { x.length } // oops
// R8 devs say they tried the "implicit knowledge" way but failed and decided to add all the edges - bad sign...
private fun addExceptionEdgesFrom(node: CFGNode<*>) {
if (!node.canThrow) return
val nextCatch = catchNodes.topOrNull()
if (nextCatch != null) {
if (!nextCatch.isNullOrEmpty() && nextCatch.first().level > levelOfNextExceptionCatchingGraph()) {
for (catchEnterNode in nextCatch) {
addEdge(node, catchEnterNode, propagateDeadness = false)
}
@@ -1249,17 +1231,14 @@ class ControlFlowGraphBuilder {
private fun addNewSimpleNode(node: CFGNode<*>, isDead: Boolean = false) {
addEdge(lastNodes.pop(), node, preferredKind = if (isDead) EdgeKind.DeadForward else EdgeKind.Forward)
lastNodes.push(node)
addExceptionEdgesFrom(node)
}
private fun addNonTerminatingNode(node: CFGNode<*>) {
private fun addNonSuccessfullyTerminatingNode(node: CFGNode<*>) {
popAndAddEdge(node)
val stub = createStubNode()
addEdge(node, stub)
lastNodes.push(stub)
}
private fun addNonSuccessfullyTerminatingNode(node: CFGNode<*>) {
addNonTerminatingNode(node)
addExceptionEdgesFrom(node)
}
@@ -13,7 +13,9 @@ import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.resolve.dfa.PersistentFlow
import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
import org.jetbrains.kotlin.fir.types.FirTypeRef
import org.jetbrains.kotlin.fir.types.coneType
import org.jetbrains.kotlin.fir.types.impl.FirImplicitNothingTypeRef
import org.jetbrains.kotlin.fir.types.isNothing
import org.jetbrains.kotlin.fir.visitors.FirTransformer
@@ -27,6 +29,8 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
@OptIn(CfgInternals::class)
val id = owner.nodeCount++
open val canThrow: Boolean get() = false
companion object {
@CfgInternals
fun addEdge(
@@ -151,7 +155,10 @@ class FunctionEnterNode(owner: ControlFlowGraph, override val fir: FirFunction,
}
class FunctionExitNode(owner: ControlFlowGraph, override val fir: FirFunction, level: Int) : CFGNode<FirFunction>(owner, level),
GraphExitNodeMarker {
GraphExitNodeMarker, EdgeLabel {
override val label: String
get() = "return@${fir.symbol.callableId}"
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitFunctionExitNode(this, data)
}
@@ -418,7 +425,10 @@ class LoopBlockExitNode(owner: ControlFlowGraph, override val fir: FirLoop, leve
}
}
class LoopConditionEnterNode(owner: ControlFlowGraph, override val fir: FirExpression, val loop: FirLoop, level: Int) : CFGNode<FirExpression>(owner, level),
EnterNodeMarker {
EnterNodeMarker, EdgeLabel {
override val label: String
get() = loop.label?.let { "continue@${it.name}" } ?: "continue"
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitLoopConditionEnterNode(this, data)
}
@@ -430,7 +440,10 @@ class LoopConditionExitNode(owner: ControlFlowGraph, override val fir: FirExpres
}
}
class LoopExitNode(owner: ControlFlowGraph, override val fir: FirLoop, level: Int) : CFGNode<FirLoop>(owner, level),
ExitNodeMarker {
ExitNodeMarker, EdgeLabel {
override val label: String
get() = fir.label?.let { "break@${it.name}" } ?: "break"
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitLoopExitNode(this, data)
}
@@ -555,18 +568,27 @@ class BinaryOrExitNode(owner: ControlFlowGraph, override val fir: FirBinaryLogic
// ----------------------------------- Operator call -----------------------------------
class TypeOperatorCallNode(owner: ControlFlowGraph, override val fir: FirTypeOperatorCall, level: Int) : CFGNode<FirTypeOperatorCall>(owner, level) {
override val canThrow: Boolean
get() = fir.operation == FirOperation.AS
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitTypeOperatorCallNode(this, data)
}
}
class ComparisonExpressionNode(owner: ControlFlowGraph, override val fir: FirComparisonExpression, level: Int) : CFGNode<FirComparisonExpression>(owner, level) {
override val canThrow: Boolean
get() = true // TODO? only overridden compareTo
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitComparisonExpressionNode(this, data)
}
}
class EqualityOperatorCallNode(owner: ControlFlowGraph, override val fir: FirEqualityOperatorCall, level: Int) : AbstractBinaryExitNode<FirEqualityOperatorCall>(owner, level) {
override val canThrow: Boolean
get() = true // TODO? only overridden equals
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitEqualityOperatorCallNode(this, data)
}
@@ -589,6 +611,9 @@ class ConstExpressionNode(owner: ControlFlowGraph, override val fir: FirConstExp
class CheckNotNullCallNode(owner: ControlFlowGraph, override val fir: FirCheckNotNullCall, level: Int)
: CFGNode<FirCheckNotNullCall>(owner, level), UnionNodeMarker {
override val canThrow: Boolean
get() = true
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitCheckNotNullCallNode(this, data)
}
@@ -601,6 +626,9 @@ class QualifiedAccessNode(
override val fir: FirQualifiedAccessExpression,
level: Int
) : CFGNode<FirQualifiedAccessExpression>(owner, level) {
override val canThrow: Boolean
get() = fir.toResolvedCallableSymbol() is FirPropertySymbol
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitQualifiedAccessNode(this, data)
}
@@ -618,6 +646,9 @@ class ResolvedQualifierNode(
class FunctionCallNode(owner: ControlFlowGraph, override val fir: FirFunctionCall, level: Int)
: CFGNode<FirFunctionCall>(owner, level), UnionNodeMarker {
override val canThrow: Boolean
get() = true
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitFunctionCallNode(this, data)
}
@@ -641,6 +672,9 @@ class GetClassCallNode(owner: ControlFlowGraph, override val fir: FirGetClassCal
class DelegatedConstructorCallNode(owner: ControlFlowGraph, override val fir: FirDelegatedConstructorCall, level: Int)
: CFGNode<FirDelegatedConstructorCall>(owner, level), UnionNodeMarker {
override val canThrow: Boolean
get() = true // shouldn't matter since delegated constructor calls aren't wrapped in try-finally, but still
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitDelegatedConstructorCallNode(this, data)
}
@@ -658,6 +692,9 @@ class ThrowExceptionNode(
override val fir: FirThrowExpression,
level: Int
) : CFGNode<FirThrowExpression>(owner, level) {
override val canThrow: Boolean
get() = true
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitThrowExceptionNode(this, data)
}
@@ -761,6 +798,9 @@ class FakeExpressionEnterNode(owner: ControlFlowGraph, level: Int) : CFGNode<Fir
// ----------------------------------- Smart-cast node -----------------------------------
class SmartCastExpressionExitNode(owner: ControlFlowGraph, override val fir: FirSmartCastExpression, level: Int) : CFGNode<FirSmartCastExpression>(owner, level) {
override val canThrow: Boolean
get() = fir.typeRef.coneType.isNothing
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitSmartCastExpressionExitNode(this, data)
}
@@ -6,8 +6,6 @@
package org.jetbrains.kotlin.fir.resolve.dfa.cfg
import org.jetbrains.kotlin.fir.declarations.FirDeclaration
import org.jetbrains.kotlin.fir.expressions.FirLoop
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
class ControlFlowGraph(val declaration: FirDeclaration?, val name: String, val kind: Kind) {
@set:CfgInternals
@@ -91,19 +89,17 @@ data class Edge(
}
}
sealed class EdgeLabel(val label: String?) {
override fun toString(): String {
return label ?: ""
}
sealed interface EdgeLabel {
val label: String?
}
object NormalPath : EdgeLabel(label = null)
object UncaughtExceptionPath : EdgeLabel(label = "onUncaughtException")
object NormalPath : EdgeLabel {
override val label: String? get() = null
}
// TODO: Label `return`ing edge with this.
data class ReturnPath(val target: FirFunctionSymbol<*>) : EdgeLabel(label = "return@${target.callableId}")
data class LoopBreakPath(val loop: FirLoop) : EdgeLabel(loop.label?.let { "break@${it.name}" } ?: "break")
data class LoopContinuePath(val loop: FirLoop) : EdgeLabel(loop.label?.let { "continue@${it.name}" } ?: "continue")
object UncaughtExceptionPath : EdgeLabel {
override val label: String get() = "onUncaughtException"
}
enum class EdgeKind(
val usedInDfa: Boolean, // propagate flow to alive nodes
@@ -13,7 +13,7 @@ fun f1(flag: Boolean) {
}
catch (e: Exception) {
// KT-13612: reassignment
n = 3
<!VAL_REASSIGNMENT!>n<!> = 3
}
<!UNINITIALIZED_VARIABLE!>n<!>.hashCode()
}
@@ -30,7 +30,7 @@ fun f2(flag: Boolean) {
}
catch (e: Exception) {
// KT-13612: reassignment
n = 3
<!VAL_REASSIGNMENT!>n<!> = 3
}
<!UNINITIALIZED_VARIABLE!>n<!>.hashCode()
}
@@ -48,7 +48,7 @@ fun f3(flag: Boolean) {
}
catch (e: Exception) {
// KT-13612: reassignment
n = 3
<!VAL_REASSIGNMENT!>n<!> = 3
}
<!UNINITIALIZED_VARIABLE!>n<!>.hashCode()
}
@@ -0,0 +1,21 @@
// !SKIP_TXT
fun exc(flag: Boolean) {
if (flag) throw RuntimeException()
}
fun test(flag: Boolean) {
var x: Any?
x = ""
try {
x = null
exc(flag)
x = 1
exc(!flag)
x = ""
} catch (e: Throwable) {
// all bad - could come here from either call
x.<!UNRESOLVED_REFERENCE!>length<!>
x.<!UNRESOLVED_REFERENCE!>inc<!>()
}
}
@@ -0,0 +1,21 @@
// !SKIP_TXT
fun exc(flag: Boolean) {
if (flag) throw RuntimeException()
}
fun test(flag: Boolean) {
var x: Any?
x = ""
try {
<!UNUSED_VALUE!>x =<!> null
exc(flag)
<!UNUSED_VALUE!>x =<!> 1
exc(!flag)
x = ""
} catch (e: Throwable) {
// all bad - could come here from either call
<!DEBUG_INFO_SMARTCAST!>x<!>.length
x.<!UNRESOLVED_REFERENCE!>inc<!>()
}
}
@@ -6082,6 +6082,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/scopeOfAnonymousInitializer.kt");
}
@Test
@TestMetadata("smartCastInCatch.kt")
public void testSmartCastInCatch() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/smartCastInCatch.kt");
}
@Test
@TestMetadata("throwInLambda.kt")
public void testThrowInLambda() throws Exception {