[go] runtime: fix nosplit stack overflow

[Dmitry Vyukov (Gerrit)]

Reviewers: Russ Cox

Dmitry Vyukov uploaded a change:
https://go-review.googlesource.com/3418

runtime: fix nosplit stack overflow

The overflow happens only with -gcflags="-N -l"
and can be reproduced with:

$ go test -gcflags="-N -l" -a -run=none net

runtime.cgocall: nosplit stack overflow
504 assumed on entry to runtime.cgocall
480 after runtime.cgocall uses 24
472 on entry to runtime.cgocall_errno
408 after runtime.cgocall_errno uses 64
400 on entry to runtime.exitsyscall
288 after runtime.exitsyscall uses 112
280 on entry to runtime.exitsyscallfast
152 after runtime.exitsyscallfast uses 128
144 on entry to runtime.writebarrierptr
88 after runtime.writebarrierptr uses 56
80 on entry to runtime.writebarrierptr_nostore1
24 after runtime.writebarrierptr_nostore1 uses 56
16 on entry to runtime.acquirem
-24 after runtime.acquirem uses 40

Move closure creation into separate function so that
frames of writebarrierptr_shadow and writebarrierptr_nostore1
are overlapped.

Change-Id: I40851f0786763ee964af34814edbc3e3d73cf4e7
---
M src/runtime/mbarrier.go
1 file changed, 21 insertions(+), 16 deletions(-)



diff --git a/src/runtime/mbarrier.go b/src/runtime/mbarrier.go
index c9ed035..33d67c4 100644
--- a/src/runtime/mbarrier.go
+++ b/src/runtime/mbarrier.go
@@ -110,28 +110,33 @@
}

if mheap_.shadow_enabled {
- systemstack(func() {
- addr := uintptr(unsafe.Pointer(dst))
- shadow := shadowptr(addr)
- if shadow == nil {
- return
- }
- // There is a race here but only if the program is using
- // racy writes instead of sync/atomic. In that case we
- // don't mind crashing.
- if *shadow != *dst && *shadow != noShadow && istrackedptr(*dst) {
- mheap_.shadow_enabled = false
- print("runtime: write barrier dst=", dst, " old=", hex(*dst), "
shadow=", shadow, " old=", hex(*shadow), " new=", hex(src), "\n")
- throw("missed write barrier")
- }
- *shadow = src
- })
+ writebarrierptr_shadow(dst, src)
}

*dst = src
writebarrierptr_nostore1(dst, src)
}

+//go:nosplit
+func writebarrierptr_shadow(dst *uintptr, src uintptr) {
+ systemstack(func() {
+ addr := uintptr(unsafe.Pointer(dst))
+ shadow := shadowptr(addr)
+ if shadow == nil {
+ return
+ }
+ // There is a race here but only if the program is using
+ // racy writes instead of sync/atomic. In that case we
+ // don't mind crashing.
+ if *shadow != *dst && *shadow != noShadow && istrackedptr(*dst) {
+ mheap_.shadow_enabled = false
+ print("runtime: write barrier dst=", dst, " old=", hex(*dst), "
shadow=", shadow, " old=", hex(*shadow), " new=", hex(src), "\n")
+ throw("missed write barrier")
+ }
+ *shadow = src
+ })
+}
+
// Like writebarrierptr, but the store has already been applied.
// Do not reapply.
//go:nosplit

--
https://go-review.googlesource.com/3418
Gerrit-Reviewer: Russ Cox <r...@golang.org>

[Dmitry Vyukov (Gerrit)]

Dmitry Vyukov has posted comments on this change.

runtime: fix nosplit stack overflow

Patch Set 1:

I guess we will always be on the edge whatever the limit is.
So could leave it as 128 as well :)

--
https://go-review.googlesource.com/3418
Gerrit-Reviewer: Dmitry Vyukov <dvy...@google.com>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-HasComments: No

[Dmitry Vyukov (Gerrit)]

Dmitry Vyukov uploaded a new patch set:

https://go-review.googlesource.com/3418

runtime: fix nosplit stack overflow

The overflow happens only with -gcflags="-N -l"
and can be reproduced with:

$ go test -gcflags="-N -l" -a -run=none net

runtime.cgocall: nosplit stack overflow
504 assumed on entry to runtime.cgocall
480 after runtime.cgocall uses 24
472 on entry to runtime.cgocall_errno
408 after runtime.cgocall_errno uses 64
400 on entry to runtime.exitsyscall
288 after runtime.exitsyscall uses 112
280 on entry to runtime.exitsyscallfast
152 after runtime.exitsyscallfast uses 128
144 on entry to runtime.writebarrierptr
88 after runtime.writebarrierptr uses 56
80 on entry to runtime.writebarrierptr_nostore1
24 after runtime.writebarrierptr_nostore1 uses 56
16 on entry to runtime.acquirem
-24 after runtime.acquirem uses 40

Move closure creation into separate function so that
frames of writebarrierptr_shadow and writebarrierptr_nostore1
are overlapped.

Fixes #9721

Change-Id: I40851f0786763ee964af34814edbc3e3d73cf4e7
---
M src/runtime/mbarrier.go
1 file changed, 21 insertions(+), 16 deletions(-)

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

runtime: fix nosplit stack overflow

Patch Set 2: Code-Review+2

--
https://go-review.googlesource.com/3418
Gerrit-Reviewer: Dmitry Vyukov <dvy...@google.com>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Dmitry Vyukov (Gerrit)]

Dmitry Vyukov has submitted this change and it was merged.

runtime: fix nosplit stack overflow

The overflow happens only with -gcflags="-N -l"
and can be reproduced with:

$ go test -gcflags="-N -l" -a -run=none net

runtime.cgocall: nosplit stack overflow
504 assumed on entry to runtime.cgocall
480 after runtime.cgocall uses 24
472 on entry to runtime.cgocall_errno
408 after runtime.cgocall_errno uses 64
400 on entry to runtime.exitsyscall
288 after runtime.exitsyscall uses 112
280 on entry to runtime.exitsyscallfast
152 after runtime.exitsyscallfast uses 128
144 on entry to runtime.writebarrierptr
88 after runtime.writebarrierptr uses 56
80 on entry to runtime.writebarrierptr_nostore1
24 after runtime.writebarrierptr_nostore1 uses 56
16 on entry to runtime.acquirem
-24 after runtime.acquirem uses 40

Move closure creation into separate function so that
frames of writebarrierptr_shadow and writebarrierptr_nostore1
are overlapped.

Fixes #9721

Change-Id: I40851f0786763ee964af34814edbc3e3d73cf4e7
Reviewed-on: https://go-review.googlesource.com/3418
Reviewed-by: Russ Cox <r...@golang.org>

---
M src/runtime/mbarrier.go
1 file changed, 21 insertions(+), 16 deletions(-)

Approvals:
Russ Cox: Looks good to me, approved