[depends on #2338] os, runtime: enable native Windows filesystem and process exit - #2340
[depends on #2338] os, runtime: enable native Windows filesystem and process exit#2340cpunion wants to merge 84 commits into
Conversation
There was a problem hiding this comment.
Review Summary
Reviewed the full diff (base 7f954d6...head 4fd4c72) across four dimensions — code quality, performance, security, and documentation accuracy. This is a large, well-constructed PR adding Windows support to the llgo runtime and build pipeline (cabi MSVC, crosscompile, goarch config, Windows threads/sync/setjmp/TLS/fault handling, SEH unwinding, os/syscall shims, and smoke tests).
Overall: strong. No blocking issues. Build-tag/file-suffix partitioning (_windows_386/_amd64/_arm64 vs _default/_unix) is internally consistent and mutually exclusive; platformFaultCallers/platformCallers and the sync API are defined exactly once per build configuration; the mmap'd meta parser now bounds-validates untrusted file bytes; the variable-arity syscall bridge is bounded (windowsSyscallMaxArgs = 42) to match the fixed asm stack reservation; and C wrappers pair Heap allocations with frees on all error paths. Performance-sensitive paths (native SRW/CONDITION_VARIABLE-backed sync, cached QueryPerformanceFrequency, SEH unwinding only on panic/fault paths) look sound. The CI windows.yml is safe by construction (pull_request, permissions: contents: read, no untrusted ${{ }} interpolation into run: steps).
A few low-severity notes are inline below.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
1b06e4a to
6f0dcc9
Compare
Depends on #2338.
Part of #2325.
Summary
os,syscall,filepath, andinternal/pollimplementations by supplying the remaining LLGo runtime boundarysyscall.GetpagesizethroughGetSystemInfoandsyscall.ExitthroughExitProcessinternal/polllinkname surface while explicitly reportingERROR_NOT_SUPPORTEDuntil the scheduler-integrated IOCP stage landsScope
This is the filesystem and process-exit stage of the Windows proposal. It depends only on the native runtime core in #2338 and is independent of the callback work in #2352.
The PR does not copy or replace the Windows standard library. The behavior remains in the pinned Go 1.26 sources; LLGo only supplies the runtime symbols those sources expect. Scheduler-integrated IOCP, sockets/networking, child-process creation, console handling, and broader GOROOT qualification remain separate stages.
Validation
On macOS ARM64 with Go 1.26.5 and LLVM 19:
git range-diffgofmtandgit diff --checkpassi686,x86_64, andaarch64MSVC triplesgo test -count=1 ./internal/ffi ./internal/lib/runtimepasses from the runtime moduleOn a native Windows 11 ARM64 VM with Go 1.26.5, MSVC Build Tools, and LLVM 19.1.7:
internal/cabi: 439.291 seconds from a cold build/cache directory)llgo (devel) windows/arm64msys-2.0.dll,cygwin1.dll, orlibwinpthreaddependency in the standard-library smoke or itslibuv-1.dlldependencyGitHub CI and Codecov results will be recorded after the PR is opened.