runtime: make it easier to connect runtime types to DWARF types #24814
Comments
|
The runtime type structures are the same as the reflect type structures, and the reflect package uses these strings for the Also I note that these strings are a surprisingly large percentage of a Go binary, so we want to be careful about adding in a lot of duplicate package paths. |
|
Ugh. Of course. So we'd have to store two strings for anonymous structs, or do some substantial rewriting for them in reflect. And obviously the reason that package path is a separate field is so it can be shared. I don't know why I didn't realize that. Okay, I will think harder about this. Maybe we should include |
|
Can we include the address of the runtime._type in the DIE?
…On Wed, Apr 11, 2018 at 4:28 PM, Heschi Kreinick ***@***.***> wrote:
Ugh. Of course. So we'd have to store two strings for anonymous structs,
or do some substantial rewriting for them in reflect. And obviously the
reason that package path is a separate field is so it can be shared. I
don't know why I didn't realize that.
Okay, I will think harder about this. Maybe we should include
runtime._type.hash in the type DIE or something.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#24814 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGkgICMBHgNk7g0I2TUtrcX90kTUvk3Wks5tnpGGgaJpZM4TQ2Xf>
.
|
|
+1. I've been bitten by this, too. I agree that we should put the extra information in the DIEs if at all possible to keep the growth in strippable sections. I think adding the address of the runtime._type should be relatively straightforward, since the DWARF type DIEs are all being generated by the linker anyway. However, it may make it more difficult if we ever wanted to pull that out to the compiler. One disadvantage of using a custom field in the DIE is that the GDB Python integration script won't be able to take advantage of it. |
|
Change https://golang.org/cl/106775 mentions this issue: |
|
I gave it a try. For a linker change, it was actually pretty easy to get something that seems to mostly work. Would appreciate any experienced eyes. |
ianlancetaylor
added
the
NeedsInvestigation
Technically this isn't true, delve scans each struct field individually and recursively reconstructs its name. Writing that code wasn't especially fun though and since anonymous types are relatively rare who knows if it actually is bug free. I too would vote for an extra DIE attribute. |
heschi
changed the title
When inspecting an interface object, debuggers need to tie the interface's dynamic type to a DWARF type so that they know how to decode the object the interface points to. This turns out to be quite tricky, because there isn't anything that directly ties a
runtime._typeto a DWARF DIE. Silly as it sounds, this is one of the more annoying parts of writing a Go debugging tool.Both Delve and x/debug/gocore need to address this somehow, and both use the type name to do it. The problem is that the runtime type string and the DWARF type name are different. DWARF type names include the full package path (e.g.
golang.org/x/debug/core.Process), but the runtime only has the package (e.g.core.Process).runtime._typeandruntime.uncommontypehave a package path, but that isn't good enough, because anonymous struct types are named by their fields (e.g.struct { sync.Mutex; runtime/pprof.profiling bool; runtime/pprof.done chan bool }), and there's no way to recover the package paths of the individual fields.Because the runtime's information is incomplete, there's no reliable way to do the mapping. gocore strips the package paths off with some string manipulation, and Delve tries to guess the package path based only on the package. Both will fail in the presence of packages with the same name.
I propose we change the runtime's type strings to include package paths, and therefore match the names used in the DWARF. AFAICT the runtime uses these strings primarily for panic messages, so the primary user-visible effect of the change will be longer, but less ambiguous, panics. The other (minor) usage is to link methods across plugin boundaries, but this should be a strict improvement there, because it won't need to look at the package path specially. In fact, it might be possible to eliminate package paths from
_typeanduncommontypeentirely./cc @hyangah @aarzilli @derekparker @aclements @crawshaw
The text was updated successfully, but these errors were encountered: