Skip to content

Commit 80e76cd

Browse files
medvednikovmedvednikov
authored
cgen: emit interface type-table index as a real symbol under -usecache (fix undefined _IError_None___index) (vlang#27381)
1 parent 6b7ec83 commit 80e76cd

3 files changed

Lines changed: 254 additions & 8 deletions

File tree

‎vlib/v/gen/c/cgen.v‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ mut:
7878
pcs_declarations strings.Builder // -prof profile counter declarations for each function
7979
cov_declarations strings.Builder // -cov coverage
8080
embedded_data strings.Builder // data to embed in the executable/binary
81+
interface_index_definitions strings.Builder // real interface index symbols for -parallel-cc helpers
8182
shared_types strings.Builder // shared/lock types
8283
shared_functions strings.Builder // shared constructors
8384
out_options_forward strings.Builder // forward `option_xxxx` types
@@ -399,6 +400,7 @@ pub fn gen(files []&ast.File, mut table ast.Table, pref_ &pref.Preferences) GenO
399400
pcs_declarations: strings.new_builder(100)
400401
cov_declarations: strings.new_builder(100)
401402
embedded_data: strings.new_builder(1000)
403+
interface_index_definitions: strings.new_builder(100)
402404
out_options_forward: strings.new_builder(100)
403405
out_options: strings.new_builder(100)
404406
out_results_forward: strings.new_builder(100)
@@ -899,6 +901,10 @@ pub fn gen(files []&ast.File, mut table ast.Table, pref_ &pref.Preferences) GenO
899901
if g.embedded_data.len > 0 {
900902
helpers.write_string2('\n// V embedded data:\n', g.embedded_data.str())
901903
}
904+
if g.interface_index_definitions.len > 0 {
905+
helpers.write_string2('\n// V interface index definitions:\n',
906+
g.interface_index_definitions.str())
907+
}
902908
if g.pref.parallel_cc {
903909
helpers.writeln('\n// V global/const non-precomputed definitions:')
904910
for var_name in g.sorted_global_const_names {
@@ -1036,6 +1042,7 @@ fn cgen_process_one_file_cb(mut p pool.PoolProcessor, idx int, wid int) voidptr
10361042
cov_declarations: strings.new_builder(100)
10371043
hotcode_definitions: strings.new_builder(100)
10381044
embedded_data: strings.new_builder(1000)
1045+
interface_index_definitions: strings.new_builder(100)
10391046
out_options_forward: strings.new_builder(100)
10401047
out_options: strings.new_builder(100)
10411048
out_results_forward: strings.new_builder(100)
@@ -1124,6 +1131,7 @@ pub fn (mut g Gen) free_builders() {
11241131
g.cov_declarations.free()
11251132
g.hotcode_definitions.free()
11261133
g.embedded_data.free()
1134+
g.interface_index_definitions.free()
11271135
g.shared_types.free()
11281136
g.shared_functions.free()
11291137
g.channel_definitions.free()
@@ -14147,6 +14155,11 @@ fn (mut g Gen) interface_table() string {
1414714155
// That keeps stray bytes from overlapping storage, like unions, from
1414814156
// aliasing a valid concrete interface variant.
1414914157
interface_index_name := '_${interface_name}_${cctype}_index'
14158+
interface_index_case_name := if g.pref.use_cache {
14159+
'${interface_index_name}_enum'
14160+
} else {
14161+
interface_index_name
14162+
}
1415014163
if already_generated_mwrappers[interface_index_name] > 0 {
1415114164
continue
1415214165
}
@@ -14236,8 +14249,15 @@ static inline __shared__${interface_name} ${shared_fn_name}(__shared__${cctype}*
1423614249
return ${cast_shared_struct_str};
1423714250
}')
1423814251
if shared_interface_mtx_helper_needed {
14239-
shared_interface_mtx_cases.writeln('\t\tcase ${interface_index_name}:')
14240-
shared_interface_mtx_cases.writeln('\t\t\treturn &(((__shared__${cctype}*)((char*)x->val._${cctype} - __offsetof(__shared__${cctype}, val)))->mtx);')
14252+
mtx_expr := '&(((__shared__${cctype}*)((char*)x->val._${cctype} - __offsetof(__shared__${cctype}, val)))->mtx)'
14253+
if g.pref.build_mode == .build_module {
14254+
shared_interface_mtx_cases.writeln('\tif (x->val._typ == ${interface_index_name}) {')
14255+
shared_interface_mtx_cases.writeln('\t\treturn ${mtx_expr};')
14256+
shared_interface_mtx_cases.writeln('\t}')
14257+
} else {
14258+
shared_interface_mtx_cases.writeln('\t\tcase ${interface_index_case_name}:')
14259+
shared_interface_mtx_cases.writeln('\t\t\treturn ${mtx_expr};')
14260+
}
1424114261
}
1424214262
}
1424314263

@@ -14501,7 +14521,24 @@ return ${cast_shared_struct_str};
1450114521
}
1450214522
iin_idx := already_generated_mwrappers[interface_index_name] - iinidx_minimum_base + 1
1450314523
if g.pref.build_mode != .build_module {
14504-
sb.writeln('enum { ${interface_index_name} = ${iin_idx} };')
14524+
if g.pref.use_cache {
14525+
// With -usecache, modules like `builtin` are compiled separately
14526+
// in build_module mode, where the index is emitted as
14527+
// `extern const u32 ..._index;` and referenced. The main program
14528+
// must therefore provide a real, externally-linked definition
14529+
// (not a compile-time `enum` constant, which has no linker
14530+
// symbol), otherwise the reference is undefined at link time -
14531+
// e.g. `undefined symbol: _IError_None___index` on FreeBSD/clang.
14532+
sb.writeln('enum { ${interface_index_case_name} = ${iin_idx} };')
14533+
if g.pref.parallel_cc {
14534+
sb.writeln('extern const u32 ${interface_index_name};')
14535+
g.interface_index_definitions.writeln('const u32 ${interface_index_name} = ${interface_index_case_name};')
14536+
} else {
14537+
sb.writeln('const u32 ${interface_index_name} = ${interface_index_case_name};')
14538+
}
14539+
} else {
14540+
sb.writeln('enum { ${interface_index_name} = ${iin_idx} };')
14541+
}
1450514542
} else {
1450614543
sb.writeln('extern const u32 ${interface_index_name};')
1450714544
}
@@ -14566,11 +14603,16 @@ return ${cast_shared_struct_str};
1456614603
cast_functions.writeln('
1456714604
static inline sync__RwMutex* ${shared_interface_mtx_helper_name}(__shared__${interface_name}* x) {')
1456814605
if shared_interface_mtx_cases.len > 0 {
14569-
cast_functions.writeln('\tswitch (x->val._typ) {')
14570-
cast_functions.write_string(shared_interface_mtx_cases.str())
14571-
cast_functions.writeln('\t\tdefault:')
14572-
cast_functions.writeln('\t\t\treturn &x->mtx;')
14573-
cast_functions.writeln('\t}')
14606+
if g.pref.build_mode == .build_module {
14607+
cast_functions.write_string(shared_interface_mtx_cases.str())
14608+
cast_functions.writeln('\treturn &x->mtx;')
14609+
} else {
14610+
cast_functions.writeln('\tswitch (x->val._typ) {')
14611+
cast_functions.write_string(shared_interface_mtx_cases.str())
14612+
cast_functions.writeln('\t\tdefault:')
14613+
cast_functions.writeln('\t\t\treturn &x->mtx;')
14614+
cast_functions.writeln('\t}')
14615+
}
1457414616
} else {
1457514617
cast_functions.writeln('\treturn &x->mtx;')
1457614618
}

‎vlib/v/gen/c/link_generated_c_files_test.v‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,31 @@ fn main() {
8989
assert header.contains('#ifdef _VPARALLELCC\n\t\t#define VV_LOC\n\t#else\n\t\t#define VV_LOC static\n\t#endif'), header
9090
assert header.contains('VV_LOC string main__helper(void);'), header
9191
}
92+
93+
fn test_parallel_cc_usecache_interface_index_definition_stays_out_of_header() {
94+
tmp_dir := os.join_path(os.vtmp_dir(), 'parallel_cc_usecache_interface_index_${os.getpid()}')
95+
os.mkdir_all(tmp_dir)!
96+
defer {
97+
os.rmdir_all(tmp_dir) or {}
98+
}
99+
source_path := os.join_path(tmp_dir, 'main.v')
100+
os.write_file(source_path, "fn main() {\n\tprintln('hello world')\n}\n")!
101+
mut prefs, _ := pref.parse_args_and_show_errors([], [
102+
'',
103+
'-usecache',
104+
'-parallel-cc',
105+
source_path,
106+
], false)
107+
mut b := builder.new_builder(prefs)
108+
mut files := b.get_builtin_files()
109+
files << b.get_user_files()
110+
b.set_module_lookup_paths()
111+
b.front_and_middle_stages(files)!
112+
result := c.gen(b.parsed_files, mut b.table, b.pref)
113+
header := result.header.replace('\r\n', '\n')
114+
out0 := result.out0_str.replace('\r\n', '\n')
115+
assert header.contains('enum { _IError_None___index_enum ='), header
116+
assert header.contains('extern const u32 _IError_None___index;'), header
117+
assert !header.contains('const u32 _IError_None___index = _IError_None___index_enum;')
118+
assert out0.contains('const u32 _IError_None___index = _IError_None___index_enum;'), out0
119+
}

‎vlib/v/tests/usecache_interface_index_symbol_test.v‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import os
2+
3+
const vexe = @VEXE
4+
5+
// Regression test for https://github.com/vlang/v/issues/27330
6+
//
7+
// With -usecache, modules like `builtin` are compiled separately in
8+
// build_module mode, where the interface type-table index is emitted as
9+
// `extern const u32 ..._index;` and referenced. The main program must therefore
10+
// provide a real, externally-linked `const u32 ..._index = N;` definition - a
11+
// compile-time `enum` constant has no linker symbol, so the reference would be
12+
// undefined at link time (e.g. `undefined symbol: _IError_None___index` on
13+
// FreeBSD/clang).
14+
fn test_usecache_interface_index_is_real_symbol() {
15+
tmp_dir := os.join_path(os.vtmp_dir(), 'v_issue_27330')
16+
os.mkdir_all(tmp_dir) or { panic(err) }
17+
defer {
18+
os.rmdir_all(tmp_dir) or {}
19+
}
20+
source_path := os.join_path(tmp_dir, 'issue_27330.v')
21+
os.write_file(source_path, "fn main() {\n\tprintln('hello world')\n}\n") or { panic(err) }
22+
23+
// -o - dumps the generated C of the main program to stdout.
24+
res := os.execute('${os.quoted_path(vexe)} -usecache -o - ${os.quoted_path(source_path)}')
25+
if res.exit_code != 0 {
26+
panic(res.output)
27+
}
28+
// The index must be a real (externally-linked) definition, not a bare enum.
29+
// The separate enum keeps an integer constant expression available for C
30+
// contexts like switch case labels.
31+
assert res.output.contains('enum { _IError_None___index_enum =')
32+
assert res.output.contains('const u32 _IError_None___index = _IError_None___index_enum;')
33+
assert !res.output.contains('enum { _IError_None___index =')
34+
35+
// Sanity check: without -usecache the compile-time enum form is kept (it is
36+
// the tcc-friendly form and needs no external symbol in a single build).
37+
res2 := os.execute('${os.quoted_path(vexe)} -o - ${os.quoted_path(source_path)}')
38+
if res2.exit_code != 0 {
39+
panic(res2.output)
40+
}
41+
assert res2.output.contains('enum { _IError_None___index =')
42+
}
43+
44+
fn test_usecache_shared_interface_lock_uses_enum_index_in_case_labels() {
45+
tmp_dir := os.join_path(os.vtmp_dir(), 'v_issue_27330_shared')
46+
os.mkdir_all(tmp_dir) or { panic(err) }
47+
defer {
48+
os.rmdir_all(tmp_dir) or {}
49+
}
50+
source_path := os.join_path(tmp_dir, 'shared_interface.v')
51+
os.write_file(source_path, '
52+
interface MyInterface {
53+
foo() string
54+
}
55+
56+
struct MyStruct {
57+
pub mut:
58+
fooer shared MyInterface
59+
}
60+
61+
struct MyImplementor {
62+
mut:
63+
num int
64+
}
65+
66+
fn (m MyImplementor) foo() string {
67+
return "Hello World!"
68+
}
69+
70+
fn main() {
71+
shared imp := MyImplementor{
72+
num: 1
73+
}
74+
s := MyStruct{
75+
fooer: imp
76+
}
77+
lock s.fooer {
78+
println(s.fooer.foo())
79+
}
80+
}
81+
') or {
82+
panic(err)
83+
}
84+
85+
res := os.execute('${os.quoted_path(vexe)} -usecache -o - ${os.quoted_path(source_path)}')
86+
if res.exit_code != 0 {
87+
panic(res.output)
88+
}
89+
assert res.output.contains('enum { _main__MyInterface_main__MyImplementor_index_enum =')
90+
assert res.output.contains('const u32 _main__MyInterface_main__MyImplementor_index = _main__MyInterface_main__MyImplementor_index_enum;')
91+
assert res.output.contains('case _main__MyInterface_main__MyImplementor_index_enum:')
92+
assert !res.output.contains('case _main__MyInterface_main__MyImplementor_index:')
93+
}
94+
95+
fn test_usecache_build_module_shared_interface_lock_uses_canonical_index_symbol() {
96+
root := os.join_path(os.vtmp_dir(), 'v_issue_27330_shared_module_${os.getpid()}')
97+
cache_dir := os.join_path(root, '.cache')
98+
vtmp_dir := os.join_path(root, '.vtmp')
99+
os.rmdir_all(root) or {}
100+
defer {
101+
os.rmdir_all(root) or {}
102+
}
103+
os.mkdir_all(os.join_path(root, 'maker'))!
104+
os.mkdir_all(vtmp_dir)!
105+
os.write_file(os.join_path(root, 'v.mod'),
106+
"Module {\n\tname: 'v_issue_27330_shared_module'\n}\n") or { panic(err) }
107+
os.write_file(os.join_path(root, 'maker', 'maker.v'), '
108+
module maker
109+
110+
interface MyInterface {
111+
foo() string
112+
}
113+
114+
struct MyStruct {
115+
pub mut:
116+
fooer shared MyInterface
117+
}
118+
119+
struct MyImplementor {
120+
mut:
121+
num int
122+
}
123+
124+
fn (m MyImplementor) foo() string {
125+
return "Hello World!"
126+
}
127+
128+
pub fn exercise() {
129+
shared imp := MyImplementor{
130+
num: 1
131+
}
132+
s := MyStruct{
133+
fooer: imp
134+
}
135+
lock s.fooer {
136+
println(s.fooer.foo())
137+
}
138+
}
139+
') or {
140+
panic(err)
141+
}
142+
143+
old_vcache := os.getenv_opt('VCACHE') or { '' }
144+
old_vtmp := os.getenv_opt('VTMP') or { '' }
145+
os.setenv('VCACHE', cache_dir, true)
146+
os.setenv('VTMP', vtmp_dir, true)
147+
defer {
148+
if old_vcache.len == 0 {
149+
os.unsetenv('VCACHE')
150+
} else {
151+
os.setenv('VCACHE', old_vcache, true)
152+
}
153+
if old_vtmp.len == 0 {
154+
os.unsetenv('VTMP')
155+
} else {
156+
os.setenv('VTMP', old_vtmp, true)
157+
}
158+
}
159+
mut p := os.new_process(vexe)
160+
p.set_work_folder(root)
161+
p.set_args(['-keepc', 'build-module', 'maker'])
162+
p.set_redirect_stdio()
163+
p.wait()
164+
stdout := p.stdout_slurp()
165+
stderr := p.stderr_slurp()
166+
exit_code := p.code
167+
p.close()
168+
assert exit_code == 0, '${stdout}${stderr}'
169+
generated_c_path := os.join_path(vtmp_dir, 'maker.tmp.c')
170+
assert os.exists(generated_c_path)
171+
generated_c := os.read_file(generated_c_path)!
172+
assert generated_c.contains('extern const u32 _maker__MyInterface_maker__MyImplementor_index;')
173+
assert generated_c.contains('if (x->val._typ == _maker__MyInterface_maker__MyImplementor_index) {')
174+
assert !generated_c.contains('enum { _maker__MyInterface_maker__MyImplementor_index_enum =')
175+
assert !generated_c.contains('case _maker__MyInterface_maker__MyImplementor_index_enum:')
176+
}

0 commit comments

Comments
 (0)