Skip to content

Commit 42b023d

Browse files
committed
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 golang#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 golang#40954 Change-Id: I44a3b9bc2513d4287107e39d0cbbd0efd46a3aae Reviewed-on: https://go-review.googlesource.com/c/go/+/250940 Run-TryBot: Emmanuel Odeke <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Keith Randall <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 4f91591 commit 42b023d

File tree

4 files changed

+55
-1
lines changed

4 files changed

+55
-1
lines changed

src/cmd/cgo/gcc.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2448,6 +2448,18 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ
24482448
tt := *t
24492449
tt.C = &TypeRepr{"%s %s", []interface{}{dt.Kind, tag}}
24502450
tt.Go = c.Ident("struct{}")
2451+
if dt.Kind == "struct" {
2452+
// We don't know what the representation of this struct is, so don't let
2453+
// anyone allocate one on the Go side. As a side effect of this annotation,
2454+
// pointers to this type will not be considered pointers in Go. They won't
2455+
// get writebarrier-ed or adjusted during a stack copy. This should handle
2456+
// all the cases badPointerTypedef used to handle, but hopefully will
2457+
// continue to work going forward without any more need for cgo changes.
2458+
tt.NotInHeap = true
2459+
// TODO: we should probably do the same for unions. Unions can't live
2460+
// on the Go heap, right? It currently doesn't work for unions because
2461+
// they are defined as a type alias for struct{}, not a defined type.
2462+
}
24512463
typedef[name.Name] = &tt
24522464
break
24532465
}
@@ -2518,6 +2530,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ
25182530
}
25192531
t.Go = name
25202532
t.BadPointer = sub.BadPointer
2533+
t.NotInHeap = sub.NotInHeap
25212534
if unionWithPointer[sub.Go] {
25222535
unionWithPointer[t.Go] = true
25232536
}
@@ -2528,6 +2541,7 @@ func (c *typeConv) loadType(dtype dwarf.Type, pos token.Pos, parent string) *Typ
25282541
tt := *t
25292542
tt.Go = sub.Go
25302543
tt.BadPointer = sub.BadPointer
2544+
tt.NotInHeap = sub.NotInHeap
25312545
typedef[name.Name] = &tt
25322546
}
25332547

@@ -3026,6 +3040,7 @@ func (c *typeConv) anonymousStructTypedef(dt *dwarf.TypedefType) bool {
30263040
// non-pointers in this type.
30273041
// TODO: Currently our best solution is to find these manually and list them as
30283042
// they come up. A better solution is desired.
3043+
// Note: DEPRECATED. There is now a better solution. Search for NotInHeap in this file.
30293044
func (c *typeConv) badPointerTypedef(dt *dwarf.TypedefType) bool {
30303045
if c.badCFType(dt) {
30313046
return true

src/cmd/cgo/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ type Type struct {
151151
Go ast.Expr
152152
EnumValues map[string]int64
153153
Typedef string
154-
BadPointer bool
154+
BadPointer bool // this pointer type should be represented as a uintptr (deprecated)
155+
NotInHeap bool // this type should have a go:notinheap annotation
155156
}
156157

157158
// A FuncType collects information about a function type in both the C and Go worlds.

src/cmd/cgo/out.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ func (p *Package) writeDefs() {
108108
sort.Strings(typedefNames)
109109
for _, name := range typedefNames {
110110
def := typedef[name]
111+
if def.NotInHeap {
112+
fmt.Fprintf(fgo2, "//go:notinheap\n")
113+
}
111114
fmt.Fprintf(fgo2, "type %s ", name)
112115
// We don't have source info for these types, so write them out without source info.
113116
// Otherwise types would look like:

test/fixedbugs/issue40954.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// run
2+
3+
// Copyright 2020 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"unsafe"
11+
)
12+
13+
//go:notinheap
14+
type S struct{ x int }
15+
16+
func main() {
17+
var i int
18+
p := (*S)(unsafe.Pointer(uintptr(unsafe.Pointer(&i))))
19+
v := uintptr(unsafe.Pointer(p))
20+
// p is a pointer to a go:notinheap type. Like some C libraries,
21+
// we stored an integer in that pointer. That integer just happens
22+
// to be the address of i.
23+
// v is also the address of i.
24+
// p has a base type which is marked go:notinheap, so it
25+
// should not be adjusted when the stack is copied.
26+
recurse(100, p, v)
27+
}
28+
func recurse(n int, p *S, v uintptr) {
29+
if n > 0 {
30+
recurse(n-1, p, v)
31+
}
32+
if uintptr(unsafe.Pointer(p)) != v {
33+
panic("adjusted notinheap pointer")
34+
}
35+
}

0 commit comments

Comments
 (0)