Repository navigation
[WASM W2-A] Complete host integration and reflection - #2601
Merged
xushiwei merged 35 commits intoSep 17, 2026
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Review: [WASM W2-A] Complete host integration and reflection
Reviewed the full diff (~12.5k lines, 165 files) across code quality, performance, security, and documentation. This is a large, carefully written change. The tricky new invariants all hold up under close reading:
- Wide Go word storage (
ssa/storage.go,ssa/memory.go): static struct form{i8*, i32}vs. dynamic i64 access are internally consistent (both 8 bytes, struct-typed alloca aliases correctly); integer narrowing inserts runtime range assertions / compile-time panics. - Reflection bridges (
ssa/reflect_bridge.go,internal/build/wasm_reflect.go): bijective base-62 IDs and shape-based dedup are correct; bridge generation is memoized by lowered-signature shape key. WASI-only gating matches the docs. - JS/WASM host boundary (
syscall/js,emval.cpp,host.c): theCopyBytes/Lengthrewrites re-acquire heap views after growth, clamp copy lengths withmin, and the new panic-on-non-array behavior matches upstream Go semantics. Embedded-NUL strings are handled by byte-length rather than C-string scanning. - Reflect call bridges (
call_bridge_wasm.go,makefunc*.go): arg-count validated before the call; result copies usemin(ffiType.Size, typ.Size_); indirect args are deep-copied off the transient frame.
No blocking correctness, security, or performance issues found.
Notes (non-blocking, no action required):
runtime/internal/ffi/pointer_array_wasm32.go: the wasm32valuePointerArrayadds a per-call[]uint32allocation on the FFI call path (Go 8-byte pointers → 4-byte libffi slots). This is functionally required; flagging only in case it shows up in profiles.runtime/internal/wasmjs/_wrap/host.c(hostFinalize): decrements_goRefCounts[id]with no bounds/underflow guard. Mirrors upstreamwasm_exec.jsandidcomes from the trusted Go runtime, so not attacker-reachable; a defensiveid < lengthguard would match the other new bounds handling.
One minor consistency item is inline below.
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
Reuse the browser driver already validated in W3 instead of waiting for Chrome dump-dom shutdown. Keep the deadline bounded and exercise explicit page-load failures in CI.
cpunion
force-pushed
the
codex/wasm-w2-host-reflect-20260913
branch
from
September 17, 2026 00:31
1eb979a to
5659f7f
Compare
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracks #2152. Depends on W1 #2578. Replaces the closed #2579 with a clean review thread; its fixes remain in the branch.
Scope
syscall/jssemantics for J32/GoJS (wasm32 with the Go-compatible JavaScript host); retain the adapters for J32/Emscripten (wasm32 with the Emscripten JavaScript host) and J64/Emscripten (wasm64 with the Emscripten JavaScript host). Exercise Node and real Chrome.MakeFunc, and runtime-created function descriptors, with GC-root and Asyncify lifetime handling.Call_/Make_bridges, deduplicated by lowered signature and GC-root shape, with compact names and cache-aware backend selection.Review boundary
Head
1eb979a9515f, based on W1cb7214ae45faand main2db247e43848. The dedicated range iscb7214ae45fa..1eb979a9515f: 35 commits, 83 files, +4,360/-369. The original 33 feature commits are unchanged; follow-ups add the review-requested license header and move the already-validated browser completion driver from W3 into this host layer. The full diff against main also includes W1; single-worker runtime/GC and broad compatibility acceptance belong to the subsequent PRs.Validation
The previous CI failure was a build-time integration error: newly merged
llgo envreferenced the removedConfig.WasmABIfield. W1 now reportsWasmProfileandWasmProvider, with regression tests for all named profiles, both aliases, and a non-WASM target. This branch inherits that fix.The runtime job on the previous head passed the Node scheduler, timers, GC, lifecycle, and callback fixtures, then timed out waiting for Chrome. The browser driver now uses the page-result protocol already present in W3, rather than coupling test completion to
--dump-domand browser shutdown. The 60-second deadline remains unchanged; explicit failures and missing results still fail. A new negative check requires a missing module to produce a page-load failure, not a timeout. No compiler/runtime implementation or WASM fixture code changes in this follow-up.Current-head local compiler construction, reflection regressions, real Chrome callback checks on J32/Emscripten (wasm32 with the Emscripten JavaScript host) and J32/GoJS (wasm32 with the Go-compatible JavaScript host), and the explicit page-load failure check pass. The fork-only final-candidate validation passed the complete runtime gate on LLVM 22, Emscripten 4.0.21, Node 24, and Chrome for Testing 153.0.8010.36, including the negative load check and both real-browser providers. Its candidate differs from this head only by the validation-only workflow; that workflow is not included in this PR.
The upstream current-head runtime job and test-command job also pass. Remaining current-head CI, patch coverage, review, and paired size/performance benchmarks remain required gates. Previous measurements and resolved review history remain in #2579; they are not treated as new-head passes. Benchmarks include
cprintf,println,fmtprintf, and active reflection, with particular attention to keeping WASI bridge costs out of JavaScript providers.