From cc700bdc269edc5fd29b14c1866c7f57f6f9b526 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Fri, 19 Jun 2020 17:59:16 -0400 Subject: cmd/trace: move viewer data structs into cmd/internal/traceviewer The ViewerEvent, ViewerData and ViewerFrame structs are moved into cmd/internal/traceviewer, and renamed Event, Data, and Frame. The structs are the same, except for the following: A definition for the JSON "bp" field that's defined in the trace format, but missing in the structs has been added. Also, the Tid and Pid fields on Event have been renamed TID and PID to better match Go style. Finally, the footer field on ViewerData, which hasn't been used for a while, has been removed. This CL is in preparation for the usage of these structs by cmd/go's tracing functionality. Updates #38714 Change-Id: I345f23617b96d4629b876ae717f89d56a67e05a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/239098 Run-TryBot: Michael Matloob TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills Reviewed-by: Jay Conrod --- src/cmd/internal/traceviewer/format.go | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/cmd/internal/traceviewer/format.go (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/traceviewer/format.go b/src/cmd/internal/traceviewer/format.go new file mode 100644 index 0000000000..871477447f --- /dev/null +++ b/src/cmd/internal/traceviewer/format.go @@ -0,0 +1,38 @@ +// Copyright 2020 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. + +// Package traceviewer provides definitions of the JSON data structures +// used by the Chrome trace viewer. +// +// The official description of the format is in this file: +// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview +package traceviewer + +type Data struct { + Events []*Event `json:"traceEvents"` + Frames map[string]Frame `json:"stackFrames"` + TimeUnit string `json:"displayTimeUnit"` +} + +type Event struct { + Name string `json:"name,omitempty"` + Phase string `json:"ph"` + Scope string `json:"s,omitempty"` + Time float64 `json:"ts"` + Dur float64 `json:"dur,omitempty"` + PID uint64 `json:"pid"` + TID uint64 `json:"tid"` + ID uint64 `json:"id,omitempty"` + BindPoint string `json:"bp,omitempty"` + Stack int `json:"sf,omitempty"` + EndStack int `json:"esf,omitempty"` + Arg interface{} `json:"args,omitempty"` + Cname string `json:"cname,omitempty"` + Category string `json:"cat,omitempty"` +} + +type Frame struct { + Name string `json:"name"` + Parent int `json:"parent,omitempty"` +} -- cgit v1.3 From c2e73fb446bffd02c651e51c6641cc90fd065b70 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Tue, 23 Jun 2020 08:46:36 -0400 Subject: cmd/compile: remove AttrSeenGlobl (use AttrOnList instead) Minor cleanup: remove the symbol attribute AttrSeenGlobal, since it is redundant with the existing attribute AttrOnList (no need to have what amounts to a separate flag for checking the same property). Change-Id: Ia269b64de37c2bb4a2314bbecf3d2091c6d57424 Reviewed-on: https://go-review.googlesource.com/c/go/+/239477 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/compile/internal/gc/obj.go | 2 +- src/cmd/internal/obj/link.go | 3 --- src/cmd/internal/obj/plist.go | 4 ---- 3 files changed, 1 insertion(+), 8 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index 0826b04e33..af5037c5a8 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -352,7 +352,7 @@ func stringsym(pos src.XPos, s string) (data *obj.LSym) { symdata := Ctxt.Lookup(symdataname) - if !symdata.SeenGlobl() { + if !symdata.OnList() { // string data off := dsname(symdata, 0, s, pos, "string") ggloblsym(symdata, int32(off), obj.DUPOK|obj.RODATA|obj.LOCAL) diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index dc47e51be9..311e5ae2e8 100644 --- a/src/cmd/internal/obj/link.go +++ b/src/cmd/internal/obj/link.go @@ -480,7 +480,6 @@ const ( AttrWrapper AttrNeedCtxt AttrNoFrame - AttrSeenGlobl AttrOnList AttrStatic @@ -537,7 +536,6 @@ func (a Attribute) MakeTypelink() bool { return a&AttrMakeTypelink != 0 } func (a Attribute) CFunc() bool { return a&AttrCFunc != 0 } func (a Attribute) NoSplit() bool { return a&AttrNoSplit != 0 } func (a Attribute) Leaf() bool { return a&AttrLeaf != 0 } -func (a Attribute) SeenGlobl() bool { return a&AttrSeenGlobl != 0 } func (a Attribute) OnList() bool { return a&AttrOnList != 0 } func (a Attribute) ReflectMethod() bool { return a&AttrReflectMethod != 0 } func (a Attribute) Local() bool { return a&AttrLocal != 0 } @@ -574,7 +572,6 @@ var textAttrStrings = [...]struct { {bit: AttrCFunc, s: "CFUNC"}, {bit: AttrNoSplit, s: "NOSPLIT"}, {bit: AttrLeaf, s: "LEAF"}, - {bit: AttrSeenGlobl, s: ""}, {bit: AttrOnList, s: ""}, {bit: AttrReflectMethod, s: "REFLECTMETHOD"}, {bit: AttrLocal, s: "LOCAL"}, diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go index afe0ee4ee0..6e33f29959 100644 --- a/src/cmd/internal/obj/plist.go +++ b/src/cmd/internal/obj/plist.go @@ -145,10 +145,6 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) { } func (ctxt *Link) Globl(s *LSym, size int64, flag int) { - if s.SeenGlobl() { - fmt.Printf("duplicate %v\n", s) - } - s.Set(AttrSeenGlobl, true) if s.OnList() { ctxt.Diag("symbol %s listed multiple times", s.Name) } -- cgit v1.3 From 7d7bd5abc7f7ac901830b79496f63ce86895e262 Mon Sep 17 00:00:00 2001 From: Lynn Boger Date: Tue, 11 Aug 2020 12:04:25 -0400 Subject: cmd/internal/obj/ppc64: don't remove NOP in assembler Previously, the assembler removed NOPs from the Prog list in obj9.go. NOPs shouldn't be removed if they were added as an inline mark, as described in the issue below. Fixes #40689 Once the NOPs were left in the Prog list, some instructions were flagged as invalid because they had an operand which was not represented in optab. In order to preserve the previous assembler behavior, entries were added to optab for those operand cases. They were not flagged as errors before because the NOP instructions were removed before the code to check the valid opcode/operand combinations. Change-Id: Iae5145f94459027cf458e914d7c5d6089807ccf8 Reviewed-on: https://go-review.googlesource.com/c/go/+/247842 Run-TryBot: Lynn Boger TryBot-Result: Gobot Gobot Reviewed-by: Paul Murphy Reviewed-by: Michael Munday Reviewed-by: Keith Randall --- src/cmd/internal/obj/ppc64/asm9.go | 3 +++ src/cmd/internal/obj/ppc64/obj9.go | 11 +++-------- 2 files changed, 6 insertions(+), 8 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/ppc64/asm9.go b/src/cmd/internal/obj/ppc64/asm9.go index 0fd0744a42..238ca8f0b7 100644 --- a/src/cmd/internal/obj/ppc64/asm9.go +++ b/src/cmd/internal/obj/ppc64/asm9.go @@ -613,6 +613,9 @@ var optab = []Optab{ {obj.APCDATA, C_LCON, C_NONE, C_NONE, C_LCON, 0, 0, 0}, {obj.AFUNCDATA, C_SCON, C_NONE, C_NONE, C_ADDR, 0, 0, 0}, {obj.ANOP, C_NONE, C_NONE, C_NONE, C_NONE, 0, 0, 0}, + {obj.ANOP, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // NOP operand variations added for #40689 + {obj.ANOP, C_REG, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // to preserve previous behavior + {obj.ANOP, C_FREG, C_NONE, C_NONE, C_NONE, 0, 0, 0}, {obj.ADUFFZERO, C_NONE, C_NONE, C_NONE, C_LBRA, 11, 4, 0}, // same as ABR/ABL {obj.ADUFFCOPY, C_NONE, C_NONE, C_NONE, C_LBRA, 11, 4, 0}, // same as ABR/ABL {obj.APCALIGN, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // align code diff --git a/src/cmd/internal/obj/ppc64/obj9.go b/src/cmd/internal/obj/ppc64/obj9.go index 16881c634b..749f7066de 100644 --- a/src/cmd/internal/obj/ppc64/obj9.go +++ b/src/cmd/internal/obj/ppc64/obj9.go @@ -429,7 +429,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { /* * find leaf subroutines - * strip NOPs * expand RET * expand BECOME pseudo */ @@ -559,10 +558,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { q = p q1 = p.Pcond if q1 != nil { - for q1.As == obj.ANOP { - q1 = q1.Link - p.Pcond = q1 - } + // NOPs are not removed due to #40689. if q1.Mark&LEAF == 0 { q1.Mark |= LABEL @@ -589,9 +585,8 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { continue case obj.ANOP: - q1 = p.Link - q.Link = q1 /* q is non-nop */ - q1.Mark |= p.Mark + // NOPs are not removed due to + // #40689 continue default: -- cgit v1.3 From d0d6593d1d4e81acd073244f42b6893fa65c99d8 Mon Sep 17 00:00:00 2001 From: Michael Munday Date: Mon, 10 Aug 2020 08:01:21 -0700 Subject: cmd/internal/obj: fix inline marker issue on s390x The optimization that replaces inline markers with pre-existing instructions assumes that 'Prog' values produced by the compiler are still reachable after the assembler has run. This was not true on s390x where the assembler was removing NOP instructions from the linked list of 'Prog' values. This led to broken inlining data which in turn caused an infinite loop in the runtime traceback code. Fix this by stopping the s390x assembler backend removing NOP values. It does not make any difference to the output of the assembler because NOP instructions are 0 bytes long anyway. Fixes #40473. Change-Id: Ib4fabadd1de8adb80421f75950ee9aad2111147a Reviewed-on: https://go-review.googlesource.com/c/go/+/247697 Run-TryBot: Michael Munday TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/internal/obj/pcln.go | 15 +++++++++++++++ src/cmd/internal/obj/s390x/objz.go | 11 ----------- 2 files changed, 15 insertions(+), 11 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go index 1f7ccf47ef..bffeda041d 100644 --- a/src/cmd/internal/obj/pcln.go +++ b/src/cmd/internal/obj/pcln.go @@ -278,6 +278,21 @@ func linkpcln(ctxt *Link, cursym *LSym) { funcpctab(ctxt, &pcln.Pcfile, cursym, "pctofile", pctofileline, pcln) funcpctab(ctxt, &pcln.Pcline, cursym, "pctoline", pctofileline, nil) + // Check that all the Progs used as inline markers are still reachable. + // See issue #40473. + inlMarkProgs := make(map[*Prog]struct{}, len(cursym.Func.InlMarks)) + for _, inlMark := range cursym.Func.InlMarks { + inlMarkProgs[inlMark.p] = struct{}{} + } + for p := cursym.Func.Text; p != nil; p = p.Link { + if _, ok := inlMarkProgs[p]; ok { + delete(inlMarkProgs, p) + } + } + if len(inlMarkProgs) > 0 { + ctxt.Diag("one or more instructions used as inline markers are no longer reachable") + } + pcinlineState := new(pcinlineState) funcpctab(ctxt, &pcln.Pcinline, cursym, "pctoinline", pcinlineState.pctoinline, nil) for _, inlMark := range cursym.Func.InlMarks { diff --git a/src/cmd/internal/obj/s390x/objz.go b/src/cmd/internal/obj/s390x/objz.go index b14dc810fa..ef6335d849 100644 --- a/src/cmd/internal/obj/s390x/objz.go +++ b/src/cmd/internal/obj/s390x/objz.go @@ -283,17 +283,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { ACMPUBNE: q = p p.Mark |= BRANCH - if p.Pcond != nil { - q := p.Pcond - for q.As == obj.ANOP { - q = q.Link - p.Pcond = q - } - } - - case obj.ANOP: - q.Link = p.Link /* q is non-nop */ - p.Link.Mark |= p.Mark default: q = p -- cgit v1.3 From a22ec6e650669f5101c7e0955d82e29d644eef4e Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 17 Aug 2020 13:17:26 +0000 Subject: Revert "cmd/internal/obj: fix inline marker issue on s390x" This reverts CL 247697. Reason for revert: This change broke the linux-arm builder. Change-Id: I8ca0d5b3b2ea0109ffbfadeab1406a1b60e7d18d Reviewed-on: https://go-review.googlesource.com/c/go/+/248718 Reviewed-by: Michael Munday Run-TryBot: Michael Munday TryBot-Result: Gobot Gobot --- src/cmd/internal/obj/pcln.go | 15 --------------- src/cmd/internal/obj/s390x/objz.go | 11 +++++++++++ 2 files changed, 11 insertions(+), 15 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go index bffeda041d..1f7ccf47ef 100644 --- a/src/cmd/internal/obj/pcln.go +++ b/src/cmd/internal/obj/pcln.go @@ -278,21 +278,6 @@ func linkpcln(ctxt *Link, cursym *LSym) { funcpctab(ctxt, &pcln.Pcfile, cursym, "pctofile", pctofileline, pcln) funcpctab(ctxt, &pcln.Pcline, cursym, "pctoline", pctofileline, nil) - // Check that all the Progs used as inline markers are still reachable. - // See issue #40473. - inlMarkProgs := make(map[*Prog]struct{}, len(cursym.Func.InlMarks)) - for _, inlMark := range cursym.Func.InlMarks { - inlMarkProgs[inlMark.p] = struct{}{} - } - for p := cursym.Func.Text; p != nil; p = p.Link { - if _, ok := inlMarkProgs[p]; ok { - delete(inlMarkProgs, p) - } - } - if len(inlMarkProgs) > 0 { - ctxt.Diag("one or more instructions used as inline markers are no longer reachable") - } - pcinlineState := new(pcinlineState) funcpctab(ctxt, &pcln.Pcinline, cursym, "pctoinline", pcinlineState.pctoinline, nil) for _, inlMark := range cursym.Func.InlMarks { diff --git a/src/cmd/internal/obj/s390x/objz.go b/src/cmd/internal/obj/s390x/objz.go index ef6335d849..b14dc810fa 100644 --- a/src/cmd/internal/obj/s390x/objz.go +++ b/src/cmd/internal/obj/s390x/objz.go @@ -283,6 +283,17 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { ACMPUBNE: q = p p.Mark |= BRANCH + if p.Pcond != nil { + q := p.Pcond + for q.As == obj.ANOP { + q = q.Link + p.Pcond = q + } + } + + case obj.ANOP: + q.Link = p.Link /* q is non-nop */ + p.Link.Mark |= p.Mark default: q = p -- cgit v1.3 From 1b86bdbdc3991c13c6ed156100a5f4918fdd9c6b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 14 Aug 2020 17:44:22 -0400 Subject: cmd/test2json: do not emit a final Action if the result is not known MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we are parsing a test output, and the test does not end in the usual PASS or FAIL line (say, because it panicked), then we need the exit status of the test binary in order to determine whether the test passed or failed. If we don't have that status available, we shouldn't guess arbitrarily — instead, we should omit the final "pass" or "fail" action entirely. (In practice, we nearly always DO have the final status, such as when running 'go test' or 'go tool test2json some.exe'.) Fixes #40132 Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de Reviewed-on: https://go-review.googlesource.com/c/go/+/248624 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- src/cmd/go/internal/test/test.go | 8 +- src/cmd/go/testdata/script/test_json_exit.txt | 102 +++++++++++++++++++++ src/cmd/internal/test2json/test2json.go | 44 +++++---- .../internal/test2json/testdata/benchshort.json | 1 - src/cmd/internal/test2json/testdata/empty.json | 1 - src/cmd/test2json/main.go | 6 +- 6 files changed, 139 insertions(+), 23 deletions(-) create mode 100644 src/cmd/go/testdata/script/test_json_exit.txt (limited to 'src/cmd/internal') diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 9c120e08dc..9cef8cf89c 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -1098,9 +1098,13 @@ func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work. } var stdout io.Writer = os.Stdout + var err error if testJSON { json := test2json.NewConverter(lockedStdout{}, a.Package.ImportPath, test2json.Timestamp) - defer json.Close() + defer func() { + json.Exited(err) + json.Close() + }() stdout = json } @@ -1204,7 +1208,7 @@ func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work. } t0 := time.Now() - err := cmd.Start() + err = cmd.Start() // This is a last-ditch deadline to detect and // stop wedged test binaries, to keep the builders diff --git a/src/cmd/go/testdata/script/test_json_exit.txt b/src/cmd/go/testdata/script/test_json_exit.txt new file mode 100644 index 0000000000..dc7ffb06cf --- /dev/null +++ b/src/cmd/go/testdata/script/test_json_exit.txt @@ -0,0 +1,102 @@ +[short] skip + +go test -c -o mainpanic.exe ./mainpanic & +go test -c -o mainexit0.exe ./mainexit0 & +go test -c -o testpanic.exe ./testpanic & +go test -c -o testbgpanic.exe ./testbgpanic & +wait + +# Test binaries that panic in TestMain should be marked as failing. + +! go test -json ./mainpanic +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +! go tool test2json ./mainpanic.exe +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +# Test binaries that exit with status 0 should be marked as passing. + +go test -json ./mainexit0 +stdout '"Action":"pass"' +! stdout '"Action":"fail"' + +go tool test2json ./mainexit0.exe +stdout '"Action":"pass"' +! stdout '"Action":"fail"' + +# Test functions that panic should never be marked as passing +# (https://golang.org/issue/40132). + +! go test -json ./testpanic +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +! go tool test2json ./testpanic.exe -test.v +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +! go tool test2json ./testpanic.exe +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +# Tests that panic in a background goroutine should be marked as failing. + +! go test -json ./testbgpanic +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +! go tool test2json ./testbgpanic.exe -test.v +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +! go tool test2json ./testbgpanic.exe +stdout '"Action":"fail"' +! stdout '"Action":"pass"' + +-- go.mod -- +module m +go 1.14 +-- mainpanic/mainpanic_test.go -- +package mainpanic_test + +import "testing" + +func TestMain(m *testing.M) { + panic("haha no") +} +-- mainexit0/mainexit0_test.go -- +package mainexit0_test + +import ( + "fmt" + "os" + "testing" +) + +func TestMain(m *testing.M) { + fmt.Println("nothing to do") + os.Exit(0) +} +-- testpanic/testpanic_test.go -- +package testpanic_test + +import "testing" + +func TestPanic(*testing.T) { + panic("haha no") +} +-- testbgpanic/testbgpanic_test.go -- +package testbgpanic_test + +import "testing" + +func TestPanicInBackground(*testing.T) { + c := make(chan struct{}) + go func() { + panic("haha no") + close(c) + }() + <-c +} diff --git a/src/cmd/internal/test2json/test2json.go b/src/cmd/internal/test2json/test2json.go index a01a8900e8..4eb6dd4838 100644 --- a/src/cmd/internal/test2json/test2json.go +++ b/src/cmd/internal/test2json/test2json.go @@ -45,10 +45,10 @@ type textBytes []byte func (b textBytes) MarshalText() ([]byte, error) { return b, nil } -// A converter holds the state of a test-to-JSON conversion. +// A Converter holds the state of a test-to-JSON conversion. // It implements io.WriteCloser; the caller writes test output in, // and the converter writes JSON output to w. -type converter struct { +type Converter struct { w io.Writer // JSON output stream pkg string // package to name in events mode Mode // mode bits @@ -100,9 +100,9 @@ var ( // // The pkg string, if present, specifies the import path to // report in the JSON stream. -func NewConverter(w io.Writer, pkg string, mode Mode) io.WriteCloser { - c := new(converter) - *c = converter{ +func NewConverter(w io.Writer, pkg string, mode Mode) *Converter { + c := new(Converter) + *c = Converter{ w: w, pkg: pkg, mode: mode, @@ -122,11 +122,20 @@ func NewConverter(w io.Writer, pkg string, mode Mode) io.WriteCloser { } // Write writes the test input to the converter. -func (c *converter) Write(b []byte) (int, error) { +func (c *Converter) Write(b []byte) (int, error) { c.input.write(b) return len(b), nil } +// Exited marks the test process as having exited with the given error. +func (c *Converter) Exited(err error) { + if err == nil { + c.result = "pass" + } else { + c.result = "fail" + } +} + var ( // printed by test on successful run. bigPass = []byte("PASS\n") @@ -160,7 +169,7 @@ var ( // handleInputLine handles a single whole test output line. // It must write the line to c.output but may choose to do so // before or after emitting other events. -func (c *converter) handleInputLine(line []byte) { +func (c *Converter) handleInputLine(line []byte) { // Final PASS or FAIL. if bytes.Equal(line, bigPass) || bytes.Equal(line, bigFail) || bytes.HasPrefix(line, bigFailErrorPrefix) { c.flushReport(0) @@ -286,7 +295,7 @@ func (c *converter) handleInputLine(line []byte) { } // flushReport flushes all pending PASS/FAIL reports at levels >= depth. -func (c *converter) flushReport(depth int) { +func (c *Converter) flushReport(depth int) { c.testName = "" for len(c.report) > depth { e := c.report[len(c.report)-1] @@ -298,23 +307,22 @@ func (c *converter) flushReport(depth int) { // Close marks the end of the go test output. // It flushes any pending input and then output (only partial lines at this point) // and then emits the final overall package-level pass/fail event. -func (c *converter) Close() error { +func (c *Converter) Close() error { c.input.flush() c.output.flush() - e := &event{Action: "pass"} if c.result != "" { - e.Action = c.result - } - if c.mode&Timestamp != 0 { - dt := time.Since(c.start).Round(1 * time.Millisecond).Seconds() - e.Elapsed = &dt + e := &event{Action: c.result} + if c.mode&Timestamp != 0 { + dt := time.Since(c.start).Round(1 * time.Millisecond).Seconds() + e.Elapsed = &dt + } + c.writeEvent(e) } - c.writeEvent(e) return nil } // writeOutputEvent writes a single output event with the given bytes. -func (c *converter) writeOutputEvent(out []byte) { +func (c *Converter) writeOutputEvent(out []byte) { c.writeEvent(&event{ Action: "output", Output: (*textBytes)(&out), @@ -323,7 +331,7 @@ func (c *converter) writeOutputEvent(out []byte) { // writeEvent writes a single event. // It adds the package, time (if requested), and test name (if needed). -func (c *converter) writeEvent(e *event) { +func (c *Converter) writeEvent(e *event) { e.Package = c.pkg if c.mode&Timestamp != 0 { t := time.Now() diff --git a/src/cmd/internal/test2json/testdata/benchshort.json b/src/cmd/internal/test2json/testdata/benchshort.json index 28e287c848..34b03b9362 100644 --- a/src/cmd/internal/test2json/testdata/benchshort.json +++ b/src/cmd/internal/test2json/testdata/benchshort.json @@ -4,4 +4,3 @@ {"Action":"output","Output":"# but to avoid questions of timing, we just use a file with no \\n at all.\n"} {"Action":"output","Output":"BenchmarkFoo \t"} {"Action":"output","Output":"10000 early EOF"} -{"Action":"pass"} diff --git a/src/cmd/internal/test2json/testdata/empty.json b/src/cmd/internal/test2json/testdata/empty.json index 80b5217501..e69de29bb2 100644 --- a/src/cmd/internal/test2json/testdata/empty.json +++ b/src/cmd/internal/test2json/testdata/empty.json @@ -1 +0,0 @@ -{"Action":"pass"} diff --git a/src/cmd/test2json/main.go b/src/cmd/test2json/main.go index 0385d8f246..57a874193e 100644 --- a/src/cmd/test2json/main.go +++ b/src/cmd/test2json/main.go @@ -118,12 +118,16 @@ func main() { w := &countWriter{0, c} cmd.Stdout = w cmd.Stderr = w - if err := cmd.Run(); err != nil { + err := cmd.Run() + if err != nil { if w.n > 0 { // Assume command printed why it failed. } else { fmt.Fprintf(c, "test2json: %v\n", err) } + } + c.Exited(err) + if err != nil { c.Close() os.Exit(1) } -- cgit v1.3 From 93eeb819cab491d4e429b7aa85a864a045979a18 Mon Sep 17 00:00:00 2001 From: Meng Zhuo Date: Mon, 8 Jun 2020 13:58:53 +0800 Subject: cmd/asm: Add SHA512 hardware instructions for ARM64 ARMv8.2-SHA add SHA512 intructions: 1. SHA512H Vm.D2, Vn, Vd 2. SHA512H2 Vm.D2, Vn, Vd 3. SHA512SU0 Vn.D2, Vd.D2 4. SHA512SU1 Vm.D2, Vn.D2, Vd.D2 ARMv8 Architecture Reference Manual C7.2.234-C7.2.234 Change-Id: Ie970fef1bba5312ad466f246035da4c40a1bbb39 Reviewed-on: https://go-review.googlesource.com/c/go/+/180057 Reviewed-by: Cherry Zhang Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot --- src/cmd/asm/internal/asm/testdata/arm64.s | 4 ++++ src/cmd/internal/obj/arm64/a.out.go | 4 ++++ src/cmd/internal/obj/arm64/anames.go | 4 ++++ src/cmd/internal/obj/arm64/asm7.go | 16 ++++++++++++++++ 4 files changed, 28 insertions(+) (limited to 'src/cmd/internal') diff --git a/src/cmd/asm/internal/asm/testdata/arm64.s b/src/cmd/asm/internal/asm/testdata/arm64.s index 69267bfa63..5a6db05074 100644 --- a/src/cmd/asm/internal/asm/testdata/arm64.s +++ b/src/cmd/asm/internal/asm/testdata/arm64.s @@ -77,6 +77,10 @@ TEXT foo(SB), DUPOK|NOSPLIT, $-8 SHA1H V5, V4 // a408285e SHA1M V8.S4, V7, V6 // e620085e SHA1P V11.S4, V10, V9 // 49110b5e + SHA512H V2.D2, V1, V0 // 208062ce + SHA512H2 V4.D2, V3, V2 // 628464ce + SHA512SU0 V9.D2, V8.D2 // 2881c0ce + SHA512SU1 V7.D2, V6.D2, V5.D2 // c58867ce VADDV V0.S4, V0 // 00b8b14e VMOVI $82, V0.B16 // 40e6024f VUADDLV V6.B16, V6 // c638306e diff --git a/src/cmd/internal/obj/arm64/a.out.go b/src/cmd/internal/obj/arm64/a.out.go index 152c493a65..03e0278a33 100644 --- a/src/cmd/internal/obj/arm64/a.out.go +++ b/src/cmd/internal/obj/arm64/a.out.go @@ -946,6 +946,10 @@ const ( ASHA256H2 ASHA256SU0 ASHA256SU1 + ASHA512H + ASHA512H2 + ASHA512SU0 + ASHA512SU1 AVADD AVADDP AVAND diff --git a/src/cmd/internal/obj/arm64/anames.go b/src/cmd/internal/obj/arm64/anames.go index 565f70aaf9..65ecd007ea 100644 --- a/src/cmd/internal/obj/arm64/anames.go +++ b/src/cmd/internal/obj/arm64/anames.go @@ -453,6 +453,10 @@ var Anames = []string{ "SHA256H2", "SHA256SU0", "SHA256SU1", + "SHA512H", + "SHA512H2", + "SHA512SU0", + "SHA512SU1", "VADD", "VADDP", "VAND", diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index df17729a76..8f8981479b 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -2747,6 +2747,7 @@ func buildop(ctxt *obj.Link) { oprangeset(AAESIMC, t) oprangeset(ASHA1SU1, t) oprangeset(ASHA256SU0, t) + oprangeset(ASHA512SU0, t) case ASHA1C: oprangeset(ASHA1P, t) @@ -2754,9 +2755,12 @@ func buildop(ctxt *obj.Link) { case ASHA256H: oprangeset(ASHA256H2, t) + oprangeset(ASHA512H, t) + oprangeset(ASHA512H2, t) case ASHA1SU0: oprangeset(ASHA256SU1, t) + oprangeset(ASHA512SU1, t) case AVADDV: oprangeset(AVUADDLV, t) @@ -5391,6 +5395,18 @@ func (c *ctxt7) oprrr(p *obj.Prog, a obj.As) uint32 { case ASHA256SU0: return 0x5E<<24 | 2<<20 | 8<<16 | 2<<12 | 2<<10 + case ASHA512H: + return 0xCE<<24 | 3<<21 | 8<<12 + + case ASHA512H2: + return 0xCE<<24 | 3<<21 | 8<<12 | 4<<8 + + case ASHA512SU1: + return 0xCE<<24 | 3<<21 | 8<<12 | 8<<8 + + case ASHA512SU0: + return 0xCE<<24 | 3<<22 | 8<<12 + case AFCVTZSD: return FPCVTI(1, 0, 1, 3, 0) -- cgit v1.3 From 31da1d993a498acefcf3dd5fdfbefa8eec132791 Mon Sep 17 00:00:00 2001 From: Tao Qingyun Date: Wed, 19 Aug 2020 01:38:43 +0000 Subject: cmd/internal/objfile: cache computation of goobj.Arch Change-Id: I23774cf185e5fa6b89398001cd0655fb0c5bdb46 GitHub-Last-Rev: ca8cae2469b5fad84bd636a3305a484dfdcb0db2 GitHub-Pull-Request: golang/go#40877 Reviewed-on: https://go-review.googlesource.com/c/go/+/249180 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/internal/archive/archive.go | 6 ++++++ src/cmd/internal/objfile/goobj.go | 32 +++++++++++++------------------- 2 files changed, 19 insertions(+), 19 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/archive/archive.go b/src/cmd/internal/archive/archive.go index db67ce424b..c1661d7711 100644 --- a/src/cmd/internal/archive/archive.go +++ b/src/cmd/internal/archive/archive.go @@ -17,6 +17,7 @@ import ( "log" "os" "strconv" + "strings" "time" "unicode/utf8" ) @@ -83,6 +84,7 @@ func (e *Entry) String() string { type GoObj struct { TextHeader []byte + Arch string Data } @@ -404,6 +406,10 @@ func (r *objReader) parseObject(o *GoObj, size int64) error { } } o.TextHeader = h + hs := strings.Fields(string(h)) + if len(hs) >= 4 { + o.Arch = hs[3] + } o.Offset = r.offset o.Size = size - int64(len(h)) diff --git a/src/cmd/internal/objfile/goobj.go b/src/cmd/internal/objfile/goobj.go index e838f58aed..af9ada3324 100644 --- a/src/cmd/internal/objfile/goobj.go +++ b/src/cmd/internal/objfile/goobj.go @@ -17,13 +17,13 @@ import ( "fmt" "io" "os" - "strings" ) type goobjFile struct { goobj *archive.GoObj r *goobj.Reader f *os.File + arch *sys.Arch } func openGoFile(f *os.File) (*File, error) { @@ -45,9 +45,16 @@ L: return nil, err } r := goobj.NewReaderFromBytes(b, false) + var arch *sys.Arch + for _, a := range sys.Archs { + if a.Name == e.Obj.Arch { + arch = a + break + } + } entries = append(entries, &Entry{ name: e.Name, - raw: &goobjFile{e.Obj, r, f}, + raw: &goobjFile{e.Obj, r, f, arch}, }) continue case archive.EntryNativeObj: @@ -223,17 +230,8 @@ func (f *goobjFile) pcln() (textStart uint64, symtab, pclntab []byte, err error) // Returns "",0,nil if unknown. // This function implements the Liner interface in preference to pcln() above. func (f *goobjFile) PCToLine(pc uint64) (string, int, *gosym.Func) { - // TODO: this is really inefficient. Binary search? Memoize last result? r := f.r - var arch *sys.Arch - archname := f.goarch() - for _, a := range sys.Archs { - if a.Name == archname { - arch = a - break - } - } - if arch == nil { + if f.arch == nil { return "", 0, nil } pcdataBase := r.PcdataBase() @@ -264,10 +262,10 @@ func (f *goobjFile) PCToLine(pc uint64) (string, int, *gosym.Func) { lengths := info.ReadFuncInfoLengths(b) off, end := info.ReadPcline(b) pcline := r.BytesAt(pcdataBase+off, int(end-off)) - line := int(pcValue(pcline, pc-addr, arch)) + line := int(pcValue(pcline, pc-addr, f.arch)) off, end = info.ReadPcfile(b) pcfile := r.BytesAt(pcdataBase+off, int(end-off)) - fileID := pcValue(pcfile, pc-addr, arch) + fileID := pcValue(pcfile, pc-addr, f.arch) globalFileID := info.ReadFile(b, lengths.FileOff, uint32(fileID)) fileName := r.File(int(globalFileID)) // Note: we provide only the name in the Func structure. @@ -332,11 +330,7 @@ func (f *goobjFile) text() (textStart uint64, text []byte, err error) { } func (f *goobjFile) goarch() string { - hs := strings.Fields(string(f.goobj.TextHeader)) - if len(hs) >= 4 { - return hs[3] - } - return "" + return f.goobj.Arch } func (f *goobjFile) loadAddress() (uint64, error) { -- cgit v1.3 From 64350f1eabeb688e997c6cd0c103e21c02ab2a46 Mon Sep 17 00:00:00 2001 From: "Paul E. Murphy" Date: Tue, 18 Aug 2020 16:26:42 -0500 Subject: cmd/asm,cmd/internal/obj/ppc64: add {l,st}xvx power9 instructions These are the indexed vsx load operations with the same endian and alignment benefits of {l,st}vx. Likewise, cleanup redundant comments in op{load,store}x and fix ISA 3.0 typos nearby. Change-Id: Ie1ace17c6150cf9168a834e435114028ff6eb07c Reviewed-on: https://go-review.googlesource.com/c/go/+/249025 Run-TryBot: Lynn Boger TryBot-Result: Gobot Gobot Reviewed-by: Lynn Boger --- src/cmd/asm/internal/asm/testdata/ppc64.s | 2 ++ src/cmd/asm/internal/asm/testdata/ppc64enc.s | 2 ++ src/cmd/internal/obj/ppc64/asm9.go | 30 ++++++++++++++++------------ 3 files changed, 21 insertions(+), 13 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/asm/internal/asm/testdata/ppc64.s b/src/cmd/asm/internal/asm/testdata/ppc64.s index b3736bf6a4..ba64d84a35 100644 --- a/src/cmd/asm/internal/asm/testdata/ppc64.s +++ b/src/cmd/asm/internal/asm/testdata/ppc64.s @@ -1037,6 +1037,7 @@ label1: // VSX load with length X-form (also left-justified) LXVL R3,R4, VS0 LXVLL R3,R4, VS0 + LXVX R3,R4, VS0 // VSX load, DQ-form // DQ(RA), XS produces // XS, DQ(RA) @@ -1060,6 +1061,7 @@ label1: // VSX store with length, X-form (also left-justified) STXVL VS0, R3,R4 STXVLL VS0, R3,R4 + STXVX VS0, R3,R4 // VSX move from VSR, XX1-form // XS,RA produces diff --git a/src/cmd/asm/internal/asm/testdata/ppc64enc.s b/src/cmd/asm/internal/asm/testdata/ppc64enc.s index 07a8a540cd..10a05ec402 100644 --- a/src/cmd/asm/internal/asm/testdata/ppc64enc.s +++ b/src/cmd/asm/internal/asm/testdata/ppc64enc.s @@ -595,11 +595,13 @@ TEXT asmtest(SB),DUPOK|NOSPLIT,$0 LXV 16(R3), VS1 // f4230011 LXVL R3, R4, VS1 // 7c23221a LXVLL R3, R4, VS1 // 7c23225a + LXVX R3, R4, VS1 // 7c232218 LXSDX (R3)(R4), VS1 // 7c241c98 STXVD2X VS1, (R3)(R4) // 7c241f98 STXV VS1,16(R3) // f4230015 STXVL VS1, R3, R4 // 7c23231a STXVLL VS1, R3, R4 // 7c23235a + STXVX VS1, R3, R4 // 7c232318 STXSDX VS1, (R3)(R4) // 7c241d98 LXSIWAX (R3)(R4), VS1 // 7c241898 STXSIWX VS1, (R3)(R4) // 7c241918 diff --git a/src/cmd/internal/obj/ppc64/asm9.go b/src/cmd/internal/obj/ppc64/asm9.go index 238ca8f0b7..3c82477fc4 100644 --- a/src/cmd/internal/obj/ppc64/asm9.go +++ b/src/cmd/internal/obj/ppc64/asm9.go @@ -1584,8 +1584,9 @@ func buildop(ctxt *obj.Link) { case ALXV: /* lxv */ opset(ALXV, r0) - case ALXVL: /* lxvl */ + case ALXVL: /* lxvl, lxvll, lxvx */ opset(ALXVLL, r0) + opset(ALXVX, r0) case ASTXVD2X: /* stxvd2x, stxvdsx, stxvw4x, stxvh8x, stxvb16x */ opset(ASTXVW4X, r0) @@ -1595,8 +1596,9 @@ func buildop(ctxt *obj.Link) { case ASTXV: /* stxv */ opset(ASTXV, r0) - case ASTXVL: /* stxvl, stxvll */ + case ASTXVL: /* stxvl, stxvll, stvx */ opset(ASTXVLL, r0) + opset(ASTXVX, r0) case ALXSDX: /* lxsdx */ opset(ALXSDX, r0) @@ -5020,11 +5022,13 @@ func (c *ctxt9) opload(a obj.As) uint32 { case AMOVW: return OPVCC(58, 0, 0, 0) | 1<<1 /* lwa */ case ALXV: - return OPDQ(61, 1, 0) /* lxv - ISA v3.00 */ + return OPDQ(61, 1, 0) /* lxv - ISA v3.0 */ case ALXVL: - return OPVXX1(31, 269, 0) /* lxvl - ISA v3.00 */ + return OPVXX1(31, 269, 0) /* lxvl - ISA v3.0 */ case ALXVLL: - return OPVXX1(31, 301, 0) /* lxvll - ISA v3.00 */ + return OPVXX1(31, 301, 0) /* lxvll - ISA v3.0 */ + case ALXVX: + return OPVXX1(31, 268, 0) /* lxvx - ISA v3.0 */ /* no AMOVWU */ case AMOVB, AMOVBZ: @@ -5122,8 +5126,6 @@ func (c *ctxt9) oploadx(a obj.As) uint32 { return OPVCC(31, 309, 0, 0) /* ldmx */ /* Vector (VMX/Altivec) instructions */ - /* ISA 2.03 enables these for PPC970. For POWERx processors, these */ - /* are enabled starting at POWER6 (ISA 2.05). */ case ALVEBX: return OPVCC(31, 7, 0, 0) /* lvebx - v2.03 */ case ALVEHX: @@ -5141,7 +5143,8 @@ func (c *ctxt9) oploadx(a obj.As) uint32 { /* End of vector instructions */ /* Vector scalar (VSX) instructions */ - /* ISA 2.06 enables these for POWER7. */ + case ALXVX: + return OPVXX1(31, 268, 0) /* lxvx - ISA v3.0 */ case ALXVD2X: return OPVXX1(31, 844, 0) /* lxvd2x - v2.06 */ case ALXVW4X: @@ -5208,6 +5211,8 @@ func (c *ctxt9) opstore(a obj.As) uint32 { return OPVXX1(31, 397, 0) /* stxvl ISA 3.0 */ case ASTXVLL: return OPVXX1(31, 429, 0) /* stxvll ISA 3.0 */ + case ASTXVX: + return OPVXX1(31, 396, 0) /* stxvx - ISA v3.0 */ } @@ -5271,8 +5276,6 @@ func (c *ctxt9) opstorex(a obj.As) uint32 { return OPVCC(31, 181, 0, 0) /* stdux */ /* Vector (VMX/Altivec) instructions */ - /* ISA 2.03 enables these for PPC970. For POWERx processors, these */ - /* are enabled starting at POWER6 (ISA 2.05). */ case ASTVEBX: return OPVCC(31, 135, 0, 0) /* stvebx - v2.03 */ case ASTVEHX: @@ -5286,15 +5289,16 @@ func (c *ctxt9) opstorex(a obj.As) uint32 { /* End of vector instructions */ /* Vector scalar (VSX) instructions */ - /* ISA 2.06 enables these for POWER7. */ + case ASTXVX: + return OPVXX1(31, 396, 0) /* stxvx - v3.0 */ case ASTXVD2X: return OPVXX1(31, 972, 0) /* stxvd2x - v2.06 */ case ASTXVW4X: return OPVXX1(31, 908, 0) /* stxvw4x - v2.06 */ case ASTXVH8X: - return OPVXX1(31, 940, 0) /* stxvh8x - v3.00 */ + return OPVXX1(31, 940, 0) /* stxvh8x - v3.0 */ case ASTXVB16X: - return OPVXX1(31, 1004, 0) /* stxvb16x - v3.00 */ + return OPVXX1(31, 1004, 0) /* stxvb16x - v3.0 */ case ASTXSDX: return OPVXX1(31, 716, 0) /* stxsdx - v2.06 */ -- cgit v1.3 From 46ca7b5ee2a8582736f1ddac27d8660e1104c345 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sat, 15 Aug 2020 08:08:37 -0700 Subject: cmd/internal/obj: stop removing NOPs from instruction stream This has already been done for s390x, ppc64. This CL is for all the other architectures. Fixes #40796 Change-Id: Idd1816e057df63022d47e99fa06617811d8c8489 Reviewed-on: https://go-review.googlesource.com/c/go/+/248684 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/cmd/internal/obj/arm/asm5.go | 3 ++ src/cmd/internal/obj/arm/obj5.go | 46 ------------------------------- src/cmd/internal/obj/arm64/asm7.go | 3 ++ src/cmd/internal/obj/arm64/obj7.go | 56 ++------------------------------------ src/cmd/internal/obj/mips/asm0.go | 3 ++ src/cmd/internal/obj/mips/obj0.go | 26 ++---------------- 6 files changed, 14 insertions(+), 123 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/arm/asm5.go b/src/cmd/internal/obj/arm/asm5.go index f66f8aaf84..7b7e42ee2e 100644 --- a/src/cmd/internal/obj/arm/asm5.go +++ b/src/cmd/internal/obj/arm/asm5.go @@ -327,6 +327,9 @@ var optab = []Optab{ {obj.APCDATA, C_LCON, C_NONE, C_LCON, 0, 0, 0, 0, 0, 0}, {obj.AFUNCDATA, C_LCON, C_NONE, C_ADDR, 0, 0, 0, 0, 0, 0}, {obj.ANOP, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0, 0}, + {obj.ANOP, C_LCON, C_NONE, C_NONE, 0, 0, 0, 0, 0, 0}, // nop variants, see #40689 + {obj.ANOP, C_REG, C_NONE, C_NONE, 0, 0, 0, 0, 0, 0}, + {obj.ANOP, C_FREG, C_NONE, C_NONE, 0, 0, 0, 0, 0, 0}, {obj.ADUFFZERO, C_NONE, C_NONE, C_SBRA, 5, 4, 0, 0, 0, 0}, // same as ABL {obj.ADUFFCOPY, C_NONE, C_NONE, C_SBRA, 5, 4, 0, 0, 0, 0}, // same as ABL {obj.AXXX, C_NONE, C_NONE, C_NONE, 0, 4, 0, 0, 0, 0}, diff --git a/src/cmd/internal/obj/arm/obj5.go b/src/cmd/internal/obj/arm/obj5.go index 008118c47b..86831f2b44 100644 --- a/src/cmd/internal/obj/arm/obj5.go +++ b/src/cmd/internal/obj/arm/obj5.go @@ -276,67 +276,21 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { /* * find leaf subroutines - * strip NOPs - * expand RET - * expand BECOME pseudo */ - var q1 *obj.Prog - var q *obj.Prog for p := cursym.Func.Text; p != nil; p = p.Link { switch p.As { case obj.ATEXT: p.Mark |= LEAF - case obj.ARET: - break - case ADIV, ADIVU, AMOD, AMODU: - q = p cursym.Func.Text.Mark &^= LEAF - continue - - case obj.ANOP: - q1 = p.Link - q.Link = q1 /* q is non-nop */ - if q1 != nil { - q1.Mark |= p.Mark - } - continue case ABL, ABX, obj.ADUFFZERO, obj.ADUFFCOPY: cursym.Func.Text.Mark &^= LEAF - fallthrough - - case AB, - ABEQ, - ABNE, - ABCS, - ABHS, - ABCC, - ABLO, - ABMI, - ABPL, - ABVS, - ABVC, - ABHI, - ABLS, - ABGE, - ABLT, - ABGT, - ABLE: - q1 = p.Pcond - if q1 != nil { - for q1.As == obj.ANOP { - q1 = q1.Link - p.Pcond = q1 - } - } } - - q = p } var q2 *obj.Prog diff --git a/src/cmd/internal/obj/arm64/asm7.go b/src/cmd/internal/obj/arm64/asm7.go index 8f8981479b..7a5a8ff38c 100644 --- a/src/cmd/internal/obj/arm64/asm7.go +++ b/src/cmd/internal/obj/arm64/asm7.go @@ -837,6 +837,9 @@ var optab = []Optab{ {obj.APCDATA, C_VCON, C_NONE, C_NONE, C_VCON, 0, 0, 0, 0, 0}, {obj.AFUNCDATA, C_VCON, C_NONE, C_NONE, C_ADDR, 0, 0, 0, 0, 0}, {obj.ANOP, C_NONE, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, + {obj.ANOP, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, // nop variants, see #40689 + {obj.ANOP, C_REG, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, + {obj.ANOP, C_VREG, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, {obj.ADUFFZERO, C_NONE, C_NONE, C_NONE, C_SBRA, 5, 4, 0, 0, 0}, // same as AB/ABL {obj.ADUFFCOPY, C_NONE, C_NONE, C_NONE, C_SBRA, 5, 4, 0, 0, 0}, // same as AB/ABL {obj.APCALIGN, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, // align code diff --git a/src/cmd/internal/obj/arm64/obj7.go b/src/cmd/internal/obj/arm64/obj7.go index b046685ada..0d74430053 100644 --- a/src/cmd/internal/obj/arm64/obj7.go +++ b/src/cmd/internal/obj/arm64/obj7.go @@ -468,73 +468,21 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { /* * find leaf subroutines - * strip NOPs - * expand RET */ - q := (*obj.Prog)(nil) - var q1 *obj.Prog for p := c.cursym.Func.Text; p != nil; p = p.Link { switch p.As { case obj.ATEXT: p.Mark |= LEAF - case obj.ARET: - break - - case obj.ANOP: - if p.Link != nil { - q1 = p.Link - q.Link = q1 /* q is non-nop */ - q1.Mark |= p.Mark - } - continue - case ABL, obj.ADUFFZERO, obj.ADUFFCOPY: c.cursym.Func.Text.Mark &^= LEAF - fallthrough - - case ACBNZ, - ACBZ, - ACBNZW, - ACBZW, - ATBZ, - ATBNZ, - AB, - ABEQ, - ABNE, - ABCS, - ABHS, - ABCC, - ABLO, - ABMI, - ABPL, - ABVS, - ABVC, - ABHI, - ABLS, - ABGE, - ABLT, - ABGT, - ABLE, - AADR, /* strange */ - AADRP: - q1 = p.Pcond - - if q1 != nil { - for q1.As == obj.ANOP { - q1 = q1.Link - p.Pcond = q1 - } - } - - break } - - q = p } + var q *obj.Prog + var q1 *obj.Prog var retjmp *obj.LSym for p := c.cursym.Func.Text; p != nil; p = p.Link { o := p.As diff --git a/src/cmd/internal/obj/mips/asm0.go b/src/cmd/internal/obj/mips/asm0.go index faa12bf133..faa827da9f 100644 --- a/src/cmd/internal/obj/mips/asm0.go +++ b/src/cmd/internal/obj/mips/asm0.go @@ -391,6 +391,9 @@ var optab = []Optab{ {obj.APCDATA, C_LCON, C_NONE, C_LCON, 0, 0, 0, 0, 0}, {obj.AFUNCDATA, C_SCON, C_NONE, C_ADDR, 0, 0, 0, 0, 0}, {obj.ANOP, C_NONE, C_NONE, C_NONE, 0, 0, 0, 0, 0}, + {obj.ANOP, C_LCON, C_NONE, C_NONE, 0, 0, 0, 0, 0}, // nop variants, see #40689 + {obj.ANOP, C_REG, C_NONE, C_NONE, 0, 0, 0, 0, 0}, + {obj.ANOP, C_FREG, C_NONE, C_NONE, 0, 0, 0, 0, 0}, {obj.ADUFFZERO, C_NONE, C_NONE, C_LBRA, 11, 4, 0, 0, 0}, // same as AJMP {obj.ADUFFCOPY, C_NONE, C_NONE, C_LBRA, 11, 4, 0, 0, 0}, // same as AJMP diff --git a/src/cmd/internal/obj/mips/obj0.go b/src/cmd/internal/obj/mips/obj0.go index 3106143844..77cad979a6 100644 --- a/src/cmd/internal/obj/mips/obj0.go +++ b/src/cmd/internal/obj/mips/obj0.go @@ -158,19 +158,14 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { /* * find leaf subroutines - * strip NOPs * expand RET * expand BECOME pseudo */ - var q *obj.Prog - var q1 *obj.Prog for p := c.cursym.Func.Text; p != nil; p = p.Link { switch p.As { /* too hard, just leave alone */ case obj.ATEXT: - q = p - p.Mark |= LABEL | LEAF | SYNC if p.Link != nil { p.Link.Mark |= LABEL @@ -179,7 +174,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { /* too hard, just leave alone */ case AMOVW, AMOVV: - q = p if p.To.Type == obj.TYPE_REG && p.To.Reg >= REG_SPECIAL { p.Mark |= LABEL | SYNC break @@ -195,11 +189,9 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { ATLBWI, ATLBP, ATLBR: - q = p p.Mark |= LABEL | SYNC case ANOR: - q = p if p.To.Type == obj.TYPE_REG { if p.To.Reg == REGZERO { p.Mark |= LABEL | SYNC @@ -235,8 +227,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { } else { p.Mark |= BRANCH } - q = p - q1 = p.Pcond + q1 := p.Pcond if q1 != nil { for q1.As == obj.ANOP { q1 = q1.Link @@ -254,24 +245,11 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { if q1 != nil { q1.Mark |= LABEL } - continue case ARET: - q = p if p.Link != nil { p.Link.Mark |= LABEL } - continue - - case obj.ANOP: - q1 = p.Link - q.Link = q1 /* q is non-nop */ - q1.Mark |= p.Mark - continue - - default: - q = p - continue } } @@ -284,6 +262,8 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { mov = AMOVW } + var q *obj.Prog + var q1 *obj.Prog autosize := int32(0) var p1 *obj.Prog var p2 *obj.Prog -- cgit v1.3 From 268dd2e5a7a8919bd26f0a59c847f8268a2437d1 Mon Sep 17 00:00:00 2001 From: Michael Munday Date: Mon, 10 Aug 2020 08:01:21 -0700 Subject: cmd/internal/obj: fix inline marker issue on s390x The optimization that replaces inline markers with pre-existing instructions assumes that 'Prog' values produced by the compiler are still reachable after the assembler has run. This was not true on s390x where the assembler was removing NOP instructions from the linked list of 'Prog' values. This led to broken inlining data which in turn caused an infinite loop in the runtime traceback code. Fix this by stopping the s390x assembler backend removing NOP values. It does not make any difference to the output of the assembler because NOP instructions are 0 bytes long anyway. Fixes #40473. Change-Id: I9b97c494afaae2d5ed6bca4cd428b4132b5f8133 Reviewed-on: https://go-review.googlesource.com/c/go/+/249448 Run-TryBot: Michael Munday TryBot-Result: Gobot Gobot Reviewed-by: Keith Randall --- src/cmd/internal/obj/pcln.go | 15 +++++++++++++++ src/cmd/internal/obj/s390x/objz.go | 11 ----------- 2 files changed, 15 insertions(+), 11 deletions(-) (limited to 'src/cmd/internal') diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go index 1f7ccf47ef..bffeda041d 100644 --- a/src/cmd/internal/obj/pcln.go +++ b/src/cmd/internal/obj/pcln.go @@ -278,6 +278,21 @@ func linkpcln(ctxt *Link, cursym *LSym) { funcpctab(ctxt, &pcln.Pcfile, cursym, "pctofile", pctofileline, pcln) funcpctab(ctxt, &pcln.Pcline, cursym, "pctoline", pctofileline, nil) + // Check that all the Progs used as inline markers are still reachable. + // See issue #40473. + inlMarkProgs := make(map[*Prog]struct{}, len(cursym.Func.InlMarks)) + for _, inlMark := range cursym.Func.InlMarks { + inlMarkProgs[inlMark.p] = struct{}{} + } + for p := cursym.Func.Text; p != nil; p = p.Link { + if _, ok := inlMarkProgs[p]; ok { + delete(inlMarkProgs, p) + } + } + if len(inlMarkProgs) > 0 { + ctxt.Diag("one or more instructions used as inline markers are no longer reachable") + } + pcinlineState := new(pcinlineState) funcpctab(ctxt, &pcln.Pcinline, cursym, "pctoinline", pcinlineState.pctoinline, nil) for _, inlMark := range cursym.Func.InlMarks { diff --git a/src/cmd/internal/obj/s390x/objz.go b/src/cmd/internal/obj/s390x/objz.go index b14dc810fa..ef6335d849 100644 --- a/src/cmd/internal/obj/s390x/objz.go +++ b/src/cmd/internal/obj/s390x/objz.go @@ -283,17 +283,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { ACMPUBNE: q = p p.Mark |= BRANCH - if p.Pcond != nil { - q := p.Pcond - for q.As == obj.ANOP { - q = q.Link - p.Pcond = q - } - } - - case obj.ANOP: - q.Link = p.Link /* q is non-nop */ - p.Link.Mark |= p.Mark default: q = p -- cgit v1.3