Go version
go version go1.22.3 darwin/arm64
Output of go env in your module/workspace:
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/jhorsts/Library/Caches/go-build'
GOENV='/Users/jhorsts/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/jhorsts/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/jhorsts/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/jhorsts/projects/playground/x/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g -I/Users/jhorsts/projects/others/freetds-1.3.11/include -I/opt/homebrew/Cellar/unixodbc/2.3.12/include'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/v0/8cfqwhrd5zdg6q7vz501vgl80000gn/T/go-build2944401573=/tmp/go-build -gno-record-gcc-switches -fno-common'
What did you do?
In Found, the bounds check is not eliminated in the switch having multiple values per case. In NotFound, it is.
package x
func Found(x []string) string {
switch len(x) {
default:
return x[0] // ./x.go:6:11: Found IsInBounds
case 0, 1:
return ""
}
}
func NotFound(x []string) string {
switch len(x) {
default:
return x[0]
case 0:
return ""
case 1:
return ""
}
}
What did you see happen?
➜ go build -gcflags="-d=ssa/check_bce/debug=1" .
./x.go:6:11: Found IsInBounds
➜
What did you expect to see?
➜ go build -gcflags="-d=ssa/check_bce/debug=1" .
➜
No bounds check.
Comment From: zigo101
Such cases might be too vast to be handled by the compiler. Consider that the constant index may be any positive integer ... So my suggestion is to write code in BCE friendly way.
[edit] But here, as the second way has been handled whereas the first one has not, maybe it is worth making them consistent.
Comment From: janishorsts
@go101, I noticed the missed BCE in a much more complex switch statement - I provided minimal code to demonstrate it. In the end, I got rid of the switch statement after refactoring.
GitHub reveals it is more widespread than I anticipated (not all found files contain missed BCE) - https://github.com/search?q=%22case+0%2C+1%22+AND+%22len%28%22+AND+language%3AGo+&type=code
Comment From: randall77
In fact there is no bounds check in the generated code. -d=ssa/check_bce/debug=1 is lying. Or at least, it isn't correctly reporting the removal of the bounds check after the check_bce detection pass.
So it might help if someone wants to look into how we might fix that reporting issue.
The issue itself reminds me of #66826, which I've already started on a strategy to fix. This reporting issue may resolve itself after that issue is fixed, as I think it will push the bounds check removal earlier.
Comment From: janishorsts
Indeed. go tool compile -S x.go reveals no bound check. It slipped my mind to check the compiled output.
<unlinkable>.Found STEXT size=48 args=0x18 locals=0x0 funcid=0x0 align=0x0 leaf
0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) TEXT <unlinkable>.Found(SB), LEAF|NOFRAME|ABIInternal, $0-24
0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) MOVD R0, <unlinkable>.x(FP)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) FUNCDATA $0, gclocals·wgcWObbY2HYnK2SU/U22lA==(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) FUNCDATA $1, gclocals·J5F+7Qw7O7ve2QcWC7DpeQ==(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) FUNCDATA $5, <unlinkable>.Found.arginfo1(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) FUNCDATA $6, <unlinkable>.Found.argliveinfo(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:3) PCDATA $3, $1
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:7) CMP $1, R1
0x0008 00008 (/Users/jhorsts/projects/playground/bce-switch/x.go:7) BGT 24
0x000c 00012 (/Users/jhorsts/projects/playground/bce-switch/x.go:8) MOVD ZR, R0
0x0010 00016 (/Users/jhorsts/projects/playground/bce-switch/x.go:8) MOVD ZR, R1
0x0014 00020 (/Users/jhorsts/projects/playground/bce-switch/x.go:8) RET (R30)
0x0018 00024 (/Users/jhorsts/projects/playground/bce-switch/x.go:6) MOVD (R0), R2
0x001c 00028 (/Users/jhorsts/projects/playground/bce-switch/x.go:6) MOVD 8(R0), R1
0x0020 00032 (/Users/jhorsts/projects/playground/bce-switch/x.go:6) MOVD R2, R0
0x0024 00036 (/Users/jhorsts/projects/playground/bce-switch/x.go:6) RET (R30)
0x0000 e0 07 00 f9 3f 04 00 f1 8c 00 00 54 e0 03 1f aa ....?......T....
0x0010 e1 03 1f aa c0 03 5f d6 02 00 40 f9 01 04 40 f9 ......_...@...@.
0x0020 e0 03 02 aa c0 03 5f d6 00 00 00 00 00 00 00 00 ......_.........
<unlinkable>.NotFound STEXT size=64 args=0x18 locals=0x0 funcid=0x0 align=0x0 leaf
0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) TEXT <unlinkable>.NotFound(SB), LEAF|NOFRAME|ABIInternal, $0-24
0x0000 00000 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) MOVD R0, <unlinkable>.x(FP)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) FUNCDATA $0, gclocals·wgcWObbY2HYnK2SU/U22lA==(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) FUNCDATA $1, gclocals·J5F+7Qw7O7ve2QcWC7DpeQ==(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) FUNCDATA $5, <unlinkable>.NotFound.arginfo1(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) FUNCDATA $6, <unlinkable>.NotFound.argliveinfo(SB)
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:12) PCDATA $3, $1
0x0004 00004 (/Users/jhorsts/projects/playground/bce-switch/x.go:16) CBZ R1, 44
0x0008 00008 (/Users/jhorsts/projects/playground/bce-switch/x.go:18) CMP $1, R1
0x000c 00012 (/Users/jhorsts/projects/playground/bce-switch/x.go:18) BNE 28
0x0010 00016 (/Users/jhorsts/projects/playground/bce-switch/x.go:19) MOVD ZR, R0
0x0014 00020 (/Users/jhorsts/projects/playground/bce-switch/x.go:19) MOVD ZR, R1
0x0018 00024 (/Users/jhorsts/projects/playground/bce-switch/x.go:19) RET (R30)
0x001c 00028 (/Users/jhorsts/projects/playground/bce-switch/x.go:15) MOVD (R0), R2
0x0020 00032 (/Users/jhorsts/projects/playground/bce-switch/x.go:15) MOVD 8(R0), R1
0x0024 00036 (/Users/jhorsts/projects/playground/bce-switch/x.go:15) MOVD R2, R0
0x0028 00040 (/Users/jhorsts/projects/playground/bce-switch/x.go:15) RET (R30)
0x002c 00044 (/Users/jhorsts/projects/playground/bce-switch/x.go:17) MOVD ZR, R0
0x0030 00048 (/Users/jhorsts/projects/playground/bce-switch/x.go:17) MOVD ZR, R1
0x0034 00052 (/Users/jhorsts/projects/playground/bce-switch/x.go:17) RET (R30)
0x0000 e0 07 00 f9 41 01 00 b4 3f 04 00 f1 81 00 00 54 ....A...?......T
0x0010 e0 03 1f aa e1 03 1f aa c0 03 5f d6 02 00 40 f9 .........._...@.
0x0020 01 04 40 f9 e0 03 02 aa c0 03 5f d6 e0 03 1f aa ..@......._.....
0x0030 e1 03 1f aa c0 03 5f d6 00 00 00 00 00 00 00 00 ......_.........
go:cuinfo.producer.<unlinkable> SDWARFCUINFO dupok size=0
0x0000 72 65 67 61 62 69 regabi
go:cuinfo.packagename.<unlinkable> SDWARFCUINFO dupok size=0
0x0000 78 x
gclocals·wgcWObbY2HYnK2SU/U22lA== SRODATA dupok size=10
0x0000 02 00 00 00 01 00 00 00 01 00 ..........
gclocals·J5F+7Qw7O7ve2QcWC7DpeQ== SRODATA dupok size=8
0x0000 02 00 00 00 00 00 00 00 ........
<unlinkable>.Found.arginfo1 SRODATA static dupok size=9
0x0000 fe 00 08 08 08 10 08 fd ff .........
<unlinkable>.Found.argliveinfo SRODATA static dupok size=2
0x0000 00 00 ..
<unlinkable>.NotFound.arginfo1 SRODATA static dupok size=9
0x0000 fe 00 08 08 08 10 08 fd ff .........
<unlinkable>.NotFound.argliveinfo SRODATA static dupok size=2
0x0000 00 00 ..
Comment From: cuonglm
So it might help if someone wants to look into how we might fix that reporting issue.
For this issue, could we just move the check bce pass after late use, then just skipping invalid block when running checkbce?
Comment From: randall77
Yes, pushing checkbce later may help. I think it won't help -d=ssa/prove/debug=1 output though, so that may not be a full fix. (But maybe a full fix isn't needed.)
Comment From: gopherbot
Change https://go.dev/cl/598635 mentions this issue: cmd/compile: run checkbce after fuseLate pass