Skip to content

cmd/compile: missing write barrier on zeroing/assigning global #18956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cherrymui opened this issue Feb 6, 2017 · 5 comments
Closed

cmd/compile: missing write barrier on zeroing/assigning global #18956

cherrymui opened this issue Feb 6, 2017 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.9Early

Comments

@cherrymui
Copy link
Member

cherrymui commented Feb 6, 2017 •
edited
Loading

What version of Go are you using (go version)?

tip (go version devel +47d2a4dafa Mon Feb 6 07:21:14 2017 +0000 darwin/amd64)

For this code

type T struct {
        p *int
}

var t T

func f() {
        t = T{}
}

the compiler does not generate write barrier.

before f
.   AS l(10) tc(1)
.   .   NAME-main.t u(1) a(true) l(7) x(0) class(PEXTERN) tc(1) assigned main.T
.   .   STRUCTLIT l(10) tc(1) main.T
.   .   .   TYPE main.T u(1) a(true) l(3) x(0) class(PEXTERN) tc(1) type=main.T main.T main.T
after walk f
.   EMPTY-init
.   .   AS u(2) l(10) tc(1)
.   .   .   NAME-main.t u(1) a(true) l(7) x(0) class(PEXTERN) tc(1) assigned main.T
.   EMPTY u(100) l(10) tc(1)
.   .   NAME-main.t u(1) a(true) l(7) x(0) class(PEXTERN) tc(1) assigned main.T

   	00000 (x.go:9)	TEXT	"".f(SB)
   	00001 (x.go:9)	FUNCDATA	$0, "".gcargs·0(SB)
   	00002 (x.go:9)	FUNCDATA	$1, "".gclocals·1(SB)
v8	00003 (x.go:10)	MOVQ	$0, "".t(SB)
b1	00004 (x.go:11)	RET

But I believe it should have write barrier for the new hybrid barrier.
cc @aclements
@cherrymui cherrymui added this to the Go1.8 milestone Feb 6, 2017
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/36355 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/36410 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/36531 mentions this issue.

@rsc rsc reopened this Feb 7, 2017
@rsc
Copy link
Contributor

rsc commented Feb 7, 2017

CL 36530 pending for cherry-pick. Then this bug should be moved to Go 1.9 for the followup CL.

gopherbot pushed a commit that referenced this issue Feb 7, 2017
@cherrymui
[release-branch.go1.8] cmd/compile: do not use "oaslit" for global
1edfd64 
The compiler did not emit write barrier for assigning global with
struct literal, like global = T{} where T contains pointer.

The relevant code path is:
walkexpr OAS var_ OSTRUCTLIT
    oaslit
        anylit OSTRUCTLIT
            walkexpr OAS var_ nil
            return without adding write barrier
    return true
break (without adding write barrier)

This CL makes oaslit not apply to globals. See also CL
https://go-review.googlesource.com/c/36355/ for an alternative
fix.

The downside of this is that it generates static data for zeroing
struct now. Also this only covers global. If there is any lurking
bug with implicit zeroing other than globals, this doesn't fix.

Fixes #18956.

Change-Id: Ibcd27e4fae3aa38390ffa94a32a9dd7a802e4b37
Reviewed-on: https://go-review.googlesource.com/36410
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
(cherry picked from commit 160914e)
Reviewed-on: https://go-review.googlesource.com/36531
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Feb 7, 2017
@cherrymui
Copy link
Member Author

The new write barrier insertion in SSA shouldn't have this problem. Closing.

@golang golang locked and limited conversation to collaborators Mar 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9Early
Development

No branches or pull requests
3 participants
@rsc @gopherbot @cherrymui