Skip to content

Commit 0d6a2d5

Browse files
runtime: skip writes to persistent memory in cgo checker
Fixes #23899 Fixes #28458 Change-Id: Ie177f2d4c399445d8d5e1a327f2419c7866cb45e Reviewed-on: https://go-review.googlesource.com/c/155697 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent 95a6f11 commit 0d6a2d5

File tree

3 files changed

+63
-17
lines changed

3 files changed

+63
-17
lines changed

misc/cgo/errors/ptr_test.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,24 @@ var ptrTests = []ptrTest{
406406
body: `var b bytes.Buffer; b.WriteString("a"); C.f(unsafe.Pointer(&b.Bytes()[0]))`,
407407
fail: false,
408408
},
409+
{
410+
// Test that bgsweep releasing a finalizer is OK.
411+
name: "finalizer",
412+
c: `// Nothing to declare.`,
413+
imports: []string{"os"},
414+
support: `func open() { os.Open(os.Args[0]) }; var G [][]byte`,
415+
body: `for i := 0; i < 10000; i++ { G = append(G, make([]byte, 4096)); if i % 100 == 0 { G = nil; open() } }`,
416+
fail: false,
417+
},
418+
{
419+
// Test that converting generated struct to interface is OK.
420+
name: "structof",
421+
c: `// Nothing to declare.`,
422+
imports: []string{"reflect"},
423+
support: `type MyInt int; func (i MyInt) Get() int { return int(i) }; type Getter interface { Get() int }`,
424+
body: `t := reflect.StructOf([]reflect.StructField{{Name: "MyInt", Type: reflect.TypeOf(MyInt(0)), Anonymous: true}}); v := reflect.New(t).Elem(); v.Interface().(Getter).Get()`,
425+
fail: false,
426+
},
409427
}
410428

411429
func TestPointerChecks(t *testing.T) {
@@ -478,7 +496,7 @@ func testOne(t *testing.T, pt ptrTest) {
478496

479497
cmd := exec.Command("go", "build")
480498
cmd.Dir = src
481-
cmd.Env = addEnv("GOPATH", gopath)
499+
cmd.Env = append(os.Environ(), "GOPATH="+gopath)
482500
buf, err := cmd.CombinedOutput()
483501
if err != nil {
484502
t.Logf("%#q:\n%s", args(cmd), buf)
@@ -550,16 +568,5 @@ func testOne(t *testing.T, pt ptrTest) {
550568
}
551569

552570
func cgocheckEnv(val string) []string {
553-
return addEnv("GODEBUG", "cgocheck="+val)
554-
}
555-
556-
func addEnv(key, val string) []string {
557-
env := []string{key + "=" + val}
558-
look := key + "="
559-
for _, e := range os.Environ() {
560-
if !strings.HasPrefix(e, look) {
561-
env = append(env, e)
562-
}
563-
}
564-
return env
571+
return append(os.Environ(), "GODEBUG=cgocheck="+val)
565572
}

src/runtime/cgocheck.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ func cgoCheckWriteBarrier(dst *uintptr, src uintptr) {
4343
return
4444
}
4545

46+
// It's OK if writing to memory allocated by persistentalloc.
47+
// Do this check last because it is more expensive and rarely true.
48+
// If it is false the expense doesn't matter since we are crashing.
49+
if inPersistentAlloc(uintptr(unsafe.Pointer(dst))) {
50+
return
51+
}
52+
4653
systemstack(func() {
4754
println("write of Go pointer", hex(src), "to non-Go memory", hex(uintptr(unsafe.Pointer(dst))))
4855
throw(cgoWriteBarrierFail)

src/runtime/malloc.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,15 @@ var globalAlloc struct {
11671167
persistentAlloc
11681168
}
11691169

1170+
// persistentChunkSize is the number of bytes we allocate when we grow
1171+
// a persistentAlloc.
1172+
const persistentChunkSize = 256 << 10
1173+
1174+
// persistentChunks is a list of all the persistent chunks we have
1175+
// allocated. The list is maintained through the first word in the
1176+
// persistent chunk. This is updated atomically.
1177+
var persistentChunks *notInHeap
1178+
11701179
// Wrapper around sysAlloc that can allocate small chunks.
11711180
// There is no associated free operation.
11721181
// Intended for things like function/type/debug-related persistent data.
@@ -1187,7 +1196,6 @@ func persistentalloc(size, align uintptr, sysStat *uint64) unsafe.Pointer {
11871196
//go:systemstack
11881197
func persistentalloc1(size, align uintptr, sysStat *uint64) *notInHeap {
11891198
const (
1190-
chunk = 256 << 10
11911199
maxBlock = 64 << 10 // VM reservation granularity is 64K on windows
11921200
)
11931201

@@ -1218,15 +1226,24 @@ func persistentalloc1(size, align uintptr, sysStat *uint64) *notInHeap {
12181226
persistent = &globalAlloc.persistentAlloc
12191227
}
12201228
persistent.off = round(persistent.off, align)
1221-
if persistent.off+size > chunk || persistent.base == nil {
1222-
persistent.base = (*notInHeap)(sysAlloc(chunk, &memstats.other_sys))
1229+
if persistent.off+size > persistentChunkSize || persistent.base == nil {
1230+
persistent.base = (*notInHeap)(sysAlloc(persistentChunkSize, &memstats.other_sys))
12231231
if persistent.base == nil {
12241232
if persistent == &globalAlloc.persistentAlloc {
12251233
unlock(&globalAlloc.mutex)
12261234
}
12271235
throw("runtime: cannot allocate memory")
12281236
}
1229-
persistent.off = 0
1237+
1238+
// Add the new chunk to the persistentChunks list.
1239+
for {
1240+
chunks := uintptr(unsafe.Pointer(persistentChunks))
1241+
*(*uintptr)(unsafe.Pointer(persistent.base)) = chunks
1242+
if atomic.Casuintptr((*uintptr)(unsafe.Pointer(&persistentChunks)), chunks, uintptr(unsafe.Pointer(persistent.base))) {
1243+
break
1244+
}
1245+
}
1246+
persistent.off = sys.PtrSize
12301247
}
12311248
p := persistent.base.add(persistent.off)
12321249
persistent.off += size
@@ -1242,6 +1259,21 @@ func persistentalloc1(size, align uintptr, sysStat *uint64) *notInHeap {
12421259
return p
12431260
}
12441261

1262+
// inPersistentAlloc reports whether p points to memory allocated by
1263+
// persistentalloc. This must be nosplit because it is called by the
1264+
// cgo checker code, which is called by the write barrier code.
1265+
//go:nosplit
1266+
func inPersistentAlloc(p uintptr) bool {
1267+
chunk := atomic.Loaduintptr((*uintptr)(unsafe.Pointer(&persistentChunks)))
1268+
for chunk != 0 {
1269+
if p >= chunk && p < chunk+persistentChunkSize {
1270+
return true
1271+
}
1272+
chunk = *(*uintptr)(unsafe.Pointer(chunk))
1273+
}
1274+
return false
1275+
}
1276+
12451277
// linearAlloc is a simple linear allocator that pre-reserves a region
12461278
// of memory and then maps that region as needed. The caller is
12471279
// responsible for locking.

0 commit comments

Comments
 (0)