proposal: x/debug: make the core and gocore packages public
#57447
Comments
|
This would be an incredible help to the ecosystem! I created some basic |
|
Change https://go.dev/cl/468916 mentions this issue: |
|
I think the main thing missing here is actually what the final API itself will look like. After an offline discussion today with @aclements and @prattmic, I think we've decided the right next step is to fork these x/debug-internal libraries into x/exp where we can iterate on them freely. The API is far too large to try to summarize for a proposal and have everyone comment on it. Meanwhile, x/exp will allow us to put it out there in a way that will allow for experimentation and trial. The reason we chose x/exp over x/debug is because x/exp has clear expectations with respect to backwards compatibility (i.e. there is none). Meanwhile, x/debug contains only one exported package with zero imports in the Go ecosystem and a handful in the main Go repository (for testing). Although x/debug has a disclaimer on it with respect to compatibility, it is less clearly experimental in its import path and long-term we do want it to maintain backwards compatibility, at least at compile time. By experimenting directly in x/debug, we risk tarnishing its reputation, so-to-speak. To keep viewcore working as a tool, I think our plan is to temporarily have x/debug import x/exp, so it can take advantage of improvements we make. Frankly, I don't expect the API to change all that much, but simultaneously it's hard to get a sense of the API in totality without first using it and iterating on it. |
|
I updated the proposal to reflect these changes, and also updated the proposal's compatibility story based on our current thinking. |
|
Change https://go.dev/cl/474541 mentions this issue: |
|
Change https://go.dev/cl/474543 mentions this issue: |
|
Change https://go.dev/cl/474542 mentions this issue: |
|
Change https://go.dev/cl/506558 mentions this issue: |
|
Change https://go.dev/cl/506757 mentions this issue: |
Loading a new core.Process in core.Core is a rather long, complex process. Currently, this function creates a stub Process with very few fields set, and then subsequent method calls initially the remaining fields. The problem with this approach is that the inputs/requirements and outputs of these initialization methods is poorly documented. These methods (e.g., readCore) usually have some prerequisite fields in Process that must be set on entry, and some fields that they set prior to return. Neither of these are well documented, and many are far from obvious. For example, Process.mainExecName is initialized in readCore -> readNote -> readNTFile -> openMappedFile. This is made more of a mess by the fact that Process tries to do most initialization with a single pass, resulting in fields getting initialized in unintuitive locations (such as mainExecName). These combine to make refactoring to support new functionality difficult, as changing ordering breaks implicit dependencies between methods. This CL addresses these issues by eliminating the iterative initialization of Process. Instead, Process is only instantiated once all fields are ready. During loading, all work is done by free functions/types, where inputs and outputs are explicit via function arguments/returns. This may inadvertently improve golang/go#44757, as it makes main binary lookup more explicit. This CL intends to keep behavior the same, but there are a few changes: * If the entry point is unknown, We no longer apply the heuristic of assuming that first executable mapping is the main binary. This could be added back, but it is probably better to ensure that the entry point is available on all architectures. * Failure to ELF parse a file for symbols isn't a hard error (as the mapped file might not even be an ELF). For golang/go#57447. Change-Id: I7c3712ccb99ceddddc6adbae57d477c25ff4e5ba Reviewed-on: https://go-review.googlesource.com/c/debug/+/506558 Reviewed-by: Michael Knyszek <[email protected]> Run-TryBot: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
|
@mknyszek @jordanlewis is there any update now about viewcore in golang.org/x/debug or golang.org/x/exp, please? do we plan to add one tool to parse the dump file from debug.WriteHeapDump()？ |
|
Hey @superajun-wsj, unfortunately the focus on diagnostics lately has been on execution tracing. We still plan to come back to this in the near future and there's been a little bit of progress in June by @prattmic, but this is still a little ways off. Thanks for your interest, and stay tuned. |
|
Change https://go.dev/cl/547536 mentions this issue: |
|
Hi, thought you might like to know I got It's somewhat empirical, definitely not complete, and I haven't tried to make it compatible with multiple versions, but my branch it's at golang/debug@master...bboreham:go-debug:go-1-20 |
In the hchan struct, buf field is an unsefa.Pointer, and its actual type is an array of length hchan.dataqsiz and type hchan.elemtype. Correct its type so that the object it points to can continue to be analyzed. For golang/go#57447. Change-Id: I47969a9a8e2870bb9573587bac5416fbea9db9ac Reviewed-on: https://go-review.googlesource.com/c/debug/+/547536 Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
|
I'm running into this issue with the latest version of go-debug and a dump generated by go 1.21: #33661 |
|
Change https://go.dev/cl/589035 mentions this issue: |
|
Hi, @mknyszek , I've been trying to use viewcore recently, and I've found some strange errors. I'm also trying to fix some of them. |
|
@WangLeonard The current problem with viewcore is that the packages that back the implementation (mainly |
CL 354011 remove unnecessary funcdata alignment. Update references. For golang/go#57447. Change-Id: I14414ea4be048c3bb8bbde711c10474f7f0515e4 Reviewed-on: https://go-review.googlesource.com/c/debug/+/589035 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]>
|
Change https://go.dev/cl/593680 mentions this issue: |
|
Change https://go.dev/cl/593681 mentions this issue: |
|
Hi all, I recently implemented a heap reference analysis tool for dwarf type retrieval based on delve. It might be relevant to the problem mentioned in this issue. Welcome to discuss and exchange ideas with us. |
|
Change https://go.dev/cl/600796 mentions this issue: |
|
There are feature requests on vulncheck (cc @zpavlinovic) is particularly interested in this work, as they have custom forks of |
Use AttrGoKind instead of the name matching to check the slice and string type. For golang/go#57447. Change-Id: I8765aa1a6315609b3476b8b84c27130629847235 Reviewed-on: https://go-review.googlesource.com/c/debug/+/593680 Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Tim King <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]>
Authors: @mknyszek @prattmic @aclements
Purpose
The purpose of this proposal is to help fill in an important gap in Go debugging by providing a foundation for Go heap analysis. The primary use-cases for heap analysis (vs. heap profiles) are to pinpoint memory leak issues and debug out-of-memory errors by providing an in-depth look at the memory graph. Other use-cases exist as well, mostly related to the large amount of detail available in typical heap analysis software.
Go currently has two heap analysis options: analyzing core files for Go processes (via
golang.org/x/debug/cmd/viewcore) or analyzing heap dumps (produced byruntime/debug.WriteHeapDump). As of today, both of these options are not easily available to users. The former has bit-rotted, while the latter lacks working tooling (and so may not actually work either).Some companies using Go have expressed interest in building their own heap analysis tooling, while members of the Go community have contributed time and effort to updating
viewcore. The utility of such functionality is lauded in the Java world, especially when it's available for servers over a connection (e.g.net/http/pprof).Plan
We propose the following plan for improving the state of Go heap analysis, with the core API change here being to make the
golang.org/x/debug/internal/coreandgolang.org/x/debug/internal/gocorepackages public.Before that can happen, however, we also need to fix them and prevent them from breaking again by:
gocoreandcoreto work with Go at tip.gocoreandcoreare well-tested, and tested at tip.After that, I propose making
gocoreandcoreexternally-visible APIs, allowing Go developers to build on top of it with some fairly minor changes to the API that I would like to hash out. To identify the full scope of the API changes and get feedback on the design, we plan to first fork the API intox/expbefore moving them back tox/debug. To letviewcoretake advantage of improvements inx/exp, and as a data point in trialing the API,x/debugwill temporarily depend onx/expin the process.Once the API moves to its final home in
x/debug, I would like to lastly proposegolang.org/x/debugbuilder to run against the main Go repository.in order to identify future breakage of these APIs as early as possible in the development of the Go toolchain.
These small steps would bring back heap analysis to the Go ecosystem and will ensure it continues to work.
Since one goal of this package is to support third-party tooling, we need a compatibility story. Simultaneously, these packages rely on and poke at Go internals that are subject to change with each release. Exposing these internals to the Go 1 compatibility promise would likely be too large of maintenance burden to carry forever across all Go versions. As a compromise, I would like to propose the following policy, which will be posted in the package's documentation:
x/debugversions will never break code at compile-time.x/debugversion, it will be reported by the gocore package.viewcore, though minimalistic, provides a proof-of-concept for thegocorelibrary that we hope will motivate Go developers to build fancier tooling for heap analysis. Documentation will be critical to making this tooling more widely known ("how to debug a memory leak withviewcore").Potential future work
I want to also mention some ideas for future work to gauge interest, even though they're not relevant to the proposal directly.
runtime/debug.WriteHeapDumpto reduce eliminate some technical debt.net/http(perhaps viaWriteHeapDump).The goal of (1) and (2) is to replace WriteHeapDump with a way to produce a process snapshot that works with existing tooling (i.e. gocore and viewcore). Constructing our own core file isn't terribly difficult, provides us with a stable binary format specification so we don't have to go through the trouble of defining our own, and is a format already automatically generated by Linux, so we have fewer formats to support.
The goal of (3) would be to improve the debugging experience with core dumps from stripped binaries. This might not need any additional API, but should be considered up-front.
The text was updated successfully, but these errors were encountered: