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.

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

cmd/compile/internal/ssa: combined loads occur before some bounds checking #15002

Closed
mdempsky opened this issue Mar 29, 2016 · 1 comment

Comments

@mdempsky
Copy link
Member

commented Mar 29, 2016

Compiling:

package binary

func Uint16(b []byte) uint16 {
    return uint16(b[0]) | uint16(b[1])<<8
}

produces:

0x0000 00000 (binary.go:3)  TEXT    "".Uint16(SB), $0-32
0x0000 00000 (binary.go:3)  MOVQ    (TLS), CX
0x0009 00009 (binary.go:3)  CMPQ    SP, 16(CX)
0x000d 00013 (binary.go:3)  JLS 53
0x000f 00015 (binary.go:3)  NOP
0x000f 00015 (binary.go:3)  NOP
0x000f 00015 (binary.go:3)  FUNCDATA    $0, gclocals·2fccd208efe70893f9ac8d682812ae72(SB)
0x000f 00015 (binary.go:3)  FUNCDATA    $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x000f 00015 (binary.go:4)  MOVQ    "".b+16(FP), CX
0x0014 00020 (binary.go:4)  CMPQ    CX, $0
0x0018 00024 (binary.go:4)  JLS $0, 46
0x001a 00026 (binary.go:4)  MOVQ    "".b+8(FP), DX
0x001f 00031 (binary.go:4)  MOVWLZX (DX), DX
0x0022 00034 (binary.go:4)  CMPQ    CX, $1
0x0026 00038 (binary.go:4)  JLS $0, 46
0x0028 00040 (binary.go:4)  MOVW    DX, "".~r1+32(FP)
0x002d 00045 (binary.go:4)  RET
0x002e 00046 (binary.go:4)  PCDATA  $0, $0
0x002e 00046 (binary.go:4)  CALL    runtime.panicindex(SB)
0x0033 00051 (binary.go:4)  UNDEF
0x0035 00053 (binary.go:4)  NOP
0x0035 00053 (binary.go:3)  CALL    runtime.morestack_noctxt(SB)
0x003a 00058 (binary.go:3)  JMP 0

This is wrong, because the CMPQ CX, $1 check needs to happen before the MOVWLZX (DX), DX combined load. This could result in triggering a SIGSEGV failure when we should be raising a panicindex failure instead.

See also #15001.

@mdempsky mdempsky added this to the Go1.7 milestone Mar 29, 2016
@gopherbot

This comment has been minimized.

Copy link

commented Mar 29, 2016

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

@gopherbot gopherbot closed this in b81f2f1 Mar 31, 2016
@golang golang locked and limited conversation to collaborators Mar 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.