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

cmd/compile: generate correct DW_AT_external flag #22399

Open
thanm opened this issue Oct 23, 2017 · 4 comments
Open

cmd/compile: generate correct DW_AT_external flag #22399

thanm opened this issue Oct 23, 2017 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@thanm
Copy link
Contributor

thanm commented Oct 23, 2017

The current (1.9) Go compiler emits DWARF records for subprograms and variables that contain the "DW_AT_external" attribute, a flag which DWARF defines as:

If the name of the subroutine described by an entry with the tag ... is visible
outside of its containing compilation unit, that entry has a DW_AT_external attribute,
which is a flag.

The compiler is assigning a value to DW_AT_external based on whether the LSym in question is marked as static (AttrStatic), which I don't think is the same as the definition above. When I look at the generated DWARF for things like the Go compiler, the only things being marked as non-external are a small number of vars and functions from assembly files.

Not sure this makes a huge amount of difference (as far as I can tell there is no degradation to the debugging experience as a result of package-local functions being marked as external), but it might be worth fixing this up just for consistency / POLA.
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 29, 2018
@odeke-em odeke-em changed the title compiler: generated correct DW_AT_external flag cmd/compile: generate correct DW_AT_external flag Apr 24, 2018
@odeke-em
Copy link
Member

Thank you @thanm for the report!

I've tweaked the title just a little and I'll page some of compiler folks @mdempsky @heschik @randall77 @griesemer.

@heschi
Copy link
Contributor

heschi commented Apr 25, 2018

If it matters, I presume this is just a matter of checking the first rune of the name to see if it's upper case. But unless the flag is actually used for something somewhere, I might be inclined to just get rid of it instead.

@thanm
Copy link
Contributor Author

thanm commented Apr 25, 2018

Well, hmm. I looked at the delve sources and it looks as though it's being relied on in github.com/derekparker/delve/pkg/dwarf/reader/reader.go right at the moment for variables. So at the moment it is being interpreted as a "this is interesting" flag and not a "this symbol visible outside its translation unit" flag. hmm.

@mdempsky
Copy link
Member

If it matters, I presume this is just a matter of checking the first rune of the name to see if it's upper case.

FWIW, it's unfortunately a little trickier than that, because of method symbols (e.g., "T.m"), method expression wrappers ("T.m-fm"), function value symbols ("f·f"), and function closures ("f.func1").

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 30, 2018
@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@thanm thanm modified the milestones: Go1.14, Unplanned Sep 11, 2019
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Debugging NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@mdempsky @andybons @ianlancetaylor @odeke-em @gopherbot @thanm @heschi