Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

proposal: debug/gosym: expose function start line and inline tree #58474

Open
brancz opened this issue Feb 11, 2023 · 15 comments · May be fixed by #58864
Open

proposal: debug/gosym: expose function start line and inline tree #58474

brancz opened this issue Feb 11, 2023 · 15 comments · May be fixed by #58864
Labels
Proposal
Milestone
Proposal

Comments

@brancz
Copy link
Contributor

brancz commented Feb 11, 2023

Expose function start line through an API in the debug/gosym package.

The .gopclntab section includes data for the starting line number of a function, however, the debug/gosym package does not export an API that allows reading this data. I propose adding it to the API surface.

Use case: I happen to work on the Parca open-source continuous-profiling project and we quite frequently run into Go binaries, that don't include DWARF debuginfos (which we discourage but it's out of our control) but of course keep the .gopclntab, so we would like to be able to support those binaries as best as possible. The function starting line number in particular is important for the new PGO features, as noted in the documentation appendix.
@seankhliao
Copy link
Member

What's the exact api that would be added for this?

@seankhliao seankhliao changed the title debug/gosym: Expose function start line through API debug/gosym: expose function start line Feb 11, 2023
@brancz
Copy link
Contributor Author

brancz commented Feb 11, 2023

Adding a StartLine field to the Func struct, and load it around here in the go12Funcs() function by adding a new function to funcData struct, called startLine, which like cuOffset would load the startLine (just field 9 instead of field 8). Doing all of this would make it be loaded when instantiating the table in NewTable.

This would only work for binaries that are Go >=1.3 as far as I understand, which would be fine for my case, since I'm targeting >=1.20 binaries in my use case.

@ianlancetaylor ianlancetaylor changed the title debug/gosym: expose function start line proposal: debug/gosym: expose function start line Feb 12, 2023
@gopherbot gopherbot added this to the Proposal milestone Feb 12, 2023
@ianlancetaylor
Copy link
Contributor

CC @golang/runtime

@prattmic
Copy link
Member

For PGO, profiles need to contain inlined frames as well. Thus, I imagine that you want to add access to the inline tree to debug/gosym as well?

(This came up recently in another context that wants inlining information and we were considering a proposal to add it to debug/gosym, or perhaps an x/debug package).

@brancz
Copy link
Contributor Author

brancz commented Feb 14, 2023

I was going to open a separate proposal for that, but yes!

@cherrymui
Copy link
Member

This would only work for binaries that are Go >=1.3 as far as I understand, which would be fine for my case, since I'm targeting >=1.20 binaries in my use case.

The start line in the pclntab is added last cycle (Go 1.20, specifically for PGO). So this would only work with Go 1.20+ binaries. For older binaries maybe fall back to 0?

@brancz
Copy link
Contributor Author

brancz commented Feb 18, 2023

The start line in the pclntab is added last cycle (Go 1.20, specifically for PGO). So this would only work with Go 1.20+ binaries. For older binaries maybe fall back to 0?

I think that would make sense.

brancz added a commit to polarsignals/go that referenced this issue Mar 4, 2023
@brancz
debug/gosym: read start line of a function from gopclntab
7f5f178 
This field was introduced in go 1.20 therefore we need to safeguard it
to only be read for pclntabs of that version or higher, as otherwise
nonsensical bytes would be read. The fallback is now the default value
of 0.

Fixes golang#58474
@brancz brancz linked a pull request Mar 4, 2023 that will close this issue
Open
@gopherbot
Copy link

Change https://go.dev/cl/473455 mentions this issue: debug/gosym: read start line of a function from gopclntab

@prattmic
Copy link
Member

prattmic commented Mar 8, 2023

@aclements, @mknyszek, and I are thinking of tying this in with #57447 (comment). i.e., instead of continuing to extend debug/gosym's strained API (particularly difficult w.r.t. inlined functions), new packages initially in x/exp/debug, and eventually in x/debug will provide access to a more complete set of data.

@brancz
Copy link
Contributor Author

brancz commented Mar 8, 2023

I’m ok with forking debug/gosym for the moment (that’s what I’m doing right now anyway), while the new package API is being worked, out if that’s the stance of the project. I just thought I’d attempt to upstream this since already had it.

@gopherbot
Copy link

Change https://go.dev/cl/474541 mentions this issue: debug: add module

@gopherbot
Copy link

Change https://go.dev/cl/474543 mentions this issue: debug/gobinary: add initial package sketch

@gopherbot
Copy link

Change https://go.dev/cl/474542 mentions this issue: debug/gobinary: add package

@prattmic
Copy link
Member

Just as a point of reference, Go 1.21 adds start line to DWARF (#57308), so this data is now available, just not from debug/gosym.

(We should still expose the gosym version. I'm just leaving this breadcrumb in case anyone is blocked on this.)

@prattmic prattmic changed the title proposal: debug/gosym: expose function start line proposal: debug/gosym: expose function start line and inline tree Oct 30, 2023
@prattmic
Copy link
Member

x/vuln has a forked version of debug/gosym, which it uses to extract data from the inline tree: https://go.googlesource.com/vuln/+/refs/heads/master/internal/vulncheck/internal/buildinfo/additions_scan.go#49

One pain point that came up there is that the references to funcdata (where the inline tree lines) in the pclntab section are stored as offsets from the go:func.* symbol. If the binary is stripped, we can't find the symbol and thus can't get the inline tree.

One possible resolution here is to change go:func.* from just a symbol to a full section (like .gopclntab), as section headers are generally not removed by stripping.

(cc @zpavlinovic)

@adonovan adonovan mentioned this issue Feb 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Incoming
Milestone
Proposal
Development

Successfully merging a pull request may close this issue.

debug/gosym: read start line of a function from gopclntab polarsignals/go
6 participants
@prattmic @ianlancetaylor @brancz @gopherbot @seankhliao @cherrymui