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

runtime, cmd/compile: remove all pointers from type information #17724

Open
crawshaw opened this Issue Nov 1, 2016 · 2 comments

Comments

Projects
None yet
5 participants
@crawshaw
Contributor

crawshaw commented Nov 1, 2016

For Go 1.7 I started replacing the pointers in Go's run time type information with offsets from the beginning of the type section. (The motivation was significantly reduced binary size, as 8-bytes for the pointer + 32-bytes for the dynamic relocation is significantly more than a 4-byte offset multiplied through by several pointers in each type and thousands of types.)

This work is not yet complete, and in its partial state it leads to subtle bugs like #17709. When using a binary built with -buildmode=shared or -buildmode=plugin, there are multiple type information sections, one for each module. An offset into this region (represented by either a typeOff or a nameOff) needs to be resolved against the correct type region. But when you follow one of the old pointers in a type, it is entirely possible the dynamic relocation processed by ld.so will jump you to the types section of a different module.

This can lead to an offset from one module being resolved against the wrong type region, meaning garbage data is used for the type. This will manifest as subtle bugs: types incorrectly mapping (or not) onto interfaces, the reflect package returning wrong results, or bad garbage collection.

The likelihood of misplacing an offset decreases significantly if we remove the pointers, as we will not have to consider the interaction of two separate relocation systems. Let's do that.

@crawshaw crawshaw added this to the Go1.9 milestone Nov 1, 2016

@crawshaw crawshaw self-assigned this Nov 1, 2016

@gopherbot

This comment has been minimized.

gopherbot commented Nov 1, 2016

CL https://golang.org/cl/32513 mentions this issue.

@crawshaw

This comment has been minimized.

Contributor

crawshaw commented Nov 1, 2016

I have perhaps overstated the risk: this only applies to parts of the runtime that have access to non-canonical *_type values. Almost all of the runtime receives a *_type via module lookup, and moduledata.typemap ensures that the type is canonical.

Because the canonical *_type is the version of it from the earliest loaded moduledata, and because ld.so always resolves to the earliest loaded module, the pointers in a typical *_type point to the module that *_type originates from. It is definitely not an issue outside the runtime as all *_type values are canonical there, and even relatively high-level runtime code like iface.go only sees canonical *_type values.

In fact, runtime.typesEqual is one of the only functions that is exposed to non-canonical *_type values. Even so, I'd hate for someone else to have to debug something like this.

gopherbot pushed a commit that referenced this issue Nov 1, 2016

runtime: resolve type offsets using source module
The runtime.typeEquals function is used during typelinksinit to
determine the canonical set of *_type values to use throughout the
runtime. As such, it is run against non-canonical *_type values, that
is, types from modules that are duplicates of a type from another
module that was loaded earlier in the program life.

These non-canonical *_type values sometimes contain pointers. These
pointers are pointing to position-independent data, and so they are set
by ld.so using dynamic relocations when the module is loaded. As such,
the pointer can point to the equivalent memory from a previous module.

This means if typesEqual follows a pointer inside a *_type, it can end
up at a piece of memory from another module. If it reads a typeOff or
nameOff from that memory and attempts to resolve it against the
non-canonical *_type from the later module, it will end up with a
reference to junk memory.

Instead, resolve against the pointer the offset was read from, so the
data is valid.

Fixes #17709.
Should no longer matter after #17724 is resolved in a later Go.

Change-Id: Ie88b151a3407d82ac030a97b5b6a19fc781901cb
Reviewed-on: https://go-review.googlesource.com/32513
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 May 24, 2017

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017

@aclements aclements modified the milestones: Go1.11, Unplanned Jun 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment