Repository navigation
windows: qualify debuggers and host shells (R12; depends on #2463) - #2465
Merged
xushiwei merged 54 commits intoAug 31, 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
Reviewed the full diff (0b99651...5ce1444, ~2k lines) across four dimensions: code quality, performance, security, and documentation accuracy.
This is a large, well-structured PR adding Windows LLVM-MinGW cross-compilation, COFF PDB/CodeView debug-info handling, link-option parsing, and extensive CI/toolchain scripting. Strong points:
- Security: all new binary downloads pin a version and verify a SHA-256 before use; vcpkg is cloned unpinned but immediately detached to a hardcoded baseline SHA. Actions interpolate only trusted matrix values, not untrusted PR context. No
curl | sh, no injection surface introduced. - Performance: no hot-path regressions.
needDebugAddrshort-circuits before the one LLVMSizeOfcall; the per-C-filedebugInfoCompilerArgsre-parse is negligible against the clang subprocess it accompanies. - Documentation: new exported symbols (
Program.EnableCodeViewDebugInfo,Config.EmitCodeView) and renamed helpers (usesWindowsToolchainProfile,ResolveWindows) have accurate, updated doc comments. User-facing docs remain consistent. - Tests: the "last /debug wins", "-w overrides external PDB", CodeView-retains-DWARF, and 386 wide-integer debug-value cases are all covered by new unit tests.
Only minor, non-blocking readability suggestions below. Nothing blocks merge.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
1 task done
This was referenced Sep 30, 2026
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
2 participants
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.
Purpose
Advance R12 of the Windows support proposal by qualifying debugger metadata and native host-shell invocation across the supported Windows profiles and architectures.
Related proposal: #2325
Depends on:
Current scope
LLGO_LLDBbefore probingPATH, so a cross-architecture lane cannot silently select the host debugger;-wor default release builds;llvm-dwarfdump --verifymisdiagnoses while normal LLDB tests continue to exercise section GC;/debuglinker choice and emit CodeView file/line records for the companion PDB while retaining embedded DWARF;msys-2.0.dll,cygwin1.dll, and, for MSVC,libwinpthreaddependencies in shell-produced executables.The explicit-PDB path does not introduce a second Go type system in CodeView. The companion PDB provides discovery, public symbols, and source/line mappings; embedded DWARF remains the type authority. Default release builds and explicit
/debug:nonebuilds do not emit the extra metadata.Validation
rebased without conflicts onto R11 head
14aedcde5;range-diffconfirms all 25 functional R12 commits are unchanged apart from their rewritten parents;the host-shell fix preserves architecture-selected compilers for non-amd64 targets and passes native Windows output/inspection paths explicitly from Cygwin, which unlike MSYS2 does not translate native-program arguments; no speculative DLL-path override remains;
go test -count=1 ./test/go ./ssa ./internal/build ./internal/debuginfopasses on macOS ARM64 (internal/buildcompleted in 446s);Windows/386 MSVC with official Win32 LLDB 19.1.7: 224/224 variable assertions and 3/3 mixed Go/C callback/fault-stack assertions pass, including all auxiliary marker/plugin checks;
Windows/386 MinGW with official Win32 LLDB 19.1.7: the same 224/224, 3/3, and auxiliary checks pass;
the MinGW/386 suite also passes with the host
lldbfirst onPATHand the target-native debugger selected only throughLLGO_LLDB;Windows/ARM64 MinGW with native Go 1.27 and CLANGARM64 22.1.8: the complete
test/gopackage passes withLLGO_TEST_JOBS=1;Windows/amd64 MSVC
TestStandardDWARFpasses at O0, O1, O2, O3, Os, and Oz;Python bytecode validation, YAML parsing,
actionlint, andgit diff --checkpass;the complete cpunion CI matrix passed on functional head
5ce14445eafter retrying one transient Go 1.20 goroot hang: https://github.com/cpunion/llgo/actions/runs/33392286531;same-runner benchmark comparison reports 0 B file-size and text-size changes for every Linux, macOS, Windows MinGW/MSVC, amd64/386/ARM64, LTO, and non-LTO workload: windows: qualify debuggers and host shells (R12; depends on xgo-dev/llgo#2463) cpunion/llgo#216 (comment).
final review-only head
9f0a5af09passesgo test -count=1 ./ssa ./internal/build; the complete upstream matrix passed after retrying one transient macOS test-process exit and one transient Windows module download: https://github.com/xgo-dev/llgo/actions/runs/33402640338.