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: annotation for sysAlloced types #13386

Open
aclements opened this Issue Nov 24, 2015 · 3 comments

Comments

Projects
None yet
4 participants
@aclements
Member

aclements commented Nov 24, 2015

@randall77 recently found some subtle issues related to write barriers on fields that point from runtime structures allocated from non-GCed (and hence non-scanned) memory to GCed heap memory. I manually audited all non-GCed structures to find any other such pointers, but obviously manual auditing can get out of date.

One possible way to help automate this is to add an annotation on types that are allocated from non-GCed memory. This annotation would disallow pointers to types that do not have this annotation and would disallow calling new on this type (implicit heap allocations are already disallowed in the runtime). It should probably also disallow unsafe.Pointer in annotated types (effectively requiring uintptr instead).

/cc @randall77 @ianlancetaylor @rsc @RLH

@aclements aclements added the Proposal label Nov 24, 2015

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Nov 24, 2015

We could also change fixalloc to take a _type* rather than a size, and then perhaps we could somehow ensure that the type had the new annotation.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 28, 2015

This doesn't need the formality of the proposal process. Anything that works and that you are willing to implement is fine.

@rsc rsc removed the Proposal label Dec 28, 2015

@rsc rsc changed the title from proposal: runtime: annotation for sysAlloced types to runtime: annotation for sysAlloced types Dec 28, 2015

@rsc rsc added this to the Unplanned milestone Dec 28, 2015

@gopherbot

This comment has been minimized.

gopherbot commented Oct 12, 2016

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

gopherbot pushed a commit that referenced this issue Oct 15, 2016

cmd/compile: add go:notinheap type pragma
This adds a //go:notinheap pragma for declarations of types that must
not be heap allocated. We ensure these rules by disallowing new(T),
make([]T), append([]T), or implicit allocation of T, by disallowing
conversions to notinheap types, and by propagating notinheap to any
struct or array that contains notinheap elements.

The utility of this pragma is that we can eliminate write barriers for
writes to pointers to go:notinheap types, since the write barrier is
guaranteed to be a no-op. This will let us mark several scheduler and
memory allocator structures as go:notinheap, which will let us
disallow write barriers in the scheduler and memory allocator much
more thoroughly and also eliminate some problematic hybrid write
barriers.

This also makes go:nowritebarrierrec and go:yeswritebarrierrec much
more powerful. Currently we use go:nowritebarrier all over the place,
but it's almost never what you actually want: when write barriers are
illegal, they're typically illegal for a whole dynamic scope. Partly
this is because go:nowritebarrier has been around longer, but it's
also because go:nowritebarrierrec couldn't be used in situations that
had no-op write barriers or where some nested scope did allow write
barriers. go:notinheap eliminates many no-op write barriers and
go:yeswritebarrierrec makes it possible to opt back in to write
barriers, so these two changes will let us use go:nowritebarrierrec
far more liberally.

This updates #13386, which is about controlling pointers from non-GC'd
memory to GC'd memory. That would require some additional pragma (or
pragmas), but could build on this pragma.

Change-Id: I6314f8f4181535dd166887c9ec239977b54940bd
Reviewed-on: https://go-review.googlesource.com/30939
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment