Skip to content

runtime: preserve FuncForPC names for function values#1924

Closed
cpunion wants to merge 3 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-fixedbugs-funcforpc-names
Closed

runtime: preserve FuncForPC names for function values#1924
cpunion wants to merge 3 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-fixedbugs-funcforpc-names

Conversation

@cpunion

@cpunion cpunion commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add runtime FuncForPC name lookup backed by debug symbol info
  • keep command-line function-value wrappers distinct for FuncForPC so folded functions still report separate names
  • cover the issue58300 pattern in test/go and remove issue58300/issue58300b goroot xfails

Tests

  • go test ./test/go -run TestRuntimeFuncForPCNamesWithLLGo -count=1
  • go test ./test/go -count=1
  • go test ./test/goroot -count=1 -run TestGoRootRunCases -args -goroot $(go env GOROOT) -dirs fixedbugs -case '^fixedbugs/issue58300(b)?.go$' -directive-mode=ci -xfail /tmp/llgo-no-xfail.yaml -run-timeout=2m
  • go test ./test/goroot -count=1 -run TestGoRootRunCases -args -goroot $(go env GOROOT) -dirs fixedbugs -case '^fixedbugs/issue58300(b)?.go$' -directive-mode=ci -run-timeout=2m
  • go test ./ssa -count=1
  • go test ./cl -run 'TestRunAndTestFromTestgo|TestRunAndTestFromTestrt|TestRunAndTestFromTestdata' -count=1
  • git diff --check

@cpunion cpunion force-pushed the codex/goroot-fixedbugs-funcforpc-names branch from 3b9293f to 97d74a8 Compare May 24, 2026 14:32
@codecov-commenter

codecov-commenter commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cpunion

cpunion commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator Author

Added package-level coverage for the FuncForPC/function-value interface paths that Codecov flagged.

Changes:

  • cl: compile-level fixture now asserts reflect.ValueOf(function).Pointer() uses a __llgo_funcpc_stub.* wrapper, with direct helper coverage for reflect, command-line-arguments, ordinary function-to-any, and non-function conversions.
  • ssa: MakeInterfaceFuncPC is covered by checking that it creates a FuncForPC-preserving wrapper distinct from the normal closure wrapper path.

Local verification:

  • go test ./cl -run 'TestCompileFuncForPCInterfaceUsesFuncPCWrapper|TestMakeInterfaceNeedsFuncPC' -coverprofile=/tmp/llgo-cl-funcpc.cover
  • go tool cover -func=/tmp/llgo-cl-funcpc.cover shows makeInterfaceNeedsFuncPC at 100%.
  • go test ./ssa -run 'TestMakeInterfaceKinds|TestClosureWrapCache' -coverprofile=/tmp/llgo-ssa-funcpc.cover
  • go tool cover -func=/tmp/llgo-ssa-funcpc.cover shows MakeInterfaceFuncPC at 100%.
  • go test ./test/go -run TestRuntimeFuncForPCNamesWithLLGo -count=1
  • go test ./test/goroot -run TestGoRootRunCases -count=1 -goroot "$(go env GOROOT)" -dirs fixedbugs -case 'fixedbugs/issue58300(b)?\\.go' -progress 30s
  • go test ./cl ./ssa -count=1
  • git diff --check

Pushed commit: 6aab2c8. CI/Codecov are pending on the new head.

@cpunion

cpunion commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed the refreshed-head CI build failure.

Changes:

  • Merged latest xgo-dev/main into codex/goroot-fixedbugs-funcforpc-names.
  • Updated the new ssa/abitype.go closureWrapDecl call to pass keepFuncPC=false, matching the normal ABI method wrapper path.
  • Removed the duplicate empty runtime.Func.FileLine stub left after the merge, keeping main's clitedebug implementation.

Validation:

  • go test ./ssa -run 'TestMakeInterfaceKinds|TestClosureWrapCache|TestMakeInterfaceFuncPC' -count=1\n- go test ./cl -run 'TestCompileFuncForPCInterfaceUsesFuncPCWrapper|TestMakeInterfaceNeedsFuncPC' -count=1\n- go test ./test/go -run TestRuntimeFuncForPCNamesWithLLGo -count=1\n- git diff --check\n\nCommit: a91f4c8d6f1495b4146227c0c3cda69ddc36068f

@cpunion cpunion force-pushed the codex/goroot-fixedbugs-funcforpc-names branch from a91f4c8 to 8871031 Compare June 6, 2026 09:11
@cpunion cpunion marked this pull request as ready for review June 28, 2026 06:07
@cpunion cpunion marked this pull request as draft June 28, 2026 14:44
cpunion added a commit to cpunion/llgo that referenced this pull request Jul 4, 2026
…root xfails

Three conformance residuals left by the earlier shadow-stack-era PRs
(xgo-dev#1903, xgo-dev#1924), reimplemented on the unwinder base:

- //line directive filenames follow gc's spelling: relative directives
  stay relative (the package loader expands them to absolute paths),
  empty directives report "??" — and an empty filename must still emit
  its statement anchor or attribution falls through to a neighboring
  record (fixedbugs/issue18149, issue22662).
- Linking uses --icf=none: Go semantics require distinct functions to
  keep distinct pcs (FuncForPC names, function-value identity —
  fixedbugs/issue58300 printed main.f for both f and g). lld's safe mode
  folded llgo-emitted same-body functions anyway; gc never folds.
  Hello-world cost: +1.6%.
- Remove 12 goroot xfails now passing: the two //line cases, issue58300,
  and nine that the stage5+acceptance line fixed outright (inline_caller,
  inline_callers, inline_literal, issue7690, issue17381, issue21879,
  issue22083, issue29735, issue58300b).

Still xfailed, tracked for the panic-snapshot follow-up: bug347, bug348,
issue14646, issue27201, issue29504, issue33724, issue4562, issue5856
(deferred runtime.Caller during panic unwinding); heapsampling
(memprofile attribution).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cpunion added a commit to cpunion/llgo that referenced this pull request Jul 4, 2026
…root xfails

Three conformance residuals left by the earlier shadow-stack-era PRs
(xgo-dev#1903, xgo-dev#1924), reimplemented on the unwinder base:

- //line directive filenames follow gc's spelling: relative directives
  stay relative (the package loader expands them to absolute paths),
  empty directives report "??" — and an empty filename must still emit
  its statement anchor or attribution falls through to a neighboring
  record (fixedbugs/issue18149, issue22662).
- Linking uses --icf=none: Go semantics require distinct functions to
  keep distinct pcs (FuncForPC names, function-value identity —
  fixedbugs/issue58300 printed main.f for both f and g). lld's safe mode
  folded llgo-emitted same-body functions anyway; gc never folds.
  Hello-world cost: +1.6%.
- Remove 12 goroot xfails now passing: the two //line cases, issue58300,
  and nine that the stage5+acceptance line fixed outright (inline_caller,
  inline_callers, inline_literal, issue7690, issue17381, issue21879,
  issue22083, issue29735, issue58300b).

Still xfailed, tracked for the panic-snapshot follow-up: bug347, bug348,
issue14646, issue27201, issue29504, issue33724, issue4562, issue5856
(deferred runtime.Caller during panic unwinding); heapsampling
(memprofile attribution).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cpunion

cpunion commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #2024 (FuncForPC naming is covered by pc-identity + funcinfo display-name normalization there).

@cpunion cpunion closed this Jul 6, 2026
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