go: math: Sqrt() can generate invalid byte code for wasm

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

$ go version
go version go1.11 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

GOARCH="amd64"
GOBIN="/home/jc/go/bin"
GOCACHE="/home/jc/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jc/git_repos:/home/jc/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build142808779=/tmp/go-build -gno-record-gcc-switches"

What did you do?

From investigating a problem reported by @bobcob7 in the WebAssembly Slack channel, it’s turned out to be a case of the Go compiler generating what seems to be wrong byte code for the math.Sqrt() function.

https://github.com/justinclift/wasmBug1/blob/f39948cac99853a6f2a99b81b3e03afbf24e6fc7/main.go#L250-L251

In that example code there are two lines that should do the same thing. One enabled (line 250), one commented out (line 251). In the repo as-is, the wasm runs ok.

However if line 250 is commented out, with the line after it then enabled, the byte code generated for wasm output seems wrong. Instead of running the same, Firefox gives an error in the javascript console:

CompileError: wasm validation error: at offset 1269295: type mismatch: expression has type i64 but expected f64

Chrome gives a different error, seemingly meaning the same thing:

CompileError: AsyncCompile: Compiling wasm function #1694 failed: f32.demote/f64[0] expected type f64, found get_local of type i64 @+1269409

After further drilling down, it’s probably something to do with math.Sqrt()'s Go implementation using “unsafe” pieces:

But, no idea why the exact same values called from the same code (but done from a function) are making it go wrong.

Hopefully someone more familiar with the wasm byte code generation can take a look. It’s probably not a hard fix. 😄

Note - As a thought, this seems like it might be a blocker for 1.11.x? Having any of the main math.* functions going wrong seems a tad non-optimal. 😉

What did you expect to see?

Working code running in the browser (eg at https://justinclift.github.io/wasmBug1/)

What did you see instead?

A compile error in the javascript console.

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 4
Comments: 15 (9 by maintainers)

Commits related to this issue

[release-branch.go1.11] cmd/compile: fix type of OffPtr in some optimization rules In some optimization rules the type of generated OffPtr was incorrectly set to the type of the pointee, instead of t... — committed to golang/go by cherrymui 6 years ago
go1.11.2 updates tracking (#2) * [release-branch.go1.11] doc/go1.11, cmd/go: elaborate on new GOFLAGS environment variable In Go 1.11, cmd/go gained support for the GOFLAGS environment variable. ... — committed to changkun/go by changkun 6 years ago

Most upvoted comments

I managed to reduce the test case to this:

package main

import "math"

type Vec2 [2]float64

func main() {
	var a Vec2
	a.A().B().C().D()
}

func (v Vec2) A() Vec2 {
	return Vec2{v[0], v[0]}
}

func (v Vec2) B() Vec2 {
	return Vec2{1.0 / v.D(), 0}
}

func (v Vec2) C() Vec2 {
	return Vec2{v[0], v[0]}
}

func (v Vec2) D() float64 {
	return math.Sqrt(v[0])
}

I couldn’t reduce it any further. I’m still investigating. It seems to be related to inlining and autotmp values.

neelance on Oct 2, 2018

Probably. Perhaps @gopherbot should reply when it doesn’t understand something.

bradfitz on Oct 4, 2018

@gopehrbot wasn’t behaving, though, so I created https://go-review.googlesource.com/c/go/+/139104 manually.

/cc @andybons @dmitshur

@bradfitz, did you mean to say “backport” instead of “cherry pick”? See https://golang.org/wiki/gopherbot#backporting-issues and source code.

dmitshur on Oct 4, 2018

The offending instruction is

00027 (9) Get	R0
00028 (9) F64Store	""..autotmp_7-40(SP)

which tries to store an integer value (R0) as float64. This comes from spilling v86 (9) = I64AddConst <float64> [8] v2. This value looks problematic, since it is an integer operation with a float type.

This value is from lowering v86 (9) = OffPtr <float64> [8] v2, which is also problematic, since OffPtr should not be float typed. This seems coming from generic opt.

cherrymui on Oct 3, 2018

@justinclift, yes, hence my comment immediately preceding yours. 😃

bradfitz on Oct 3, 2018

Change https://golang.org/cl/139257 mentions this issue: cmd/compile: fix type of OffPtr in some optimization rules

gopherbot on Oct 3, 2018