cgen,builder,markused,vcache: make -usecache correct for multi-module builds (fix #27592)#27658
cgen,builder,markused,vcache: make -usecache correct for multi-module builds (fix #27592)#27658eptx wants to merge 7 commits into
Conversation
…_module
Fixed-size C arrays ([N]T) are not assignable with `=`, so the
`dst = *(T*)&((T[]){..}[0])` runtime-init form emitted illegal C
("array type ... is not assignable") whenever globals are zero-initialised
in _vinit rather than at declaration — the path taken under -usecache and
build_module. Emit memcpy for fixed-array globals, mirroring the existing
{E_STRUCT} memcpy path.
…builds
Fix a family of pre-existing -usecache bugs that made cached module builds
fail to link, miscompile, or crash. All share one root cause: under
-usecache the program translation unit is compiled with skip_unused ON
while each cached module (`v build-module`) is compiled with skip_unused
OFF, so the two disagree on what is defined vs referenced and on type
index numbering. The fix consistently routes every cross-TU definition and
initialization through the program TU as the single owner, with cached
modules referencing `extern`.
gen/c/infix.v
Sumtype `is` compared a module-local type index (int(right_type)) while
the matching cast/`match` used the unified g.type_sidx() expression, so
`is` silently mismatched across TUs. Use g.type_sidx() for both.
gen/c/cgen.v
- Interface (interface,concrete) dispatch index: emit as a program-TU
`const u32` definition, referenced `extern` by cached modules, instead
of a per-TU enum/local that does not survive linking. Guard the
interface-table skip_unused prune so cached builds keep referenced
method decls and complete iface/variant types.
- Per-module const init: emit `void <mod>__init_consts(void)` in each
cached module and call all of them from the program TU's init, before
the inline const inits, so cached-module statics are initialized
(previously left 0). Fixes edwards25519 uninit-read segfault, etc.
- Closures: gate closure_init on table.used_features.anon_fn so the
trampoline pool is initialized even when the program TU itself has
nr_closures == 0 but a cached module uses closures.
- Definition #include (".c" amalgamations, e.g. zstd) emitted only in
the owning TU to avoid duplicate symbols across cached objects.
- Sumtype cast fns, v_typeof_interface helper, and embed metadata made
`static` / `extern`-declared appropriately under use_cache.
gen/c/consts_and_globals.v
argv/argc globals defined in the program TU (extern in cached modules);
extern globals emit decl-only (no re-init).
gen/c/fn.v
Per-`_test.v` functions and non-owning-module methods emit decl-only in
cached builds; guarded so builtin methods are not over-skipped. Bundled
modules (util.bundle_modules, e.g. builtin.closure) are exempted from the
use_cache builtin decl-only gate: they are skipped in every build-module
compile, so no cached object carries their bodies and the program TU must
emit them — otherwise closure_create_with_data etc. are undefined at link
for any non-test -usecache build that uses closures.
gen/c/auto_str_methods.v, gen/c/embed.v
Auto str fns and embed metadata use the non-parallel/static owner path.
markused/markused.v
Mark non-generic interfaces under use_cache so their tables are complete.
builder/rebuilding.v
handle_usecache: apply the import-loop guards (help / builtin /
already-built / should-bundle) to the test's own non-main modules too.
Without them a test build cached builtin a second time via its absolute
path, linking two builtin objects and duplicating ~9000 symbols.
…ing, compiler-identity salt Two cache-invalidation bugs that each leave a silently wrong (or unlinkable) binary until the user manually wipes the cache: 1. CACHE POISONING: source hashes were saved BEFORE invalidated modules were rebuilt, and v_build_module ignored the rebuild exit status. One failed rebuild (racing edit, transient cc error) left the stale object in place with hashes already recorded as current — every later build silently linked the stale module object until a manual cache wipe (observed as a permanent link failure; an ABI-compatible variant is a silently WRONG binary). Now: failed rebuilds are loud, the stale object is removed (nothing stale can be served), and hashes are recorded only once nothing stale survives — the next run re-detects and retries. Wipe-free recovery verified. 2. COMPILER IDENTITY: cache keys ignored which compiler built the object, so a rebuilt v (v self / make) reused objects generated by an older cgen. The baked git hash is insufficient (v self from uncommitted changes keeps it): the cache namespace is now salted with the running v executable size+mtime.
A small multi-module project compiled with -usecache as a normal program and as a test build, cold and warm cache each, with an isolated VCACHE. Exercises, across the program-TU/cached-module boundary: sumtype `is`, interface dispatch, closures created in the cached module, module consts / array growth (max_int), and float formatting (strconv's const tables in the cached builtin object). Fails on master (undefined symbols for the program build, duplicate symbols for the test build); passes with the preceding fixes.
|
V1 is going to be removed this month, so it's better to implement such things in V3. |
|
Well it'll still stay in oldv/ for 1 year, and will be available with -oldv. But it's no longer going to be developed. |
How do I use v3 now |
Cached module objects emitted tentative definitions (common symbols) for interface name tables, bundled-module globals (g_closure), and the argv globals — relying on the linker merging commons across objects. Current Apple ld and lld no longer merge cross-TU tentative definitions; they error with 'N duplicate symbols', which broke every -usecache build (2 dups for a tiny program, 328 for a real test binary). Ownership now matches the -usecache design exactly, with no commons: - interface name tables: build_module objects emit 'extern T x_name_table[N];' (program TU keeps the single initialized definition); - bundled-module globals (builtin.closure / builtin.overflow): build_module objects reference extern; the program TU under -usecache is the single definition site and now also runs their initializers; - g_main_argc/g_main_argv: builtin's own build_module compile no longer emits 'extern ... = 0;' (a definition) — declaration only, the generated main() in the program TU owns them. Verified: nm on cached objects shows U (was __DATA,__common) for IError_name_table / g_closure / g_main_argv; cold and warm -usecache builds link and run.
|
Added one commit ( While validating downstream we hit another instance of the same class this PR addresses, worth fixing here too: cached module objects emit tentative definitions (common symbols) for interface Validation on the branch: the multi-module regression test from this PR, the #27381 test, and |
|
…t cache invalidation
Two related defects made -usecache module objects disagree with the
program TU about $embed_file content:
1. RESOLUTION: a relative embed path probed the process CWD first and
bound to it whenever a file existed there, using the documented
source-relative location ("Paths can be absolute or relative to the
source file", doc/docs.md) only as a fallback. Two consequences:
- the -usecache `v build-module` child runs chdir-ed to VROOT
(Builder.v_build_module), so a CWD-relative twin of the asset
under VROOT's parent tree was silently baked into the cached
module object while the program TU embedded the intended file —
divergent embedded content across objects of one binary;
- the "source file" anchor used for the fallback was Checker.file,
which is the IMPORTING file's context when a module const
initializer is checked from a use site, not the declaring file.
Now the parser records the declaring file on ast.EmbeddedFile
(source_file, like HashStmt.source_file), and the checker resolves
relative paths against it FIRST, keeping CWD-relative resolution
only as a backwards-compatibility fallback. Resolution is thereby
CWD-independent: parent, build-module child and any later compile
agree on the embedded file.
2. INVALIDATION: embedded assets are invisible to the .v source
hashes, so editing ONLY the embedded file kept serving the stale
cached module object forever. Every build-module object now records
a .embeds.txt manifest (content hash + absolute path per checker-
resolved $embed_file target, via the new ast.File.embedded_apaths),
and Builder.rebuild_cached_module verifies it before serving a
cache hit, rebuilding on any mismatch. A failed rebuild removes the
stale object instead of serving it (the same poisoning class as the
eager-hash-save bug).
Regression test: vlib/v/tests/usecache_embed_file_test.v builds a
module whose embed has a CWD-relative decoy twin from a foreign CWD
(resolution), then edits only the asset under a warm cache
(invalidation). Also green: vlib/v/embed_file 6/6, vlib/v/vcache.
|
Pushed one more Symptom class: with Root causes (two, related):
Regression test: Found in the wild: a git-worktree checkout sharing one V binary with its main checkout — |
…v globals object-local
Three CI-surfaced fixes:
1. v_build_module: clear VFLAGS around the spawned 'v build-module' child.
The parent already folds VFLAGS into the relayed b.pref.build_options;
letting the child re-apply VFLAGS duplicates values ('-cflags x' -> 'x x'),
so the child derives a different vcache key than the parent looks up and
the freshly built object is never found ('could not rebuild cache module
... does not exist yet'). Pre-existing on master (reproducible with
VFLAGS='-cc clang -cflags -fno-omit-frame-pointer' v -usecache run x.v);
exposed by the new usecache tests under the sanitized CI jobs, which set
exactly such a VFLAGS.
2. consts_and_globals: exclude -is_o outputs from the program-owned argv
global definition — each -is_o output is a standalone hostable TU and
several are expected to be linked together (link_generated_c_files_test),
so g_main_argc/g_main_argv keep their object-local (static) linkage there.
3. consts_and_globals: the bundled-module should_init clause was dangling
(comment lines between '||' continuations terminated the expression, so
the final '|| (...)' parsed as a discarded statement — flagged by
v_parser_test). Joined properly; the program TU now emits the initialized
definition for bundled-module globals under -usecache as documented.
Validated: link_generated_c_files_test, v_parser_test, usecache_multi_module
+ usecache_embed_file (plain AND under the sanitize jobs' VFLAGS), vcache,
embed_file 6/6, fmt -verify + vet clean.
|
Pushed
The remaining red I could find is master-side, not from this branch: Validated locally: |
4 participants
Fixes the family of
-usecachecorrectness bugs root-caused in #27592 (duplicate__globalsymbols there are one instance of a general pattern), and makes cache invalidation sound. Includes a multi-module regression test.Root cause
Under
-usecachethe program TU is compiled withskip_unusedON while each cached module (v build-module) is compiled withskip_unusedOFF, so the two disagree about what is defined vs referenced (and about type-index numbering). This PR consistently routes every cross-TU definition/initialization through a single owner — the program TU — with cached modules referencingextern.Layers fixed
__globals (-usecachelink failure on the clang/gcc backend (macOS arm64): duplicate__globalsymbols (g_autostr_*_stack) from cached builtin.o #27592): extern globals emit decl-only in the program TU instead of a duplicate definition;g_main_argc/g_main_argvare defined by the program TU (they otherwise have no definition at all under-usecache)._vinit-time init formdst = *(T*)&((T[]){..}[0])is illegal C for arrays (not assignable) — emit memcpy, mirroring the existing struct path.is: the fallthrough compared a module-localint(right_type)literal while the cast wrappers andmatchuse the unified_v_type_idx_<T>()value — sox is Tsilently disagrees with how the value was tagged across cached objects (misdispatch, no diagnostic). Nowtype_sidx()(a no-op for plain builds).statics, but only the program TU has_vinitconst-init code — a cached module's consts stay zeroed (e.g.max_int == 0insidebuiltin.omakesarray.pushpanic on any growth;strconv's tables read as zeros). Each cached object now emits an idempotent<mod>__init_consts(), and the program TU calls them all before its own inline const inits (so const-generator functions that run in cached objects, e.g. ed25519'sd_const, see live values).closure_init()was only called when the program TU itself generated a closure (nr_closures > 0), so a closure created inside a cached module ran on an uninitialized trampoline pool → segfault; now gated on the whole-programused_features.anon_fn. (b) Bundled modules (util.bundle_modules:builtin.closure,builtin.overflow) are skipped in every build-module compile, yet the program TU also emitted them decl-only for non-test builds — their bodies existed in no object:undefined symbol: builtin__closure__closure_create_with_datafor any non-test-usecachebuild that uses closures.#includes (C amalgamations): emitted only in the owning TU, so the same C definitions don't land in several cached objects.handle_usecachecachedbuiltina second time via its absolute path (missing the import-loop guards on the test-module loop) → both objects linked, ~9000 duplicate symbols. Also,-usecachetest builds re-emitted every used cached-module body in the program TU (blanket!is_test); now only fns defined in_test.vfiles get bodies there.v_build_module's exit status was ignored — one failed rebuild (racing edit, transient cc error) leaves a stale object that every later build silently links until a manualv wipe-cache. And cache keys ignored which compiler built an object, so a rebuiltv(afterv selffrom uncommitted changes) reuses objects produced by an older cgen. Failed rebuilds are now loud + remove the stale object before hashes are recorded, and the cache namespace is salted with the running v executable's size+mtime.Regression test
vlib/v/tests/usecache_multi_module_test.v+testdata/usecache_multi_module/: a small multi-module project exercising, across the program-TU/cached-module boundary: sumtypeis, interface dispatch, closures created in the cached module, module consts / array growth (max_int), and float formatting (strconv's const tables in the cached builtin object). Compiled as a normal program and as a test build, cold and warm cache each, with an isolatedVCACHE/VTMP. On vanilla master the program build fails to link and the test build has ~9000 duplicate symbols; with this PR everything passes.Validation
(macOS arm64,
-cc clang; every-usecacheitem below fails on vanilla master)v wipe-cache && v -cc clang -usecache run hello.v— the-usecachelink failure on the clang/gcc backend (macOS arm64): duplicate__globalsymbols (g_autostr_*_stack) from cached builtin.o #27592 repro — links and runs, cold and warm.v -cc clang -usecache -o v2 cmd/v— the compiler itself builds under-usecachewith clang and the resulting binary works (the scenario the CI task disabled on macOS in ci: comment out the taskV self compilation with -usecacheon macos for now #23145).v test vlib/v/tests/usecache_interface_index_symbol_test.v(the cgen: emit interface type-table index as a real symbol under-usecache(fix undefined_IError_None___index) #27381 regression test),v test vlib/v/vcache/— pass;v test vlib/builtin/— identical results to master;v fmt -verifyclean on all touched files;v selffine.Happy to split this if preferred — the builder/vcache invalidation fixes and the cgen cross-TU fixes are cleanly separable.
Fixes #27592