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: 100 < 1 is true on GOARCH=arm #19403

Closed
hajimehoshi opened this issue Mar 5, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@hajimehoshi
Copy link
Contributor

commented Mar 5, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/hajimehoshi/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build115404782=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Compiled https://play.golang.org/p/BtBt5z7tgz with GOARCH=arm

GOARCH=arm go build -o test test.go
qemu-arm test

I used qemu here, but I confirmed the same problem happens on actual ARM devices like ZenFone Go using gomobile.

What did you expect to see?

No panics

What did you see instead?

Panic:

100 < 1 = true
panic: 100 < 1

goroutine 1 [running]:
main.vertices(0xa0080, 0xae450, 0x10, 0x10, 0x10333fa0, 0x6, 0x6)
        /home/hajimehoshi/test/test.go:61 +0x4a8
main.main()
        /home/hajimehoshi/test/test.go:72 +0x64

@ALTree ALTree changed the title build: 100 < 1 is true on GOARCH=arm cmd/compile: 100 < 1 is true on GOARCH=arm Mar 5, 2017

@josharian josharian added this to the Go1.8.1 milestone Mar 5, 2017

@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2017

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2017

The assembler back end uses F15 as a temporary register for int-float conversion (which I didn't notice before). The value happens to live in F15 and get clobbered. Will send a fix.

@cherrymui cherrymui self-assigned this Mar 5, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Mar 5, 2017

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

@gopherbot gopherbot closed this in 6fd5e25 Mar 5, 2017

@josharian

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2017

Reopening for 1.8.1 cherry-pick.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 6, 2017

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

gopherbot pushed a commit that referenced this issue Mar 6, 2017

test/fixedbugs: add test for #19403
Change-Id: Ie52dac8eb4daed95e049ad74d5ae101e8a5cb854
Reviewed-on: https://go-review.googlesource.com/37725
Run-TryBot: Quentin Smith <quentin@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@gopherbot

This comment has been minimized.

Copy link

commented Apr 5, 2017

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

@gopherbot

This comment has been minimized.

Copy link

commented Apr 5, 2017

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

@aclements

This comment has been minimized.

Copy link
Member

commented Apr 5, 2017

Cherry-picked to release.

@aclements aclements closed this Apr 5, 2017

gopherbot pushed a commit that referenced this issue Apr 5, 2017

[release-branch.go1.8] cmd/compile: mark MOVWF/MOVFW clobbering F15 o…
…n ARM

The assembler back end uses F15 as a temporary register in these
instructions.

Checked the assembler back end and made sure that this is the
only case clobbering F15.

Fixes #19403.

Change-Id: I02b9e00fdd9229db899f501c8e9b306e02912d83
Reviewed-on: https://go-review.googlesource.com/39598
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

gopherbot pushed a commit that referenced this issue Apr 5, 2017

[release-branch.go1.8] test/fixedbugs: add test for #19403
Change-Id: Ie52dac8eb4daed95e049ad74d5ae101e8a5cb854
Reviewed-on: https://go-review.googlesource.com/39599
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Apr 10, 2017

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

gopherbot pushed a commit that referenced this issue Apr 10, 2017

[dev.typealias] dev.typealias: merge go1.8.1 into dev.typealias
This also includes fixes since Go 1.8rc3.

a4c18f0 [release-branch.go1.8] go1.8.1
8babce2 [release-branch.go1.8] doc: document go1.8.1
853d533 [release-branch.go1.8] cmd/go: add test for test -race -i behavior
166f215 [release-branch.go1.8] cmd/go: do not install broken libraries during 'go test -i -race'
95a5b80 [release-branch.go1.8] cmd/compile: added special case for reflect header fields to esc
fe79c75 [release-branch.go1.8] cmd/compile: add missing WBs for reflect.{Slice,String}Header.Data
d7989b7 [release-branch.go1.8] cmd/link: skip TestDWARF when cgo is disabled
056be9f [release-branch.go1.8] cmd/link: skip TestDWARF on Plan 9
0224040 [release-branch.go1.8] encoding/xml: disable checking of attribute syntax, like Go 1.7
04017ff [release-branch.go1.8] reflect: fix out-of-bounds pointers calling no-result method
2d00430 [release-branch.go1.8] cmd/link: emit a mach-o dwarf segment that dsymutil will accept
3ca0d34 [release-branch.go1.8] cmd/link: make mach-o dwarf segment properly aligned
84192f2 [release-branch.go1.8] cmd/link: disable mach-o dwarf munging with -w (in addition to -s)
752b8b7 [release-branch.go1.8] cmd/compile: don't crash when slicing non-slice
ff5695d [release-branch.go1.8] runtime: print user stack on other threads during GOTRACBEACK=crash
517a38c [release-branch.go1.8] test/fixedbugs: add test for #19403
dc70a5e [release-branch.go1.8] cmd/compile: mark MOVWF/MOVFW clobbering F15 on ARM
77476e8 [release-branch.go1.8] cmd/compile,runtime: fix atomic And8 for mipsle
bf71119 [release-branch.go1.8] cmd/compile: repaired loop-finder to handle trickier nesting
11a224b [release-branch.go1.8] cmd/compile: add opcode flag hasSideEffects for do-not-remove
3a8841b [release-branch.go1.8] cmd/link: do not pass -s through to host linker on macOS
6c5abcf [release-branch.go1.8] text/template: fix handling of empty blocks
43fa04c [release-branch.go1.8] image/png: restore Go 1.7 rejection of transparent gray8 images
e35c01b [release-branch.go1.8] net, net/http: adjust time-in-past constant even earlier
c955eb1 [release-branch.go1.8] cmd/compile/internal/ssa: don't schedule values after select
f8ed453 [release-branch.go1.8] os/exec: deflake TestStdinCloseRace
d431307 [release-branch.go1.8] cmd/link: put plt stubs first in Textp on ppc64x
0a5cec7 [release-branch.go1.8] doc: reorganize the contribution guidelines into a guide
8890527 [release-branch.go1.8] time: make the ParseInLocation test more robust
ea6781b [release-branch.go1.8] crypto/tls: make Config.Clone also clone the GetClientCertificate field
2327d69 [release-branch.go1.8] cmd/compile: do not fold offset into load/store for args on ARM64
ba48d20 [release-branch.go1.8] cmd/compile: check both syms when folding address into load/store on ARM64
b43fabf [release-branch.go1.8] cmd/compile: add zero-extension before right shift when lowering Lrot on ARM
6a712df [release-branch.go1.8] cmd/compile: fix merging of s390x conditional moves into branch conditions
865536b [release-branch.go1.8] cmd/compile: remove unnecessary type conversions on s390x
bae53da [release-branch.go1.8] runtime: avoid O(n) semaphore list walk in contention profiling
d4ee1f4 [release-branch.go1.8] website: mention go1.8 in project page
991ee8f [release-branch.go1.8] doc: fix broken link in go1.8.html
cd6b620 [release-branch.go1.8] go1.8
606eb9b [release-branch.go1.8] doc: document go1.8
bcda91c [release-branch.go1.8] runtime: do not call wakep from enlistWorker, to avoid possible deadlock
7d7a0a9 [release-branch.go1.8] doc: update Code of Conduct wording and scope
cedc511 [release-branch.go1.8] encoding/xml: fix incorrect indirect code in chardata, comment, innerxml fields
ae13ccf [release-branch.go1.8] database/sql: convert test timeouts to explicit waits with checks
7cec9a5 [release-branch.go1.8] reflect: clear ptrToThis in Ptr when allocating result on heap
d84dee0 [release-branch.go1.8] database/sql: ensure driverConns are closed if not returned to pool
f1e44a4 [release-branch.go1.8] database/sql: do not exhaust connection pool on conn request timeout
3ade540 [release-branch.go1.8] database/sql: record the context error in Rows if canceled
0545006 [release-branch.go1.8] crypto/x509: check for new tls-ca-bundle.pem last
1363eeb [release-branch.go1.8] cmd/go, go/build: better defenses against GOPATH=GOROOT
1edfd64 [release-branch.go1.8] cmd/compile: do not use "oaslit" for global
6eb0f54 [release-branch.go1.8] cmd/compile/internal/syntax: avoid follow-up error for incorrect if statement
c543cc3 [release-branch.go1.8] cmd/compile/internal/syntax: make a parser error "1.7 compliant"
f0749fe [release-branch.go1.8] cmd/link: use external linking for PIE by default
ba878ac [release-branch.go1.8] doc: remove inactive members of the CoC working group
6177f6d [release-branch.go1.8] vendor/golang.org/x/crypto/curve25519: avoid loss of R15 in -dynlink mode
67cd1fa [release-branch.go1.8] cmd/compile: do not fold large offset on ARM64

Change-Id: I907afba886429c4feb36c9895f16046eeab4ad5f

@golang golang locked and limited conversation to collaborators Apr 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.