Skip to content
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

cmd/compile: bad compilation #15854

Closed
randall77 opened this issue May 26, 2016 · 3 comments
Closed

cmd/compile: bad compilation #15854

randall77 opened this issue May 26, 2016 · 3 comments
Labels
FrozenDueToAge
Milestone
Go1.7Beta

Comments

@randall77
Copy link
Contributor 

package main

import (
    "fmt"
)

type CIString struct {
    _   [0]struct{ notComparable []byte }
    Val string
}

// New creates a new CIString.
func New(str string) CIString {
    return CIString{Val: str}
}

var a = New("ID")

func main() {
    if a.Val != "ID" {
        fmt.Printf("bad: %d %s\n", len(a.Val), a.Val)
    }
}

When run, it often (with GOGC=1) prints the "bad" message and has varying junk for the string contents (but the length is always right).
The bug goes away with -gcflags=-ssa=0

@dsnet
@randall77 randall77 added this to the Go1.7 milestone May 26, 2016
@randall77 randall77 self-assigned this May 26, 2016
@randall77
Copy link
Contributor Author

GOSSAHASH says it is an init function that is bad. There are 11 of them :( Maybe time to add the package name to the thing GOSSAHASH checks...

@randall77
Copy link
Contributor Author

Looks like the write barrier is wrong for the store to the global. It's storing junk instead of the result of the New("ID") call.

    00146 (init.go:24)  TEXT    "".init(SB), $0
    00147 (init.go:24)  FUNCDATA    $0, "".gcargs·4(SB)
    00148 (init.go:24)  FUNCDATA    $1, "".gclocals·5(SB)
    00149 (init.go:24)  TYPE    "".autotmp_9(SP)type.*"".CIString, $8
v25 00150 (init.go:17)  LEAQ    16(SP), AX
v42 00151 (init.go:17)  MOVQ    AX, "".autotmp_9(SP)
v5  00152 (init.go:24)  MOVBLZX "".initdone·(SB), CX
v43 00153 (init.go:24)  CMPB    CX, $1
b1  00154 (init.go:24)  JLS $0, 156
b2  00155 (init.go:24)  RET
b3  00156 (init.go:24)  JNE $0, 158
v14 00157 (init.go:24)  CALL    runtime.throwinit(SB)
v18 00158 (init.go:24)  MOVB    $1, "".initdone·(SB)
v19 00159 (init.go:24)  CALL    fmt.init(SB)
v35 00160 (init.go:17)  LEAQ    go.string."ID"(SB), AX
v22 00161 (init.go:17)  MOVQ    AX, (SP)
v23 00162 (init.go:17)  MOVQ    $2, 8(SP)
v24 00163 (init.go:17)  CALL    "".New(SB)
v30 00164 (init.go:17)  MOVL    runtime.writeBarrier(SB), AX
v9  00165 (init.go:17)  TESTB   AX, AX
b8  00166 (init.go:17)  JNE $0, 171
v33 00167 (init.go:17)  MOVUPS  16(SP), X0
v41 00168 (init.go:17)  MOVUPS  X0, "".a(SB)
v46 00169 (init.go:24)  MOVB    $2, "".initdone·(SB)
b11 00170 (init.go:24)  RET
v21 00171 (init.go:17)  LEAQ    type."".CIString(SB), AX
v34 00172 (init.go:17)  MOVQ    AX, (SP)
v10 00173 (init.go:17)  LEAQ    "".a(SB), AX
v36 00174 (init.go:17)  MOVQ    AX, 8(SP)
v40 00175 (init.go:17)  MOVQ    "".autotmp_9(SP), AX
v38 00176 (init.go:17)  MOVQ    AX, 16(SP)
v39 00177 (init.go:17)  CALL    runtime.typedmemmove(SB)
b9  00178 (init.go:17)  JMP 169

The result of the New is 16 bytes big and at 16(SP) right after the call. When we branch to the write barrier, it sets up the call to typedmemmove. To do that, it needs to write 24 bytes of arguments. The last 8 of those overwrite the result of New (and hence clobber the string pointer). The result of New must be moved out of the return args section before the call to typedmemmove.

@rsc rsc modified the milestones: Go1.7Beta, Go1.7 May 27, 2016
@dsnet dsnet closed this as completed May 27, 2016
@dsnet dsnet reopened this May 27, 2016
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators May 27, 2017
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.7Beta
Development

No branches or pull requests
4 participants
@rsc @dsnet @randall77 @gopherbot