Skip to content

Commit 5c4151a

Browse files
committed
v3: stop re-reading released worker args in the monomorphize merges
The monomorphize worker merge copied its `[]string` tables shallowly: the array was copied, but every element still pointed into the worker scratch arena that is released right after the merge. A later pass (the driver compiles twice on this path) re-seeds its specializations from `generic_specialization_args` / `monomorph_cache_specs`, so it read freed type arguments: they turn into NUL bytes, and V3 then reports bogus `unknown function` diagnostics for perfectly valid calls (`ctrl.use` / `ctrl.route_use` in veb apps) and silently falls back to the compatibility compiler. Deep-copy the merged arguments into the parent arena, re-record every emitted specification from the master (instead of skipping keys a worker already recorded) so the last, parent-owned copy wins, and copy the emitted specialization keys/arguments into the parent arena while the worker scope is still alive but suspended. Needed to compile RuoQi-v (https://github.com/RuoQi-DoDo/RuoQi-v) with `-new-compiler`. Fixes #28489.
1 parent c56c69a commit 5c4151a

4 files changed

Lines changed: 162 additions & 7 deletions

File tree

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import os
2+
3+
const scoped_monomorph_vexe = @VEXE
4+
const scoped_monomorph_tests_dir = os.dir(@FILE)
5+
const scoped_monomorph_v3_dir = os.dir(scoped_monomorph_tests_dir)
6+
const scoped_monomorph_vlib_dir = os.dir(scoped_monomorph_v3_dir)
7+
const scoped_monomorph_v3_src = os.join_path(scoped_monomorph_v3_dir, 'v.v')
8+
9+
$if windows {
10+
const scoped_monomorph_bin_suffix = '.exe'
11+
} $else {
12+
const scoped_monomorph_bin_suffix = ''
13+
}
14+
15+
fn scoped_monomorph_v3_bin_path() string {
16+
return os.join_path(os.temp_dir(), 'v3_scoped_monomorphize_closure_test${scoped_monomorph_bin_suffix}')
17+
}
18+
19+
fn scoped_monomorph_v3_bin() string {
20+
bin := scoped_monomorph_v3_bin_path()
21+
if os.exists(bin) {
22+
return bin
23+
}
24+
// `-prealloc` is what enables `scope_parallel_workers` and the scoped
25+
// monomorphize path, matching how the distributed compiler is built.
26+
build := os.execute('${os.quoted_path(scoped_monomorph_vexe)} -gc none -prealloc -path "${scoped_monomorph_vlib_dir}|@vlib|@vmodules" -o ${os.quoted_path(bin)} ${os.quoted_path(scoped_monomorph_v3_src)}')
27+
assert build.exit_code == 0, build.output
28+
return bin
29+
}
30+
31+
fn testsuite_begin() {
32+
os.rm(scoped_monomorph_v3_bin_path()) or {}
33+
}
34+
35+
// Compiler builds use `-prealloc`, and the memory-bounded monomorphize path (the
36+
// fix for vlang/v#28564) runs for every non-empty specialization batch there.
37+
// That path used to give two different lifted closures the same `__anon_fn_N`
38+
// name - the module-keyed signature table then mixed their signatures up and the
39+
// generated C did not compile - and merged specialization arguments as shallow
40+
// `[]string` copies that still pointed into the released worker arena, so a later
41+
// pass read freed arguments (bogus `unknown function` diagnostics or a crash).
42+
// This is the small `veb` program from vlang/v#28489, which exercises closures
43+
// lifted while specializing a generic helper.
44+
fn test_scoped_monomorphize_keeps_closure_signatures_and_args() {
45+
v3_bin := scoped_monomorph_v3_bin()
46+
dir := os.join_path(os.temp_dir(), 'v3_scoped_monomorphize_closure')
47+
os.rmdir_all(dir) or {}
48+
os.mkdir_all(dir) or { panic(err) }
49+
defer {
50+
os.rmdir_all(dir) or {}
51+
}
52+
os.write_file(os.join_path(dir, 'main.v'), "module main
53+
54+
import veb
55+
56+
pub struct Ctx {
57+
veb.Context
58+
}
59+
60+
pub struct ModelApp {
61+
veb.Middleware[Ctx]
62+
veb.Controller
63+
}
64+
65+
pub struct Item {
66+
ModelApp
67+
}
68+
69+
pub struct MainApp {
70+
veb.Middleware[Ctx]
71+
veb.Controller
72+
}
73+
74+
fn mw() veb.MiddlewareOptions[Ctx] {
75+
return veb.MiddlewareOptions[Ctx]{
76+
handler: fn (mut ctx Ctx) bool {
77+
return true
78+
}
79+
}
80+
}
81+
82+
fn (mut app MainApp) common_middleware[T](mut ctrl T) {
83+
ctrl.use(mw())
84+
}
85+
86+
fn (mut app MainApp) register_routes_no_auth[T, U](mut ctrl T, url_path string) {
87+
app.common_middleware[T](mut ctrl)
88+
app.register_controller[T, U](url_path, mut ctrl) or { panic(err) }
89+
ctrl.route_use('/item/*', veb.encode_auto[Ctx]())
90+
}
91+
92+
fn main() {
93+
mut app := &MainApp{}
94+
app.register_routes_no_auth[Item, Ctx](mut &Item{}, '/item')
95+
veb.run_at[MainApp, Ctx](mut app, port: 9001) or { panic(err) }
96+
}
97+
") or { panic(err) }
98+
out := os.join_path(dir, 'app${scoped_monomorph_bin_suffix}')
99+
compile := os.execute('${os.quoted_path(v3_bin)} -nocache -o ${os.quoted_path(out)} ${os.quoted_path(dir)}')
100+
assert compile.exit_code == 0, compile.output
101+
assert !compile.output.contains('C compilation failed'), compile.output
102+
assert os.is_file(out), 'the compile produced no binary'
103+
}

‎vlib/v/transform/monomorphize.v‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,13 +588,21 @@ fn (mut t Transformer) request_generic_fn_specialization(decl GenericFnDecl, arg
588588
}
589589

590590
fn (mut t Transformer) record_monomorph_cache_spec(key string, decl_key string, module_name string, args []string) {
591-
if key.len == 0 || decl_key.len == 0 || key in t.monomorph_cache_specs {
591+
if key.len == 0 || decl_key.len == 0 {
592592
return
593593
}
594-
t.monomorph_cache_specs[key] = MonomorphCacheSpec{
595-
decl_key: decl_key
596-
module: module_name
597-
args: args.clone()
594+
// The strings must not reference a worker arena: a scoped batch releases its
595+
// arenas right after the merge, and a later pass re-seeds its specializations
596+
// from this cache. Re-record instead of skipping an existing key so the last
597+
// (parent-arena) copy wins over one a worker recorded for the same spec.
598+
mut owned_args := []string{cap: args.len}
599+
for arg in args {
600+
owned_args << arg.clone()
601+
}
602+
t.monomorph_cache_specs[key.clone()] = MonomorphCacheSpec{
603+
decl_key: decl_key.clone()
604+
module: module_name.clone()
605+
args: owned_args
598606
}
599607
}
600608

‎vlib/v/transform/monomorphize_test.v‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,3 +674,24 @@ fn test_free_generic_map_suffix_preserves_qualified_value_type() {
674674
decoded := generic_type_arg_from_suffix_with_containers(suffix)
675675
assert decoded == 'map[string]binary.St'
676676
}
677+
678+
// The monomorph cache outlives the worker arena that produced an entry: a scoped
679+
// batch records a spec, releases its arena and a later pass re-seeds from this
680+
// table (vlang/v#28489). Recording has to own key, module, declaration key and
681+
// every argument, and a re-recorded key must keep the last copy instead of being
682+
// skipped.
683+
fn test_record_monomorph_cache_spec_replaces_existing_entry() {
684+
mut a := flat.FlatAst.new()
685+
mut tc := types.TypeChecker.new(&a)
686+
mut t := new_transformer(mut a, &tc, map[string]bool{})
687+
688+
t.record_monomorph_cache_spec('main.f_T_int', 'main.f', 'main', ['int'])
689+
t.record_monomorph_cache_spec('main.f_T_int', 'main.f', 'main', ['iam.Token'])
690+
spec := t.monomorph_cache_specs['main.f_T_int'] or {
691+
assert false, 'the re-recorded spec disappeared'
692+
return
693+
}
694+
assert spec.args == ['iam.Token']
695+
assert spec.decl_key == 'main.f'
696+
assert spec.module == 'main'
697+
}

‎vlib/v/transform/transform_parallel_notd_v3_no_parallel.v‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,7 +1412,10 @@ fn (mut t Transformer) run_parallel_monomorphize_specs(specs []PendingGenericFnS
14121412
for name in w.generic_specialization_args_log {
14131413
spec_args := w.generic_specialization_args[name] or { continue }
14141414
if name !in t.generic_specialization_args {
1415-
t.generic_specialization_args[name.clone()] = spec_args.clone()
1415+
// Deep-copy the elements: the worker's copy of this array is
1416+
// backed by its scratch arena, which is released after the
1417+
// merge (a later pass re-seeds from these recorded args).
1418+
t.generic_specialization_args[name.clone()] = clone_monomorph_specialization_args(spec_args)
14161419
}
14171420
}
14181421
// Every emitted worker specialization is registered by the master below.
@@ -1567,6 +1570,21 @@ fn (mut t Transformer) run_scoped_monomorphize_specs(specs []PendingGenericFnSpe
15671570
roots << root
15681571
emitted_specs << spec
15691572
}
1573+
// The emitted specs can carry worker-owned argument strings. Copy key and
1574+
// arguments while the worker scope is still alive, but with that scope
1575+
// suspended so the copies land in the arena the merge loops below use:
1576+
// reading the worker strings after `transform_worker_scope_leave()` would
1577+
// be a use-after-free (vlang/v#28489).
1578+
mut owned_emitted_specs := []PendingGenericFnSpec{cap: emitted_specs.len}
1579+
worker_scope_state := transform_stage_scope_suspend(scope)
1580+
for spec in emitted_specs {
1581+
owned_emitted_specs << PendingGenericFnSpec{
1582+
decl: spec.decl
1583+
args: clone_monomorph_specialization_args(spec.args)
1584+
key: spec.key.clone()
1585+
}
1586+
}
1587+
transform_stage_scope_resume(scope, worker_scope_state)
15701588
w.worker_scope = scope
15711589
transform_worker_scope_leave(scope)
15721590

@@ -1594,8 +1612,13 @@ fn (mut t Transformer) run_scoped_monomorphize_specs(specs []PendingGenericFnSpe
15941612
}
15951613
t.request_generic_fn_specialization(pending.decl, owned_args)
15961614
}
1597-
for idx, spec in emitted_specs {
1615+
for idx, spec in owned_emitted_specs {
15981616
root := flat.NodeId(int(roots[idx]) + node_shift)
1617+
// Re-record the spec from the master: `record_monomorph_cache_spec`
1618+
// deep-copies the argument strings into the arena that is current here
1619+
// (the parent), while the worker's copies die with `scope`.
1620+
t.record_monomorph_cache_spec(spec.key.clone(), spec.decl.key, spec.decl.module,
1621+
spec.args)
15991622
if !t.generic_specialization_registered(spec.decl, spec.args) {
16001623
value := specialized_generic_fn_value(spec.decl.node.value, spec.args)
16011624
t.register_specialized_fn_signature_value(spec.decl, value, spec.args)

0 commit comments

Comments
 (0)