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

cmd/compile, cmd/link: remove use of Automs #34554

Closed
thanm opened this issue Sep 26, 2019 · 7 comments
Closed

cmd/compile, cmd/link: remove use of Automs #34554

thanm opened this issue Sep 26, 2019 · 7 comments

Comments

@thanm
Copy link
Contributor

thanm commented Sep 26, 2019

This is a tracking issue/bug for a modest compiler/linker enhancement involving DWARF type generation.

Background / motivation

Consider the following Go program:

package main

type astruct struct {
	a, b int
}
type bstruct struct {
	c, d byte
}

func dead() {
	var iface = []bstruct{}
	println(iface)

}

func main() {
	var iface interface{} = map[string]astruct{}
	println(iface)
}

We have two functions, "dead" and "main". The main function is obviously useful, but the function 'dead' is uncalled, and will be eliminated by the linker's dead code elimination phase.

When generating DWARF for 'main', we would like to insure that the types used by main are included, that is, that we emit DWARF DIEs for each interesting type. The tricky part here is that there are no variables (autos or parameters) in main that refer to types like astruct or map[string]astruct, which means that these types are also vulnerable to linker deadcode.

The compiler and linker currently handle this via a construct known as an "Autom"; the compiler emits an Autom record for each auto-tmp variable used by the function; these autom's get placed into the object file entry for the function. In the case of 'main' above, the compiler-generated temp var used to construct the value map[string]astruct{} triggers generation of an Autom.

The linker then reads in autom's when it reads the object file, and later on during linker processing it walks the Autom's for each live (non-dead-coded) function and generates a DWARF type DIE for its type if needed.

Note that for the example program above, we'll get type information for astruct (via the autom mechanism) but not for bstruct, since that type is not referenced by any live function.

Proposed change

What the section above leaves out is that the autom mechanism is cumbersome and heavyweight -- Autom's in the object file have a lot more information than strictly needed (this has happened gradually over time due to more DWARF generation moving from the compiler to the linker). This results in object file bloat and extra processing time in the linker.

What would work equally well is to attach dummy relocations to each function symbol that record the Go types used by its associated autotmps, then use these relocations to drive the DWARF type generation. This would speed up the compiler and linker and reduce object file size.

Issues/caveats/problem

There is a small chunk of code in the linker specific to the Plan9 target that does processing of Autom's to create directives or symbols for Autom's in the Plan9 object file. It's hard to tell from the code whether this is at all useful, since we haven't been able to find someone who knows how to run the Plan9 debugger. Given the current usage of autom's (e.g. for non-user-visible variables) it seems unlikely that removing the Plan9 autom handling will cause any debugging issues, so the proposal is to just drop this code.

@thanm thanm self-assigned this Sep 26, 2019
@aclements
Copy link
Member

Did you look into whether the function DIE trees already cause the appropriate DWARF types to be included? I don't understand how they couldn't, but the linker is full of surprises. Are the dummy relocations even necessary? If they are necessary, wouldn't it be better to make the relocations attached to the function DIEs cause the types to be included?

@thanm
Copy link
Contributor Author

thanm commented Sep 26, 2019

@aclements The example I posted illustrates:

func main() {
	var iface interface{} = map[string]astruct{}
	println(iface)
}

There is no place in main's subrogram DIE to hang a type reference, since we never actually declare a named variable (parameter or auto) that has the type of interest (astruct or equivalent). So yes, if we want to preserve the current compiler/linker behavior we need the dummy relocations.

I did also as an experiment try running all.bash after hacking the linker to ignore autom's completely. When I did this I got a failure in the runtime's gdb test (TestGdbAutotmpTypes).

@thanm
Copy link
Contributor Author

thanm commented Sep 26, 2019

If they are necessary, wouldn't it be better to make the relocations attached to the function DIEs cause the types to be included?

This might be a good idea. The advantage of putting them on the function symbol itself is that there are no changes needed to dead code elimination; if I put them on the subprogram DIE then I'd have to add special processing. With that said, it would be easy to do.

@gopherbot
Copy link

Change https://golang.org/cl/197499 mentions this issue: cmd/compile: don't emit autom's into object file

@gopherbot
Copy link

Change https://golang.org/cl/197498 mentions this issue: cmd/link: create DWARF types for autos based R_USETYPE relocs

@gopherbot
Copy link

Change https://golang.org/cl/197497 mentions this issue: cmd/compile: add R_USETYPE relocs to func syms for autom types

@gopherbot
Copy link

Change https://golang.org/cl/197500 mentions this issue: cmd/link: remove reading/processing of function Autom records

gopherbot pushed a commit that referenced this issue Sep 27, 2019
During DWARF processing, keep track of the go type symbols for types
directly or indirectly referenced by auto variables in a function,
and add a set of dummy R_USETYPE relocations to the function's DWARF
subprogram DIE symbol.

This change is not useful on its own, but is part of a series of
changes intended to clean up handling of autom's in the compiler
and linker.

Updates #34554.

Change-Id: I974afa9b7092aa5dba808f74e00aa931249d6fe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197497
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
gopherbot pushed a commit that referenced this issue Sep 27, 2019
Switch the linker over to use dummy R_USETYPE relocations on DWARF
subprogram DIEs as a means of insuring that DWARF types are created
for types of autotmp values used in live functions.

This change is part of a series intended to clean up handling of
autotmp types and remove use of autom's in the compiler and linker.

Updates #34554.

Change-Id: Ic74da6bd723ab7e4d8a16ad46e23228650d4b525
Reviewed-on: https://go-review.googlesource.com/c/go/+/197498
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
gopherbot pushed a commit that referenced this issue Sep 27, 2019
Don't write Autom records when writing a function to the object file;
we no longer need them in the linker for DWARF processing. So as to
keep the object file format unchanged, write out a zero-length list of
automs to the object, as opposed to removing all references.

Updates #34554.

Change-Id: I42a1d67207ea7114ae4f3a315cf37effba57f190
Reviewed-on: https://go-review.googlesource.com/c/go/+/197499
Reviewed-by: Jeremy Faller <jeremy@golang.org>
gopherbot pushed a commit that referenced this issue Sep 27, 2019
Remove linker reading and processing of automs (no longer needed, now
that the compiler is emitting R_USETYPE relocations on functions). So
as to avoid changing the object file format, the object still contains
a count of automs, but this count is required to be zero.

Updates #34554.

Change-Id: I10230e191057c5c5705541eeb06f747d5f73c42d
Reviewed-on: https://go-review.googlesource.com/c/go/+/197500
Reviewed-by: Jeremy Faller <jeremy@golang.org>
@thanm thanm closed this as completed Sep 27, 2019
@golang golang locked and limited conversation to collaborators Sep 26, 2020
@rsc rsc unassigned thanm Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants