Skip to content

Commit 6ddd96b

Browse files
alexmarkovCommit Queue
authored andcommitted
[vm,compiler] Treat 'Never' return type as unreachable
When building a flow graph, break control flow (close fragment) after a call with return type 'Never'. Append 'Stop' instruction after the call to make sure the return address is still within function code range. TEST=runtime/tests/vm/dart/regress_59941_il_test.dart Fixes #59941 Issue #56969 Change-Id: I5bbc0b9740a4f7e06b8560589daeb2b44f13d084 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405604 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 83f540b commit 6ddd96b

File tree

6 files changed

+82
-1
lines changed

6 files changed

+82
-1
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Verify that calling a method which returns Never breaks control flow.
6+
// Regression test for https://github.com/dart-lang/sdk/issues/59941.
7+
8+
import 'package:expect/expect.dart';
9+
import 'package:vm/testing/il_matchers.dart';
10+
11+
@pragma('vm:never-inline')
12+
void myprint(Object o) {
13+
print(o);
14+
}
15+
16+
@pragma('vm:never-inline')
17+
Never bar() {
18+
throw 'baz';
19+
}
20+
21+
@pragma('vm:never-inline')
22+
@pragma('vm:testing:print-flow-graph')
23+
int foo(bool condition, int arg) {
24+
int i = arg;
25+
if (condition) {
26+
i = 2;
27+
bar(); // <-- Return type `Never`, so it always throws and can never fall through.
28+
}
29+
return i;
30+
}
31+
32+
main() {
33+
Expect.equals(42, foo(false, 42));
34+
Expect.equals(43, foo(false, 43));
35+
Expect.throws(() {
36+
foo(true, 44);
37+
});
38+
}
39+
40+
void matchIL$foo(FlowGraph graph) {
41+
graph.dump();
42+
graph.match([
43+
match.block('Graph', []),
44+
match.block('Function', [
45+
'condition' << match.Parameter(index: 0),
46+
'arg' << match.Parameter(index: 1),
47+
match.CheckStackOverflow(),
48+
match.Branch(
49+
match.StrictCompare('condition', match.any, kind: '==='),
50+
ifTrue: 'B3',
51+
ifFalse: 'B4',
52+
),
53+
]),
54+
'B3' << match.block('Target', [match.StaticCall(), match.Stop()]),
55+
'B4' << match.block('Target', [match.DartReturn('arg')]),
56+
]);
57+
}

runtime/vm/compiler/backend/block_scheduler.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ class AOTBlockScheduler {
286286
if ((marks & kVisitedMark) == 0) {
287287
marks |= kVisitedMark;
288288

289-
if (last->IsThrow() || last->IsReThrow()) {
289+
if (last->IsThrow() || last->IsReThrow() || last->IsStop()) {
290290
marks |= kColdMark;
291291
} else {
292292
// When visiting a block inside a loop with two successors

runtime/vm/compiler/backend/flow_graph_checker.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ static bool DefDominatesUse(Definition* def, Instruction* instruction) {
117117
static bool IsControlFlow(Instruction* instruction) {
118118
return instruction->IsBranch() || instruction->IsGoto() ||
119119
instruction->IsIndirectGoto() || instruction->IsReturnBase() ||
120+
(instruction->IsStop() && instruction->next() == nullptr) ||
120121
instruction->IsThrow() || instruction->IsReThrow() ||
121122
instruction->IsTailCall();
122123
}

runtime/vm/compiler/frontend/base_flow_graph_builder.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,10 @@ Fragment BaseFlowGraphBuilder::Return(TokenPosition position) {
224224
return instructions.closed();
225225
}
226226

227+
Fragment BaseFlowGraphBuilder::Stop(const char* message) {
228+
return Fragment(new (Z) StopInstr(message)).closed();
229+
}
230+
227231
bool BaseFlowGraphBuilder::ShouldOmitCheckBoundsIn(const Function& function,
228232
const Function* caller) {
229233
auto& state = CompilerState::Current();
@@ -1233,6 +1237,13 @@ Fragment BaseFlowGraphBuilder::ClosureCall(
12331237
instructions += Drop();
12341238
instructions += Constant(result_type->constant_value);
12351239
}
1240+
if (!target_function.IsNull()) {
1241+
const auto& return_type =
1242+
AbstractType::Handle(Z, target_function.result_type());
1243+
if (return_type.IsNeverType() && return_type.IsNonNullable()) {
1244+
instructions += Stop("unreachable after returning Never");
1245+
}
1246+
}
12361247
return instructions;
12371248
}
12381249

runtime/vm/compiler/frontend/base_flow_graph_builder.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ class BaseFlowGraphBuilder {
344344
Fragment BranchIfStrictEqual(TargetEntryInstr** then_entry,
345345
TargetEntryInstr** otherwise_entry);
346346
Fragment Return(TokenPosition position);
347+
Fragment Stop(const char* message);
347348
Fragment CheckStackOverflow(TokenPosition position,
348349
intptr_t stack_depth,
349350
intptr_t loop_depth);

runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,13 @@ Fragment FlowGraphBuilder::InstanceCall(
418418
instructions += Drop();
419419
instructions += Constant(result_type->constant_value);
420420
}
421+
if (!interface_target.IsNull()) {
422+
const auto& return_type =
423+
AbstractType::Handle(Z, interface_target.result_type());
424+
if (return_type.IsNeverType() && return_type.IsNonNullable()) {
425+
instructions += Stop("unreachable after returning Never");
426+
}
427+
}
421428
return instructions;
422429
}
423430

@@ -666,6 +673,10 @@ Fragment FlowGraphBuilder::StaticCall(TokenPosition position,
666673
instructions += Drop();
667674
instructions += Constant(result_type->constant_value);
668675
}
676+
const auto& return_type = AbstractType::Handle(Z, target.result_type());
677+
if (return_type.IsNeverType() && return_type.IsNonNullable()) {
678+
instructions += Stop("unreachable after returning Never");
679+
}
669680
return instructions;
670681
}
671682

0 commit comments

Comments
 (0)