Skip to content

fix bounds panic messages#1901

Draft
cpunion wants to merge 4 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-boundsmsg-coverage
Draft

fix bounds panic messages#1901
cpunion wants to merge 4 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-boundsmsg-coverage

Conversation

@cpunion
Copy link
Copy Markdown
Collaborator

@cpunion cpunion commented May 22, 2026

Summary

  • preserve failing bounds operands and signedness in generated bounds checks
  • route slice/string bounds failures through runtime boundsError messages with len/cap-specific codes
  • add stable test/go coverage for bounds panic text and un-xfail fixed GOROOT issue30116 cases

GOROOT note

Full GOROOT CI is slow/disabled locally, so I ran targeted fixedbugs cases only. Machine GOROOT: /opt/homebrew/Cellar/go@1.24/1.24.11/libexec (go1.24.11 darwin/arm64). Normal PR CI should run from the fork branch.

Test/go coverage

Added test/go/bounds_panic_text_test.go covering runtime.Error panic text for signed/unsigned index checks, slice/string/array slicing, len vs capacity wording, and full slice max bounds.

Targeted commands run

  • go test ./test/go -run TestBoundsPanicText -count=1 -v
  • go test ./test/go -run 'Test(BoundsPanicText|UnsignedIndexBoundsCheck|FullSliceBoundsPanicText)' -count=1 -v\n- go run ./cmd/llgo test -run TestBoundsPanicText -count=1 ./test/go\n- go run ./cmd/llgo test -run 'Test(BoundsPanicText|UnsignedIndexBoundsCheck|FullSliceBoundsPanicText)' -count=1 ./test/go\n- go test ./ssa -run 'TestFromTestdata/utf8|TestFromTestgo/cursor' -count=1\n- go test ./ssa -run Test -count=1\n- go test ./cl -run 'TestRunAndTestFrom(Testgo|Testrt|Testdata|Testlibc)' -count=1\n- go test ./test/goroot -run TestGoRootRunCases -count=1 -v -args -goroot "$(go env GOROOT)" -dirs fixedbugs -case '^fixedbugs/issue30116(u)?\.go$' -xfail /tmp/llgo-no-xfail.yaml -run-timeout 5m -build-timeout 3m\n- go test ./test/goroot -run TestGoRootRunCases -count=1 -v -args -goroot "$(go env GOROOT)" -dirs fixedbugs -case '^fixedbugs/issue30116(u)?\.go$' -run-timeout 5m -build-timeout 3m\n\n## Local note\nA direct go test ./runtime/internal/runtime -count=1 is not applicable in this module layout; that runtime path is validated through LLGo test/go, SSA/CL checks, and the targeted GOROOT run.

2026-05-24 issue15002 update

  • Classified fixedbugs/issue15002.go as bounds/index load ordering coverage: it protects the final byte of a readable page and expects the x[1] bounds panic before any widened byte load can cross into the protected page.
  • Added test/go/bounds_load_order_test.go with mmap/mprotect coverage for uint16/uint32/uint64 byte assembly using both constant and variable indexes.
  • Removed only the fixedbugs/issue15002.go linux/amd64 xfail now covered by test/go.
  • Local limitation: this machine is darwin/arm64 and Docker is unavailable, so the exact linux/amd64 goroot run is left to CI. I did run the focused goroot case through a temporary GOARCH=amd64 baseline wrapper plus native LLGo explicit-file execution, and the case passed locally.

Additional commands run:

  • go test ./test/go -run TestBoundsCheckBeforeWiderByteLoad -count=1 -v
  • go run ./cmd/llgo test -run TestBoundsCheckBeforeWiderByteLoad -count=1 ./test/go
  • go test ./test/go -count=1
  • go test ./test/goroot -run TestGoRootRunCases -count=1 -v -args -goroot "$(go env GOROOT)" -go /tmp/go-amd64-wrapper-llgo-bounds -llgo /tmp/llgo-bounds-issue15002 -dirs fixedbugs -case '^fixedbugs/issue15002.go$' -run-timeout=30s -build-timeout=3m
  • go test ./ssa -count=1
  • go test ./cl -count=1
  • go test ./runtime/internal/runtime -count=1 (not applicable: package is not in the main module)

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 22, 2026

PR #1901 follow-up for head b072c41.

Root cause: the failing test (ubuntu-latest, 19) job failed in cl at TestRunAndTestFromTestlibgo/mapzero. The runtime/compiler change now reports the precise Go-style bounds panic runtime error: index out of range [0] with length 0, but cl/_testlibgo/mapzero/expect.txt still expected the old shorter runtime error: index out of range.

Fix: updated the mapzero golden output to match the new bounds panic message. The functional bounds-message coverage remains in test/go/bounds_panic_text_test.go.

Local tests run:

  • go test ./test/go -run TestBoundsPanicText -count=1
  • go test ./cl -run 'TestRunAndTestFromTestlibgo/mapzero' -count=1\n- go test ./test/go -count=1\n- go test ./ssa -run 'Test.*' -count=1\n- go test ./cl -run TestRunAndTestFromTestlibgo -count=1\n\nCI status: new head checks have started and are currently pending/queued; I am monitoring run set for b072c41.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 24, 2026

Index0 follow-up for 525647b:

  • Classification: reproduced on xgo-dev/main with xfail disabled using go1.24.11 darwin/arm64. The failure is a GOROOT index0.go runoutput stderr mismatch: llgo panics with type assertion interface{} -> error failed while the upstream harness expects the recovered bounds panic to implement error and contain out of range. This is the same bounds/index panic value/message root as this PR.
  • Change: removed only the covered index0.go runoutput xfails for darwin/arm64 and linux/amd64.
  • Coverage: test/go/bounds_panic_text_test.go covers runtime.Error panic values and exact Go-compatible bounds/index/slice panic text; the focused GOROOT index0.go case now passes with the default xfail file.
  • Xfail scan: no remaining index0/bounds/index xfail entries found after this removal.
  • Not covered by this PR: GC/liveness/finalizer/goroutine and nil/chan/print/recover areas remain intentionally untouched. Direct go test ./runtime/... is not applicable in this module layout; runtime behavior is covered through test/go plus ssa/cl and focused GOROOT runs.

Local commands run:

  • go test ./test/goroot -run TestGoRootRunCases -count=1 -args -goroot "/opt/homebrew/Cellar/go@1.24/1.24.11/libexec" -dirs . -case '^index0.go$' -directive-mode ci -xfail test/goroot/no-such-xfail.yaml -progress=0
  • go test ./test/goroot -run TestGoRootRunCases -count=1 -args -goroot "/opt/homebrew/Cellar/go@1.24/1.24.11/libexec" -dirs . -case '^index0.go$' -directive-mode ci -progress=0
  • go test ./test/go -count=1
  • go test ./ssa ./cl -count=1
  • go test ./runtime/... -count=1 (not applicable: module does not contain package github.com/goplus/llgo/runtime)

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 24, 2026

Update for issue15002 bounds load ordering coverage:

  • Classified fixedbugs/issue15002.go as in-scope bounds/index load ordering coverage. The test guards a page boundary and expects the x[1] bounds panic before any widened byte load crosses into the protected page.
  • Added test/go/bounds_load_order_test.go covering uint16/uint32/uint64 byte assembly with constant and variable indexes.
  • Removed only the fixedbugs/issue15002.go linux/amd64 xfail.
  • Local limitation: exact linux/amd64 goroot execution is left to CI because this machine is darwin/arm64 and Docker is unavailable. Local focused goroot execution via GOARCH=amd64 baseline wrapper plus native LLGo explicit-file run passed.

Additional local results: go test ./test/go -count=1 PASS; go test ./ssa -count=1 PASS; go test ./cl -count=1 PASS; direct go test ./runtime/internal/runtime is not applicable in this module layout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants