From 96983721d42e9e238ea9fec720849ab7bb616122 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 26 Aug 2020 14:17:35 -0700 Subject: [PATCH] [release-branch.go1.15] cmd/cgo: use go:notinheap for anonymous structs They can't reasonably be allocated on the heap. Not a huge deal, but it has an interesting and useful side effect. After CL 249917, the compiler and runtime treat pointers to go:notinheap types as uintptrs instead of real pointers (no write barrier, not processed during stack scanning, ...). That feature is exactly what we want for cgo to fix #40954. All the cases we have of pointers declared in C, but which might actually be filled with non-pointer data, are of this form (JNI's jobject heirarch, Darwin's CFType heirarchy, ...). Fixes #40954 Change-Id: I44a3b9bc2513d4287107e39d0cbbd0efd46a3aae Reviewed-on: https://go-review.googlesource.com/c/go/+/250940 Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Trust: Keith Randall Reviewed-by: Ian Lance Taylor Reviewed-on: https://go-review.googlesource.com/c/go/+/255321 --- src/cmd/cgo/gcc.go | 15 +++++++++++++++ src/cmd/cgo/main.go | 3 ++- src/cmd/cgo/out.go | 3 +++ test/fixedbugs/issue40954.go | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue40954.go diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index a59534ebd0..e512d51fda 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -2434,6 +2434,18 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ tt := *t tt.C = &TypeRepr{"%s %s", []interface{}{dt.Kind, tag}} tt.Go = c.Ident("struct{}") + if dt.Kind == "struct" { + // We don't know what the representation of this struct is, so don't let + // anyone allocate one on the Go side. As a side effect of this annotation, + // pointers to this type will not be considered pointers in Go. They won't + // get writebarrier-ed or adjusted during a stack copy. This should handle + // all the cases badPointerTypedef used to handle, but hopefully will + // continue to work going forward without any more need for cgo changes. + tt.NotInHeap = true + // TODO: we should probably do the same for unions. Unions can't live + // on the Go heap, right? It currently doesn't work for unions because + // they are defined as a type alias for struct{}, not a defined type. + } typedef[name.Name] = &tt break } @@ -2504,6 +2516,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ } t.Go = name t.BadPointer = sub.BadPointer + t.NotInHeap = sub.NotInHeap if unionWithPointer[sub.Go] { unionWithPointer[t.Go] = true } @@ -2514,6 +2527,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ tt := *t tt.Go = sub.Go tt.BadPointer = sub.BadPointer + tt.NotInHeap = sub.NotInHeap typedef[name.Name] = &tt } @@ -3022,6 +3036,7 @@ func (c *typeConv) anonymousStructTypedef(dt *dwarf.TypedefType) bool { // non-pointers in this type. // TODO: Currently our best solution is to find these manually and list them as // they come up. A better solution is desired. +// Note: DEPRECATED. There is now a better solution. Search for NotInHeap in this file. func (c *typeConv) badPointerTypedef(dt *dwarf.TypedefType) bool { if c.badCFType(dt) { return true diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 890daf65c4..c72c9abcc1 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -151,7 +151,8 @@ type Type struct { Go ast.Expr EnumValues map[string]int64 Typedef string - BadPointer bool + BadPointer bool // this pointer type should be represented as a uintptr (deprecated) + NotInHeap bool // this type should have a go:notinheap annotation } // A FuncType collects information about a function type in both the C and Go worlds. diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 4064f0ae41..cbdb4e0f5b 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -108,6 +108,9 @@ func (p *Package) writeDefs() { sort.Strings(typedefNames) for _, name := range typedefNames { def := typedef[name] + if def.NotInHeap { + fmt.Fprintf(fgo2, "//go:notinheap\n") + } fmt.Fprintf(fgo2, "type %s ", name) // We don't have source info for these types, so write them out without source info. // Otherwise types would look like: diff --git a/test/fixedbugs/issue40954.go b/test/fixedbugs/issue40954.go new file mode 100644 index 0000000000..53e9ccf387 --- /dev/null +++ b/test/fixedbugs/issue40954.go @@ -0,0 +1,35 @@ +// run + +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "unsafe" +) + +//go:notinheap +type S struct{ x int } + +func main() { + var i int + p := (*S)(unsafe.Pointer(uintptr(unsafe.Pointer(&i)))) + v := uintptr(unsafe.Pointer(p)) + // p is a pointer to a go:notinheap type. Like some C libraries, + // we stored an integer in that pointer. That integer just happens + // to be the address of i. + // v is also the address of i. + // p has a base type which is marked go:notinheap, so it + // should not be adjusted when the stack is copied. + recurse(100, p, v) +} +func recurse(n int, p *S, v uintptr) { + if n > 0 { + recurse(n-1, p, v) + } + if uintptr(unsafe.Pointer(p)) != v { + panic("adjusted notinheap pointer") + } +} -- 2.50.0