go: cmd/compile: go binaries not working on exynos 64 bit CPUs

Go binaries have stopped working on Galaxy S9+. The breaking commit seems to be 0a7ac93c27c9ade79fe0f66ae0bb81484c241ae5.

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

go 1.11.1 (worked in go 1.10.4)

Does this issue reproduce with the latest release?

Yes

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

Android on Arm64 (Android 8.0 on a Galaxy S9+)

What did you do?

Used gomobile to compile the following code and used it in an Android app to display a toast notification.

package hello

import (
	"runtime"
)

func Message() string {
	return "Hello: " + runtime.Version()
}

What did you expect to see?

The app displaying a Toast with the text “Hello: <Go Version>”

What did you see instead?

The app crashing with a SIGILL.

Credits

The problem was brought to our attention in the gophers slack channel by @shipit.

Git bisect output

0a7ac93c27c9ade79fe0f66ae0bb81484c241ae5 is the first bad commit
commit 0a7ac93c27c9ade79fe0f66ae0bb81484c241ae5
Author: Wei Xiao <wei.xiao@arm.com>
Date:   Fri Nov 3 02:05:28 2017 +0000

    cmd/compile: improve atomic add intrinsics with ARMv8.1 new instruction
    
    ARMv8.1 has added new instruction (LDADDAL) for atomic memory operations. This
    CL improves existing atomic add intrinsics with the new instruction. Since the
    new instruction is only guaranteed to be present after ARMv8.1, we guard its
    usage with a conditional on CPU feature.
    
    Performance result on ARMv8.1 machine:
    name        old time/op  new time/op  delta
    Xadd-224    1.05µs ± 6%  0.02µs ± 4%  -98.06%  (p=0.000 n=10+8)
    Xadd64-224  1.05µs ± 3%  0.02µs ±13%  -98.10%  (p=0.000 n=9+10)
    [Geo mean]  1.05µs       0.02µs       -98.08%
    
    Performance result on ARMv8.0 machine:
    name        old time/op  new time/op  delta
    Xadd-46      538ns ± 1%   541ns ± 1%  +0.62%  (p=0.000 n=9+9)
    Xadd64-46    505ns ± 1%   508ns ± 0%  +0.48%  (p=0.003 n=9+8)
    [Geo mean]   521ns        524ns       +0.55%
    
    Change-Id: If4b5d8d0e2d6f84fe1492a4f5de0789910ad0ee9
    Reviewed-on: https://go-review.googlesource.com/81877
    Run-TryBot: Cherry Zhang <cherryyz@google.com>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: Cherry Zhang <cherryyz@google.com>

:040000 040000 848bfd65d49b5fe8b5472954c50e6596a4ad15a6 c77dcbdac660c41fb79e07785745459b5d704489 M	src

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 27 (10 by maintainers)

Most upvoted comments

The easy sledgehammer is disabling the optimization if runtime.GOOS == “android”.

@gopherbot please consider this for backport to 1.11. It’s an unavoidable crash on a popular Android phone and easy to avoid from Go.

Read the docs for that new instruction (LDADDAL)? I notice that your chip has two different speed cores on it. Maybe some support LDADDAL and some don’t? Read through src/internal/cpu/cpu_arm64.go and look for the atomics flag. See if the constants there agree with the hwcap bits we should be checking. Compile a binary and make sure the guard code around the new instruction looks correct.