aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Randall <khr@golang.org>2020-08-26 14:17:35 -0700
committerDmitri Shuralyov <dmitshur@golang.org>2020-10-09 17:25:11 +0000
commit96983721d42e9e238ea9fec720849ab7bb616122 (patch)
tree1664581ba93f3ffb6401e7cd90d0ff9f41ee7280
parent142027888a7d3da0494d77581166f1d120317864 (diff)
downloadgo-96983721d42e9e238ea9fec720849ab7bb616122.tar.gz
go-96983721d42e9e238ea9fec720849ab7bb616122.zip
[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 <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-on: https://go-review.googlesource.com/c/go/+/255321
-rw-r--r--src/cmd/cgo/gcc.go15
-rw-r--r--src/cmd/cgo/main.go3
-rw-r--r--src/cmd/cgo/out.go3
-rw-r--r--test/fixedbugs/issue40954.go35
4 files changed, 55 insertions, 1 deletions
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")
+ }
+}