Skip to content

cmd/compile/internal/walk: unreachable branch generated for select cases #50823

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
zhouguangyuan0718 opened this issue Jan 26, 2022 · 4 comments
Closed

cmd/compile/internal/walk: unreachable branch generated for select cases #50823

zhouguangyuan0718 opened this issue Jan 26, 2022 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.19

Comments

@zhouguangyuan0718
Copy link
Contributor

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

$ go version
go version go1.17.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/mnt/d/go/"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/mnt/d/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/mnt/d/demo/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1730011016=/tmp/go-build -gno-record-gcc-switches"

What did you do?

demo.go:

package demo

func F(a, b chan int) int {
        select {
        case t := <-a:
                return t
        case t := <-b:
                return t
        }
}

Then I run "go tool compile -S demo.go" and got:

"".F STEXT size=180 args=0x10 locals=0x70 funcid=0x0
        0x0000 00000 (demo.go:3)        TEXT    "".F(SB), ABIInternal, $112-16
        0x0000 00000 (demo.go:3)        CMPQ    SP, 16(R14)
        0x0004 00004 (demo.go:3)        PCDATA  $0, $-2
        0x0004 00004 (demo.go:3)        JLS     145
        0x000a 00010 (demo.go:3)        PCDATA  $0, $-1
        0x000a 00010 (demo.go:3)        SUBQ    $112, SP
        0x000e 00014 (demo.go:3)        MOVQ    BP, 104(SP)
        0x0013 00019 (demo.go:3)        LEAQ    104(SP), BP
        0x0018 00024 (demo.go:3)        FUNCDATA        $0, gclocals·dc9b0298814590ca3ffc3a889546fc8b(SB)
        0x0018 00024 (demo.go:3)        FUNCDATA        $1, gclocals·955e25ebd21d9d330257196a49a276e5(SB)
        0x0018 00024 (demo.go:3)        FUNCDATA        $2, "".F.stkobj(SB)
        0x0018 00024 (demo.go:3)        FUNCDATA        $5, "".F.arginfo1(SB)
        0x0018 00024 (demo.go:4)        LEAQ    ""..autotmp_9+72(SP), DX
        0x001d 00029 (demo.go:4)        MOVUPS  X15, (DX)
        0x0021 00033 (demo.go:4)        LEAQ    ""..autotmp_9+88(SP), DX
        0x0026 00038 (demo.go:4)        MOVUPS  X15, (DX)
        0x002a 00042 (demo.go:5)        MOVQ    AX, ""..autotmp_9+88(SP)
        0x002f 00047 (demo.go:5)        LEAQ    ""..autotmp_6+56(SP), DX
        0x0034 00052 (demo.go:5)        MOVQ    DX, ""..autotmp_9+96(SP)
        0x0039 00057 (demo.go:7)        MOVQ    BX, ""..autotmp_9+72(SP)
        0x003e 00062 (demo.go:7)        LEAQ    ""..autotmp_8+48(SP), DX
        0x0043 00067 (demo.go:7)        MOVQ    DX, ""..autotmp_9+80(SP)
        0x0048 00072 (demo.go:4)        LEAQ    ""..autotmp_9+72(SP), AX
        0x004d 00077 (demo.go:4)        LEAQ    ""..autotmp_10+64(SP), BX
        0x0052 00082 (demo.go:4)        XORL    CX, CX
        0x0054 00084 (demo.go:4)        XORL    DI, DI
        0x0056 00086 (demo.go:4)        MOVL    $2, SI
        0x005b 00091 (demo.go:4)        MOVL    $1, R8
        0x0061 00097 (demo.go:4)        PCDATA  $1, $1
        0x0061 00097 (demo.go:4)        CALL    runtime.selectgo(SB)
        0x0066 00102 (demo.go:7)        TESTQ   AX, AX
        0x0069 00105 (demo.go:7)        JEQ     129
        0x006b 00107 (demo.go:5)        CMPQ    AX, $1
        0x006f 00111 (demo.go:5)        JNE     144
        0x0071 00113 (demo.go:5)        MOVQ    ""..autotmp_6+56(SP), AX
        0x0076 00118 (demo.go:6)        MOVQ    104(SP), BP
        0x007b 00123 (demo.go:6)        ADDQ    $112, SP
        0x007f 00127 (demo.go:6)        NOP
        0x0080 00128 (demo.go:6)        RET
        0x0081 00129 (demo.go:7)        MOVQ    ""..autotmp_8+48(SP), AX
        0x0086 00134 (demo.go:8)        MOVQ    104(SP), BP
        0x008b 00139 (demo.go:8)        ADDQ    $112, SP
        0x008f 00143 (demo.go:8)        RET
        0x0090 00144 (demo.go:5)        PCDATA  $1, $-1
        0x0090 00144 (demo.go:5)        XCHGL   AX, AX
        0x0091 00145 (demo.go:5)        NOP
        0x0091 00145 (demo.go:3)        PCDATA  $1, $-1
        0x0091 00145 (demo.go:3)        PCDATA  $0, $-2
        0x0091 00145 (demo.go:3)        MOVQ    AX, 8(SP)
        0x0096 00150 (demo.go:3)        MOVQ    BX, 16(SP)
        0x009b 00155 (demo.go:3)        NOP
        0x00a0 00160 (demo.go:3)        CALL    runtime.morestack_noctxt(SB)
        0x00a5 00165 (demo.go:3)        MOVQ    8(SP), AX
        0x00aa 00170 (demo.go:3)        MOVQ    16(SP), BX
        0x00af 00175 (demo.go:3)        PCDATA  $0, $-1
        0x00af 00175 (demo.go:3)        JMP     0
        0x0000 49 3b 66 10 0f 86 87 00 00 00 48 83 ec 70 48 89  I;f.......H..pH.
        0x0010 6c 24 68 48 8d 6c 24 68 48 8d 54 24 48 44 0f 11  l$hH.l$hH.T$HD..
        0x0020 3a 48 8d 54 24 58 44 0f 11 3a 48 89 44 24 58 48  :H.T$XD..:H.D$XH
        0x0030 8d 54 24 38 48 89 54 24 60 48 89 5c 24 48 48 8d  .T$8H.T$`H.\$HH.
        0x0040 54 24 30 48 89 54 24 50 48 8d 44 24 48 48 8d 5c  T$0H.T$PH.D$HH.\
        0x0050 24 40 31 c9 31 ff be 02 00 00 00 41 b8 01 00 00  $@1.1......A....
        0x0060 00 e8 00 00 00 00 48 85 c0 74 16 48 83 f8 01 75  ......H..t.H...u
        0x0070 1f 48 8b 44 24 38 48 8b 6c 24 68 48 83 c4 70 90  .H.D$8H.l$hH..p.
        0x0080 c3 48 8b 44 24 30 48 8b 6c 24 68 48 83 c4 70 c3  .H.D$0H.l$hH..p.
        0x0090 90 48 89 44 24 08 48 89 5c 24 10 0f 1f 44 00 00  .H.D$.H.\$...D..
        0x00a0 e8 00 00 00 00 48 8b 44 24 08 48 8b 5c 24 10 e9  .....H.D$.H.\$..
        0x00b0 4c ff ff ff                                      L...
        rel 98+4 t=7 runtime.selectgo+0
        rel 161+4 t=7 runtime.morestack_noctxt+0

What did you expect to see?

There is one CMP or TEST instruction after call runtime.selectgo, because we can distinguish two branch by comparing once.

What did you see instead?

        0x0061 00097 (demo.go:4)        CALL    runtime.selectgo(SB)
        0x0066 00102 (demo.go:7)        TESTQ   AX, AX
        0x0069 00105 (demo.go:7)        JEQ     129
        0x006b 00107 (demo.go:5)        CMPQ    AX, $1
        0x006f 00111 (demo.go:5)        JNE     144
        0x0071 00113 (demo.go:5)        MOVQ    ""..autotmp_6+56(SP), AX
        0x0076 00118 (demo.go:6)        MOVQ    104(SP), BP
        0x007b 00123 (demo.go:6)        ADDQ    $112, SP
        0x007f 00127 (demo.go:6)        NOP
        0x0080 00128 (demo.go:6)        RET
        0x0081 00129 (demo.go:7)        MOVQ    ""..autotmp_8+48(SP), AX
        0x0086 00134 (demo.go:8)        MOVQ    104(SP), BP
        0x008b 00139 (demo.go:8)        ADDQ    $112, SP
        0x008f 00143 (demo.go:8)        RET
        0x0090 00144 (demo.go:5)        PCDATA  $1, $-1
        0x0090 00144 (demo.go:5)        XCHGL   AX, AX
        0x0091 00145 (demo.go:5)        NOP
        0x0091 00145 (demo.go:3)        PCDATA  $1, $-1
        0x0091 00145 (demo.go:3)        PCDATA  $0, $-2
        0x0091 00145 (demo.go:3)        MOVQ    AX, 8(SP)
        0x0096 00150 (demo.go:3)        MOVQ    BX, 16(SP)
        0x009b 00155 (demo.go:3)        NOP
        0x00a0 00160 (demo.go:3)        CALL    runtime.morestack_noctxt(SB)
        0x00a5 00165 (demo.go:3)        MOVQ    8(SP), AX
        0x00aa 00170 (demo.go:3)        MOVQ    16(SP), BX
        0x00af 00175 (demo.go:3)        PCDATA  $0, $-1
        0x00af 00175 (demo.go:3)        JMP     0

Thers is an unreachable branch after call runtime.selectgo, it's "0x006f JNE 144", 144 is the instruction between ret and morestack check. The instruction "CMPQ AX, $1" and "JNE 144" should not been generated or it should jmp to a panic call.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/380894 mentions this issue: cmd/compile: avoid generating unreachable branch for select cases

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 26, 2022
@mknyszek mknyszek added this to the Backlog milestone Jan 26, 2022
@mknyszek
Copy link
Contributor

CC @randall77 and @cherrymui maybe?

@zhouguangyuan0718
Copy link
Contributor Author

With my patch, the result for the example in #50823 (comment) is:

"".F STEXT size=165 args=0x10 locals=0x70 funcid=0x0 align=0x0
        0x0000 00000 (issue50823.go:3)  TEXT    "".F(SB), ABIInternal, $112-16
        0x0000 00000 (issue50823.go:3)  CMPQ    SP, 16(R14)
        0x0004 00004 (issue50823.go:3)  PCDATA  $0, $-2
        0x0004 00004 (issue50823.go:3)  JLS     131
        0x0006 00006 (issue50823.go:3)  PCDATA  $0, $-1
        0x0006 00006 (issue50823.go:3)  SUBQ    $112, SP
        0x000a 00010 (issue50823.go:3)  MOVQ    BP, 104(SP)
        0x000f 00015 (issue50823.go:3)  LEAQ    104(SP), BP
        0x0014 00020 (issue50823.go:3)  FUNCDATA        $0, gclocals·dc9b0298814590ca3ffc3a889546fc8b(SB)
        0x0014 00020 (issue50823.go:3)  FUNCDATA        $1, gclocals·955e25ebd21d9d330257196a49a276e5(SB)
        0x0014 00020 (issue50823.go:3)  FUNCDATA        $2, "".F.stkobj(SB)
        0x0014 00020 (issue50823.go:3)  FUNCDATA        $5, "".F.arginfo1(SB)
        0x0014 00020 (issue50823.go:3)  FUNCDATA        $6, "".F.argliveinfo(SB)
        0x0014 00020 (issue50823.go:3)  PCDATA  $3, $1
        0x0014 00020 (issue50823.go:4)  MOVUPS  X15, ""..autotmp_9+72(SP)
        0x001a 00026 (issue50823.go:4)  MOVUPS  X15, ""..autotmp_9+88(SP)
        0x0020 00032 (issue50823.go:5)  MOVQ    AX, ""..autotmp_9+88(SP)
        0x0025 00037 (issue50823.go:5)  LEAQ    ""..autotmp_6+56(SP), DX
        0x002a 00042 (issue50823.go:5)  MOVQ    DX, ""..autotmp_9+96(SP)
        0x002f 00047 (issue50823.go:7)  MOVQ    BX, ""..autotmp_9+72(SP)
        0x0034 00052 (issue50823.go:7)  LEAQ    ""..autotmp_8+48(SP), DX
        0x0039 00057 (issue50823.go:7)  MOVQ    DX, ""..autotmp_9+80(SP)
        0x003e 00062 (issue50823.go:4)  LEAQ    ""..autotmp_9+72(SP), AX
        0x0043 00067 (issue50823.go:4)  LEAQ    ""..autotmp_10+64(SP), BX
        0x0048 00072 (issue50823.go:4)  XORL    CX, CX
        0x004a 00074 (issue50823.go:4)  XORL    DI, DI
        0x004c 00076 (issue50823.go:4)  MOVL    $2, SI
        0x0051 00081 (issue50823.go:4)  MOVL    $1, R8
        0x0057 00087 (issue50823.go:4)  PCDATA  $1, $1
        0x0057 00087 (issue50823.go:4)  CALL    runtime.selectgo(SB)
        0x005c 00092 (issue50823.go:4)  NOP
        0x0060 00096 (issue50823.go:7)  TESTQ   AX, AX
        0x0063 00099 (issue50823.go:7)  JNE     116
        0x0065 00101 (issue50823.go:7)  MOVQ    ""..autotmp_8+48(SP), AX
        0x006a 00106 (issue50823.go:8)  MOVQ    104(SP), BP
        0x006f 00111 (issue50823.go:8)  ADDQ    $112, SP
        0x0073 00115 (issue50823.go:8)  RET
        0x0074 00116 (issue50823.go:5)  MOVQ    ""..autotmp_6+56(SP), AX
        0x0079 00121 (issue50823.go:6)  MOVQ    104(SP), BP
        0x007e 00126 (issue50823.go:6)  ADDQ    $112, SP
        0x0082 00130 (issue50823.go:6)  RET
        0x0083 00131 (issue50823.go:6)  NOP
        0x0083 00131 (issue50823.go:3)  PCDATA  $1, $-1
        0x0083 00131 (issue50823.go:3)  PCDATA  $0, $-2
        0x0083 00131 (issue50823.go:3)  MOVQ    AX, 8(SP)
        0x0088 00136 (issue50823.go:3)  MOVQ    BX, 16(SP)
        0x008d 00141 (issue50823.go:3)  CALL    runtime.morestack_noctxt(SB)
        0x0092 00146 (issue50823.go:3)  MOVQ    8(SP), AX
        0x0097 00151 (issue50823.go:3)  MOVQ    16(SP), BX
        0x009c 00156 (issue50823.go:3)  PCDATA  $0, $-1
        0x009c 00156 (issue50823.go:3)  NOP
        0x00a0 00160 (issue50823.go:3)  JMP     0

We only need one comparation for two cases.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/385516 mentions this issue: test: add a testcase for #50823

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 4, 2022
@dmitshur dmitshur modified the milestones: Backlog, Go1.19 Mar 4, 2022
@golang golang locked and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @gopherbot @zhouguangyuan0718