Skip to content

fix nil receiver method dispatch checks#1885

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

fix nil receiver method dispatch checks#1885
cpunion wants to merge 4 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-method-coverage

Conversation

@cpunion
Copy link
Copy Markdown
Collaborator

@cpunion cpunion commented May 22, 2026

Summary

  • Emit explicit nil-deref guards for Go pointer indirections, while preserving the array-pointer range skip.
  • Implement ssa:wrapnilchk so value-receiver method wrappers panic on nil pointer receivers.
  • Add focused test/go coverage for promoted methods, method expressions/values, and interface calls on nil receivers.
  • Remove the fixed method.go and method5.go GOROOT xfails.

Testing

  • go test ./cl -run 'TestCompileSmallNilDerefGuards|TestCompileLargeNilDerefInterfaceGuards|TestIsLargeNonPointerValue' -count=1
  • go test ./test/go -run 'Test(Promoted|ValueReceiver|InterfaceCall)' -count=1
  • ./dev/llgo.sh test -run 'Test(Promoted|ValueReceiver|InterfaceCall)' ./test/go
  • go test ./test/goroot -count=1 -timeout 20m -run TestGoRoot -args -goroot "$(go env GOROOT)" -dirs . -case '^method\.go$' -xfail /tmp/llgo-no-xfail.yaml
  • go test ./test/goroot -count=1 -timeout 20m -run TestGoRoot -args -goroot "$(go env GOROOT)" -dirs . -case '^method5\.go$' -xfail /tmp/llgo-no-xfail.yaml
  • go test ./test/goroot -count=1 -timeout 20m -run TestGoRoot -args -goroot "$(go env GOROOT)" -dirs . -case '^method(5)?\.go$'

GOROOT CI is slow/disabled, so this PR keeps stable coverage in test/go and lists the exact targeted GOROOT commands above. I did not push any branch to xgo-dev/llgo; normal PR CI should run from the fork branch.

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 22, 2026

Updated this draft PR with commit 90947ed92164517dff3dab36c3ea98b3e5cdd942 pushed to cpunion/llgo:codex/goroot-method-coverage.

Root cause: the initial nil receiver method-dispatch change guarded ordinary pointer dereferences too broadly, which shifted existing FileCheck output, while the real missing behavior was value-receiver dispatch through nil pointer/promoted embedded pointer paths that must panic under Go semantics.

Fix: narrow the nil-deref guard to the method dispatch cases that need Go-compatible panic behavior and update the affected FileCheck expectations.

Local verification reported from the worktree:

  • go test ./cl ./ssa ./test/go -count=1

CI has been re-triggered on the new head; I will continue monitoring it.

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 22, 2026

Update pushed to cpunion:codex/goroot-method-coverage at 251419cb0360b1dc3cf16839669e821a91359549.

Root cause: the failed CI job was testing the GitHub PR merge commit, not just the branch head. Current xgo-dev/main has a newer cl/_testgo/genericembediface FileCheck test that was not present in the branch head. After this PR's nil method dispatch changes, the generated value-receiver wrappers now include runtime.AssertNilDeref, so the base test's old IR expectations failed in the merge commit.

Fix:

  • merged current xgo-dev/main into the PR branch so the base-only test is present on the branch without add/add merge risk;
  • updated cl/_testgo/genericembediface/in.go expectations for (*server).ServerReflectionInfo and (*stream).Context to include the nil deref checks;
  • kept behavior coverage in test/go/method_dispatch_test.go from the previous commit.

Local verification:

  • go test ./ssa -run 'TestFromTestgo/genericembediface' -count=1
  • go test ./cl -run 'TestRunAndTestFromTestgo/genericembediface' -count=1
  • go test ./test/go -count=1
  • go test ./cl ./ssa -count=1

CI status: new head checks are running. The replacement for the failed job is Go / test (ubuntu-latest, 19) at https://github.com/xgo-dev/llgo/actions/runs/26305192373/job/77440063804 and is currently in progress/queued. PR is still draft.

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 22, 2026

CI update for head 251419cb0360b1dc3cf16839669e821a91359549:

  • The replacement for the previous failure, Go / test (ubuntu-latest, 19), completed successfully: https://github.com/xgo-dev/llgo/actions/runs/26305192373/job/77440063804
  • Its Test step (go test -timeout 30m ./...) passed, so the merge-only FileCheck failure is resolved.
  • Current checks summary shows no failures; remaining macOS jobs are still pending/queued.

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.

1 participant