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

EVEX instructions that use RIP addressing can be incorrectly assembled leading to program crash #31001

Closed
markdryan opened this issue Mar 22, 2019 · 3 comments

Comments

@markdryan
Copy link
Contributor

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

$ go version
go version go1.12 linux/amd64

Does this issue reproduce with the latest release?

Yes (and on master (go version devel +c1320ec Fri Mar 22 08:51:29 2019 +0100 linux/amd64))

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/markus/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/markus/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-build243001176=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I built and ran the following on a SkylakeX Core i9 that supports AVX512 instructions

main.go

package main

// go:noescape
func good()

// go:noescape
func bad()

func main() {
	good()
	bad()
}

bug.s

#include "textflag.h"
#include "funcdata.h"

DATA bInitX<>+0x00(SB)/8, $0x0000000100000000
DATA bInitX<>+0x08(SB)/8, $0x0000000300000002
DATA bInitX<>+0x10(SB)/8, $0x0000000500000004
DATA bInitX<>+0x18(SB)/8, $0x0000000700000006
DATA bInitX<>+0x20(SB)/8, $0x0000000900000008
DATA bInitX<>+0x28(SB)/8, $0x0000000B0000000A
DATA bInitX<>+0x30(SB)/8, $0x0000000D0000000C
DATA bInitX<>+0x38(SB)/8, $0x0000000F0000000E

GLOBL bInitX<>(SB), (NOPTR+RODATA), $64	

// func good()
TEXT ·good(SB), NOSPLIT, $0
	NO_LOCAL_POINTERS

	VMOVUPS bInitX<>+0(SB), Z0
	RET

// func bad()
TEXT ·bad(SB), NOSPLIT, $0
	NO_LOCAL_POINTERS

	VMOVUPS bInitX<>+0(SB), Z8
	RET

What did you expect to see?

The program should run correctly and exit cleanly.

What did you see instead?

unexpected fault address 0x32e4b2a
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0x32e4b2a pc=0x44f520]

goroutine 1 [running]:
runtime.throw(0x46f117, 0x5)
	/usr/local/go/src/runtime/panic.go:617 +0x72 fp=0xc00005c750 sp=0xc00005c720 pc=0x423272
runtime.sigpanic()
	/usr/local/go/src/runtime/signal_unix.go:397 +0x401 fp=0xc00005c780 sp=0xc00005c750 pc=0x434bf1
main.bad(0xc00005c790, 0x424bbc, 0xc00009c000, 0x0, 0xc00009c000, 0x0, 0x0, 0x0, 0xc000000300, 0x0, ...)
	/home/user/play/avx512bug/bug.s:26 fp=0xc00005c788 sp=0xc00005c780 pc=0x44f520
main.main()
	/home/user/play/avx512bug/main.go:11 +0x25 fp=0xc00005c798 sp=0xc00005c788 pc=0x44f4a5
runtime.main()
	/usr/local/go/src/runtime/proc.go:200 +0x20c fp=0xc00005c7e0 sp=0xc00005c798 pc=0x424bbc
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc00005c7e8 sp=0xc00005c7e0 pc=0x449181

Analysis

Looking at the disassembly in objdump it's easy to see what the problem is. The first VMOVUPS instruction

VMOVUPS bInitX<>+0(SB), Z0

  44f510:       62 f1 7c 48 10 05 66    vmovups 0x2e966(%rip),%zmm0        # 47de80 <bInitX>
  44f517:       e9 02 00 
  44f51a:       c3                      retq   

is assembled correctly, while the second

VMOVUPS bInitX<>+0(SB), Z8

  44f520:       62 71 7c 48 10 05 00    vmovups 0x2e95600(%rip),%zmm8        # 32e4b2a <runtime.end+0x2e05772>
  44f527:       56 e9 02 
  44f52a:       00 cc                   add    %cl,%ah

is not.

Note that there is an extra 00 byte inserted after the ModR/M byte (05). This leads to an incorrect relative address being encoded into the instruction and the corruption of the subsequent instruction. Instead of a retq we now have an add.

@markdryan
Copy link
Contributor Author

I've submitted a patch for the issue here:

https://go-review.googlesource.com/c/go/+/168562

@josharian
Copy link
Contributor

cc @quasilyte @TocarIP

@gopherbot
Copy link

Change https://golang.org/cl/168562 mentions this issue: cmd/asm: Fix EVEX RIP-relative addressing

@golang golang locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants