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: |
Under -gc e, gen_free_for_struct emitted the pointee's free call
through reference-typed fields unconditionally. A field still holding
its `unsafe { nil }` default (e.g. `meta &ElementMeta = unsafe { nil }`)
dereferenced NULL — SIGSEGV whenever such a value was freed (the
module-internal `v test` compilation inserts a free of the
option-field unwrap copy at test scope exit; any other free of such a
value crashes the same way).
The guard wraps every pointer-field free in both emission arms (the
plain struct arm and the struct-option container arm):
if (it->meta != 0) { ElementMeta_free(it->meta); }
No refcount bookkeeping rides these emission sites, so the guard on
the call is complete. Behavior test: the 2-file minimal repro from #2
as testdata + a wrapper asserting the -gc e module-internal test run
exits 0 (pre-fix: signal 11).
Root cause of cx-private#737; upstream-bundle candidate (precedent:
vlang#27458, vlang#27658).
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bbd24692c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| emb_sc := if g.pref.use_cache || g.pref.build_mode == .build_module { 'static ' } else { '' } | ||
| g.embedded_data.writeln('${emb_sc}v__embed_file__EmbedFileData _v_embed_file_metadata(u64 ef_hash) {') |
There was a problem hiding this comment.
When users pass -usecache -parallel-cc in that order, option parsing leaves both modes enabled (vlib/v/pref/pref.v:1024-1054). Parallel C generation places embedded_data in out_0.c while calls are distributed among the other translation units, but this line gives _v_embed_file_metadata internal linkage even though those units receive an extern declaration. Consequently, any such build using $embed_file fails to link because the callers cannot resolve the static definition; use parallel-aware linkage or a per-object unique external name instead.
Useful? React with 👍 / 👎.
| // Under -usecache these per-variant cast fns are generated independently in | ||
| // every object that uses the sumtype (program TU + each cached module), so | ||
| // external linkage collides at link. Emit them file-local instead. | ||
| sc := if g.pref.use_cache || g.pref.build_mode == .build_module { 'static ' } else { '' } |
There was a problem hiding this comment.
For -usecache -parallel-cc builds, this unconditionally makes generated sumtype cast helpers static. These helper definitions are collected into auto_fn_definitions and emitted in out_0.c, while functions invoking them can be assigned to the other parallel C files (vlib/v/builder/cbuilder/parallel_cc.v:46-72); their static declarations therefore cannot be satisfied by the definition in out_0.o, causing link failures whenever the program translation unit performs a sumtype conversion. The qualifier needs to preserve cross-file linkage in parallel C mode while still avoiding collisions between cached objects.
Useful? React with 👍 / 👎.
Under -gc e, gen_free_for_struct emitted the pointee's free call
through reference-typed fields unconditionally. A field still holding
its `unsafe { nil }` default (e.g. `meta &ElementMeta = unsafe { nil }`)
dereferenced NULL — SIGSEGV whenever such a value was freed (the
module-internal `v test` compilation inserts a free of the
option-field unwrap copy at test scope exit; any other free of such a
value crashes the same way).
The guard wraps every pointer-field free in both emission arms (the
plain struct arm and the struct-option container arm):
if (it->meta != 0) { ElementMeta_free(it->meta); }
No refcount bookkeeping rides these emission sites, so the guard on
the call is complete. Behavior test: the 2-file minimal repro from #2
as testdata + a wrapper asserting the -gc e module-internal test run
exits 0 (pre-fix: signal 11).
Root cause of cx-private#737; upstream-bundle candidate (precedent:
vlang#27458, vlang#27658).
This branch has not been deployed
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