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

Jump to bottom

cmd/compile: -B on 32bit GOARCHs causes internal compiler error #48092

Closed
divVerent opened this issue Aug 31, 2021 · 6 comments
Closed

cmd/compile: -B on 32bit GOARCHs causes internal compiler error #48092

divVerent opened this issue Aug 31, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@divVerent
Copy link

divVerent commented Aug 31, 2021 •
edited

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

$ go version
go version go1.17 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="/home/rpolzer/.cache/go-build"
GOENV="/home/rpolzer/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rpolzer/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/rpolzer/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/rpolzer/homedir/src/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/rpolzer/homedir/src/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build1448838183=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ cat > hello.go
package main

func main() {
}
^D
$ GOOS=windows GOARCH=386 go build -gcflags='all=-B' hello.go

What did you expect to see?

A Win32 PE EXE file that does nothing.

What did you see instead?

# internal/abi
/home/rpolzer/homedir/src/go/src/internal/abi/abi.go:52:30: internal compiler error: '(*IntArgRegBitmap).Get': not lowered: v16, Unknown UINT8

Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new

Note: without the -B flag, the issue does not occur. On linux/amd64 and windows/amd64 -B is fine to use and improves performance.
@ALTree ALTree changed the title -gcflags -B on Win32 causes internal error cmd/compile: -B on GOARCH=386 causes internal compiler error Aug 31, 2021
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 31, 2021
@ALTree
Copy link
Member

ALTree commented Aug 31, 2021

It looks like the crash is triggered when compiling internal/abi:

$ GOARCH=386 go build -gcflags=-B internal/abi

# internal/abi
/usr/local/go/src/internal/abi/abi.go:42:29: internal compiler error: '(*IntArgRegBitmap).Set': not lowered: v14, Unknown UINT8

Tip also crashes. Other GOARCHs are also affected. Both arm and mipsle crash too.

cc @randall77 @mknyszek @aclements

@ALTree ALTree changed the title cmd/compile: -B on GOARCH=386 causes internal compiler error cmd/compile: -B on 32bit GOARCHs causes internal compiler error Aug 31, 2021
@ALTree ALTree added this to the Go1.18 milestone Aug 31, 2021
@randall77
Copy link
Contributor

This happens when we do:

type A [0]int
var a A
x := a[i]

The compiler uses an explicit "never use me" value for x, because it knows that the bounds check can never succeed, and thus the value of x must be on a dead path in the CFG. But then with -B, uses of x aren't on dead paths, and the compiler rightly complains it is trying to use a "never use me" value.

This happens in internal/abi for all the architectures that haven't been converted yet (everything except amd64 and arm64), because IntArgsReg is 0, and thus IntArgRegBitmap is [0]uint8.

It's not entirely clear to me that this is a bug worth fixing. -B is really a bleeding edge kind of request. Asserting to the compiler that all bounds checks will pass, while giving it code where the bounds check cannot possibly pass, is asking for trouble. That said, the fix is easy (use the zero value of x's type), and it's probably worth having the stdlib at least compile with -B, even if it would never run correctly like that.

@gopherbot
Copy link

Change https://golang.org/cl/346589 mentions this issue: cmd/compile: use the zero value for results of impossible indexing

@randall77
Copy link
Contributor

I don't think this is worth backporting, though. It's easy enough to work around by either not indexing 0-length arrays, or not using -B.

@divVerent
Copy link
Author

divVerent commented Aug 31, 2021 via email

@randall77
Copy link
Contributor

-B isn't really intended to be used by anybody. When you use -B the compiler isn't adhering to the Go spec any more. It's intended just to be used for measuring the overhead of bounds checks. We've been discussing removing -B altogether.

facebook-github-bot pushed a commit to facebook/openbmc that referenced this issue May 6, 2022
@doranand @facebook-github-bot
attempt to fix build in Sandcastle
513b661 
Summary:
The -B option under gcflags disables runtime bounds checking on arrays.
In itself this is not ideal plus doing so can apparently trigger the bug
mentioned at:

	golang/go#48092

Unfortunately this change does add some overhead.  Size comparison of old
and new binaries:

```
0 /tmp $ ls -l pp/flashy qq/flashy
-rwxr-xr-x 1 doranand users 2555904 May  2 04:14 pp/flashy
-rwxr-xr-x 1 doranand users 2949120 May  6 06:52 qq/flashy
```

Test Plan:
```
0 ~/local/openbmc/tools/flashy $ ./build.sh && ./build_dev.sh && go test ./... && echo it wokred
ok      github.com/facebook/openbmc/tools/flashy        1.288s
ok      github.com/facebook/openbmc/tools/flashy/checks_and_remediations/common (cached)
ok      github.com/facebook/openbmc/tools/flashy/checks_and_remediations/galaxy100      (cached)
ok      github.com/facebook/openbmc/tools/flashy/checks_and_remediations/wedge100       (cached)
ok      github.com/facebook/openbmc/tools/flashy/checks_and_remediations/yamp   (cached)
?       github.com/facebook/openbmc/tools/flashy/flash_procedure        [no test files]
ok      github.com/facebook/openbmc/tools/flashy/install        0.022s
ok      github.com/facebook/openbmc/tools/flashy/lib/fileutils  (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/flash      (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/flash/flashcp      (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/flash/flashutils   (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/flash/flashutils/devices   (cached)
?       github.com/facebook/openbmc/tools/flashy/lib/logger     [no test files]
ok      github.com/facebook/openbmc/tools/flashy/lib/step       (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/utils      (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/validate   (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/validate/image     (cached)
ok      github.com/facebook/openbmc/tools/flashy/lib/validate/partition (cached)
?       github.com/facebook/openbmc/tools/flashy/tests  [no test files]
?       github.com/facebook/openbmc/tools/flashy/utilities      [no test files]
it wokred
```

Build ephemeral fbpkg and use to test upgrade a BMC:

```
0 ~/local/openbmc $ fbpkg build -E openbmc.utils.flashy
fbpkg/client.py:5112: DeprecationWarning: 'click.get_terminal_size()' is deprecated and will be removed in Click 8.1. Use 'shutil.get_terminal_size()' instead.
2022-05-06 06:52:11,363 fbpkg.build INFO: found configerator config: openbmc/fbpkgs/utils/openbmc.utils.flashy.fbpkg
There are uncommitted changes in this repository.  See: https://fburl.com/fbpkg_local_changes . This may make debugging this version impossible.
Proceed? [y/N]: y
2022-05-06 06:52:31,432 fbpkg.build INFO: Building: openbmc.utils.flashy
2022-05-06 06:52:32,127 fbpkg.build INFO: creating fbpkg archive for openbmc.utils.flashy:1b41dcd
2022-05-06 06:52:32,139 fbpkg.util INFO: generating CHECKSUMS file for openbmc.utils.flashy:1b41dcd
2022-05-06 06:52:32,139 fbpkg.compression INFO: compressing /tmp/tmpnaugfb08/fbpkg__openbmc.utils.flashy:1b41dcd.tar using /usr/bin/pigz
2022-05-06 06:52:32,192 fbpkg.build INFO: generating fbpkg MetaData
2022-05-06 06:52:33,022 fbpkg.meta INFO: creating package versions in staging
2022-05-06 06:52:40,147 fbpkg.meta INFO: creating package versions complete
openbmc.utils.flashy:1b41dcd
0 ~/local/openbmc $ oobgrader --wait --flashy-tag 1b41dcd --host fboss8845197-oob.snc1 --force
...
Host                   Workflow ID                           Progress    Status                   Result
---------------------  ------------------------------------  ----------  -----------------------  ----------------------
fboss8845197-oob.snc1  f4af702d-9834-4ee0-a5b1-ee26eb9c7af4  finished    WorkflowStatus.FINISHED  FinishStatus.SUCCEEDED
```

Reviewed By: deathowl

fbshipit-source-id: 849a4423478b8e517a41937163909964eaeaf57b
@golang golang locked and limited conversation to collaborators Aug 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@divVerent @ALTree @randall77 @gopherbot