-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: record optimization level in binary #22168
Comments
One place to put this kind of information in DWARF is in the |
That doesn't let us record per-package information, though. |
A package is a compilation unit. |
In gc's DWARF output there's just one big compilation unit. Though it would be reasonable to change that. |
I was also confused by this... right now the gc compiler doesn't emit the comp unit DIE-- rather a single unit is emitted by the linker. Having the compiler emit a compilation unit DIE seems like it might be nice. |
I looked into creating a separate CU for each package a bit. It's a little tricky since, as far as I can tell, the linker has lost track of which package a symbol belongs to by the point it's stitching together the DWARF. Symbols from different packages are also interleaved, so we'd have to produce |
Maybe the CU ranges wouldn't be so bad. In the readelf --debug-dump=info go | awk 'x {x=0;print $4} /subprogram/ {x=1}' | sed 's/\..*//' | grep -v ^type$ | sort | uniq | wc -l
readelf --debug-dump=info go | awk 'x {x=0;print $4} /subprogram/ {x=1}' | sed 's/\..*//' | grep -v ^type$ | uniq | wc -l |
My understanding was that the hi/lo PC range for the CU DIE was optional -- perhaps it would make sense to just leave it out altogether. Certainly for C++ programs where there is linker reordering of functions (for example, based on profile data), hi/lo PC or ranges would not be generated. |
Ah, well that would be more convenient. Still, I think the harder part is that it's lost track of the packages when it creates the CU. I compared the symbols in Given that the CU DIE is constructed by the linker, we would also need a way to communicate the optimization level from the compiler to the linker to be put in this DIE. |
I would normally assume that we want this on a per-function granularity, for the use case of "this fails in production, optimization hides the debugging info, but I cannot afford to run the whole thing unoptimized, therefore I will use the (as-yet non-existent) //go:noopt comment to disable optimization of the single function I really need to catch in the act". However, if we plan to get our optimized-debugging story good enough, we don't want to introduce a feature that we hope to deprecate within a year. Do I have this right? |
I should point out that #19340 is about insuring that optimized and non-optimized packages aren't mixed together accidentally.
I don't think there is a standard way to say this in DWARF, it would have to be an extension attribute. |
Right. Russ is hoping to get something out for 1.10 that will prevent accidentally mixing and matching gcflags. (Though it's possible that, in the future, there will be a way to intentionally specify gcflags for specific packages in a way that plays nicely with caching.)
Yes. I believe this is true whether this is function granularity or package granularity or anything else. I'm open to suggestions for what this attribute should be, especially if there are already existing vendor attributes like this. |
I discussed this with several other runtime and compiler people today and we decided to split up the packages into separate compilation unit DIEs (which I've already implemented), and add the compile command line flags to |
Change https://golang.org/cl/69973 mentions this issue: |
Currently, the linker generates one huge DWARF compilation unit for the entire Go binary. This commit creates a separate compilation unit and line table per Go package. We temporarily lose compilation unit PC range information, since it's now discontiguous, so harder to emit. We'll bring it back in the next commit. Beyond being "more traditional", this has various technical advantages: * It should speed up line table lookup, since that requires a sequential scan of the line table. With this change, a debugger can first locate the per-package line table and then scan only that line table. * Once we emit compilation unit PC ranges again, this should also speed up various other debugger reverse PC lookups. * It puts us in a good position to move more DWARF generation into the compiler, which could produce at least the CU header, per-function line table fragments, and per-function frame unwinding info that the linker could simply paste together. * It will let us record a per-package compiler command-line flags (#22168). Change-Id: Ibac642890984636b3ef1d4b37fe97f4453c2cc84 Reviewed-on: https://go-review.googlesource.com/69973 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
Change https://golang.org/cl/71430 mentions this issue: |
Go's debug information for optimized binaries needs a lot of improvement (we're working on it). In the mean time, it would be valuable for debuggers to know whether a binary optimized or unoptimized. For example, this could be used (in combination with the toolchain version already recorded in the binary) to warn users that local variable information may be incorrect and that they should consider recompiling with optimizations disabled.
One complication is that it's currently possible (and fairly common) to compile different packages with different optimization levels. If the
cmd/go
changes go as planned, this will no longer be possible in 1.10, but may again become possible in the future.Some reasonable ways to record this information are:
Create a per-package symbol to indicate the optimization level. We could create this symbol only if the package is unoptimized, so we don't bloat optimized binaries with lots of extra symbols. This would be easy to get at from any symbolic debugger.
Record this as a custom attribute in each DWARF function (there is no DIE corresponding to a package). This is arguably the "right" place to record this, but it requires help from the debugger's DWARF decoder to access.
I think both are pretty easy to implement. I would lean toward solution 1.
/cc @rsc @dr2chase @zombiezen
The text was updated successfully, but these errors were encountered: