Repository navigation
fix: handle failures exposed by full GOROOT coverage - #2741
Merged
cpunion merged 5 commits intoOct 6, 2026
Merged
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review summary
The core fix in ssa/di.go is sound and well-reasoned: for empty/zero-sized aggregates it backs the debug-only address with a byte and stores a zero while keeping the true source type in the variable metadata, which correctly avoids the LLVM 22 SelectionDAGBuilder::resolveDanglingDebugInfo crash. The new cl/debug_zero_size_test.go exercises real object emission across targets and opt levels — the right call, since module verification alone doesn't reproduce the crash. The test/goroot YAML and unit-test additions are internally consistent (selectors, timeouts, and reason strings all match).
Verified across quality, performance, security, and documentation passes:
- Performance: the added
SizeOfis O(1) and off the hot path (debug builds only, once per variable); the 12-iteration test is test-only and reuses the SSA build. - Security: no type-confusion / over-read risk — a byte-sized alloca is larger than the zero-sized declared element, so every downstream access touches 0 bytes within the allocation.
- Documentation: comments and YAML reason strings accurately describe the code.
One minor, non-blocking note left inline. No correctness regressions found.
| entryBuilder := *b | ||
| entryBuilder.impl = b.Func.entryAllocaBuilder() | ||
| dbgPtr = entryBuilder.AllocaT(ty) | ||
| store = b.Store(dbgPtr, storedValue) |
Contributor
There was a problem hiding this comment.
Minor / non-blocking: the Store is now intentionally emitted while dbgPtr.Type is still Pointer(ty) (the alloca'd type), before it's reset to Pointer(v.Type). For most types where ty != v.Type (slice, string, named) this is benign — the same store is emitted. For signature/closure values it's a real emitted-IR change: Store now takes the closure branch and writes a full {funcptr, nil} aggregate, whereas the old order stored only the function pointer and left the $data slot as uninitialized stack garbage. This is actually an improvement (debug-only), but the surrounding comment only explains the zero-sized rationale. Consider a one-line note that the Store deliberately runs under the alloca type ty so closure-typed debug values are fully initialized.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
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
1 participant
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.
Fix failures exposed by restoring full daily coverage in llgo-compat/ci#37.
abi/f_ret_z_not.gorangegen.go, Windows 64-bitfixedbugs/issue39541.go, Windows amd64reflect.MakeFunccalls exceeded the minute limit under CI load.psquery blocked process completion and deadline handling.fixedbugs/issue75764.goValidation: the ARM64 regression crashes before the change and passes after it for Darwin, Linux, Windows MSVC and MinGW at O0/O2/Os (12 object-emission checks). The original Go 1.27 GOROOT case, existing SSA/CL debug tests, all GOROOT runner unit tests, resource/deadline regression tests with the race detector, and
git diff --checkpass.Targeted CI passed 6/6 jobs, covering the four cases against Go 1.26/1.27 on macOS ARM64, Windows MSVC amd64/386, and MinGW amd64/386/ARM64 (commit
1b0a2e0). The 2 GiB heap trial passed MinGW ARM64 but still reached the RSS guard on MSVC ARM64/Go 1.26. Final MSVC/MinGW ARM64 validation passed 2/2 jobs with the 1 GiB heap target and 4 GiB RSS guard retained (commit5c4020c). Both Go versions pass; peak LLGo RSS is 3116 MiB on MSVC and 2825 MiB on MinGW, with builds taking 3m29s–3m37s. CI #37’s actual case-selection preflight correctly serializes both versions.