Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
e92e918 to
b55d1a1
Compare
6803f47 to
519f0f0
Compare
There was a problem hiding this comment.
Review summary
Large PR (~175k insertions across 518 files). The bulk is generated discovery testdata (testdata/discovery/ledger/**) and highly repetitive AMD64/ARM instruction-lowering tables, both of which I skimmed rather than reviewed line-by-line. I focused on the high-signal changes: the new discovery/scan tooling (cmd/plan9asmcorpus/discovery.go, cmd/plan9asmdiscover, cmd/plan9asmll, cmd/plan9asmscan), internal/discoverymeta, and the core translate/parser changes.
Overall this is solid, defensively-written code: file handles are consistently closed on error paths, zip extraction blocks zip-slip (proxy-prefix + path.Clean + filepath.Rel + O_EXCL), external commands use fixed argv with escaped module paths, and remote fetches are size-bounded. Doc/AGENTS.md claims (flags, scripts, committed manifest counts, test names) were verified accurate.
One inline correctness/consistency finding below, plus a few non-blocking notes.
Non-blocking notes:
-
Exponential build-tag search —
cmd/plan9asmcorpus/discovery.go(findDiscoveryBuildTags). The search iterates1 << len(customTags)masks, each doing filesystem-backedctx.MatchFilecalls, and the outerwantedCountloop re-walks the mask space ~len(customTags)+1times. With the 16-tag cap this is bounded but can reach millions ofMatchFilecalls for a package near the limit. Usually fine (tags are typically 0–2); consider iterating masks once (prioritizing by popcount) if large-tag modules become common. -
Symlink escape in local include expansion —
cmd/plan9asmll/main.go(pathWithinRoot/expandAsmIncludes). Containment is checked lexically viafilepath.Rel; a#includetarget that is a symlink pointing outside the source root would pass the check and be read (nofilepath.EvalSymlinks). Blast radius is contained (read-only, inlined into a discarded IR artifact, ephemeral CI against public modules; the zip extractor already rejects non-regular files), so this is defense-in-depth only. Resolving symlinks before the containment check would close the gap. -
Readability —
cmd/plan9asmll/main.go:988(and:1023).op == "JMP" || op == "B" || op == "RET" && len(ins.Args) == 1relies on&&binding tighter than||. The current behavior is correct, but explicit parentheses around(op == "RET" && len(ins.Args) == 1)would guard against a future||term silently changing the grouping.
68e5f23 to
11d9c12
Compare
# Conflicts: # cross_runtime_test.go
# Conflicts: # docs/development/current-work.md
Summary
This draft expands typed Go assembler coverage across 386, amd64, ARM, ARM64, and wasm, and imports an independently discovered third-party assembly corpus. LLVM 22 is required; missing tools, translation failures, object-compilation failures, incomplete reports, and resource failures remain hard failures. The PR stays Draft until current-head external verification, CI, review, and coverage are green.
The rebased source also adds ARM64 signed-lane
SMOV/SMOVWand BF16BFDOTlowering, and fixes scalarSQSHLUcode generation so LLVM 22 does not exhaust memory on one-lane intrinsic legalization. No fallback to another LLVM version is used.Committed discovery funnel
The repository stores one bidirectional manifest and 256 module-hashed, semver-sorted JSONL record files. Negative scans remain recorded; retry failures are not marked complete. The separate assembly ledger records
pending,passed,failed, and evidence-backednot_applicablestates per matched exact version. cgo results stay outside this repository.The committed history cursor is
2026-03-18T00:17:13.411942Z(history_before), and the incremental cursor is2026-09-22T22:43:08Z(incremental_since). The manifest derives these endpoints from the earliest/latest recorded ranges and validates continuity. Historical backfill is still incomplete.Thus the pushed ledger snapshot does not claim that all discovered assembly compiles. Current CI reports are audited separately below without changing or interrupting the running source.
The last separately recorded remote-scanner snapshot (not refreshed during this CI repair) reports
latest_records=729,533,cgo_records=791,846,cgo_modules=16,127,cgo_failures=1,729, andcgo_pending=6,228. These values are operational progress only and are not mixed into the repository ledger.The generic scanner independently contains the reported issue libraries, including
github.com/coder/websocket@v1.8.15(llgo#2464),github.com/klauspost/compress@v1.20.0(llgo#2552),github.com/tmthrgd/go-hex@v0.0.0-20190904060850-447a3041c3bc(llgo#2576), andgithub.com/tailscale/wireguard-go@v0.0.0-20260911194433-e3222a3340cd. Their presence in the ledger is discovery evidence, not a current-head pass result.Official Go assembler coverage (Go 1.27.1)
The observed-form gate is green for all five architectures. The arm64 check also confirms all 98 toolchain-available required opcode families are encoder-defined, observed, and lowerable. Context-only forms require their source/function environment; they are not standalone lowering or runtime claims.
Verification
b93666f5includesmain@7cc8c0fand remains Draft. The contribution is pushed only to thecpunionfork. Current-head CI started on 2026-09-27 at 10:00 UTC. The latest checkpoint has 73 successful jobs, eight failures, six running and five queued. Let this run finish once: no push, cancellation or rerun while it is active. Verified development is being committed locally meanwhile.0d96d26: the last pre-push snapshot had 63 successful jobs and one failure. All 11 earlier failed jobs passed, including all 10 standard-library lanes and Windows. The Linux/Windows root suites, race, required cross-runtime, benchmark and coverage jobs passed. External shards 2, 6 and 7 passed; shard 3 exposed a single-character stack-slot parsing bug ingopkg.in/agiledragon/gomonkey.v2@v2.14.3, now fixed in the pushed head. These historical results are not current-head completion.a7e98ccaccepts Go-valid single-character stack annotations such asn-8(SP). Retained red/green tests cover identifier/offset forms; both affected-library assembly files now translate and compile. The full root suite passed in 631 seconds, both CLI suites passed, and Go 1.20/1.27 focused checks, three-OS LLVM 22 objects, Darwin runtime and Linux/QEMU runtime pass.7c07c82b), scalable count/address NZCV effects (0186a071) and aligned ORR/EOR relations (c433e78b) have focused full pool regressions, Go 1.20, three-OS LLVM 22 compilation and required Linux/QEMU evidence; scalar fixtures also execute on Darwin. The scalable fixture checks 132 cases at all 16 architectural vector lengths. At frozen2265cb14, the full root suite and all root subpackages pass (1,059.348 seconds); both CLI full suites pass too. The ARM64 standard-library matrix compiles 950 IR files across 21 configurations. Complete shards 15/17 now both pass. The full required Linux/QEMU runtime selection also passes in 636.849 seconds. Latest strict benchmark: 184/184 files, zero N/A, 23 target-seconds plus two build-seconds. Local verification does not imply current CI completion.46ca52a7: 218 passed versions, 56 source N/A, 4,509 pending, zero failures and 5,154 successful target translations. Both canonical and mirroredsimd@v1.21.1pass all 120 applicable files on six targets (360 translations each, zero N/A). Overallcomplete/verifiedremain false because only 2/32 shards are included.fe8f8e41,ffa77183,72789337) prevents mixed network/resource/checksum failures from becoming source N/A, rejects such N/A evidence before ledger publication, and adds bounded retries for ordinary HTTPEOF. Real proxy-disconnection tests reproduce the old one-attempt failure and verify recovery or failure after at most three attempts. Production checksum verification remains enabled. The full corpus suite, race, Go 1.20, vet and Windows test-object compilation pass; all five diagnostic functions have 100% statement coverage. A fresh complete shard 21 replay and full root regression are running. This changed source has a newly bound ledger; the SIMD checkpoint and old CI evidence are not relabeled as current-source passes.Current CI artifacts are validated at identical
b93666f5source content in a separate evidence worktree (local evidence commit1965527d). Thirteen completed shards pass (1/2/3/4/6/7/8/10/12/13/16/18/19), and eight fail (0/5/9/11/14/15/17/21). All indices are zero-based. These are translation plus LLVM object-compilation results, not execution of every external library's tests:b93666f5CI assembly evidenceThe 4,783-version accounting is complete as a progress snapshot, but both
completeandverifiedremain false.The nine current-CI failed candidates remain failures in this exact-source ledger: GoJIT searches raw function bytes for an embedded marker and enters native code mid-function; GopherJRE takes an out-of-directive-group code address and exchanges a custom register/stack contract with generated code; Sharkie switches native stacks and fabricates a code-relative return address; both case-distinct ContainerFS paths encounter 503/429 responses alongside obsolete dependency 404s after retries; both SIMD paths fail the same SVE byte-parsing file after 46/47 Darwin/ARM64 files compile; shard 21 also loses the exact Klaytn and lagpixellol/xray-core downloads to plain HTTP EOF at the checksum-proxy endpoint. The local unpushed fix now compiles the entire SVE byte file on Linux, Darwin and Windows, and its actual SVE2 parsing/formatting functions pass 3,120 runtime scenarios (130 parse and 65 format cases at each of 16 VLs) under Linux/QEMU. Both complete SIMD shards now pass locally as reported above; the newer proxy repair's shard replay is still running. Old CI evidence is not promoted to a new-source pass. Neither native-layout dependence nor infrastructure errors are silently marked passed or source N/A. Earlier
0d96d26evidence remains preserved separately atb434d36e, never mixed into the current-source accounting.