Skip to content

Commit 04a224d

Browse files
authored
HLL::Compiler.compile: balance backend per-compile state on early exit
HLL::Backend.start (MoarVM) pushes a frame list onto %COMPILING<moar><frames> at every compile's 'start' stage. The matching pop runs at the 'mast' stage in QASTCompilerMAST.to_mast. A compile that stops before 'mast' (e.g. with `:target('ast')`), or one whose stage call throws, leaves the push dangling. The next nested compile's MAST migration then attaches its frames to the dangling level rather than the outer compile's, leaving those MAST::Frames unreachable from any compunit's @!frames. The outer's deserialization code dies later with `Could not find frame <unit>`. Snapshot the backend's per-compile state before the stages loop and rebalance it afterwards via two optional backend hooks (compile_snapshot, compile_cleanup). Wrap each stage call in CATCH so the cleanup also runs when a stage throws, capture the exception, exit the stages loop, then rethrow with its original payload. The legacy Rakudo frontend never stops short of 'mast' and never throws across the stages loop boundary, so on every existing non-RakuAST compile the snapshot matches the post-loop depth, the cleanup pop loop runs no iterations, and the caught variable stays null.
1 parent e37642d commit 04a224d

4 files changed

Lines changed: 130 additions & 3 deletions

File tree

src/HLL/Compiler.nqp

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,17 @@ class HLL::Compiler does HLL::Backend::Default {
503503
nqp::unshift(@stages, 'start');
504504
}
505505

506+
# Backends may keep per-compile state on %*COMPILING (e.g. a frame
507+
# list that the 'start' stage pushes for the 'mast' stage to pop).
508+
# Snapshot that state so compile_cleanup can rebalance it if the
509+
# stages loop exits before the consuming stage runs, whether via a
510+
# 'target' adverb shorter than the final stage or an in-flight
511+
# exception.
512+
my $compile_snapshot := nqp::can($!backend, 'compile_snapshot')
513+
?? $!backend.compile_snapshot
514+
!! NQPMu;
515+
516+
my $caught := nqp::null;
506517
for @stages {
507518
$stderr.print(nqp::sprintf("Stage %-11s: ", [$_])) if nqp::defined($stagestats) && $_ ne "parse";
508519
my int $timestamp := nqp::time();
@@ -511,9 +522,22 @@ class HLL::Compiler does HLL::Backend::Default {
511522
self.execute_stage($_, $result, %adverbs)
512523
}
513524

514-
$result := %adverbs<profile-stage> eq $_
515-
?? $!backend.run_profiled(&run, %adverbs<profile-filename> || %adverbs<profile-compile>, %adverbs<profile-kind>)
516-
!! run();
525+
# Only the stage call can throw something the cleanup below
526+
# needs to run for. Wrap just that call so stagestats output
527+
# and other bookkeeping stay outside the handler scope.
528+
# The bare CATCH also matches CONTROL-category exceptions.
529+
# Rakudo's resumable warnings (CX::Warn and friends) are
530+
# absorbed by HLL-level handlers installed deeper in the
531+
# call chain before they reach the stage call site, so they
532+
# never bubble into this CATCH at runtime. If that ever
533+
# changes, this scope would have to filter the category.
534+
try {
535+
$result := %adverbs<profile-stage> eq $_
536+
?? $!backend.run_profiled(&run, %adverbs<profile-filename> || %adverbs<profile-compile>, %adverbs<profile-kind>)
537+
!! run();
538+
CATCH { $caught := $_ }
539+
}
540+
last if !nqp::isnull($caught);
517541

518542
my num $diff := nqp::div_n(nqp::time() - $timestamp,1000000000e0);
519543
if nqp::defined($stagestats) {
@@ -532,6 +556,11 @@ class HLL::Compiler does HLL::Backend::Default {
532556
last if $_ eq $target;
533557
}
534558

559+
nqp::can($!backend, 'compile_cleanup')
560+
&& $!backend.compile_cleanup($compile_snapshot);
561+
562+
nqp::rethrow($caught) if !nqp::isnull($caught);
563+
535564
if %adverbs<compunit_ok> {
536565
return $result
537566
}

src/vm/moar/HLL/Backend.nqp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,10 @@ class HLL::Backend::MoarVM {
761761
0
762762
}
763763

764+
# Push a fresh entry onto %COMPILING<moar><frames>. The matching pop
765+
# lives in QASTCompilerMAST.to_mast at the 'mast' stage. The safety
766+
# net for a stages loop that exits before 'mast' lives in
767+
# compile_snapshot / compile_cleanup below.
764768
method start($source, *%adverbs) {
765769
if nqp::existskey(%*COMPILING, 'moar') {
766770
nqp::push(%*COMPILING<moar><frames>, nqp::list);
@@ -780,6 +784,24 @@ class HLL::Backend::MoarVM {
780784
$source
781785
}
782786

787+
# compile_snapshot records the depth of %COMPILING<moar><frames>
788+
# before the stages loop runs. compile_cleanup pops anything still
789+
# above that mark, restoring the depth so an early-exit 'target' or
790+
# in-flight exception cannot leave a partial frame entry behind for
791+
# the next nested compile. Pairs with `start` above (the push) and
792+
# QASTCompilerMAST.to_mast (the pop).
793+
method compile_snapshot() {
794+
nqp::existskey(%*COMPILING, 'moar')
795+
?? nqp::elems(nqp::atkey(nqp::atkey(%*COMPILING, 'moar'), 'frames'))
796+
!! 0
797+
}
798+
799+
method compile_cleanup($snapshot) {
800+
return 0 unless nqp::existskey(%*COMPILING, 'moar');
801+
my $frames := nqp::atkey(nqp::atkey(%*COMPILING, 'moar'), 'frames');
802+
nqp::pop($frames) while nqp::elems($frames) > $snapshot;
803+
}
804+
783805
method mast($qast, *%adverbs) {
784806
nqp::getcomp('QAST').to_mast($qast);
785807
}

src/vm/moar/QAST/QASTCompilerMAST.nqp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,10 @@ my class MASTCompilerInstance {
358358
$annotations := %moar<annotations> := MAST::Bytecode.new;
359359
}
360360

361+
# Pops the entry HLL::Backend::MoarVM.start pushed for this
362+
# compile. HLL::Compiler.compile runs compile_cleanup on the
363+
# backend as a safety net for stages loops that exit before
364+
# reaching us.
361365
my @frames := nqp::pop(%moar<frames>);
362366
$!writer := MoarVM::BytecodeWriter.new(:$string-heap, :$callsites, :$annotations);
363367
$!mast_compunit := MAST::CompUnit.new(

t/moar/55-compile-frame-balance.t

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
plan(4);
2+
3+
my $comp := nqp::getcomp('nqp');
4+
my $backend := $comp.backend;
5+
6+
# HLL::Backend on MoarVM pushes a per-compile frame list onto
7+
# %COMPILING<moar><frames> at the 'start' stage and pops it at the
8+
# 'mast' stage (QASTCompilerMAST.to_mast). A compile that stops short
9+
# of 'mast' (e.g. `:target('ast')`), or one whose stages unwind via an
10+
# exception, leaves the push dangling. HLL::Compiler.compile rebalances
11+
# via compile_snapshot / compile_cleanup hooks on the backend.
12+
13+
ok(nqp::can($backend, 'compile_snapshot'),
14+
'MoarVM backend exposes compile_snapshot');
15+
ok(nqp::can($backend, 'compile_cleanup'),
16+
'MoarVM backend exposes compile_cleanup');
17+
18+
# Drive an outer compile whose BEGIN body runs several :target('ast')
19+
# compiles. Inner and outer share %*COMPILING. If the inner compiles
20+
# leak their start() push, the depth grows. The rebalance must keep
21+
# pre- and post-depth equal.
22+
my $caught := '';
23+
try {
24+
$comp.compile(q|BEGIN {
25+
my $inner := nqp::getcomp("nqp");
26+
my $before := nqp::elems(%*COMPILING<moar><frames>);
27+
$inner.compile("1", :target("ast"), :compunit_ok(1));
28+
$inner.compile("2", :target("ast"), :compunit_ok(1));
29+
$inner.compile("3", :target("ast"), :compunit_ok(1));
30+
$inner.compile("4", :target("ast"), :compunit_ok(1));
31+
$inner.compile("5", :target("ast"), :compunit_ok(1));
32+
my $after := nqp::elems(%*COMPILING<moar><frames>);
33+
if $before != $after {
34+
nqp::die(":target('ast') leaked " ~
35+
($after - $before) ~ " frame level(s) onto " ~
36+
"%COMPILING<moar><frames>");
37+
}
38+
};|);
39+
CATCH { $caught := ~$!; }
40+
}
41+
is($caught, '',
42+
"multiple :target('ast') compiles inside an outer compile's BEGIN " ~
43+
"leave %COMPILING<moar><frames> balanced");
44+
45+
# Stage exception path: an inner compile that throws during 'parse'
46+
# must run compile_cleanup before the rethrow, so the outer caller sees
47+
# the original exception and a balanced frame stack.
48+
my $caught2 := '';
49+
try {
50+
$comp.compile(q|BEGIN {
51+
my $inner := nqp::getcomp("nqp");
52+
my $before := nqp::elems(%*COMPILING<moar><frames>);
53+
my $threw := 0;
54+
try {
55+
$inner.compile("\"unterminated", :target("parse"), :compunit_ok(1));
56+
CATCH { $threw := 1 }
57+
}
58+
my $after := nqp::elems(%*COMPILING<moar><frames>);
59+
unless $threw {
60+
nqp::die("inner compile did not throw on invalid source");
61+
}
62+
if $before != $after {
63+
nqp::die("exception during 'parse' stage leaked " ~
64+
($after - $before) ~ " frame level(s) onto " ~
65+
"%COMPILING<moar><frames>");
66+
}
67+
};|);
68+
CATCH { $caught2 := ~$!; }
69+
}
70+
is($caught2, '',
71+
"an exception during a stage runs compile_cleanup and rethrows " ~
72+
"the original exception");

0 commit comments

Comments
 (0)