From 9d6427d8992b05445029f95c9555820675dd2e3e Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 24 Jan 2016 01:33:16 -0500 Subject: [PATCH] cmd/asm: reject foo(SB)(AX) instead of silently treating as foo(SB) Add test for assembly errors, to verify fix. Make sure invalid instruction errors are printed just once (was printing them once per span iteration, so typically twice). Fixes #13282. Change-Id: Id5f66f80a80b3bc4832e00084b0a91f1afec7f8f Reviewed-on: https://go-review.googlesource.com/18858 Reviewed-by: Rob Pike --- src/cmd/asm/internal/asm/endtoend_test.go | 114 +++++++++++++++++- .../asm/internal/asm/testdata/amd64error.s | 7 ++ src/cmd/asm/main.go | 2 +- src/cmd/compile/internal/gc/lex.go | 2 +- src/cmd/internal/obj/link.go | 8 +- src/cmd/internal/obj/x86/asm6.go | 9 ++ 6 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 src/cmd/asm/internal/asm/testdata/amd64error.s diff --git a/src/cmd/asm/internal/asm/endtoend_test.go b/src/cmd/asm/internal/asm/endtoend_test.go index 8f5d56d53a..4bc7e2fb74 100644 --- a/src/cmd/asm/internal/asm/endtoend_test.go +++ b/src/cmd/asm/internal/asm/endtoend_test.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "sort" "strconv" "strings" @@ -35,11 +36,10 @@ func testEndToEnd(t *testing.T, goarch, file string) { ctxt.Bso = obj.Binitw(os.Stdout) defer ctxt.Bso.Flush() failed := false - ctxt.Diag = func(format string, args ...interface{}) { + ctxt.DiagFunc = func(format string, args ...interface{}) { failed = true t.Errorf(format, args...) } - obj.Binitw(ioutil.Discard) pList.Firstpc, ok = parser.Parse() if !ok || failed { t.Errorf("asm: %s assembly failed", goarch) @@ -175,7 +175,7 @@ Diff: top := pList.Firstpc var text *obj.LSym ok = true - ctxt.Diag = func(format string, args ...interface{}) { + ctxt.DiagFunc = func(format string, args ...interface{}) { t.Errorf(format, args...) ok = false } @@ -250,8 +250,110 @@ func isHexes(s string) bool { return true } +// It would be nice if the error messages began with +// the standard file:line: prefix, +// but that's not where we are today. +// It might be at the beginning but it might be in the middle of the printed instruction. +var fileLineRE = regexp.MustCompile(`(?:^|\()(testdata[/\\][0-9a-z]+\.s:[0-9]+)(?:$|\))`) + +// Same as in test/run.go +var ( + errRE = regexp.MustCompile(`// ERROR ?(.*)`) + errQuotesRE = regexp.MustCompile(`"([^"]*)"`) +) + +func testErrors(t *testing.T, goarch, file string) { + lex.InitHist() + input := filepath.Join("testdata", file+".s") + architecture, ctxt := setArch(goarch) + lexer := lex.NewLexer(input, ctxt) + parser := NewParser(ctxt, architecture, lexer) + pList := obj.Linknewplist(ctxt) + var ok bool + testOut = new(bytes.Buffer) // The assembler writes test output to this buffer. + ctxt.Bso = obj.Binitw(os.Stdout) + defer ctxt.Bso.Flush() + failed := false + var errBuf bytes.Buffer + ctxt.DiagFunc = func(format string, args ...interface{}) { + failed = true + s := fmt.Sprintf(format, args...) + if !strings.HasSuffix(s, "\n") { + s += "\n" + } + errBuf.WriteString(s) + } + pList.Firstpc, ok = parser.Parse() + obj.Flushplist(ctxt) + if ok && !failed { + t.Errorf("asm: %s had no errors", goarch) + } + + errors := map[string]string{} + for _, line := range strings.Split(errBuf.String(), "\n") { + if line == "" || strings.HasPrefix(line, "\t") { + continue + } + m := fileLineRE.FindStringSubmatch(line) + if m == nil { + t.Errorf("unexpected error: %v", line) + continue + } + fileline := m[1] + if errors[fileline] != "" { + t.Errorf("multiple errors on %s:\n\t%s\n\t%s", fileline, errors[fileline], line) + continue + } + errors[fileline] = line + } + + // Reconstruct expected errors by independently "parsing" the input. + data, err := ioutil.ReadFile(input) + if err != nil { + t.Error(err) + return + } + lineno := 0 + lines := strings.Split(string(data), "\n") + for _, line := range lines { + lineno++ + + fileline := fmt.Sprintf("%s:%d", input, lineno) + if m := errRE.FindStringSubmatch(line); m != nil { + all := m[1] + mm := errQuotesRE.FindAllStringSubmatch(all, -1) + if len(mm) != 1 { + t.Errorf("%s: invalid errorcheck line:\n%s", fileline, line) + } else if err := errors[fileline]; err == "" { + t.Errorf("%s: missing error, want %s", fileline, all) + } else if !strings.Contains(err, mm[0][1]) { + t.Errorf("%s: wrong error for %s:\n%s", fileline, all, err) + } + } else { + if errors[fileline] != "" { + t.Errorf("unexpected error on %s: %v", fileline, errors[fileline]) + } + } + delete(errors, fileline) + } + var extra []string + for key := range errors { + extra = append(extra, key) + } + sort.Strings(extra) + for _, fileline := range extra { + t.Errorf("unexpected error on %s: %v", fileline, errors[fileline]) + } +} + func Test386EndToEnd(t *testing.T) { - testEndToEnd(t, "386", "386") + defer os.Setenv("GO386", os.Getenv("GO386")) + + for _, go386 := range []string{"387", "sse"} { + os.Setenv("GO386", go386) + t.Logf("GO386=%v", os.Getenv("GO386")) + testEndToEnd(t, "386", "386") + } } func TestARMEndToEnd(t *testing.T) { @@ -276,6 +378,10 @@ func TestAMD64Encoder(t *testing.T) { testEndToEnd(t, "amd64", "amd64enc") } +func TestAMD64Errors(t *testing.T) { + testErrors(t, "amd64", "amd64error") +} + func TestMIPS64EndToEnd(t *testing.T) { testEndToEnd(t, "mips64", "mips64") } diff --git a/src/cmd/asm/internal/asm/testdata/amd64error.s b/src/cmd/asm/internal/asm/testdata/amd64error.s new file mode 100644 index 0000000000..9895b54ab0 --- /dev/null +++ b/src/cmd/asm/internal/asm/testdata/amd64error.s @@ -0,0 +1,7 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +TEXT errors(SB),$0 + MOVL foo<>(SB)(AX), AX // ERROR "invalid instruction" + RET diff --git a/src/cmd/asm/main.go b/src/cmd/asm/main.go index 528481c132..f48050c137 100644 --- a/src/cmd/asm/main.go +++ b/src/cmd/asm/main.go @@ -54,7 +54,7 @@ func main() { lexer := lex.NewLexer(flag.Arg(0), ctxt) parser := asm.NewParser(ctxt, architecture, lexer) diag := false - ctxt.Diag = func(format string, args ...interface{}) { + ctxt.DiagFunc = func(format string, args ...interface{}) { diag = true log.Printf(format, args...) } diff --git a/src/cmd/compile/internal/gc/lex.go b/src/cmd/compile/internal/gc/lex.go index 8d1d2e2594..b9c27357bb 100644 --- a/src/cmd/compile/internal/gc/lex.go +++ b/src/cmd/compile/internal/gc/lex.go @@ -105,7 +105,7 @@ func Main() { Thearch.Linkarchinit() Ctxt = obj.Linknew(Thearch.Thelinkarch) - Ctxt.Diag = Yyerror + Ctxt.DiagFunc = Yyerror Ctxt.Bso = &bstdout bstdout = *obj.Binitw(os.Stdout) diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index bc898235c1..762a49ecf2 100644 --- a/src/cmd/internal/obj/link.go +++ b/src/cmd/internal/obj/link.go @@ -604,12 +604,13 @@ type Link struct { Autosize int32 Armsize int32 Pc int64 - Diag func(string, ...interface{}) + DiagFunc func(string, ...interface{}) Mode int Cursym *LSym Version int Textp *LSym Etextp *LSym + Errors int // state for writing objects Text *LSym @@ -618,6 +619,11 @@ type Link struct { Edata *LSym } +func (ctxt *Link) Diag(format string, args ...interface{}) { + ctxt.Errors++ + ctxt.DiagFunc(format, args...) +} + // The smallest possible offset from the hardware stack pointer to a local // variable on the stack. Architectures that use a link register save its value // on the stack in the function prologue and so always have a pointer between diff --git a/src/cmd/internal/obj/x86/asm6.go b/src/cmd/internal/obj/x86/asm6.go index f00be91b00..fdc25faf98 100644 --- a/src/cmd/internal/obj/x86/asm6.go +++ b/src/cmd/internal/obj/x86/asm6.go @@ -1856,6 +1856,7 @@ func span6(ctxt *obj.Link, s *obj.LSym) { var loop int32 var m int var p *obj.Prog + errors := ctxt.Errors for { loop = 0 for i = 0; i < len(s.R); i++ { @@ -1968,6 +1969,9 @@ func span6(ctxt *obj.Link, s *obj.LSym) { if loop == 0 { break } + if ctxt.Errors > errors { + return + } } if ctxt.Headtype == obj.Hnacl { @@ -2294,6 +2298,11 @@ func oclass(ctxt *obj.Link, p *obj.Prog, a *obj.Addr) int { return Yxxx case obj.TYPE_MEM: + if a.Name != obj.NAME_NONE { + if ctxt.Asmode == 64 && (a.Reg != REG_NONE || a.Index != REG_NONE || a.Scale != 0) { + return Yxxx + } + } return Ym case obj.TYPE_ADDR: -- 2.48.1