Gerrit Bot (Gerrit)

unread,

2:23 AM (20 hours ago) 2:23 AM

to goph...@pubsubhelper.golang.org, Zxilly Chou, golang-co...@googlegroups.com

Gerrit Bot has uploaded the change for review

Commit message

runtime: don't emit write barrier for code pointers in itabInit

itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.

Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.

Fixes #80472

Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5

GitHub-Last-Rev: d93905f293112ce7ddd6749e6c5b7d972df8f9c2

GitHub-Pull-Request: golang/go#80487

Change diff

diff --git a/src/runtime/iface.go b/src/runtime/iface.go
index 009f104..e853625 100644
--- a/src/runtime/iface.go
+++ b/src/runtime/iface.go
@@ -201,6 +201,11 @@
 // If !firstTime, itabInit will not write anything to m.Fun (see issue 65962).
 // It is ok to call this multiple times on the same m, even concurrently
 // (although it will only be called once with firstTime==true).
+//
+// itabInit stores only code pointers, so it must not emit a write barrier;
+// //go:nowritebarrier enforces that (see the methods store below).
+//
+//go:nowritebarrier
 func itabInit(m *itab, firstTime bool) string {
 	inter := m.Inter
 	typ := m.Type
@@ -214,7 +219,10 @@
 	nt := int(x.Mcount)
 	xmhdr := unsafe.Slice((*abi.Method)(add(unsafe.Pointer(x), uintptr(x.Moff))), nt)
 	j := 0
-	methods := unsafe.Slice((*unsafe.Pointer)(unsafe.Pointer(&m.Fun[0])), ni)
+	// These are code pointers, not heap pointers. Use a uintptr slice so the
+	// store emits no write barrier: on wasm a code pointer can look like a
+	// heap pointer and make the GC crash (see issue 80472).
+	methods := unsafe.Slice((*uintptr)(unsafe.Pointer(&m.Fun[0])), ni)
 	var fun0 unsafe.Pointer
 imethods:
 	for k := 0; k < ni; k++ {
@@ -240,7 +248,7 @@
 					if k == 0 {
 						fun0 = ifn // we'll set m.Fun[0] at the end
 					} else if firstTime {
-						methods[k] = ifn
+						methods[k] = uintptr(ifn)
 					}
 					continue imethods
 				}

Change information

Files:

M src/runtime/iface.go

Change size: S

Delta: 1 file changed, 10 insertions(+), 2 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

10:35 AM (12 hours ago) 10:35 AM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, Keith Randall, Carlos Amedee, Michael Knyszek, Michael Pratt, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Carlos Amedee, Michael Knyszek and Michael Pratt

Keith Randall voted and added 1 comment

Votes added by Keith Randall

Auto-Submit	+1
Code-Review	+2
Commit-Queue	+1

1 comment

File src/runtime/iface.go

Line 225, Patchset 1 (Latest): methods := unsafe.Slice((*uintptr)(unsafe.Pointer(&m.Fun[0])), ni)

Keith Randall . unresolved

This can all be just `&m.Fun[0]`, I think.

Open in Gerrit

Related details

Attention is currently required from:

Carlos Amedee
Michael Knyszek
Michael Pratt

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

10:37 AM (12 hours ago) 10:37 AM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Carlos Amedee, Michael Knyszek, Michael Pratt, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Carlos Amedee, Michael Knyszek and Michael Pratt

Keith Randall voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Carlos Amedee
Michael Knyszek
Michael Pratt

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Gerrit Bot (Gerrit)

unread,

11:10 AM (12 hours ago) 11:10 AM

to Zxilly Chou, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention needed from Carlos Amedee, Keith Randall, Keith Randall, Michael Knyszek and Michael Pratt

Gerrit Bot uploaded new patchset

Gerrit Bot uploaded patch set #2 to this change.

Following approvals got outdated and were removed:

Code-Review: +2 by Keith Randall, +1 by Keith Randall
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com

Open in Gerrit

Related details

Attention is currently required from:

Carlos Amedee
Keith Randall
Keith Randall
Michael Knyszek
Michael Pratt

Submit Requirements:

Code-Review

No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

11:59 AM (11 hours ago) 11:59 AM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, Keith Randall, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Carlos Amedee, Michael Knyszek, Michael Pratt, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Carlos Amedee, Keith Randall, Michael Knyszek and Michael Pratt

Keith Randall voted and added 1 comment

Votes added by Keith Randall

Auto-Submit	+1
Code-Review	+2
Commit-Queue	+1

1 comment

File src/runtime/iface.go

Line 225, Patchset 1: methods := unsafe.Slice((*uintptr)(unsafe.Pointer(&m.Fun[0])), ni)

Keith Randall . resolved

This can all be just `&m.Fun[0]`, I think.

Keith Randall

Done

Open in Gerrit

Related details

Attention is currently required from:

Carlos Amedee
Keith Randall
Michael Knyszek
Michael Pratt

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

12:00 PM (11 hours ago) 12:00 PM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, Keith Randall, golang...@luci-project-accounts.iam.gserviceaccount.com, Carlos Amedee, Michael Knyszek, Michael Pratt, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Carlos Amedee, Michael Knyszek and Michael Pratt

Keith Randall voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Carlos Amedee
Michael Knyszek
Michael Pratt

satisfied_requirement

unsatisfied_requirement

open

diffy

Carlos Amedee (Gerrit)

unread,

5:29 PM (5 hours ago) 5:29 PM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Keith Randall, Michael Knyszek, Michael Pratt, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Michael Knyszek and Michael Pratt

Carlos Amedee voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Michael Knyszek
Michael Pratt

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Gopher Robot (Gerrit)

unread,

5:33 PM (5 hours ago) 5:33 PM

to Zxilly Chou, Gerrit Bot, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Carlos Amedee, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Keith Randall, Michael Knyszek, Michael Pratt, golang-co...@googlegroups.com

Gopher Robot submitted the change

Change information

Commit message:

runtime: don't emit write barrier for code pointers in itabInit

itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.

Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.

Fixes #80472

Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5

GitHub-Last-Rev: 6e1374c083518bb9f994092acca4f5ea28b38dfa

GitHub-Pull-Request: golang/go#80487

Reviewed-on: https://go-review.googlesource.com/c/go/+/803460

Reviewed-by: Keith Randall <k...@google.com>

Auto-Submit: Keith Randall <k...@golang.org>

Reviewed-by: Carlos Amedee <car...@golang.org>

LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>

Reviewed-by: Keith Randall <k...@golang.org>

Files:

M src/runtime/iface.go

Change size: S

Delta: 1 file changed, 10 insertions(+), 2 deletions(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +2 by Keith Randall, +1 by Carlos Amedee, +1 by Keith Randall
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Keith Randall (Gerrit)

unread,

5:57 PM (5 hours ago) 5:57 PM

to goph...@pubsubhelper.golang.org, Zxilly Chou, Keith Randall, golang-co...@googlegroups.com

Keith Randall has uploaded the change for review

Commit message

[release-branch.go1.27] runtime: don't emit write barrier for code pointers in itabInit



itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.

Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.

Fixes #80498



Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5
GitHub-Last-Rev: 6e1374c083518bb9f994092acca4f5ea28b38dfa
GitHub-Pull-Request: golang/go#80487
Reviewed-on: https://go-review.googlesource.com/c/go/+/803460
Reviewed-by: Keith Randall <k...@google.com>
Auto-Submit: Keith Randall <k...@golang.org>
Reviewed-by: Carlos Amedee <car...@golang.org>
LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <k...@golang.org>

(cherry picked from commit 9d2ed75ac043fb1589082f51bc43d927d6c3f1c4)

Change diff

diff --git a/src/runtime/iface.go b/src/runtime/iface.go
index 009f104..6385d1c 100644


--- a/src/runtime/iface.go
+++ b/src/runtime/iface.go
@@ -201,6 +201,11 @@
 // If !firstTime, itabInit will not write anything to m.Fun (see issue 65962).
 // It is ok to call this multiple times on the same m, even concurrently
 // (although it will only be called once with firstTime==true).
+//
+// itabInit stores only code pointers, so it must not emit a write barrier;
+// //go:nowritebarrier enforces that (see the methods store below).
+//
+//go:nowritebarrier
 func itabInit(m *itab, firstTime bool) string {
 	inter := m.Inter
 	typ := m.Type
@@ -214,7 +219,10 @@
 	nt := int(x.Mcount)
 	xmhdr := unsafe.Slice((*abi.Method)(add(unsafe.Pointer(x), uintptr(x.Moff))), nt)
 	j := 0
-	methods := unsafe.Slice((*unsafe.Pointer)(unsafe.Pointer(&m.Fun[0])), ni)
+	// These are code pointers, not heap pointers. Use a uintptr slice so the
+	// store emits no write barrier: on wasm a code pointer can look like a
+	// heap pointer and make the GC crash (see issue 80472).

+	methods := unsafe.Slice(&m.Fun[0], ni)


 	var fun0 unsafe.Pointer
 imethods:
 	for k := 0; k < ni; k++ {
@@ -240,7 +248,7 @@
 					if k == 0 {
 						fun0 = ifn // we'll set m.Fun[0] at the end
 					} else if firstTime {
-						methods[k] = ifn
+						methods[k] = uintptr(ifn)
 					}
 					continue imethods
 				}

Change information

Files:

M src/runtime/iface.go

Change size: S

Delta: 1 file changed, 10 insertions(+), 2 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

5:57 PM (5 hours ago) 5:57 PM

to Keith Randall, Zxilly Chou, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Keith Randall voted

Auto-Submit	+1
Commit-Queue	+1

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

6:01 PM (5 hours ago) 6:01 PM

to goph...@pubsubhelper.golang.org, Zxilly Chou, Keith Randall, golang-co...@googlegroups.com

Keith Randall has uploaded the change for review

Commit message

[release-branch.go1.25] runtime: don't emit write barrier for code pointers in itabInit



itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.

Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.

Fixes #80500

Change-Id: If9532c01b66b8c4ceb47c932017569488b6143d5

GitHub-Last-Rev: 6e1374c083518bb9f994092acca4f5ea28b38dfa

GitHub-Pull-Request: golang/go#80487

Reviewed-on: https://go-review.googlesource.com/c/go/+/803460

Reviewed-by: Keith Randall <k...@google.com>

Auto-Submit: Keith Randall <k...@golang.org>

Reviewed-by: Carlos Amedee <car...@golang.org>

LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>

Reviewed-by: Keith Randall <k...@golang.org>

Change diff

diff --git a/src/runtime/iface.go b/src/runtime/iface.go
index 0665c4b..b810dae 100644


--- a/src/runtime/iface.go
+++ b/src/runtime/iface.go
@@ -201,6 +201,11 @@
 // If !firstTime, itabInit will not write anything to m.Fun (see issue 65962).
 // It is ok to call this multiple times on the same m, even concurrently
 // (although it will only be called once with firstTime==true).
+//
+// itabInit stores only code pointers, so it must not emit a write barrier;
+// //go:nowritebarrier enforces that (see the methods store below).
+//
+//go:nowritebarrier
 func itabInit(m *itab, firstTime bool) string {
 	inter := m.Inter
 	typ := m.Type
@@ -214,7 +219,10 @@
 	nt := int(x.Mcount)

 	xmhdr := (*[1 << 16]abi.Method)(add(unsafe.Pointer(x), uintptr(x.Moff)))[:nt:nt]
 	j := 0
-	methods := (*[1 << 16]unsafe.Pointer)(unsafe.Pointer(&m.Fun[0]))[:ni:ni]


+	// These are code pointers, not heap pointers. Use a uintptr slice so the
+	// store emits no write barrier: on wasm a code pointer can look like a
+	// heap pointer and make the GC crash (see issue 80472).

+	methods := (*[1 << 16]uintptr)(unsafe.Pointer(&m.Fun[0]))[:ni:ni]


 	var fun0 unsafe.Pointer
 imethods:
 	for k := 0; k < ni; k++ {
@@ -240,7 +248,7 @@
 					if k == 0 {
 						fun0 = ifn // we'll set m.Fun[0] at the end
 					} else if firstTime {
-						methods[k] = ifn
+						methods[k] = uintptr(ifn)
 					}
 					continue imethods
 				}

Change information

Files:

M src/runtime/iface.go

Change size: S

Delta: 1 file changed, 10 insertions(+), 2 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Keith Randall (Gerrit)

unread,

6:02 PM (5 hours ago) 6:02 PM

to goph...@pubsubhelper.golang.org, Zxilly Chou, Keith Randall, golang-co...@googlegroups.com

Keith Randall has uploaded the change for review

Commit message

[release-branch.go1.26] runtime: don't emit write barrier for code pointers in itabInit



itabInit filled m.Fun by storing code pointers through an
unsafe.Pointer slice, which makes the compiler emit a write barrier.
On wasm a code PC is a function index shifted left 16 bits, a small
value that can fall inside a live heap span, so the GC mistakes it
for a bad heap pointer and crashes.

Store through a uintptr slice instead so no write barrier is emitted,
and mark itabInit //go:nowritebarrier so the same mistake fails to
compile.

Fixes #80499

unsatisfied_requirement

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[go] runtime: don't emit write barrier for code pointers in itabInit

Gerrit Bot (Gerrit)

Gerrit Bot has uploaded the change for review

Commit message

Change diff

Change information

Related details

Keith Randall (Gerrit)

Keith Randall voted and added 1 comment

Votes added by Keith Randall

1 comment

Related details

Keith Randall (Gerrit)

Keith Randall voted Code-Review+1

Related details

Gerrit Bot (Gerrit)

Gerrit Bot uploaded new patchset

Related details

Keith Randall (Gerrit)

Keith Randall voted and added 1 comment

Votes added by Keith Randall

1 comment

Related details

Keith Randall (Gerrit)

Keith Randall voted Code-Review+1

Related details

Carlos Amedee (Gerrit)

Carlos Amedee voted Code-Review+1

Related details

Gopher Robot (Gerrit)

Gopher Robot submitted the change

Change information

Keith Randall (Gerrit)

Keith Randall has uploaded the change for review

Commit message

Change diff

Change information

Related details

Keith Randall (Gerrit)

Keith Randall voted

Related details

Keith Randall (Gerrit)

Keith Randall has uploaded the change for review

Commit message

Change diff

Change information

Related details

Keith Randall (Gerrit)

Keith Randall has uploaded the change for review

Commit message