From 4ebc6514faa8530d1a68b4e04f57dc5c25bcb01c Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 18 Mar 2019 15:48:34 +0100 Subject: [PATCH] cmd/asm: Fix EVEX RIP-relative addressing AVX-512 instructions that use RIP-relative addressing and require the R bit of the EVEX prefix to be zero, i.e., instructions that use Z8-Z15 or Z24-Z31, are incorrectly encoded by the assembler. The reason is that the location of the offset at which the relative address is to be written is incorrectly computed when the R bit is clear. For example, VMOVUPS bInitX<>+0(SB), Z0 encodes correctly to 62 f1 7c 48 10 05 66 e9 02 00 whereas VMOVUPS bInitX<>+0(SB), Z8 encodes incorrectly to 62 71 7c 48 10 05 00 56 e9 02 00 Note the extra zero byte between the ModR/M byte (05) and the relative address starting with 56. This error results in the first byte of the following instruction being overwritten and typically, a program crash. This commit fixes the issue in the same way that is fixed for VEX encoded instructions, by simply not incrementing the offset for EVEX instructions. Existing test code created for a similar VEX encoding issue (19518) has been modified to also test for the issue addressed by this commit. Fixes #31001 Change-Id: If84719ac22ebb5fb3c42ff96cd32b611ad497414 Reviewed-on: https://go-review.googlesource.com/c/go/+/168562 Run-TryBot: Ilya Tocar TryBot-Result: Gobot Gobot Reviewed-by: Ilya Tocar --- src/cmd/internal/obj/x86/asm6.go | 2 +- ...{issue19518_test.go => pcrelative_test.go} | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) rename src/cmd/internal/obj/x86/{issue19518_test.go => pcrelative_test.go} (67%) diff --git a/src/cmd/internal/obj/x86/asm6.go b/src/cmd/internal/obj/x86/asm6.go index 91a2fc22ff3..a81de43845e 100644 --- a/src/cmd/internal/obj/x86/asm6.go +++ b/src/cmd/internal/obj/x86/asm6.go @@ -5374,7 +5374,7 @@ func (ab *AsmBuf) asmins(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog) { if int64(r.Off) < p.Pc { break } - if ab.rexflag != 0 && !ab.vexflag { + if ab.rexflag != 0 && !ab.vexflag && !ab.evexflag { r.Off++ } if r.Type == objabi.R_PCREL { diff --git a/src/cmd/internal/obj/x86/issue19518_test.go b/src/cmd/internal/obj/x86/pcrelative_test.go similarity index 67% rename from src/cmd/internal/obj/x86/issue19518_test.go rename to src/cmd/internal/obj/x86/pcrelative_test.go index 174e2dd846f..51b60cf93e5 100644 --- a/src/cmd/internal/obj/x86/issue19518_test.go +++ b/src/cmd/internal/obj/x86/pcrelative_test.go @@ -6,6 +6,7 @@ package x86_test import ( "bytes" + "fmt" "internal/testenv" "io/ioutil" "os" @@ -17,7 +18,7 @@ import ( const asmData = ` GLOBL zeros<>(SB),8,$64 TEXT ·testASM(SB),4,$0 -VMOVDQU zeros<>(SB), Y8 // PC relative relocation is off by 1, for Y8-15 +VMOVUPS zeros<>(SB), %s // PC relative relocation is off by 1, for Y8-Y15, Z8-15 and Z24-Z31 RET ` @@ -31,13 +32,13 @@ func main() { } ` -func objdumpOutput(t *testing.T) []byte { - tmpdir, err := ioutil.TempDir("", "19518") +func objdumpOutput(t *testing.T, mname, source string) []byte { + tmpdir, err := ioutil.TempDir("", mname) if err != nil { t.Fatal(err) } defer os.RemoveAll(tmpdir) - err = ioutil.WriteFile(filepath.Join(tmpdir, "go.mod"), []byte("module issue19518\n"), 0666) + err = ioutil.WriteFile(filepath.Join(tmpdir, "go.mod"), []byte(fmt.Sprintf("module %s\n", mname)), 0666) if err != nil { t.Fatal(err) } @@ -46,7 +47,7 @@ func objdumpOutput(t *testing.T) []byte { t.Fatal(err) } defer tmpfile.Close() - _, err = tmpfile.WriteString(asmData) + _, err = tmpfile.WriteString(source) if err != nil { t.Fatal(err) } @@ -85,17 +86,19 @@ func objdumpOutput(t *testing.T) []byte { return objout } -func TestVexPCrelative(t *testing.T) { +func TestVexEvexPCrelative(t *testing.T) { testenv.MustHaveGoBuild(t) - objout := objdumpOutput(t) - data := bytes.Split(objout, []byte("\n")) - for idx := len(data) - 1; idx >= 0; idx-- { - // OBJDUMP doesn't know about VMOVDQU, - // so instead of checking that it was assembled correctly, - // check that RET wasn't overwritten. - if bytes.Index(data[idx], []byte("RET")) != -1 { - return +LOOP: + for _, reg := range []string{"Y0", "Y8", "Z0", "Z8", "Z16", "Z24"} { + asm := fmt.Sprintf(asmData, reg) + objout := objdumpOutput(t, "pcrelative", asm) + data := bytes.Split(objout, []byte("\n")) + for idx := len(data) - 1; idx >= 0; idx-- { + // check that RET wasn't overwritten. + if bytes.Index(data[idx], []byte("RET")) != -1 { + continue LOOP + } } + t.Errorf("VMOVUPS zeros<>(SB), %s overwrote RET", reg) } - t.Fatal("RET was overwritten") }