Flow analysis: ensure that not-yet-declared variables aren't marked as captured.

In certain error recovery situations, it's possible for code to refer
to a variable whose declaration has been lost by error recovery
mechanisms.  To prevent flow analysis from crashing when this
happened, it assumed that any reference to a variable whose
declaration had not yet been seen was valid, and implicitly added that
variable to the flow analysis state.

This created a subtle problem: if a function contained a closure that
declared (and assigned to) a local variable, at the time the closure
was entered, flow analysis would get confused and temporarily put the
variable in the "write captured" state (because it hadn't yet seen the
declaration of the variable, so it didn't realize it was local to the
closure).  Then, a boolean variable might capture that incorrect
state.  Later, upon seeing the declaration of the variable, it would
fix the incorrect state, however it was possible that a later
reference to the boolean variable would re-vivify the old incorrect
state.  This is precisely what happened in issue #47991.

This CL fixes the problem by giving flow analysis the ability to
detect, at the time the FlowAnalysis object is constructed, all
variables that are referred to but not explicitly declared, and add
them to the flow analysis state.  This allows it to safely assume that
any variables that are not yet in the flow analysis state haven't been
declared yet (and hence can be ignored), so no variable is every
erroneously placed into the "write captured" state.

Fixes #47991.

Bug: https://github.com/dart-lang/sdk/issues/47991
Change-Id: I8d84fab96fad063f1d3ade3b8b9a6e9af88c3737
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/227361
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2022-01-11 15:42:04 +00:00 committed by Commit Bot
parent fccc5fc4d5
commit 4775fa3857
5 changed files with 64 additions and 12 deletions

View file

@ -71,6 +71,7 @@ class AssignedVariables<Node extends Object, Variable extends Object> {
void declare(Variable variable) {
assert(!_isFinished);
_stack.last._declared.add(variable);
_anywhere._declared.add(variable);
}
/// This method may be called during pre-traversal, to mark the end of a
@ -1737,7 +1738,8 @@ class FlowModel<Variable extends Object, Type extends Object> {
Map<Variable?, VariableModel<Variable, Type>>? newVariableInfo;
for (Variable variable in writtenVariables) {
VariableModel<Variable, Type> info = infoFor(variable);
VariableModel<Variable, Type>? info = variableInfo[variable];
if (info == null) continue;
VariableModel<Variable, Type> newInfo =
info.discardPromotionsAndMarkNotUnassigned();
if (!identical(info, newInfo)) {
@ -1749,16 +1751,8 @@ class FlowModel<Variable extends Object, Type extends Object> {
for (Variable variable in capturedVariables) {
VariableModel<Variable, Type>? info = variableInfo[variable];
if (info == null) {
(newVariableInfo ??=
new Map<Variable?, VariableModel<Variable, Type>>.of(
variableInfo))[variable] = new VariableModel<Variable, Type>(
promotedTypes: null,
tested: const [],
assigned: false,
unassigned: false,
ssaNode: null);
} else if (!info.writeCaptured) {
if (info == null) continue;
if (!info.writeCaptured) {
(newVariableInfo ??=
new Map<Variable?, VariableModel<Variable, Type>>.of(
variableInfo))[variable] = info.writeCapture();
@ -3487,6 +3481,15 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
if (!_assignedVariables._isFinished) {
_assignedVariables.finish();
}
AssignedVariablesNodeInfo<Variable> anywhere = _assignedVariables._anywhere;
Set<Variable> implicitlyDeclaredVars = {
...anywhere._read,
...anywhere._written
};
implicitlyDeclaredVars.removeAll(anywhere._declared);
for (Variable variable in implicitlyDeclaredVars) {
declare(variable, true);
}
}
@override

View file

@ -12,3 +12,10 @@ class C {
class D {
const D(bool b) : assert(b);
}
class E {
final String a;
final String? b;
const E(this.a, {this.b});
}

View file

@ -2,6 +2,9 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
dynamic /*member: x:assigned={a}*/ x = /*declared={a, b}*/ (int a, int b) {
dynamic /*member: x1:assigned={a}*/ x1 = /*declared={a, b}*/ (int a, int b) {
a = 0;
};
/*member: x2:none*/
final x2 = new List.filled(0, null);

View file

@ -3197,6 +3197,25 @@ main() {
x.expr.as_('int').stmt, checkNotPromoted(x),
]);
});
test('issue 47991', () {
var h = Harness();
var b = Var('b', 'bool');
var i = Var('i', 'int', isFinal: true);
h.run([
localFunction([
declareInitialized(b, expr('bool').or(expr('bool'))),
declare(i, initialized: false),
if_(b.expr, [
checkUnassigned(i, true),
i.write(expr('int')).stmt,
], [
checkUnassigned(i, true),
i.write(expr('int')).stmt,
]),
]),
]);
});
});
group('Reachability', () {

View file

@ -0,0 +1,20 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// Regression test - see https://github.com/dart-lang/sdk/issues/47991
import 'dart:math';
void main() {
() {
final should = Random().nextBool() || Random().nextBool();
final int a;
if (should) {
a = 1;
} else {
a = 2;
}
};
}