From 9abbe277105f9b9a3b4c5905e7faf9a02827ce18 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sat, 19 Dec 2020 13:46:49 -0800 Subject: [PATCH 01/27] test: skip issue11656.go on mips/mips64/ppc64 For #11656 For #43283 Change-Id: I1fcf2b24800f421e36201af43130b487abe605b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/279312 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Run-TryBot: Emmanuel Odeke Reviewed-by: Emmanuel Odeke --- test/fixedbugs/issue11656.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/fixedbugs/issue11656.go b/test/fixedbugs/issue11656.go index 62b36cf790..5018263364 100644 --- a/test/fixedbugs/issue11656.go +++ b/test/fixedbugs/issue11656.go @@ -27,6 +27,13 @@ import ( ) func main() { + // This test is currently failing on some architectures. + // See issue #43283. + switch runtime.GOARCH { + case "ppc64", "mips", "mipsle", "mips64", "mips64le": + return + } + debug.SetPanicOnFault(true) defer func() { if err := recover(); err == nil { From 53c984d976f49b5671b11ff17f5d622572d4cf58 Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Sat, 5 Dec 2020 19:53:08 +0000 Subject: [PATCH 02/27] runtime: skip wakep call in wakeNetPoller on Plan 9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was part of a performance improvement made by CL 232298 to reduce timer latency. On multiprocessor Plan 9 machines, it triggers memory faults often enough that the builder test suite never completes successfully. See issue #42303 for discussion. As shown by the benchmark result below, worst case latency on plan9_arm is very bad even with the wakep call in place - in the tickers-per-P=1 case, a 3ms timer is 270ms late. Skipping the wakep call and running the benchmark again shows some cases worse, some better. The performance cost doesn't seem excessive for this temporary workaround which makes the plan9_arm builders usable again. With wakep call: cpu% go test -bench Latency time goos: plan9 goarch: arm pkg: time BenchmarkParallelTimerLatency-4 100 10985859 avg-late-ns 18630963 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-4 195 270294688 avg-late-ns 542057670 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-4 234 182452000 avg-late-ns 423933688 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-4 280 193003004 avg-late-ns 408034405 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-4 282 132819086 avg-late-ns 313624570 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-4 339 71152187 avg-late-ns 189014519 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-4 315 26860484 avg-late-ns 101759844 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-4 357 19106739 avg-late-ns 59435620 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-4 376 7246933 avg-late-ns 38888461 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-4 267 40476892 avg-late-ns 205851926 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-4 294 87836303 avg-late-ns 252059695 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-4 379 4127144 avg-late-ns 10494927 max-late-ns Without wakep call: BenchmarkParallelTimerLatency-4 61 10775151 avg-late-ns 18668517 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=1-4 199 299587535 avg-late-ns 597182307 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=2-4 272 184561831 avg-late-ns 449739837 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=3-4 235 154983257 avg-late-ns 370940553 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=4-4 290 150034689 avg-late-ns 332399843 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=5-4 298 47540764 avg-late-ns 133709031 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=6-4 350 20379394 avg-late-ns 81742809 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=7-4 363 14403223 avg-late-ns 98901212 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=8-4 375 12293090 avg-late-ns 50266552 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=9-4 336 40628820 avg-late-ns 150946099 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=300µs/tickers-per-P=10-4 289 88265539 avg-late-ns 280770418 max-late-ns BenchmarkStaggeredTickerLatency/work-dur=2ms/tickers-per-P=1-4 375 8364937 avg-late-ns 22598421 max-late-ns Fixes #42303 Change-Id: I70c63cb2a2bad46950a7cd9dfc7bb32943710d32 Reviewed-on: https://go-review.googlesource.com/c/go/+/275672 Reviewed-by: David du Colombier <0intro@gmail.com> Trust: Michael Pratt --- src/runtime/proc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 64e102fb0a..418e06932e 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -2882,7 +2882,9 @@ func wakeNetPoller(when int64) { } else { // There are no threads in the network poller, try to get // one there so it can handle new timers. - wakep() + if GOOS != "plan9" { // Temporary workaround - see issue #42303. + wakep() + } } } From cb95819cf6e969dc7dcc64ec7820d3995379c9f4 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 11 Dec 2020 14:14:30 -0500 Subject: [PATCH 03/27] runtime: detect netbsd netpoll overrun in sysmon The netbsd kernel has a bug [1] that occassionally prevents netpoll from waking with netpollBreak, which could result in missing timers for an unbounded amount of time, as netpoll can't restart with a shorter delay when an earlier timer is added. Prior to CL 232298, sysmon could detect these overrun timers and manually start an M to run them. With this fallback gone, the bug actually prevents timer execution indefinitely. As a workaround, we add back sysmon detection only for netbsd. [1] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50094 Updates #42515 Change-Id: I8391f5b9dabef03dd1d94c50b3b4b3bd4f889e66 Reviewed-on: https://go-review.googlesource.com/c/go/+/277332 Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Michael Knyszek Reviewed-by: Austin Clements Trust: Michael Pratt --- src/runtime/proc.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 418e06932e..5adcbf07dc 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -5130,6 +5130,26 @@ func sysmon() { } } mDoFixup() + if GOOS == "netbsd" { + // netpoll is responsible for waiting for timer + // expiration, so we typically don't have to worry + // about starting an M to service timers. (Note that + // sleep for timeSleepUntil above simply ensures sysmon + // starts running again when that timer expiration may + // cause Go code to run again). + // + // However, netbsd has a kernel bug that sometimes + // misses netpollBreak wake-ups, which can lead to + // unbounded delays servicing timers. If we detect this + // overrun, then startm to get something to handle the + // timer. + // + // See issue 42515 and + // https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50094. + if next, _ := timeSleepUntil(); next < now { + startm(nil, false) + } + } if atomic.Load(&scavenge.sysmonWake) != 0 { // Kick the scavenger awake if someone requested it. wakeScavenger() From 8438a5779b76620237d608282a99d17467b91f4c Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Wed, 11 Nov 2020 21:27:56 -0500 Subject: [PATCH 04/27] runtime: use _exit on darwin On darwin, where we use libc for syscalls, when the runtime exits, it calls libc exit function, which may call back into user code, e.g. invoking functions registered with atexit. In particular, it may call back into Go. But at this point, the Go runtime is already exiting, so this wouldn't work. On non-libc platforms we use exit syscall directly, which doesn't invoke any callbacks. Use _exit on darwin to achieve the same behavior. No test for now, as it doesn't pass on all platforms (see trybot run of PS2). May fix #42465. May fix #43294. Change-Id: Ia1ada22b5da8cb64fdd598d0541eb90e195367eb Reviewed-on: https://go-review.googlesource.com/c/go/+/269378 Trust: Cherry Zhang Run-TryBot: Cherry Zhang TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/runtime/sys_darwin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go index c89ce78012..55845bf2e5 100644 --- a/src/runtime/sys_darwin.go +++ b/src/runtime/sys_darwin.go @@ -467,7 +467,7 @@ func setNonblock(fd int32) { //go:cgo_import_dynamic libc_pthread_create pthread_create "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_pthread_self pthread_self "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_pthread_kill pthread_kill "/usr/lib/libSystem.B.dylib" -//go:cgo_import_dynamic libc_exit exit "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc_exit _exit "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_raise raise "/usr/lib/libSystem.B.dylib" //go:cgo_import_dynamic libc_open open "/usr/lib/libSystem.B.dylib" From 6cff874c47bdb4567f5c84bc59d93311493caefe Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 7 Dec 2020 15:11:46 +0000 Subject: [PATCH 05/27] runtime/metrics: add Read examples This change adds two examples of using the Read function: one that reads one metric and one that reads all metrics. Change-Id: I4940a44c9b1d65f3f7a1554e3145ff07e6492fc1 Reviewed-on: https://go-review.googlesource.com/c/go/+/275855 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt Trust: Michael Knyszek --- src/runtime/metrics/example_test.go | 96 +++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 src/runtime/metrics/example_test.go diff --git a/src/runtime/metrics/example_test.go b/src/runtime/metrics/example_test.go new file mode 100644 index 0000000000..cade0c38bf --- /dev/null +++ b/src/runtime/metrics/example_test.go @@ -0,0 +1,96 @@ +// 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 metrics_test + +import ( + "fmt" + "runtime/metrics" +) + +func ExampleRead_readingOneMetric() { + // Name of the metric we want to read. + const myMetric = "/memory/classes/heap/free:bytes" + + // Create a sample for the metric. + sample := make([]metrics.Sample, 1) + sample[0].Name = myMetric + + // Sample the metric. + metrics.Read(sample) + + // Check if the metric is actually supported. + // If it's not, the resulting value will always have + // kind KindBad. + if sample[0].Value.Kind() == metrics.KindBad { + panic(fmt.Sprintf("metric %q no longer supported", myMetric)) + } + + // Handle the result. + // + // It's OK to assume a particular Kind for a metric; + // they're guaranteed not to change. + freeBytes := sample[0].Value.Uint64() + + fmt.Printf("free but not released memory: %d\n", freeBytes) +} + +func ExampleRead_readingAllMetrics() { + // Get descriptions for all supported metrics. + descs := metrics.All() + + // Create a sample for each metric. + samples := make([]metrics.Sample, len(descs)) + for i := range samples { + samples[i].Name = descs[i].Name + } + + // Sample the metrics. Re-use the samples slice if you can! + metrics.Read(samples) + + // Iterate over all results. + for _, sample := range samples { + // Pull out the name and value. + name, value := sample.Name, sample.Value + + // Handle each sample. + switch value.Kind() { + case metrics.KindUint64: + fmt.Printf("%s: %d\n", name, value.Uint64()) + case metrics.KindFloat64: + fmt.Printf("%s: %f\n", name, value.Float64()) + case metrics.KindFloat64Histogram: + // The histogram may be quite large, so let's just pull out + // a crude estimate for the median for the sake of this example. + fmt.Printf("%s: %f\n", name, medianBucket(value.Float64Histogram())) + case metrics.KindBad: + // This should never happen because all metrics are supported + // by construction. + panic("bug in runtime/metrics package!") + default: + // This may happen as new metrics get added. + // + // The safest thing to do here is to simply log it somewhere + // as something to look into, but ignore it for now. + // In the worst case, you might temporarily miss out on a new metric. + fmt.Printf("%s: unexpected metric Kind: %v\n", name, value.Kind()) + } + } +} + +func medianBucket(h *metrics.Float64Histogram) float64 { + total := uint64(0) + for _, count := range h.Counts { + total += count + } + thresh := total / 2 + total = 0 + for i, count := range h.Counts { + total += count + if total > thresh { + return h.Buckets[i] + } + } + panic("should not happen") +} From 06915ac14dfb7c80f384e3446bc6fa474e6bfa94 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Sat, 19 Dec 2020 19:26:06 -0800 Subject: [PATCH 06/27] [dev.regabi] cmd/compile: move itabname call out of implements We only need to call itabname when actually creating the OCONVIFACE ops, not any time we test whether a type implements an interface. Additionally, by moving this call out of implements, we make it purely based on types, which makes it safe to move to package types. Does not pass toolstash -cmp, because it shuffles symbol creation order. Change-Id: Iea8e0c9374218f4d97b4339020ebd758d051bd03 Reviewed-on: https://go-review.googlesource.com/c/go/+/279333 Reviewed-by: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Trust: Matthew Dempsky --- src/cmd/compile/internal/gc/subr.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index 2b0047e150..48cbd2505e 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -304,6 +304,14 @@ func assignop(src, dst *types.Type) (ir.Op, string) { var missing, have *types.Field var ptr int if implements(src, dst, &missing, &have, &ptr) { + // Call itabname so that (src, dst) + // gets added to itabs early, which allows + // us to de-virtualize calls through this + // type/interface pair later. See peekitabs in reflect.go + if isdirectiface(src) && !dst.IsEmptyInterface() { + itabname(src, dst) + } + return ir.OCONVIFACE, "" } @@ -1404,14 +1412,6 @@ func implements(t, iface *types.Type, m, samename **types.Field, ptr *int) bool } } - // We're going to emit an OCONVIFACE. - // Call itabname so that (t, iface) - // gets added to itabs early, which allows - // us to de-virtualize calls through this - // type/interface pair later. See peekitabs in reflect.go - if isdirectiface(t0) && !iface.IsEmptyInterface() { - itabname(t0, iface) - } return true } From cb4898a77d79f457d75f601fad6908dd85bdc772 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 18 Dec 2020 19:38:13 -0800 Subject: [PATCH 07/27] [dev.regabi] cmd/compile: simplify declaration importing Rather than creating Names w/ ONONAME earlier and later adding in the details, this CL changes the import logic to create and add details at the same time. Passes buildall w/ toolstash -cmp. Change-Id: Ifaabade3cef8cd80ddd6644bff79393b934255d9 Reviewed-on: https://go-review.googlesource.com/c/go/+/279313 Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/export.go | 110 ++++++----------------- src/cmd/compile/internal/gc/iimport.go | 62 +++++++------ src/cmd/compile/internal/gc/typecheck.go | 13 +-- 3 files changed, 58 insertions(+), 127 deletions(-) diff --git a/src/cmd/compile/internal/gc/export.go b/src/cmd/compile/internal/gc/export.go index d26dd9af5d..6ed4327a8f 100644 --- a/src/cmd/compile/internal/gc/export.go +++ b/src/cmd/compile/internal/gc/export.go @@ -77,126 +77,70 @@ func dumpexport(bout *bio.Writer) { } } -func importsym(ipkg *types.Pkg, s *types.Sym, op ir.Op) *ir.Name { - n := ir.AsNode(s.PkgDef()) - if n == nil { - // iimport should have created a stub ONONAME - // declaration for all imported symbols. The exception - // is declarations for Runtimepkg, which are populated - // by loadsys instead. - if s.Pkg != Runtimepkg { - base.Fatalf("missing ONONAME for %v\n", s) - } +func importsym(ipkg *types.Pkg, pos src.XPos, s *types.Sym, op ir.Op, ctxt ir.Class) *ir.Name { + if n := s.PkgDef(); n != nil { + base.Fatalf("importsym of symbol that already exists: %v", n) + } - n = ir.NewDeclNameAt(src.NoXPos, s) - s.SetPkgDef(n) - s.Importdef = ipkg - } - if n.Op() != ir.ONONAME && n.Op() != op { - redeclare(base.Pos, s, fmt.Sprintf("during import %q", ipkg.Path)) - } - return n.(*ir.Name) + n := ir.NewDeclNameAt(pos, s) + n.SetOp(op) // TODO(mdempsky): Add as argument to NewDeclNameAt. + n.SetClass(ctxt) + s.SetPkgDef(n) + s.Importdef = ipkg + return n } // importtype returns the named type declared by symbol s. // If no such type has been declared yet, a forward declaration is returned. // ipkg is the package being imported -func importtype(ipkg *types.Pkg, pos src.XPos, s *types.Sym) *types.Type { - n := importsym(ipkg, s, ir.OTYPE) - if n.Op() != ir.OTYPE { - t := types.NewNamed(n) - n.SetOp(ir.OTYPE) - n.SetPos(pos) - n.SetType(t) - n.SetClass(ir.PEXTERN) - } - - t := n.Type() - if t == nil { - base.Fatalf("importtype %v", s) - } - return t +func importtype(ipkg *types.Pkg, pos src.XPos, s *types.Sym) *ir.Name { + n := importsym(ipkg, pos, s, ir.OTYPE, ir.PEXTERN) + n.SetType(types.NewNamed(n)) + return n } // importobj declares symbol s as an imported object representable by op. // ipkg is the package being imported -func importobj(ipkg *types.Pkg, pos src.XPos, s *types.Sym, op ir.Op, ctxt ir.Class, t *types.Type) ir.Node { - n := importsym(ipkg, s, op) - if n.Op() != ir.ONONAME { - if n.Op() == op && (op == ir.ONAME && n.Class() != ctxt || !types.Identical(n.Type(), t)) { - redeclare(base.Pos, s, fmt.Sprintf("during import %q", ipkg.Path)) - } - return nil - } - - n.SetOp(op) - n.SetPos(pos) - n.SetClass(ctxt) +func importobj(ipkg *types.Pkg, pos src.XPos, s *types.Sym, op ir.Op, ctxt ir.Class, t *types.Type) *ir.Name { + n := importsym(ipkg, pos, s, op, ctxt) + n.SetType(t) if ctxt == ir.PFUNC { n.Sym().SetFunc(true) } - n.SetType(t) return n } // importconst declares symbol s as an imported constant with type t and value val. // ipkg is the package being imported -func importconst(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type, val constant.Value) { +func importconst(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type, val constant.Value) *ir.Name { n := importobj(ipkg, pos, s, ir.OLITERAL, ir.PEXTERN, t) - if n == nil { // TODO: Check that value matches. - return - } - n.SetVal(val) - - if base.Flag.E != 0 { - fmt.Printf("import const %v %L = %v\n", s, t, val) - } + return n } // importfunc declares symbol s as an imported function with type t. // ipkg is the package being imported -func importfunc(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) { +func importfunc(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) *ir.Name { n := importobj(ipkg, pos, s, ir.ONAME, ir.PFUNC, t) - if n == nil { - return - } - name := n.(*ir.Name) fn := ir.NewFunc(pos) fn.SetType(t) - name.SetFunc(fn) - fn.Nname = name + n.SetFunc(fn) + fn.Nname = n - if base.Flag.E != 0 { - fmt.Printf("import func %v%S\n", s, t) - } + return n } // importvar declares symbol s as an imported variable with type t. // ipkg is the package being imported -func importvar(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) { - n := importobj(ipkg, pos, s, ir.ONAME, ir.PEXTERN, t) - if n == nil { - return - } - - if base.Flag.E != 0 { - fmt.Printf("import var %v %L\n", s, t) - } +func importvar(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) *ir.Name { + return importobj(ipkg, pos, s, ir.ONAME, ir.PEXTERN, t) } // importalias declares symbol s as an imported type alias with type t. // ipkg is the package being imported -func importalias(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) { - n := importobj(ipkg, pos, s, ir.OTYPE, ir.PEXTERN, t) - if n == nil { - return - } - - if base.Flag.E != 0 { - fmt.Printf("import type %v = %L\n", s, t) - } +func importalias(ipkg *types.Pkg, pos src.XPos, s *types.Sym, t *types.Type) *ir.Name { + return importobj(ipkg, pos, s, ir.OTYPE, ir.PEXTERN, t) } func dumpasmhdr() { diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 549751335e..76f55a44e5 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -41,18 +41,23 @@ var ( inlineImporter = map[*types.Sym]iimporterAndOffset{} ) -func expandDecl(n *ir.Name) { - if n.Op() != ir.ONONAME { - return +func expandDecl(n ir.Node) ir.Node { + if n, ok := n.(*ir.Name); ok { + return n } - r := importReaderFor(n, declImporter) + id := n.(*ir.Ident) + if n := id.Sym().PkgDef(); n != nil { + return n.(*ir.Name) + } + + r := importReaderFor(id.Sym(), declImporter) if r == nil { // Can happen if user tries to reference an undeclared name. - return + return n } - r.doDecl(n) + return r.doDecl(n.Sym()) } func expandInline(fn *ir.Func) { @@ -60,7 +65,7 @@ func expandInline(fn *ir.Func) { return } - r := importReaderFor(fn.Nname, inlineImporter) + r := importReaderFor(fn.Nname.Sym(), inlineImporter) if r == nil { base.Fatalf("missing import reader for %v", fn) } @@ -68,13 +73,13 @@ func expandInline(fn *ir.Func) { r.doInline(fn) } -func importReaderFor(n *ir.Name, importers map[*types.Sym]iimporterAndOffset) *importReader { - x, ok := importers[n.Sym()] +func importReaderFor(sym *types.Sym, importers map[*types.Sym]iimporterAndOffset) *importReader { + x, ok := importers[sym] if !ok { return nil } - return x.p.newReader(x.off, n.Sym().Pkg) + return x.p.newReader(x.off, sym.Pkg) } type intReader struct { @@ -272,11 +277,7 @@ func (r *importReader) setPkg() { r.currPkg = r.pkg() } -func (r *importReader) doDecl(n ir.Node) { - if n.Op() != ir.ONONAME { - base.Fatalf("doDecl: unexpected Op for %v: %v", n.Sym(), n.Op()) - } - +func (r *importReader) doDecl(sym *types.Sym) *ir.Name { tag := r.byte() pos := r.pos() @@ -284,24 +285,26 @@ func (r *importReader) doDecl(n ir.Node) { case 'A': typ := r.typ() - importalias(r.p.ipkg, pos, n.Sym(), typ) + return importalias(r.p.ipkg, pos, sym, typ) case 'C': typ := r.typ() val := r.value(typ) - importconst(r.p.ipkg, pos, n.Sym(), typ, val) + return importconst(r.p.ipkg, pos, sym, typ, val) case 'F': typ := r.signature(nil) - importfunc(r.p.ipkg, pos, n.Sym(), typ) + n := importfunc(r.p.ipkg, pos, sym, typ) r.funcExt(n) + return n case 'T': // Types can be recursive. We need to setup a stub // declaration before recursing. - t := importtype(r.p.ipkg, pos, n.Sym()) + n := importtype(r.p.ipkg, pos, sym) + t := n.Type() // We also need to defer width calculations until // after the underlying type has been assigned. @@ -312,7 +315,7 @@ func (r *importReader) doDecl(n ir.Node) { if underlying.IsInterface() { r.typeExt(t) - break + return n } ms := make([]*types.Field, r.uint64()) @@ -339,15 +342,18 @@ func (r *importReader) doDecl(n ir.Node) { for _, m := range ms { r.methExt(m) } + return n case 'V': typ := r.typ() - importvar(r.p.ipkg, pos, n.Sym(), typ) + n := importvar(r.p.ipkg, pos, sym, typ) r.varExt(n) + return n default: base.Fatalf("unexpected tag: %v", tag) + panic("unreachable") } } @@ -433,16 +439,11 @@ func (r *importReader) ident() *types.Sym { return pkg.Lookup(name) } -func (r *importReader) qualifiedIdent() *ir.Name { +func (r *importReader) qualifiedIdent() *ir.Ident { name := r.string() pkg := r.pkg() sym := pkg.Lookup(name) - n := sym.PkgDef() - if n == nil { - n = ir.NewDeclNameAt(src.NoXPos, sym) - sym.SetPkgDef(n) - } - return n.(*ir.Name) + return ir.NewIdent(src.NoXPos, sym) } func (r *importReader) pos() src.XPos { @@ -498,10 +499,7 @@ func (r *importReader) typ1() *types.Type { // support inlining functions with local defined // types. Therefore, this must be a package-scope // type. - n := r.qualifiedIdent() - if n.Op() == ir.ONONAME { - expandDecl(n) - } + n := expandDecl(r.qualifiedIdent()) if n.Op() != ir.OTYPE { base.Fatalf("expected OTYPE, got %v: %v, %v", n.Op(), n.Sym(), n) } diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 83939fd6bf..4fae4a0819 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -8,7 +8,6 @@ import ( "cmd/compile/internal/base" "cmd/compile/internal/ir" "cmd/compile/internal/types" - "cmd/internal/src" "fmt" "go/constant" "go/token" @@ -97,23 +96,13 @@ func resolve(n ir.Node) (res ir.Node) { if pkgName := dotImportRefs[id]; pkgName != nil { pkgName.Used = true } - - if sym.Def == nil { - if _, ok := declImporter[sym]; !ok { - return n // undeclared name - } - sym.Def = ir.NewDeclNameAt(src.NoXPos, sym) - } - n = ir.AsNode(sym.Def) } - // Stub ir.Name left for us by iimport. - n := n.(*ir.Name) if inimport { base.Fatalf("recursive inimport") } inimport = true - expandDecl(n) + n = expandDecl(n) inimport = false return n } From 94cfeca0a5b36a70a8bdd1a0015eb78c7e9a3311 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 18 Dec 2020 20:14:45 -0800 Subject: [PATCH 08/27] [dev.regabi] cmd/compile: stop using ONONAME with Name This CL changes NewDeclNameAt to take an Op argument to set the Op up front, and updates all callers to provide the appropriate Op. This allows dropping the Name.SetOp method. Passes buildall w/ toolstash -cmp. Change-Id: I20e580f62d3c8a81223d1c162327c11b37bbf3f0 Reviewed-on: https://go-review.googlesource.com/c/go/+/279314 Trust: Matthew Dempsky Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/dcl.go | 2 -- src/cmd/compile/internal/gc/export.go | 5 ++--- src/cmd/compile/internal/gc/iimport.go | 2 +- src/cmd/compile/internal/gc/noder.go | 17 +++++++---------- src/cmd/compile/internal/gc/universe.go | 6 ++---- src/cmd/compile/internal/ir/name.go | 24 +++++++++--------------- 6 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/cmd/compile/internal/gc/dcl.go b/src/cmd/compile/internal/gc/dcl.go index 64b15077cd..04e3506dba 100644 --- a/src/cmd/compile/internal/gc/dcl.go +++ b/src/cmd/compile/internal/gc/dcl.go @@ -141,7 +141,6 @@ func variter(vl []ir.Node, t ir.Ntype, el []ir.Node) []ir.Node { as2.PtrRlist().Set1(e) for _, v := range vl { v := v.(*ir.Name) - v.SetOp(ir.ONAME) declare(v, dclcontext) v.Ntype = t v.Defn = as2 @@ -166,7 +165,6 @@ func variter(vl []ir.Node, t ir.Ntype, el []ir.Node) []ir.Node { el = el[1:] } - v.SetOp(ir.ONAME) declare(v, dclcontext) v.Ntype = t diff --git a/src/cmd/compile/internal/gc/export.go b/src/cmd/compile/internal/gc/export.go index 6ed4327a8f..8a8295537c 100644 --- a/src/cmd/compile/internal/gc/export.go +++ b/src/cmd/compile/internal/gc/export.go @@ -82,9 +82,8 @@ func importsym(ipkg *types.Pkg, pos src.XPos, s *types.Sym, op ir.Op, ctxt ir.Cl base.Fatalf("importsym of symbol that already exists: %v", n) } - n := ir.NewDeclNameAt(pos, s) - n.SetOp(op) // TODO(mdempsky): Add as argument to NewDeclNameAt. - n.SetClass(ctxt) + n := ir.NewDeclNameAt(pos, op, s) + n.SetClass(ctxt) // TODO(mdempsky): Move this into NewDeclNameAt too? s.SetPkgDef(n) s.Importdef = ipkg return n diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 76f55a44e5..219ce4bdef 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -971,7 +971,7 @@ func (r *importReader) node() ir.Node { // statements case ir.ODCL: pos := r.pos() - lhs := ir.NewDeclNameAt(pos, r.ident()) + lhs := ir.NewDeclNameAt(pos, ir.ONAME, r.ident()) typ := ir.TypeNode(r.typ()) return npos(pos, liststmt(variter([]ir.Node{lhs}, typ, nil))) // TODO(gri) avoid list creation diff --git a/src/cmd/compile/internal/gc/noder.go b/src/cmd/compile/internal/gc/noder.go index ee01423833..b61f19ae2e 100644 --- a/src/cmd/compile/internal/gc/noder.go +++ b/src/cmd/compile/internal/gc/noder.go @@ -374,7 +374,7 @@ func (p *noder) importDecl(imp *syntax.ImportDecl) { } func (p *noder) varDecl(decl *syntax.VarDecl) []ir.Node { - names := p.declNames(decl.NameList) + names := p.declNames(ir.ONAME, decl.NameList) typ := p.typeExprOrNil(decl.Type) var exprs []ir.Node @@ -425,7 +425,7 @@ func (p *noder) constDecl(decl *syntax.ConstDecl, cs *constState) []ir.Node { p.checkUnused(pragma) } - names := p.declNames(decl.NameList) + names := p.declNames(ir.OLITERAL, decl.NameList) typ := p.typeExprOrNil(decl.Type) var values []ir.Node @@ -450,8 +450,6 @@ func (p *noder) constDecl(decl *syntax.ConstDecl, cs *constState) []ir.Node { if decl.Values == nil { v = ir.DeepCopy(n.Pos(), v) } - - n.SetOp(ir.OLITERAL) declare(n, dclcontext) n.Ntype = typ @@ -471,8 +469,7 @@ func (p *noder) constDecl(decl *syntax.ConstDecl, cs *constState) []ir.Node { } func (p *noder) typeDecl(decl *syntax.TypeDecl) ir.Node { - n := p.declName(decl.Name) - n.SetOp(ir.OTYPE) + n := p.declName(ir.OTYPE, decl.Name) declare(n, dclcontext) // decl.Type may be nil but in that case we got a syntax error during parsing @@ -495,16 +492,16 @@ func (p *noder) typeDecl(decl *syntax.TypeDecl) ir.Node { return nod } -func (p *noder) declNames(names []*syntax.Name) []ir.Node { +func (p *noder) declNames(op ir.Op, names []*syntax.Name) []ir.Node { nodes := make([]ir.Node, 0, len(names)) for _, name := range names { - nodes = append(nodes, p.declName(name)) + nodes = append(nodes, p.declName(op, name)) } return nodes } -func (p *noder) declName(name *syntax.Name) *ir.Name { - return ir.NewDeclNameAt(p.pos(name), p.name(name)) +func (p *noder) declName(op ir.Op, name *syntax.Name) *ir.Name { + return ir.NewDeclNameAt(p.pos(name), op, p.name(name)) } func (p *noder) funcDecl(fun *syntax.FuncDecl) ir.Node { diff --git a/src/cmd/compile/internal/gc/universe.go b/src/cmd/compile/internal/gc/universe.go index 21ddc78089..c988c575dc 100644 --- a/src/cmd/compile/internal/gc/universe.go +++ b/src/cmd/compile/internal/gc/universe.go @@ -97,8 +97,7 @@ func initUniverse() { defBasic := func(kind types.Kind, pkg *types.Pkg, name string) *types.Type { sym := pkg.Lookup(name) - n := ir.NewDeclNameAt(src.NoXPos, sym) - n.SetOp(ir.OTYPE) + n := ir.NewDeclNameAt(src.NoXPos, ir.OTYPE, sym) t := types.NewBasic(kind, n) n.SetType(t) sym.Def = n @@ -134,8 +133,7 @@ func initUniverse() { // error type s := types.BuiltinPkg.Lookup("error") - n := ir.NewDeclNameAt(src.NoXPos, s) - n.SetOp(ir.OTYPE) + n := ir.NewDeclNameAt(src.NoXPos, ir.OTYPE, s) types.ErrorType = types.NewNamed(n) types.ErrorType.SetUnderlying(makeErrorInterface()) n.SetType(types.ErrorType) diff --git a/src/cmd/compile/internal/ir/name.go b/src/cmd/compile/internal/ir/name.go index f5f4280fd0..9cf959b23d 100644 --- a/src/cmd/compile/internal/ir/name.go +++ b/src/cmd/compile/internal/ir/name.go @@ -164,13 +164,19 @@ func NewIota(pos src.XPos, sym *types.Sym) *Name { return newNameAt(pos, OIOTA, sym) } -// NewDeclNameAt returns a new ONONAME Node associated with symbol s at position pos. +// NewDeclNameAt returns a new Name associated with symbol s at position pos. // The caller is responsible for setting Curfn. -func NewDeclNameAt(pos src.XPos, sym *types.Sym) *Name { +func NewDeclNameAt(pos src.XPos, op Op, sym *types.Sym) *Name { if sym == nil { base.Fatalf("NewDeclNameAt nil") } - return newNameAt(pos, ONONAME, sym) + switch op { + case ONAME, OTYPE, OLITERAL: + // ok + default: + base.Fatalf("NewDeclNameAt op %v", op) + } + return newNameAt(pos, op, sym) } // newNameAt is like NewNameAt but allows sym == nil. @@ -207,18 +213,6 @@ func (*Name) CanBeNtype() {} func (*Name) CanBeAnSSASym() {} func (*Name) CanBeAnSSAAux() {} -func (n *Name) SetOp(op Op) { - if n.op != ONONAME { - base.Fatalf("%v already has Op %v", n, n.op) - } - switch op { - default: - panic(n.no("SetOp " + op.String())) - case OLITERAL, ONAME, OTYPE, OIOTA: - n.op = op - } -} - // Pragma returns the PragmaFlag for p, which must be for an OTYPE. func (n *Name) Pragma() PragmaFlag { return n.pragma } From bc7e4d9257693413d57ad467814ab71f1585a155 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 13:14:41 -0500 Subject: [PATCH 09/27] syscall: don't generate ptrace on iOS May fix #43302. Change-Id: I6b7ddf94495c4fa80cf8a50a38eef5f8b2872669 Reviewed-on: https://go-review.googlesource.com/c/go/+/279481 Trust: Cherry Zhang Reviewed-by: Ian Lance Taylor --- src/syscall/mksyscall.pl | 2 +- src/syscall/ptrace_darwin.go | 14 ++++++++++++++ src/syscall/ptrace_ios.go | 12 ++++++++++++ src/syscall/syscall_darwin_amd64.go | 2 +- src/syscall/syscall_darwin_arm64.go | 2 +- src/syscall/zsyscall_darwin_amd64.go | 2 +- src/syscall/zsyscall_darwin_arm64.go | 2 +- 7 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 src/syscall/ptrace_darwin.go create mode 100644 src/syscall/ptrace_ios.go diff --git a/src/syscall/mksyscall.pl b/src/syscall/mksyscall.pl index 25b40d7ba2..7e2cedfb6c 100755 --- a/src/syscall/mksyscall.pl +++ b/src/syscall/mksyscall.pl @@ -125,7 +125,7 @@ while(<>) { # without reading the header. $text .= "// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT\n\n"; - if ($darwin && $func eq "ptrace") { + if ($darwin && $func eq "ptrace1") { # The ptrace function is called from forkAndExecInChild where stack # growth is forbidden. $text .= "//go:nosplit\n" diff --git a/src/syscall/ptrace_darwin.go b/src/syscall/ptrace_darwin.go new file mode 100644 index 0000000000..a873d826b8 --- /dev/null +++ b/src/syscall/ptrace_darwin.go @@ -0,0 +1,14 @@ +// 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. + +// +build !ios + +package syscall + +// Nosplit because it is called from forkAndExecInChild. +// +//go:nosplit +func ptrace(request int, pid int, addr uintptr, data uintptr) error { + return ptrace1(request, pid, addr, data) +} diff --git a/src/syscall/ptrace_ios.go b/src/syscall/ptrace_ios.go new file mode 100644 index 0000000000..2f61a88a08 --- /dev/null +++ b/src/syscall/ptrace_ios.go @@ -0,0 +1,12 @@ +// 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 syscall + +// Nosplit because it is called from forkAndExecInChild. +// +//go:nosplit +func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { + panic("unimplemented") +} diff --git a/src/syscall/syscall_darwin_amd64.go b/src/syscall/syscall_darwin_amd64.go index 23a4e5f996..22ddb78ae5 100644 --- a/src/syscall/syscall_darwin_amd64.go +++ b/src/syscall/syscall_darwin_amd64.go @@ -21,7 +21,7 @@ func setTimeval(sec, usec int64) Timeval { //sys Stat(path string, stat *Stat_t) (err error) = SYS_stat64 //sys Statfs(path string, stat *Statfs_t) (err error) = SYS_statfs64 //sys fstatat(fd int, path string, stat *Stat_t, flags int) (err error) = SYS_fstatat64 -//sys ptrace(request int, pid int, addr uintptr, data uintptr) (err error) +//sys ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) = SYS_ptrace func SetKevent(k *Kevent_t, fd, mode, flags int) { k.Ident = uint64(fd) diff --git a/src/syscall/syscall_darwin_arm64.go b/src/syscall/syscall_darwin_arm64.go index c824f6d89d..ecb9ffff49 100644 --- a/src/syscall/syscall_darwin_arm64.go +++ b/src/syscall/syscall_darwin_arm64.go @@ -21,7 +21,7 @@ func setTimeval(sec, usec int64) Timeval { //sys Stat(path string, stat *Stat_t) (err error) //sys Statfs(path string, stat *Statfs_t) (err error) //sys fstatat(fd int, path string, stat *Stat_t, flags int) (err error) -//sys ptrace(request int, pid int, addr uintptr, data uintptr) (err error) +//sys ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) = SYS_ptrace func SetKevent(k *Kevent_t, fd, mode, flags int) { k.Ident = uint64(fd) diff --git a/src/syscall/zsyscall_darwin_amd64.go b/src/syscall/zsyscall_darwin_amd64.go index 093739ebc7..c246c3a267 100644 --- a/src/syscall/zsyscall_darwin_amd64.go +++ b/src/syscall/zsyscall_darwin_amd64.go @@ -2091,7 +2091,7 @@ func libc_fstatat64_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT //go:nosplit -func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { +func ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) { _, _, e1 := syscall6(funcPC(libc_ptrace_trampoline), uintptr(request), uintptr(pid), uintptr(addr), uintptr(data), 0, 0) if e1 != 0 { err = errnoErr(e1) diff --git a/src/syscall/zsyscall_darwin_arm64.go b/src/syscall/zsyscall_darwin_arm64.go index 7698b2503e..ede0091de2 100644 --- a/src/syscall/zsyscall_darwin_arm64.go +++ b/src/syscall/zsyscall_darwin_arm64.go @@ -2091,7 +2091,7 @@ func libc_fstatat_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT //go:nosplit -func ptrace(request int, pid int, addr uintptr, data uintptr) (err error) { +func ptrace1(request int, pid int, addr uintptr, data uintptr) (err error) { _, _, e1 := syscall6(funcPC(libc_ptrace_trampoline), uintptr(request), uintptr(pid), uintptr(addr), uintptr(data), 0, 0) if e1 != 0 { err = errnoErr(e1) From 306b2451c849c9a5835069f317dfea851e526a00 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Mon, 14 Dec 2020 10:03:37 -0500 Subject: [PATCH 10/27] [dev.regabi] runtime: fix ABI targets in runtime.panic{Index,Slice} shims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix up the assembly shim routines runtime.panic{Index,Slice} and friends so that their tail calls target ABIInternal and not ABI0 functions. This is so as to ensure that these calls don't go through an ABI0->ABIInternal wrapper (which would throw off the machinery in the called routines designed to detect whether the violation happened in the runtime). Note that when the compiler starts emitting real register calls to these routines, we'll need to rewrite them to update the arg size and ensure that args are in the correct registers. For example, the current shim TEXT runtime·panicIndex(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) JMP runtime·goPanicIndex(SB) will need to change to TEXT runtime·panicIndex(SB),NOSPLIT,$0 // AX already set up properly MOVQ CX, BX // second argument expected in BX JMP runtime·goPanicIndex(SB) Change-Id: I48d1b5138fb4d229380ad12735cfaca5c50e6cc3 Reviewed-on: https://go-review.googlesource.com/c/go/+/278755 Reviewed-by: Cherry Zhang Trust: Than McIntosh --- src/runtime/asm_amd64.s | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index 196252e1dd..53d1f8e358 100644 --- a/src/runtime/asm_amd64.s +++ b/src/runtime/asm_amd64.s @@ -1728,67 +1728,67 @@ TEXT runtime·debugCallPanicked(SB),NOSPLIT,$16-16 TEXT runtime·panicIndex(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicIndex(SB) + JMP runtime·goPanicIndex(SB) TEXT runtime·panicIndexU(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicIndexU(SB) + JMP runtime·goPanicIndexU(SB) TEXT runtime·panicSliceAlen(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSliceAlen(SB) + JMP runtime·goPanicSliceAlen(SB) TEXT runtime·panicSliceAlenU(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSliceAlenU(SB) + JMP runtime·goPanicSliceAlenU(SB) TEXT runtime·panicSliceAcap(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSliceAcap(SB) + JMP runtime·goPanicSliceAcap(SB) TEXT runtime·panicSliceAcapU(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSliceAcapU(SB) + JMP runtime·goPanicSliceAcapU(SB) TEXT runtime·panicSliceB(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicSliceB(SB) + JMP runtime·goPanicSliceB(SB) TEXT runtime·panicSliceBU(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicSliceBU(SB) + JMP runtime·goPanicSliceBU(SB) TEXT runtime·panicSlice3Alen(SB),NOSPLIT,$0-16 MOVQ DX, x+0(FP) MOVQ BX, y+8(FP) - JMP runtime·goPanicSlice3Alen(SB) + JMP runtime·goPanicSlice3Alen(SB) TEXT runtime·panicSlice3AlenU(SB),NOSPLIT,$0-16 MOVQ DX, x+0(FP) MOVQ BX, y+8(FP) - JMP runtime·goPanicSlice3AlenU(SB) + JMP runtime·goPanicSlice3AlenU(SB) TEXT runtime·panicSlice3Acap(SB),NOSPLIT,$0-16 MOVQ DX, x+0(FP) MOVQ BX, y+8(FP) - JMP runtime·goPanicSlice3Acap(SB) + JMP runtime·goPanicSlice3Acap(SB) TEXT runtime·panicSlice3AcapU(SB),NOSPLIT,$0-16 MOVQ DX, x+0(FP) MOVQ BX, y+8(FP) - JMP runtime·goPanicSlice3AcapU(SB) + JMP runtime·goPanicSlice3AcapU(SB) TEXT runtime·panicSlice3B(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSlice3B(SB) + JMP runtime·goPanicSlice3B(SB) TEXT runtime·panicSlice3BU(SB),NOSPLIT,$0-16 MOVQ CX, x+0(FP) MOVQ DX, y+8(FP) - JMP runtime·goPanicSlice3BU(SB) + JMP runtime·goPanicSlice3BU(SB) TEXT runtime·panicSlice3C(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicSlice3C(SB) + JMP runtime·goPanicSlice3C(SB) TEXT runtime·panicSlice3CU(SB),NOSPLIT,$0-16 MOVQ AX, x+0(FP) MOVQ CX, y+8(FP) - JMP runtime·goPanicSlice3CU(SB) + JMP runtime·goPanicSlice3CU(SB) #ifdef GOOS_android // Use the free TLS_SLOT_APP slot #2 on Android Q. From 9b6147120a30a8bc30a41c1651f369e8bcb80948 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 14:11:02 -0500 Subject: [PATCH 11/27] cmd/pack: treat compiler's -linkobj output as "compiler object" Treat the compiler's -linkobj output as "compiler object, which means "pack c" will "see through" the file and add individual entry to the new archive, instead of the object as a whole. This is somewhat peculiar. But Go 1.15's cmd/pack does this, although seemingly accidental. We just do the same. FWIW, it does make things more consistent with/without -linkobj flag. Fixes #43271. Change-Id: I6b2d99256db7ebf0fa430f85afa7464e334f6bcb Reviewed-on: https://go-review.googlesource.com/c/go/+/279483 Trust: Cherry Zhang Run-TryBot: Cherry Zhang Reviewed-by: Jeremy Faller Reviewed-by: Than McIntosh --- src/cmd/pack/pack.go | 31 ++++++++++-------- src/cmd/pack/pack_test.go | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go index 82546ea7dc..3dffabe5ec 100644 --- a/src/cmd/pack/pack.go +++ b/src/cmd/pack/pack.go @@ -315,20 +315,25 @@ func (ar *Archive) extractContents1(e *archive.Entry, out io.Writer) { } // isGoCompilerObjFile reports whether file is an object file created -// by the Go compiler, which is an archive file with exactly two entries: -// __.PKGDEF and _go_.o. +// by the Go compiler, which is an archive file with exactly one entry +// of __.PKGDEF, or _go_.o, or both entries. func isGoCompilerObjFile(a *archive.Archive) bool { - if len(a.Entries) != 2 { + switch len(a.Entries) { + case 1: + return (a.Entries[0].Type == archive.EntryGoObj && a.Entries[0].Name == "_go_.o") || + (a.Entries[0].Type == archive.EntryPkgDef && a.Entries[0].Name == "__.PKGDEF") + case 2: + var foundPkgDef, foundGo bool + for _, e := range a.Entries { + if e.Type == archive.EntryPkgDef && e.Name == "__.PKGDEF" { + foundPkgDef = true + } + if e.Type == archive.EntryGoObj && e.Name == "_go_.o" { + foundGo = true + } + } + return foundPkgDef && foundGo + default: return false } - var foundPkgDef, foundGo bool - for _, e := range a.Entries { - if e.Type == archive.EntryPkgDef && e.Name == "__.PKGDEF" { - foundPkgDef = true - } - if e.Type == archive.EntryGoObj && e.Name == "_go_.o" { - foundGo = true - } - } - return foundPkgDef && foundGo } diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go index 218c7acda6..16a5135800 100644 --- a/src/cmd/pack/pack_test.go +++ b/src/cmd/pack/pack_test.go @@ -302,6 +302,72 @@ func TestIssue21703(t *testing.T) { run(goBin, "tool", "compile", "-I", ".", "b.go") } +// Test the "c" command can "see through" the archive generated by the compiler. +// This is peculiar. (See issue ) +func TestCreateWithCompilerObj(t *testing.T) { + testenv.MustHaveGoBuild(t) + + dir := tmpDir(t) + defer os.RemoveAll(dir) + src := filepath.Join(dir, "p.go") + prog := "package p; var X = 42\n" + err := os.WriteFile(src, []byte(prog), 0666) + if err != nil { + t.Fatal(err) + } + + run := func(args ...string) string { + return doRun(t, dir, args...) + } + + goBin := testenv.GoToolPath(t) + run(goBin, "build", "cmd/pack") // writes pack binary to dir + run(goBin, "tool", "compile", "-pack", "-o", "p.a", "p.go") + run("./pack", "c", "packed.a", "p.a") + fi, err := os.Stat(filepath.Join(dir, "p.a")) + if err != nil { + t.Fatalf("stat p.a failed: %v", err) + } + fi2, err := os.Stat(filepath.Join(dir, "packed.a")) + if err != nil { + t.Fatalf("stat packed.a failed: %v", err) + } + // For compiler-generated object file, the "c" command is + // expected to get (essentially) the same file back, instead + // of packing it into a new archive with a single entry. + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } + + // Test -linkobj flag as well. + run(goBin, "tool", "compile", "-linkobj", "p2.a", "-o", "p.x", "p.go") + run("./pack", "c", "packed2.a", "p2.a") + fi, err = os.Stat(filepath.Join(dir, "p2.a")) + if err != nil { + t.Fatalf("stat p2.a failed: %v", err) + } + fi2, err = os.Stat(filepath.Join(dir, "packed2.a")) + if err != nil { + t.Fatalf("stat packed2.a failed: %v", err) + } + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } + + run("./pack", "c", "packed3.a", "p.x") + fi, err = os.Stat(filepath.Join(dir, "p.x")) + if err != nil { + t.Fatalf("stat p.x failed: %v", err) + } + fi2, err = os.Stat(filepath.Join(dir, "packed3.a")) + if err != nil { + t.Fatalf("stat packed3.a failed: %v", err) + } + if want, got := fi.Size(), fi2.Size(); want != got { + t.Errorf("packed file with different size: want %d, got %d", want, got) + } +} + // doRun runs a program in a directory and returns the output. func doRun(t *testing.T, dir string, args ...string) string { cmd := exec.Command(args[0], args[1:]...) From 4d27c4c223ccb7de3876abbac79b58ad9579be1a Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Wed, 9 Dec 2020 15:14:59 -0500 Subject: [PATCH 12/27] runtime: correct error handling in several FreeBSD syscall wrappers The FreeBSD syscall convention uses the carry flag to indicate whether an error has occured. The sys_umtx_op, thr_new, and pipe2 syscall wrappers were failing to account for this convention and silently suppressing errors as a result. This commit corrects these wrappers by copying the pattern used by the other fallible syscall wrappers. Note that futexsleep1 must now explicitly ignore the ETIMEDOUT error from sys_umtx_op. Previously ETIMEDOUT was implicitly ignored because sys_umtx_op never returned an error. Fixes #43106. Change-Id: I9c422b87cf4c6d308003bf42c3b419f785578b5d Reviewed-on: https://go-review.googlesource.com/c/go/+/276892 Run-TryBot: Ian Lance Taylor Reviewed-by: Austin Clements Trust: Than McIntosh --- src/runtime/defs_freebsd_386.go | 9 +++++---- src/runtime/defs_freebsd_amd64.go | 9 +++++---- src/runtime/defs_freebsd_arm.go | 9 +++++---- src/runtime/defs_freebsd_arm64.go | 9 +++++---- src/runtime/os_freebsd.go | 3 +-- src/runtime/sys_freebsd_386.s | 6 ++++++ src/runtime/sys_freebsd_amd64.s | 6 ++++++ src/runtime/sys_freebsd_arm.s | 3 +++ src/runtime/sys_freebsd_arm64.s | 6 ++++++ 9 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/runtime/defs_freebsd_386.go b/src/runtime/defs_freebsd_386.go index 767755425c..f822934d58 100644 --- a/src/runtime/defs_freebsd_386.go +++ b/src/runtime/defs_freebsd_386.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_amd64.go b/src/runtime/defs_freebsd_amd64.go index 5a833426fd..0b696cf227 100644 --- a/src/runtime/defs_freebsd_amd64.go +++ b/src/runtime/defs_freebsd_amd64.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_arm.go b/src/runtime/defs_freebsd_arm.go index b55dfd88cf..b6f3e790cf 100644 --- a/src/runtime/defs_freebsd_arm.go +++ b/src/runtime/defs_freebsd_arm.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/defs_freebsd_arm64.go b/src/runtime/defs_freebsd_arm64.go index 5b9d504ba6..0759a1238f 100644 --- a/src/runtime/defs_freebsd_arm64.go +++ b/src/runtime/defs_freebsd_arm64.go @@ -13,10 +13,11 @@ const ( ) const ( - _EINTR = 0x4 - _EFAULT = 0xe - _EAGAIN = 0x23 - _ENOSYS = 0x4e + _EINTR = 0x4 + _EFAULT = 0xe + _EAGAIN = 0x23 + _ENOSYS = 0x4e + _ETIMEDOUT = 0x3c _O_NONBLOCK = 0x4 _O_CLOEXEC = 0x100000 diff --git a/src/runtime/os_freebsd.go b/src/runtime/os_freebsd.go index 730973a202..1c60ee2a57 100644 --- a/src/runtime/os_freebsd.go +++ b/src/runtime/os_freebsd.go @@ -166,7 +166,7 @@ func futexsleep1(addr *uint32, val uint32, ns int64) { utp = &ut } ret := sys_umtx_op(addr, _UMTX_OP_WAIT_UINT_PRIVATE, val, unsafe.Sizeof(*utp), utp) - if ret >= 0 || ret == -_EINTR { + if ret >= 0 || ret == -_EINTR || ret == -_ETIMEDOUT { return } print("umtx_wait addr=", addr, " val=", val, " ret=", ret, "\n") @@ -208,7 +208,6 @@ func newosproc(mp *m) { var oset sigset sigprocmask(_SIG_SETMASK, &sigset_all, &oset) - // TODO: Check for error. ret := thr_new(¶m, int32(unsafe.Sizeof(param))) sigprocmask(_SIG_SETMASK, &oset, nil) if ret < 0 { diff --git a/src/runtime/sys_freebsd_386.s b/src/runtime/sys_freebsd_386.s index c346e719e1..97e6d9ab36 100644 --- a/src/runtime/sys_freebsd_386.s +++ b/src/runtime/sys_freebsd_386.s @@ -13,12 +13,16 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$-4 MOVL $454, AX INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, ret+20(FP) RET TEXT runtime·thr_new(SB),NOSPLIT,$-4 MOVL $455, AX INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, ret+8(FP) RET @@ -120,6 +124,8 @@ TEXT runtime·pipe2(SB),NOSPLIT,$12-16 MOVL flags+0(FP), BX MOVL BX, 8(SP) INT $0x80 + JAE 2(PC) + NEGL AX MOVL AX, errno+12(FP) RET diff --git a/src/runtime/sys_freebsd_amd64.s b/src/runtime/sys_freebsd_amd64.s index 010b2ec4d4..07734b0d7d 100644 --- a/src/runtime/sys_freebsd_amd64.s +++ b/src/runtime/sys_freebsd_amd64.s @@ -18,6 +18,8 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 MOVQ ut+24(FP), R8 MOVL $454, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, ret+32(FP) RET @@ -26,6 +28,8 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVL size+8(FP), SI MOVL $455, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, ret+16(FP) RET @@ -118,6 +122,8 @@ TEXT runtime·pipe2(SB),NOSPLIT,$0-20 MOVL flags+0(FP), SI MOVL $542, AX SYSCALL + JCC 2(PC) + NEGQ AX MOVL AX, errno+16(FP) RET diff --git a/src/runtime/sys_freebsd_arm.s b/src/runtime/sys_freebsd_arm.s index 1e12f9cfcb..b12e47c576 100644 --- a/src/runtime/sys_freebsd_arm.s +++ b/src/runtime/sys_freebsd_arm.s @@ -51,6 +51,7 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 ADD $20, R13 // arg 5 is passed on stack MOVW $SYS__umtx_op, R7 SWI $0 + RSB.CS $0, R0 SUB $20, R13 // BCS error MOVW R0, ret+20(FP) @@ -61,6 +62,7 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVW size+4(FP), R1 MOVW $SYS_thr_new, R7 SWI $0 + RSB.CS $0, R0 MOVW R0, ret+8(FP) RET @@ -144,6 +146,7 @@ TEXT runtime·pipe2(SB),NOSPLIT,$0-16 MOVW flags+0(FP), R1 MOVW $SYS_pipe2, R7 SWI $0 + RSB.CS $0, R0 MOVW R0, errno+12(FP) RET diff --git a/src/runtime/sys_freebsd_arm64.s b/src/runtime/sys_freebsd_arm64.s index 8a4f9b7fa1..1aa09e87ca 100644 --- a/src/runtime/sys_freebsd_arm64.s +++ b/src/runtime/sys_freebsd_arm64.s @@ -60,6 +60,9 @@ TEXT runtime·sys_umtx_op(SB),NOSPLIT,$0 MOVD ut+24(FP), R4 MOVD $SYS__umtx_op, R8 SVC + BCC ok + NEG R0, R0 +ok: MOVW R0, ret+32(FP) RET @@ -69,6 +72,9 @@ TEXT runtime·thr_new(SB),NOSPLIT,$0 MOVW size+8(FP), R1 MOVD $SYS_thr_new, R8 SVC + BCC ok + NEG R0, R0 +ok: MOVW R0, ret+16(FP) RET From 301af2cb71d2731baa55653df67850ce85032e16 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Wed, 16 Dec 2020 13:45:48 -0500 Subject: [PATCH 13/27] [dev.regabi] runtime/race: adjust test pattern match for ABI wrapper Adjust the pattern matching in one of the race output test to allow for the possible introduction of an ABI wrapper. Normally for tests that match traceback output wrappers are not an issue since they are screened out by Go's traceback mechanism, but in this case the race runtime is doing the unwinding, so the wrapper may be visible. Change-Id: I45413b5c4701d4c28cc760fccc8203493dbe2874 Reviewed-on: https://go-review.googlesource.com/c/go/+/278756 Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Trust: Than McIntosh --- src/runtime/race/output_test.go | 82 +++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/src/runtime/race/output_test.go b/src/runtime/race/output_test.go index 69496874c6..17dc32013f 100644 --- a/src/runtime/race/output_test.go +++ b/src/runtime/race/output_test.go @@ -7,6 +7,7 @@ package race_test import ( + "fmt" "internal/testenv" "os" "os/exec" @@ -71,9 +72,24 @@ func TestOutput(t *testing.T) { "GORACE="+test.gorace, ) got, _ := cmd.CombinedOutput() - if !regexp.MustCompile(test.re).MatchString(string(got)) { - t.Fatalf("failed test case %v, expect:\n%v\ngot:\n%s", - test.name, test.re, got) + matched := false + for _, re := range test.re { + if regexp.MustCompile(re).MatchString(string(got)) { + matched = true + break + } + } + if !matched { + exp := fmt.Sprintf("expect:\n%v\n", test.re[0]) + if len(test.re) > 1 { + exp = fmt.Sprintf("expected one of %d patterns:\n", + len(test.re)) + for k, re := range test.re { + exp += fmt.Sprintf("pattern %d:\n%v\n", k, re) + } + } + t.Fatalf("failed test case %v, %sgot:\n%s", + test.name, exp, got) } } } @@ -84,7 +100,7 @@ var tests = []struct { goos string gorace string source string - re string + re []string }{ {"simple", "run", "", "atexit_sleep_ms=0", ` package main @@ -107,7 +123,7 @@ func racer(x *int, done chan bool) { store(x, 42) done <- true } -`, `================== +`, []string{`================== WARNING: DATA RACE Write at 0x[0-9,a-f]+ by goroutine [0-9]: main\.store\(\) @@ -129,7 +145,7 @@ Goroutine [0-9] \(running\) created at: ================== Found 1 data race\(s\) exit status 66 -`}, +`}}, {"exitcode", "run", "", "atexit_sleep_ms=0 exitcode=13", ` package main @@ -143,7 +159,7 @@ func main() { x = 43 <-done } -`, `exit status 13`}, +`, []string{`exit status 13`}}, {"strip_path_prefix", "run", "", "atexit_sleep_ms=0 strip_path_prefix=/main.", ` package main @@ -157,9 +173,9 @@ func main() { x = 43 <-done } -`, ` +`, []string{` go:7 \+0x[0-9,a-f]+ -`}, +`}}, {"halt_on_error", "run", "", "atexit_sleep_ms=0 halt_on_error=1", ` package main @@ -173,10 +189,10 @@ func main() { x = 43 <-done } -`, ` +`, []string{` ================== exit status 66 -`}, +`}}, {"test_fails_on_race", "test", "", "atexit_sleep_ms=0", ` package main_test @@ -193,12 +209,12 @@ func TestFail(t *testing.T) { <-done t.Log(t.Failed()) } -`, ` +`, []string{` ================== --- FAIL: TestFail \(0...s\) .*main_test.go:14: true .*testing.go:.*: race detected during execution of test -FAIL`}, +FAIL`}}, {"slicebytetostring_pc", "run", "", "atexit_sleep_ms=0", ` package main @@ -211,11 +227,11 @@ func main() { data[0] = 1 <-done } -`, ` +`, []string{` runtime\.slicebytetostring\(\) .*/runtime/string\.go:.* main\.main\.func1\(\) - .*/main.go:7`}, + .*/main.go:7`}}, // Test for https://golang.org/issue/33309 {"midstack_inlining_traceback", "run", "linux", "atexit_sleep_ms=0", ` @@ -241,7 +257,7 @@ func g(c chan int) { func h(c chan int) { c <- x } -`, `================== +`, []string{`================== WARNING: DATA RACE Read at 0x[0-9,a-f]+ by goroutine [0-9]: main\.h\(\) @@ -261,7 +277,7 @@ Goroutine [0-9] \(running\) created at: ================== Found 1 data race\(s\) exit status 66 -`}, +`}}, // Test for https://golang.org/issue/17190 {"external_cgo_thread", "run", "linux", "atexit_sleep_ms=0", ` @@ -300,7 +316,25 @@ func main() { racy++ <- done } -`, `================== +`, []string{`================== +WARNING: DATA RACE +Read at 0x[0-9,a-f]+ by main goroutine: + main\.main\(\) + .*/main\.go:34 \+0x[0-9,a-f]+ + +Previous write at 0x[0-9,a-f]+ by goroutine [0-9]: + main\.goCallback\(\) + .*/main\.go:27 \+0x[0-9,a-f]+ + _cgoexp_[0-9a-z]+_goCallback\(\) + .*_cgo_gotypes\.go:[0-9]+ \+0x[0-9,a-f]+ + _cgoexp_[0-9a-z]+_goCallback\(\) + :1 \+0x[0-9,a-f]+ + +Goroutine [0-9] \(running\) created at: + runtime\.newextram\(\) + .*/runtime/proc.go:[0-9]+ \+0x[0-9,a-f]+ +==================`, + `================== WARNING: DATA RACE Read at 0x[0-9,a-f]+ by .*: main\..* @@ -313,7 +347,7 @@ Previous write at 0x[0-9,a-f]+ by .*: Goroutine [0-9] \(running\) created at: runtime\.newextram\(\) .*/runtime/proc.go:[0-9]+ \+0x[0-9,a-f]+ -==================`}, +==================`}}, {"second_test_passes", "test", "", "atexit_sleep_ms=0", ` package main_test import "testing" @@ -331,11 +365,11 @@ func TestFail(t *testing.T) { func TestPass(t *testing.T) { } -`, ` +`, []string{` ================== --- FAIL: TestFail \(0...s\) .*testing.go:.*: race detected during execution of test -FAIL`}, +FAIL`}}, {"mutex", "run", "", "atexit_sleep_ms=0", ` package main import ( @@ -366,7 +400,7 @@ func main() { } wg.Wait() if (data == iterations*(threads+1)) { fmt.Println("pass") } -}`, `pass`}, +}`, []string{`pass`}}, // Test for https://github.com/golang/go/issues/37355 {"chanmm", "run", "", "atexit_sleep_ms=0", ` package main @@ -395,7 +429,7 @@ func main() { wg.Wait() _ = data } -`, `================== +`, []string{`================== WARNING: DATA RACE Write at 0x[0-9,a-f]+ by goroutine [0-9]: main\.main\.func2\(\) @@ -408,5 +442,5 @@ Previous write at 0x[0-9,a-f]+ by main goroutine: Goroutine [0-9] \(running\) created at: main\.main\(\) .*/main.go:[0-9]+ \+0x[0-9,a-f]+ -==================`}, +==================`}}, } From 2755361e6abfd3a58acd5f7ebbcd05c23bc8261a Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 18 Dec 2020 20:49:50 -0800 Subject: [PATCH 14/27] [dev.regabi] cmd/compile: change noder.declNames to returns ir.Names declNames always returns a slice of *ir.Names, so return that directly rather than as []ir.Node. While here, also change iimport to directly create ir.ODCL/ir.OAS statements, rather than calling variter. Allows eliminating a use of ir.TypeNode. Passes buildall w/ toolstash -cmp. Change-Id: Icb75e993c4957b6050c797ba64ee71cfb7a19644 Reviewed-on: https://go-review.googlesource.com/c/go/+/279315 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Matthew Dempsky Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/dcl.go | 20 ++++++++------------ src/cmd/compile/internal/gc/embed.go | 4 ++-- src/cmd/compile/internal/gc/iimport.go | 10 ++++++++-- src/cmd/compile/internal/gc/noder.go | 5 ++--- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/cmd/compile/internal/gc/dcl.go b/src/cmd/compile/internal/gc/dcl.go index 04e3506dba..09d2e7d8b7 100644 --- a/src/cmd/compile/internal/gc/dcl.go +++ b/src/cmd/compile/internal/gc/dcl.go @@ -130,17 +130,16 @@ func declare(n *ir.Name, ctxt ir.Class) { // declare variables from grammar // new_name_list (type | [type] = expr_list) -func variter(vl []ir.Node, t ir.Ntype, el []ir.Node) []ir.Node { +func variter(vl []*ir.Name, t ir.Ntype, el []ir.Node) []ir.Node { var init []ir.Node doexpr := len(el) > 0 if len(el) == 1 && len(vl) > 1 { e := el[0] as2 := ir.Nod(ir.OAS2, nil, nil) - as2.PtrList().Set(vl) as2.PtrRlist().Set1(e) for _, v := range vl { - v := v.(*ir.Name) + as2.PtrList().Append(v) declare(v, dclcontext) v.Ntype = t v.Defn = as2 @@ -152,17 +151,14 @@ func variter(vl []ir.Node, t ir.Ntype, el []ir.Node) []ir.Node { return append(init, as2) } - nel := len(el) - for _, v := range vl { - v := v.(*ir.Name) + for i, v := range vl { var e ir.Node if doexpr { - if len(el) == 0 { - base.Errorf("assignment mismatch: %d variables but %d values", len(vl), nel) + if i >= len(el) { + base.Errorf("assignment mismatch: %d variables but %d values", len(vl), len(el)) break } - e = el[0] - el = el[1:] + e = el[i] } declare(v, dclcontext) @@ -180,8 +176,8 @@ func variter(vl []ir.Node, t ir.Ntype, el []ir.Node) []ir.Node { } } - if len(el) != 0 { - base.Errorf("assignment mismatch: %d variables but %d values", len(vl), nel) + if len(el) > len(vl) { + base.Errorf("assignment mismatch: %d variables but %d values", len(vl), len(el)) } return init } diff --git a/src/cmd/compile/internal/gc/embed.go b/src/cmd/compile/internal/gc/embed.go index 0d4ce83716..ea23e26069 100644 --- a/src/cmd/compile/internal/gc/embed.go +++ b/src/cmd/compile/internal/gc/embed.go @@ -24,7 +24,7 @@ const ( embedFiles ) -func varEmbed(p *noder, names []ir.Node, typ ir.Ntype, exprs []ir.Node, embeds []PragmaEmbed) (newExprs []ir.Node) { +func varEmbed(p *noder, names []*ir.Name, typ ir.Ntype, exprs []ir.Node, embeds []PragmaEmbed) (newExprs []ir.Node) { haveEmbed := false for _, decl := range p.file.DeclList { imp, ok := decl.(*syntax.ImportDecl) @@ -66,7 +66,7 @@ func varEmbed(p *noder, names []ir.Node, typ ir.Ntype, exprs []ir.Node, embeds [ return exprs } - v := names[0].(*ir.Name) + v := names[0] Target.Embeds = append(Target.Embeds, v) v.Embed = new([]ir.Embed) for _, e := range embeds { diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 219ce4bdef..cd66d39b66 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -972,8 +972,14 @@ func (r *importReader) node() ir.Node { case ir.ODCL: pos := r.pos() lhs := ir.NewDeclNameAt(pos, ir.ONAME, r.ident()) - typ := ir.TypeNode(r.typ()) - return npos(pos, liststmt(variter([]ir.Node{lhs}, typ, nil))) // TODO(gri) avoid list creation + lhs.SetType(r.typ()) + + declare(lhs, ir.PAUTO) + + var stmts ir.Nodes + stmts.Append(ir.Nod(ir.ODCL, lhs, nil)) + stmts.Append(ir.Nod(ir.OAS, lhs, nil)) + return npos(pos, liststmt(stmts.Slice())) // case OAS, OASWB: // unreachable - mapped to OAS case below by exporter diff --git a/src/cmd/compile/internal/gc/noder.go b/src/cmd/compile/internal/gc/noder.go index b61f19ae2e..97a9ac4396 100644 --- a/src/cmd/compile/internal/gc/noder.go +++ b/src/cmd/compile/internal/gc/noder.go @@ -441,7 +441,6 @@ func (p *noder) constDecl(decl *syntax.ConstDecl, cs *constState) []ir.Node { nn := make([]ir.Node, 0, len(names)) for i, n := range names { - n := n.(*ir.Name) if i >= len(values) { base.Errorf("missing value in const declaration") break @@ -492,8 +491,8 @@ func (p *noder) typeDecl(decl *syntax.TypeDecl) ir.Node { return nod } -func (p *noder) declNames(op ir.Op, names []*syntax.Name) []ir.Node { - nodes := make([]ir.Node, 0, len(names)) +func (p *noder) declNames(op ir.Op, names []*syntax.Name) []*ir.Name { + nodes := make([]*ir.Name, 0, len(names)) for _, name := range names { nodes = append(nodes, p.declName(op, name)) } From 3512cde10ac5e466527d69313b8250b2ea0146b1 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 17 Dec 2020 18:47:26 -0800 Subject: [PATCH 15/27] [dev.regabi] cmd/compile: stop reusing Ntype for OSLICELIT length For OSLICELITs, we were reusing the Ntype field after type checking to hold the length of the OSLICELIT's backing array. However, Ntype is only meant for nodes that can represent types. Today, this works only because we currently use Name for all OLITERAL constants (whether declared or not), whereas we should be able to represent them more compactly with a dedicated type that doesn't implement Ntype. Passes buildall w/ toolstash -cmp. Change-Id: I385f1d787c41b016f507a5bad9489d59ccfde7f2 Reviewed-on: https://go-review.googlesource.com/c/go/+/279152 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Matthew Dempsky Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/inl.go | 2 +- src/cmd/compile/internal/gc/order.go | 2 +- src/cmd/compile/internal/gc/sinit.go | 18 +++++++++--------- src/cmd/compile/internal/gc/typecheck.go | 3 ++- src/cmd/compile/internal/ir/expr.go | 1 + 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 33a309db87..5ada83b715 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -452,7 +452,7 @@ func (v *hairyVisitor) doNode(n ir.Node) error { // and don't charge for the OBLOCK itself. The ++ undoes the -- below. v.budget++ - case ir.OCALLPART: + case ir.OCALLPART, ir.OSLICELIT: v.budget-- // Hack for toolstash -cmp. } diff --git a/src/cmd/compile/internal/gc/order.go b/src/cmd/compile/internal/gc/order.go index 9c03a5843c..1a0f0066d0 100644 --- a/src/cmd/compile/internal/gc/order.go +++ b/src/cmd/compile/internal/gc/order.go @@ -1281,7 +1281,7 @@ func (o *Order) expr1(n, lhs ir.Node) ir.Node { n := n.(*ir.CompLitExpr) o.exprList(n.List()) if n.Transient() { - t := types.NewArray(n.Type().Elem(), ir.Int64Val(n.Right())) + t := types.NewArray(n.Type().Elem(), n.Len) n.Prealloc = o.newTemp(t, false) } return n diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go index 7b710fd511..a845bc5d75 100644 --- a/src/cmd/compile/internal/gc/sinit.go +++ b/src/cmd/compile/internal/gc/sinit.go @@ -142,8 +142,9 @@ func (s *InitSchedule) staticcopy(l *ir.Name, loff int64, rn *ir.Name, typ *type } case ir.OSLICELIT: + r := r.(*ir.CompLitExpr) // copy slice - slicesym(l, loff, s.inittemps[r], ir.Int64Val(r.Right())) + slicesym(l, loff, s.inittemps[r], r.Len) return true case ir.OARRAYLIT, ir.OSTRUCTLIT: @@ -232,14 +233,14 @@ func (s *InitSchedule) staticassign(l *ir.Name, loff int64, r ir.Node, typ *type } case ir.OSLICELIT: + r := r.(*ir.CompLitExpr) s.initplan(r) // Init slice. - bound := ir.Int64Val(r.Right()) - ta := types.NewArray(r.Type().Elem(), bound) + ta := types.NewArray(r.Type().Elem(), r.Len) ta.SetNoalg(true) a := staticname(ta) s.inittemps[r] = a - slicesym(l, loff, a, bound) + slicesym(l, loff, a, r.Len) // Fall through to init underlying array. l = a loff = 0 @@ -425,10 +426,11 @@ func getdyn(n ir.Node, top bool) initGenType { return initDynamic case ir.OSLICELIT: + n := n.(*ir.CompLitExpr) if !top { return initDynamic } - if ir.Int64Val(n.Right())/4 > int64(n.List().Len()) { + if n.Len/4 > int64(n.List().Len()) { // <25% of entries have explicit values. // Very rough estimation, it takes 4 bytes of instructions // to initialize 1 byte of result. So don't use a static @@ -603,14 +605,12 @@ func isSmallSliceLit(n *ir.CompLitExpr) bool { return false } - r := n.Right() - - return smallintconst(r) && (n.Type().Elem().Width == 0 || ir.Int64Val(r) <= smallArrayBytes/n.Type().Elem().Width) + return n.Type().Elem().Width == 0 || n.Len <= smallArrayBytes/n.Type().Elem().Width } func slicelit(ctxt initContext, n *ir.CompLitExpr, var_ ir.Node, init *ir.Nodes) { // make an array type corresponding the number of elements we have - t := types.NewArray(n.Type().Elem(), ir.Int64Val(n.Right())) + t := types.NewArray(n.Type().Elem(), n.Len) dowidth(t) if ctxt == inNonInitFunction { diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 4fae4a0819..2d383ab49e 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -2850,7 +2850,8 @@ func typecheckcomplit(n *ir.CompLitExpr) (res ir.Node) { case types.TSLICE: length := typecheckarraylit(t.Elem(), -1, n.List().Slice(), "slice literal") n.SetOp(ir.OSLICELIT) - n.SetRight(nodintconst(length)) + n.SetRight(nil) + n.Len = length case types.TMAP: var cs constSet diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 8f43eb0fb2..d74e7f8763 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -294,6 +294,7 @@ type CompLitExpr struct { Ntype Ntype List_ Nodes // initialized values Prealloc *Name + Len int64 // backing array length for OSLICELIT } func NewCompLitExpr(pos src.XPos, op Op, typ Ntype, list []Node) *CompLitExpr { From c8610e4700bee51898197987de5335b8527079e8 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 17 Dec 2020 20:17:04 -0800 Subject: [PATCH 16/27] [dev.regabi] cmd/compile: add ir.BasicLit to represent literals This CL changes so that all literals are represented with a new, smaller ir.BasicLit type, so that ir.Name is only used to represent declared constants. Passes buildall w/ toolstash -cmp. Change-Id: I4702b8e3fa945617bd05881d7a2be1205f229633 Reviewed-on: https://go-review.googlesource.com/c/go/+/279153 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Matthew Dempsky Reviewed-by: Russ Cox --- src/cmd/compile/internal/gc/universe.go | 9 +++------ src/cmd/compile/internal/ir/expr.go | 19 +++++++++++++++++++ src/cmd/compile/internal/ir/name.go | 11 +++++++++++ src/cmd/compile/internal/ir/node_gen.go | 15 +++++++++++++++ src/cmd/compile/internal/ir/val.go | 7 +------ 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/cmd/compile/internal/gc/universe.go b/src/cmd/compile/internal/gc/universe.go index c988c575dc..e11c0eb92c 100644 --- a/src/cmd/compile/internal/gc/universe.go +++ b/src/cmd/compile/internal/gc/universe.go @@ -11,6 +11,7 @@ import ( "cmd/compile/internal/ir" "cmd/compile/internal/types" "cmd/internal/src" + "go/constant" ) var basicTypes = [...]struct { @@ -163,14 +164,10 @@ func initUniverse() { } s = types.BuiltinPkg.Lookup("true") - b := nodbool(true) - b.(*ir.Name).SetSym(lookup("true")) - s.Def = b + s.Def = ir.NewConstAt(src.NoXPos, s, types.UntypedBool, constant.MakeBool(true)) s = types.BuiltinPkg.Lookup("false") - b = nodbool(false) - b.(*ir.Name).SetSym(lookup("false")) - s.Def = b + s.Def = ir.NewConstAt(src.NoXPos, s, types.UntypedBool, constant.MakeBool(false)) s = lookup("_") types.BlankSym = s diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index d74e7f8763..5937798bd4 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -136,6 +136,25 @@ func (n *AddrExpr) SetOp(op Op) { } } +// A BasicLit is a literal of basic type. +type BasicLit struct { + miniExpr + val constant.Value +} + +func NewBasicLit(pos src.XPos, val constant.Value) Node { + n := &BasicLit{val: val} + n.op = OLITERAL + n.pos = pos + if k := val.Kind(); k != constant.Unknown { + n.SetType(idealType(k)) + } + return n +} + +func (n *BasicLit) Val() constant.Value { return n.val } +func (n *BasicLit) SetVal(val constant.Value) { n.val = val } + // A BinaryExpr is a binary expression X Op Y, // or Op(X, Y) for builtin functions that do not become calls. type BinaryExpr struct { diff --git a/src/cmd/compile/internal/ir/name.go b/src/cmd/compile/internal/ir/name.go index 9cf959b23d..b0b33cccfa 100644 --- a/src/cmd/compile/internal/ir/name.go +++ b/src/cmd/compile/internal/ir/name.go @@ -179,6 +179,17 @@ func NewDeclNameAt(pos src.XPos, op Op, sym *types.Sym) *Name { return newNameAt(pos, op, sym) } +// NewConstAt returns a new OLITERAL Node associated with symbol s at position pos. +func NewConstAt(pos src.XPos, sym *types.Sym, typ *types.Type, val constant.Value) *Name { + if sym == nil { + base.Fatalf("NewConstAt nil") + } + n := newNameAt(pos, OLITERAL, sym) + n.SetType(typ) + n.SetVal(val) + return n +} + // newNameAt is like NewNameAt but allows sym == nil. func newNameAt(pos src.XPos, op Op, sym *types.Sym) *Name { n := new(Name) diff --git a/src/cmd/compile/internal/ir/node_gen.go b/src/cmd/compile/internal/ir/node_gen.go index a0fae2b949..a5959ea26f 100644 --- a/src/cmd/compile/internal/ir/node_gen.go +++ b/src/cmd/compile/internal/ir/node_gen.go @@ -116,6 +116,21 @@ func (n *AssignStmt) editChildren(edit func(Node) Node) { n.Y = maybeEdit(n.Y, edit) } +func (n *BasicLit) Format(s fmt.State, verb rune) { FmtNode(n, s, verb) } +func (n *BasicLit) copy() Node { + c := *n + c.init = c.init.Copy() + return &c +} +func (n *BasicLit) doChildren(do func(Node) error) error { + var err error + err = maybeDoList(n.init, err, do) + return err +} +func (n *BasicLit) editChildren(edit func(Node) Node) { + editList(n.init, edit) +} + func (n *BinaryExpr) Format(s fmt.State, verb rune) { FmtNode(n, s, verb) } func (n *BinaryExpr) copy() Node { c := *n diff --git a/src/cmd/compile/internal/ir/val.go b/src/cmd/compile/internal/ir/val.go index 5b0506c0d0..ff45f31074 100644 --- a/src/cmd/compile/internal/ir/val.go +++ b/src/cmd/compile/internal/ir/val.go @@ -92,12 +92,7 @@ func ValidTypeForConst(t *types.Type, v constant.Value) bool { // nodlit returns a new untyped constant with value v. func NewLiteral(v constant.Value) Node { - n := newNameAt(base.Pos, OLITERAL, nil) - if k := v.Kind(); k != constant.Unknown { - n.SetType(idealType(k)) - n.SetVal(v) - } - return n + return NewBasicLit(base.Pos, v) } func idealType(ct constant.Kind) *types.Type { From cb28c96be8b8010dd979e0723bf5a94b11962a93 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 24 Sep 2020 13:14:46 -0400 Subject: [PATCH 17/27] [dev.regabi] cmd/compile,cmd/link: initial support for ABI wrappers Add compiler support for emitting ABI wrappers by creating real IR as opposed to introducing ABI aliases. At the moment these are "no-op" wrappers in the sense that they make a simple call (using the existing ABI) to their target. The assumption here is that once late call expansion can handle both ABI0 and the "new" ABIInternal (register version), it can expand the call to do the right thing. Note that the runtime contains functions that do not strictly follow the rules of the current Go ABI0; this has been handled in most cases by treating these as ABIInternal instead (these changes have been made in previous patches). Generation of ABI wrappers (as opposed to ABI aliases) is currently gated by GOEXPERIMENT=regabi -- wrapper generation is on by default if GOEXPERIMENT=regabi is set and off otherwise (but can be turned on using "-gcflags=all=-abiwrap -ldflags=-abiwrap"). Wrapper generation currently only workd on AMD64; explicitly enabling wrapper for other architectures (via the command line) is not supported. Also in this patch are a few other command line options for debugging (tracing and/or limiting wrapper creation). These will presumably go away at some point. Updates #27539, #40724. Change-Id: I1ee3226fc15a3c32ca2087b8ef8e41dbe6df4a75 Reviewed-on: https://go-review.googlesource.com/c/go/+/270863 Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Trust: Than McIntosh --- src/cmd/compile/internal/base/debug.go | 1 + src/cmd/compile/internal/base/flag.go | 3 + src/cmd/compile/internal/gc/gsubr.go | 191 ++++++++++++++++++++---- src/cmd/compile/internal/gc/main.go | 23 +++ src/cmd/compile/internal/gc/pgen.go | 7 +- src/cmd/compile/internal/gc/racewalk.go | 2 +- src/cmd/compile/internal/gc/ssa.go | 49 +++++- src/cmd/compile/internal/types/sym.go | 17 +++ src/cmd/internal/obj/link.go | 6 + src/cmd/internal/obj/plist.go | 6 + src/cmd/internal/obj/textflag.go | 3 + src/cmd/internal/obj/x86/obj6.go | 4 +- src/cmd/link/internal/ld/main.go | 12 +- src/cmd/link/internal/ld/symtab.go | 37 ++++- src/runtime/textflag.h | 2 + test/nosplit.go | 9 +- 16 files changed, 328 insertions(+), 44 deletions(-) diff --git a/src/cmd/compile/internal/base/debug.go b/src/cmd/compile/internal/base/debug.go index 45a552a4d9..3acdcea846 100644 --- a/src/cmd/compile/internal/base/debug.go +++ b/src/cmd/compile/internal/base/debug.go @@ -51,6 +51,7 @@ type DebugFlags struct { TypeAssert int `help:"print information about type assertion inlining"` TypecheckInl int `help:"eager typechecking of inline function bodies"` WB int `help:"print information about write barriers"` + ABIWrap int `help:"print information about ABI wrapper generation"` any bool // set when any of the values have been set } diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go index aadc70f496..ce87ff730e 100644 --- a/src/cmd/compile/internal/base/flag.go +++ b/src/cmd/compile/internal/base/flag.go @@ -81,6 +81,8 @@ type CmdFlags struct { CompilingRuntime bool "flag:\"+\" help:\"compiling runtime\"" // Longer names + ABIWrap bool "help:\"enable generation of ABI wrappers\"" + ABIWrapLimit int "help:\"emit at most N ABI wrappers (for debugging)\"" AsmHdr string "help:\"write assembly header to `file`\"" Bench string "help:\"append benchmark times to `file`\"" BlockProfile string "help:\"write block profile to `file`\"" @@ -140,6 +142,7 @@ func ParseFlags() { Flag.LowerP = &Ctxt.Pkgpath Flag.LowerV = &Ctxt.Debugvlog + Flag.ABIWrap = objabi.Regabi_enabled != 0 Flag.Dwarf = objabi.GOARCH != "wasm" Flag.DwarfBASEntries = &Ctxt.UseBASEntries Flag.DwarfLocationLists = &Ctxt.Flag_locationlists diff --git a/src/cmd/compile/internal/gc/gsubr.go b/src/cmd/compile/internal/gc/gsubr.go index ddb431d5ab..f3ef14c99b 100644 --- a/src/cmd/compile/internal/gc/gsubr.go +++ b/src/cmd/compile/internal/gc/gsubr.go @@ -34,9 +34,12 @@ import ( "cmd/compile/internal/base" "cmd/compile/internal/ir" "cmd/compile/internal/ssa" + "cmd/compile/internal/types" "cmd/internal/obj" "cmd/internal/objabi" "cmd/internal/src" + "fmt" + "os" ) var sharedProgArray = new([10000]obj.Prog) // *T instead of T to work around issue 19839 @@ -187,32 +190,154 @@ func (pp *Progs) settext(fn *ir.Func) { ptxt.From.Sym = fn.LSym } +// makeABIWrapper creates a new function that wraps a cross-ABI call +// to "f". The wrapper is marked as an ABIWRAPPER. +func makeABIWrapper(f *ir.Func, wrapperABI obj.ABI) { + + // Q: is this needed? + savepos := base.Pos + savedclcontext := dclcontext + savedcurfn := Curfn + + base.Pos = autogeneratedPos + dclcontext = ir.PEXTERN + + // At the moment we don't support wrapping a method, we'd need machinery + // below to handle the receiver. Panic if we see this scenario. + ft := f.Nname.Ntype.Type() + if ft.NumRecvs() != 0 { + panic("makeABIWrapper support for wrapping methods not implemented") + } + + // Manufacture a new func type to use for the wrapper. + var noReceiver *ir.Field + tfn := ir.NewFuncType(base.Pos, + noReceiver, + structargs(ft.Params(), true), + structargs(ft.Results(), false)) + + // Reuse f's types.Sym to create a new ODCLFUNC/function. + fn := dclfunc(f.Nname.Sym(), tfn) + fn.SetDupok(true) + fn.SetWrapper(true) // ignore frame for panic+recover matching + + // Select LSYM now. + asym := base.Ctxt.LookupABI(f.LSym.Name, wrapperABI) + asym.Type = objabi.STEXT + if fn.LSym != nil { + panic("unexpected") + } + fn.LSym = asym + + // ABI0-to-ABIInternal wrappers will be mainly loading params from + // stack into registers (and/or storing stack locations back to + // registers after the wrapped call); in most cases they won't + // need to allocate stack space, so it should be OK to mark them + // as NOSPLIT in these cases. In addition, my assumption is that + // functions written in assembly are NOSPLIT in most (but not all) + // cases. In the case of an ABIInternal target that has too many + // parameters to fit into registers, the wrapper would need to + // allocate stack space, but this seems like an unlikely scenario. + // Hence: mark these wrappers NOSPLIT. + // + // ABIInternal-to-ABI0 wrappers on the other hand will be taking + // things in registers and pushing them onto the stack prior to + // the ABI0 call, meaning that they will always need to allocate + // stack space. If the compiler marks them as NOSPLIT this seems + // as though it could lead to situations where the the linker's + // nosplit-overflow analysis would trigger a link failure. On the + // other hand if they not tagged NOSPLIT then this could cause + // problems when building the runtime (since there may be calls to + // asm routine in cases where it's not safe to grow the stack). In + // most cases the wrapper would be (in effect) inlined, but are + // there (perhaps) indirect calls from the runtime that could run + // into trouble here. + // FIXME: at the moment all.bash does not pass when I leave out + // NOSPLIT for these wrappers, so all are currently tagged with NOSPLIT. + setupTextLSym(fn, obj.NOSPLIT|obj.ABIWRAPPER) + + // Generate call. Use tail call if no params and no returns, + // but a regular call otherwise. + // + // Note: ideally we would be using a tail call in cases where + // there are params but no returns for ABI0->ABIInternal wrappers, + // provided that all params fit into registers (e.g. we don't have + // to allocate any stack space). Doing this will require some + // extra work in typecheck/walk/ssa, might want to add a new node + // OTAILCALL or something to this effect. + var call ir.Node + if tfn.Type().NumResults() == 0 && tfn.Type().NumParams() == 0 && tfn.Type().NumRecvs() == 0 { + call = nodSym(ir.ORETJMP, nil, f.Nname.Sym()) + } else { + call = ir.Nod(ir.OCALL, f.Nname, nil) + call.PtrList().Set(paramNnames(tfn.Type())) + call.SetIsDDD(tfn.Type().IsVariadic()) + if tfn.Type().NumResults() > 0 { + n := ir.Nod(ir.ORETURN, nil, nil) + n.PtrList().Set1(call) + call = n + } + } + fn.PtrBody().Append(call) + + funcbody() + if base.Debug.DclStack != 0 { + testdclstack() + } + + typecheckFunc(fn) + Curfn = fn + typecheckslice(fn.Body().Slice(), ctxStmt) + + escapeFuncs([]*ir.Func{fn}, false) + + Target.Decls = append(Target.Decls, fn) + + // Restore previous context. + base.Pos = savepos + dclcontext = savedclcontext + Curfn = savedcurfn +} + // initLSym defines f's obj.LSym and initializes it based on the // properties of f. This includes setting the symbol flags and ABI and // creating and initializing related DWARF symbols. // // initLSym must be called exactly once per function and must be // called for both functions with bodies and functions without bodies. +// For body-less functions, we only create the LSym; for functions +// with bodies call a helper to setup up / populate the LSym. func initLSym(f *ir.Func, hasBody bool) { + // FIXME: for new-style ABI wrappers, we set up the lsym at the + // point the wrapper is created. + if f.LSym != nil && base.Flag.ABIWrap { + return + } + selectLSym(f, hasBody) + if hasBody { + setupTextLSym(f, 0) + } +} + +// selectLSym sets up the LSym for a given function, and +// makes calls to helpers to create ABI wrappers if needed. +func selectLSym(f *ir.Func, hasBody bool) { if f.LSym != nil { base.Fatalf("Func.initLSym called twice") } if nam := f.Nname; !ir.IsBlank(nam) { - f.LSym = nam.Sym().Linksym() - if f.Pragma&ir.Systemstack != 0 { - f.LSym.Set(obj.AttrCFunc, true) - } - var aliasABI obj.ABI - needABIAlias := false - defABI, hasDefABI := symabiDefs[f.LSym.Name] + var wrapperABI obj.ABI + needABIWrapper := false + defABI, hasDefABI := symabiDefs[nam.Sym().LinksymName()] if hasDefABI && defABI == obj.ABI0 { // Symbol is defined as ABI0. Create an // Internal -> ABI0 wrapper. - f.LSym.SetABI(obj.ABI0) - needABIAlias, aliasABI = true, obj.ABIInternal + f.LSym = nam.Sym().LinksymABI0() + needABIWrapper, wrapperABI = true, obj.ABIInternal } else { + f.LSym = nam.Sym().Linksym() // No ABI override. Check that the symbol is // using the expected ABI. want := obj.ABIInternal @@ -220,6 +345,9 @@ func initLSym(f *ir.Func, hasBody bool) { base.Fatalf("function symbol %s has the wrong ABI %v, expected %v", f.LSym.Name, f.LSym.ABI(), want) } } + if f.Pragma&ir.Systemstack != 0 { + f.LSym.Set(obj.AttrCFunc, true) + } isLinknameExported := nam.Sym().Linkname != "" && (hasBody || hasDefABI) if abi, ok := symabiRefs[f.LSym.Name]; (ok && abi == obj.ABI0) || isLinknameExported { @@ -235,32 +363,39 @@ func initLSym(f *ir.Func, hasBody bool) { // using linkname and we don't want to create // duplicate ABI wrappers. if f.LSym.ABI() != obj.ABI0 { - needABIAlias, aliasABI = true, obj.ABI0 + needABIWrapper, wrapperABI = true, obj.ABI0 } } - if needABIAlias { - // These LSyms have the same name as the - // native function, so we create them directly - // rather than looking them up. The uniqueness - // of f.lsym ensures uniqueness of asym. - asym := &obj.LSym{ - Name: f.LSym.Name, - Type: objabi.SABIALIAS, - R: []obj.Reloc{{Sym: f.LSym}}, // 0 size, so "informational" + if needABIWrapper { + if !useABIWrapGen(f) { + // Fallback: use alias instead. FIXME. + + // These LSyms have the same name as the + // native function, so we create them directly + // rather than looking them up. The uniqueness + // of f.lsym ensures uniqueness of asym. + asym := &obj.LSym{ + Name: f.LSym.Name, + Type: objabi.SABIALIAS, + R: []obj.Reloc{{Sym: f.LSym}}, // 0 size, so "informational" + } + asym.SetABI(wrapperABI) + asym.Set(obj.AttrDuplicateOK, true) + base.Ctxt.ABIAliases = append(base.Ctxt.ABIAliases, asym) + } else { + if base.Debug.ABIWrap != 0 { + fmt.Fprintf(os.Stderr, "=-= %v to %v wrapper for %s.%s\n", + wrapperABI, 1-wrapperABI, types.LocalPkg.Path, f.LSym.Name) + } + makeABIWrapper(f, wrapperABI) } - asym.SetABI(aliasABI) - asym.Set(obj.AttrDuplicateOK, true) - base.Ctxt.ABIAliases = append(base.Ctxt.ABIAliases, asym) } } +} - if !hasBody { - // For body-less functions, we only create the LSym. - return - } - - var flag int +// setupTextLsym initializes the LSym for a with-body text symbol. +func setupTextLSym(f *ir.Func, flag int) { if f.Dupok() { flag |= obj.DUPOK } diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 7f7cd63cdf..de2b3db36a 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -1144,3 +1144,26 @@ func initializeTypesPackage() { initUniverse() } + +// useNewABIWrapGen returns TRUE if the compiler should generate an +// ABI wrapper for the function 'f'. +func useABIWrapGen(f *ir.Func) bool { + if !base.Flag.ABIWrap { + return false + } + + // Support limit option for bisecting. + if base.Flag.ABIWrapLimit == 1 { + return false + } + if base.Flag.ABIWrapLimit < 1 { + return true + } + base.Flag.ABIWrapLimit-- + if base.Debug.ABIWrap != 0 && base.Flag.ABIWrapLimit == 1 { + fmt.Fprintf(os.Stderr, "=-= limit reached after new wrapper for %s\n", + f.LSym.Name) + } + + return true +} diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index 5b5288c389..dae9d79147 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -32,7 +32,6 @@ func emitptrargsmap(fn *ir.Func) { return } lsym := base.Ctxt.Lookup(fn.LSym.Name + ".args_stackmap") - nptr := int(fn.Type().ArgWidth() / int64(Widthptr)) bv := bvalloc(int32(nptr) * 2) nbitmap := 1 @@ -399,7 +398,11 @@ func debuginfo(fnsym *obj.LSym, infosym *obj.LSym, curfn interface{}) ([]dwarf.S fn := curfn.(*ir.Func) if fn.Nname != nil { - if expect := fn.Sym().Linksym(); fnsym != expect { + expect := fn.Sym().Linksym() + if fnsym.ABI() == obj.ABI0 { + expect = fn.Sym().LinksymABI0() + } + if fnsym != expect { base.Fatalf("unexpected fnsym: %v != %v", fnsym, expect) } } diff --git a/src/cmd/compile/internal/gc/racewalk.go b/src/cmd/compile/internal/gc/racewalk.go index 472deb16e3..61a65368af 100644 --- a/src/cmd/compile/internal/gc/racewalk.go +++ b/src/cmd/compile/internal/gc/racewalk.go @@ -61,7 +61,7 @@ func ispkgin(pkgs []string) bool { } func instrument(fn *ir.Func) { - if fn.Pragma&ir.Norace != 0 { + if fn.Pragma&ir.Norace != 0 || (fn.Sym().Linksym() != nil && fn.Sym().Linksym().ABIWrapper()) { return } diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index a5340e7f11..b4cf8b6dc7 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -1421,7 +1421,7 @@ func (s *state) stmt(n ir.Node) { case ir.ORETJMP: b := s.exit() b.Kind = ssa.BlockRetJmp // override BlockRet - b.Aux = n.Sym().Linksym() + b.Aux = callTargetLSym(n.Sym(), s.curfn.LSym) case ir.OCONTINUE, ir.OBREAK: var to *ssa.Block @@ -4826,11 +4826,11 @@ func (s *state) call(n *ir.CallExpr, k callKind, returnResultAddr bool) *ssa.Val } case sym != nil: if testLateExpansion { - aux := ssa.StaticAuxCall(sym.Linksym(), ACArgs, ACResults) + aux := ssa.StaticAuxCall(callTargetLSym(sym, s.curfn.LSym), ACArgs, ACResults) call = s.newValue0A(ssa.OpStaticLECall, aux.LateExpansionResultType(), aux) call.AddArgs(callArgs...) } else { - call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, ssa.StaticAuxCall(sym.Linksym(), ACArgs, ACResults), s.mem()) + call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, ssa.StaticAuxCall(callTargetLSym(sym, s.curfn.LSym), ACArgs, ACResults), s.mem()) } default: s.Fatalf("bad call type %v %v", n.Op(), n) @@ -7291,3 +7291,46 @@ func clobberBase(n ir.Node) ir.Node { } return n } + +// callTargetLSym determines the correct LSym for 'callee' when called +// from function 'caller'. There are a couple of different scenarios +// to contend with here: +// +// 1. if 'caller' is an ABI wrapper, then we always want to use the +// LSym from the Func for the callee. +// +// 2. if 'caller' is not an ABI wrapper, then we looked at the callee +// to see if it corresponds to a "known" ABI0 symbol (e.g. assembly +// routine defined in the current package); if so, we want the call to +// directly target the ABI0 symbol (effectively bypassing the +// ABIInternal->ABI0 wrapper for 'callee'). +// +// 3. in all other cases, want the regular ABIInternal linksym +// +func callTargetLSym(callee *types.Sym, callerLSym *obj.LSym) *obj.LSym { + lsym := callee.Linksym() + if !base.Flag.ABIWrap { + return lsym + } + if ir.AsNode(callee.Def) == nil { + return lsym + } + ndclfunc := ir.AsNode(callee.Def).Name().Defn + if ndclfunc == nil { + return lsym + } + // check for case 1 above + if callerLSym.ABIWrapper() { + if nlsym := ndclfunc.Func().LSym; nlsym != nil { + lsym = nlsym + } + } else { + // check for case 2 above + nam := ndclfunc.Func().Nname + defABI, hasDefABI := symabiDefs[nam.Sym().LinksymName()] + if hasDefABI && defABI == obj.ABI0 { + lsym = nam.Sym().LinksymABI0() + } + } + return lsym +} diff --git a/src/cmd/compile/internal/types/sym.go b/src/cmd/compile/internal/types/sym.go index 19f06fcf5b..c512e3a003 100644 --- a/src/cmd/compile/internal/types/sym.go +++ b/src/cmd/compile/internal/types/sym.go @@ -93,6 +93,23 @@ func (sym *Sym) Linksym() *obj.LSym { return base.Ctxt.LookupInit(sym.LinksymName(), initPkg) } +// LinksymABI0 looks up or creates an ABI0 linker symbol for "sym", +// in cases where we want to specifically select the ABI0 version of +// a symbol (typically used only for ABI wrappers). +func (sym *Sym) LinksymABI0() *obj.LSym { + if sym == nil { + return nil + } + initPkg := func(r *obj.LSym) { + if sym.Linkname != "" { + r.Pkg = "_" + } else { + r.Pkg = sym.Pkg.Prefix + } + } + return base.Ctxt.LookupABIInit(sym.LinksymName(), obj.ABI0, initPkg) +} + // Less reports whether symbol a is ordered before symbol b. // // Symbols are ordered exported before non-exported, then by name, and diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index 7b5c990a5d..977c5c3303 100644 --- a/src/cmd/internal/obj/link.go +++ b/src/cmd/internal/obj/link.go @@ -635,6 +635,10 @@ const ( // ContentAddressable indicates this is a content-addressable symbol. AttrContentAddressable + // ABI wrapper is set for compiler-generated text symbols that + // convert between ABI0 and ABIInternal calling conventions. + AttrABIWrapper + // attrABIBase is the value at which the ABI is encoded in // Attribute. This must be last; all bits after this are // assumed to be an ABI value. @@ -660,6 +664,7 @@ func (a Attribute) TopFrame() bool { return a&AttrTopFrame != 0 } func (a Attribute) Indexed() bool { return a&AttrIndexed != 0 } func (a Attribute) UsedInIface() bool { return a&AttrUsedInIface != 0 } func (a Attribute) ContentAddressable() bool { return a&AttrContentAddressable != 0 } +func (a Attribute) ABIWrapper() bool { return a&AttrABIWrapper != 0 } func (a *Attribute) Set(flag Attribute, value bool) { if value { @@ -695,6 +700,7 @@ var textAttrStrings = [...]struct { {bit: AttrTopFrame, s: "TOPFRAME"}, {bit: AttrIndexed, s: ""}, {bit: AttrContentAddressable, s: ""}, + {bit: AttrABIWrapper, s: "ABIWRAPPER"}, } // TextAttrString formats a for printing in as part of a TEXT prog. diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go index 2b096996f7..679ce7eb8f 100644 --- a/src/cmd/internal/obj/plist.go +++ b/src/cmd/internal/obj/plist.go @@ -80,6 +80,11 @@ func Flushplist(ctxt *Link, plist *Plist, newprog ProgAlloc, myimportpath string if !strings.HasPrefix(s.Name, "\"\".") { continue } + if s.ABIWrapper() { + // Don't create an args_stackmap symbol reference for an ABI + // wrapper function + continue + } found := false for p := s.Func().Text; p != nil; p = p.Link { if p.As == AFUNCDATA && p.From.Type == TYPE_CONST && p.From.Offset == objabi.FUNCDATA_ArgsPointerMaps { @@ -134,6 +139,7 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) { s.Set(AttrNoSplit, flag&NOSPLIT != 0) s.Set(AttrReflectMethod, flag&REFLECTMETHOD != 0) s.Set(AttrWrapper, flag&WRAPPER != 0) + s.Set(AttrABIWrapper, flag&ABIWRAPPER != 0) s.Set(AttrNeedCtxt, flag&NEEDCTXT != 0) s.Set(AttrNoFrame, flag&NOFRAME != 0) s.Set(AttrTopFrame, flag&TOPFRAME != 0) diff --git a/src/cmd/internal/obj/textflag.go b/src/cmd/internal/obj/textflag.go index d2cec734b1..fcc4014aa2 100644 --- a/src/cmd/internal/obj/textflag.go +++ b/src/cmd/internal/obj/textflag.go @@ -51,4 +51,7 @@ const ( // Function is the top of the call stack. Call stack unwinders should stop // at this function. TOPFRAME = 2048 + + // Function is an ABI wrapper. + ABIWRAPPER = 4096 ) diff --git a/src/cmd/internal/obj/x86/obj6.go b/src/cmd/internal/obj/x86/obj6.go index 184fb4308b..839aeb8fe3 100644 --- a/src/cmd/internal/obj/x86/obj6.go +++ b/src/cmd/internal/obj/x86/obj6.go @@ -637,7 +637,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { } } - if !p.From.Sym.NoSplit() || p.From.Sym.Wrapper() { + if !p.From.Sym.NoSplit() || (p.From.Sym.Wrapper() && !p.From.Sym.ABIWrapper()) { p = obj.Appendp(p, newprog) p = load_g_cx(ctxt, p, newprog) // load g into CX } @@ -690,7 +690,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) { p.To.Reg = REG_BP } - if cursym.Func().Text.From.Sym.Wrapper() { + if cursym.Func().Text.From.Sym.Wrapper() && !cursym.Func().Text.From.Sym.ABIWrapper() { // if g._panic != nil && g._panic.argp == FP { // g._panic.argp = bottom-of-frame // } diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go index 5c8293810f..1420030eec 100644 --- a/src/cmd/link/internal/ld/main.go +++ b/src/cmd/link/internal/ld/main.go @@ -92,11 +92,10 @@ var ( FlagRound = flag.Int("R", -1, "set address rounding `quantum`") FlagTextAddr = flag.Int64("T", -1, "set text segment `address`") flagEntrySymbol = flag.String("E", "", "set `entry` symbol name") - - cpuprofile = flag.String("cpuprofile", "", "write cpu profile to `file`") - memprofile = flag.String("memprofile", "", "write memory profile to `file`") - memprofilerate = flag.Int64("memprofilerate", 0, "set runtime.MemProfileRate to `rate`") - + cpuprofile = flag.String("cpuprofile", "", "write cpu profile to `file`") + memprofile = flag.String("memprofile", "", "write memory profile to `file`") + memprofilerate = flag.Int64("memprofilerate", 0, "set runtime.MemProfileRate to `rate`") + flagAbiWrap = false benchmarkFlag = flag.String("benchmark", "", "set to 'mem' or 'cpu' to enable phase benchmarking") benchmarkFileFlag = flag.String("benchmarkprofile", "", "emit phase profiles to `base`_phase.{cpu,mem}prof") ) @@ -135,6 +134,9 @@ func Main(arch *sys.Arch, theArch Arch) { objabi.Flagfn1("X", "add string value `definition` of the form importpath.name=value", func(s string) { addstrdata1(ctxt, s) }) objabi.Flagcount("v", "print link trace", &ctxt.Debugvlog) objabi.Flagfn1("importcfg", "read import configuration from `file`", ctxt.readImportCfg) + if objabi.Regabi_enabled != 0 { + flag.BoolVar(&flagAbiWrap, "abiwrap", true, "support ABI wrapper functions") + } objabi.Flagparse(usage) diff --git a/src/cmd/link/internal/ld/symtab.go b/src/cmd/link/internal/ld/symtab.go index c98e4de03f..3b709baf75 100644 --- a/src/cmd/link/internal/ld/symtab.go +++ b/src/cmd/link/internal/ld/symtab.go @@ -102,6 +102,41 @@ func putelfsym(ctxt *Link, x loader.Sym, typ elf.SymType, curbind elf.SymBind) { elfshnum = xosect.Elfsect.(*ElfShdr).shnum } + sname := ldr.SymExtname(x) + + // For functions with ABI wrappers, we have to make sure that we + // don't wind up with two elf symbol table entries with the same + // name (since this will generated an error from the external + // linker). In the CgoExportStatic case, we want the ABI0 symbol + // to have the primary symbol table entry (since it's going to be + // called from C), so we rename the ABIInternal symbol. In all + // other cases, we rename the ABI0 symbol, since we want + // cross-load-module calls to target ABIInternal. + // + // TODO: generalize this for non-ELF (put the rename code in the + // loader, and store the rename result in SymExtname). + // + // TODO: avoid the ldr.Lookup calls below by instead using an aux + // sym or marker relocation to associate the wrapper with the + // wrapped function. + // + if flagAbiWrap { + if !ldr.IsExternal(x) && ldr.SymType(x) == sym.STEXT { + // First case + if ldr.SymVersion(x) == sym.SymVerABIInternal { + if s2 := ldr.Lookup(sname, sym.SymVerABI0); s2 != 0 && ldr.AttrCgoExportStatic(s2) && ldr.SymType(s2) == sym.STEXT { + sname = sname + ".abiinternal" + } + } + // Second case + if ldr.SymVersion(x) == sym.SymVerABI0 && !ldr.AttrCgoExportStatic(x) { + if s2 := ldr.Lookup(sname, sym.SymVerABIInternal); s2 != 0 && ldr.SymType(s2) == sym.STEXT { + sname = sname + ".abi0" + } + } + } + } + // One pass for each binding: elf.STB_LOCAL, elf.STB_GLOBAL, // maybe one day elf.STB_WEAK. bind := elf.STB_GLOBAL @@ -140,8 +175,6 @@ func putelfsym(ctxt *Link, x loader.Sym, typ elf.SymType, curbind elf.SymBind) { other |= 3 << 5 } - sname := ldr.SymExtname(x) - // When dynamically linking, we create Symbols by reading the names from // the symbol tables of the shared libraries and so the names need to // match exactly. Tools like DTrace will have to wait for now. diff --git a/src/runtime/textflag.h b/src/runtime/textflag.h index daca36d948..e727208cd0 100644 --- a/src/runtime/textflag.h +++ b/src/runtime/textflag.h @@ -35,3 +35,5 @@ // Function is the top of the call stack. Call stack unwinders should stop // at this function. #define TOPFRAME 2048 +// Function is an ABI wrapper. +#define ABIWRAPPER 4096 diff --git a/test/nosplit.go b/test/nosplit.go index faa7b8c2d8..8a3fa9bf35 100644 --- a/test/nosplit.go +++ b/test/nosplit.go @@ -353,7 +353,14 @@ TestCases: log.Fatal(err) } - cmd := exec.Command("go", "build") + // Turn off ABI0 wrapper generation for now. The problem here is + // that in these test cases main.main is an assembly routine, + // thus calls to it will have to go through an ABI wrapper. The + // ABI wrapper will consume some stack space, which throws off + // the numbers. + workaround := "-gcflags=-abiwrap=0" + + cmd := exec.Command("go", "build", workaround) cmd.Dir = dir output, err := cmd.CombinedOutput() if err == nil { From 0aa9b4709acd609c1a3e9cb028e7f4c4da3f0357 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Tue, 22 Dec 2020 12:40:32 -0500 Subject: [PATCH 18/27] cmd/pack: r command create output file if not exist Go 1.15 pack's r command creates the output file if it does not exist. The system "ar" command does this as well. Do the same. For bazelbuild/rules_go#2762. Change-Id: Icd88396b5c714b735c859a29ab29851e4301f4d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/279516 Trust: Cherry Zhang Run-TryBot: Cherry Zhang Reviewed-by: Than McIntosh TryBot-Result: Go Bot --- src/cmd/pack/pack.go | 7 +++++-- src/cmd/pack/pack_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go index 3dffabe5ec..412ea36d60 100644 --- a/src/cmd/pack/pack.go +++ b/src/cmd/pack/pack.go @@ -43,7 +43,7 @@ func main() { ar = openArchive(os.Args[2], os.O_RDONLY, os.Args[3:]) ar.scan(ar.printContents) case 'r': - ar = openArchive(os.Args[2], os.O_RDWR, os.Args[3:]) + ar = openArchive(os.Args[2], os.O_RDWR|os.O_CREATE, os.Args[3:]) ar.addFiles() case 'c': ar = openArchive(os.Args[2], os.O_RDWR|os.O_TRUNC|os.O_CREATE, os.Args[3:]) @@ -124,10 +124,13 @@ func openArchive(name string, mode int, files []string) *Archive { log.Fatal(err) } var a *archive.Archive - if mode&os.O_CREATE != 0 { // the c command + if mode&os.O_TRUNC != 0 { // the c command a, err = archive.New(f) } else { a, err = archive.Parse(f, verbose) + if err != nil && mode&os.O_CREATE != 0 { // the r command + a, err = archive.New(f) + } } if err != nil { log.Fatal(err) diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go index 16a5135800..118376f9df 100644 --- a/src/cmd/pack/pack_test.go +++ b/src/cmd/pack/pack_test.go @@ -303,7 +303,7 @@ func TestIssue21703(t *testing.T) { } // Test the "c" command can "see through" the archive generated by the compiler. -// This is peculiar. (See issue ) +// This is peculiar. (See issue #43271) func TestCreateWithCompilerObj(t *testing.T) { testenv.MustHaveGoBuild(t) @@ -368,6 +368,29 @@ func TestCreateWithCompilerObj(t *testing.T) { } } +// Test the "r" command creates the output file if it does not exist. +func TestRWithNonexistentFile(t *testing.T) { + testenv.MustHaveGoBuild(t) + + dir := tmpDir(t) + defer os.RemoveAll(dir) + src := filepath.Join(dir, "p.go") + prog := "package p; var X = 42\n" + err := os.WriteFile(src, []byte(prog), 0666) + if err != nil { + t.Fatal(err) + } + + run := func(args ...string) string { + return doRun(t, dir, args...) + } + + goBin := testenv.GoToolPath(t) + run(goBin, "build", "cmd/pack") // writes pack binary to dir + run(goBin, "tool", "compile", "-o", "p.o", "p.go") + run("./pack", "r", "p.a", "p.o") // should succeed +} + // doRun runs a program in a directory and returns the output. func doRun(t *testing.T, dir string, args ...string) string { cmd := exec.Command(args[0], args[1:]...) From c06a354bccf60ea32ed74238be409a00aac292c5 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 21 Dec 2020 18:41:16 -0500 Subject: [PATCH 19/27] test: trigger SIGSEGV instead of SIGTRAP in issue11656.go In issue11656.go, it tests that if the runtime can get a reasonable traceback when it faults at a non-function PC. It does it by jumping to an address that contains an illegal or trap instruction. When it traps, the SIGTRAP crashes the runtime. This CL changes it to use an instruction that triggers SIGSEGV. This is due to two reasons: - currently, the handling of bad PC is done by preparePanic, which is only used for a panicking signal (SIGSEGV, SIGBUS, SIGFPE), not a fatal signal (e.g. SIGTRAP). - the test uses defer+recover to get a traceback, which only works for panicking signals, not fatal signals. Ideally, we should handle all kinds of faults (SIGSEGV, SIGBUS, SIGILL, SIGTRAP, etc.) with a nice traceback. I'll leave this for the future. This CL also adds RISCV64 support. Fixes #43283. Change-Id: I5e0fbf8530cc89d16e05c3257d282bc1d4d03405 Reviewed-on: https://go-review.googlesource.com/c/go/+/279423 Trust: Cherry Zhang Reviewed-by: Ian Lance Taylor --- test/fixedbugs/issue11656.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/test/fixedbugs/issue11656.go b/test/fixedbugs/issue11656.go index 5018263364..acd3f4f3e5 100644 --- a/test/fixedbugs/issue11656.go +++ b/test/fixedbugs/issue11656.go @@ -27,13 +27,6 @@ import ( ) func main() { - // This test is currently failing on some architectures. - // See issue #43283. - switch runtime.GOARCH { - case "ppc64", "mips", "mipsle", "mips64", "mips64le": - return - } - debug.SetPanicOnFault(true) defer func() { if err := recover(); err == nil { @@ -61,27 +54,30 @@ func f(n int) { x uintptr } - // We want to force an illegal instruction, to get a crash - // at a PC value != 0. + // We want to force a seg fault, to get a crash at a PC value != 0. // Not all systems make the data section non-executable. ill := make([]byte, 64) switch runtime.GOARCH { case "386", "amd64": - binary.LittleEndian.PutUint16(ill, 0x0b0f) // ud2 + ill = append(ill, 0x89, 0x04, 0x25, 0x00, 0x00, 0x00, 0x00) // MOVL AX, 0 case "arm": - binary.LittleEndian.PutUint32(ill, 0xe7f000f0) // no name, but permanently undefined + binary.LittleEndian.PutUint32(ill, 0xe3a00000) // MOVW $0, R0 + binary.LittleEndian.PutUint32(ill, 0xe5800000) // MOVW R0, (R0) case "arm64": - binary.LittleEndian.PutUint32(ill, 0xd4207d00) // brk #1000 + binary.LittleEndian.PutUint32(ill, 0xf90003ff) // MOVD ZR, (ZR) case "ppc64": - binary.BigEndian.PutUint32(ill, 0x7fe00008) // trap + binary.BigEndian.PutUint32(ill, 0xf8000000) // MOVD R0, (R0) case "ppc64le": - binary.LittleEndian.PutUint32(ill, 0x7fe00008) // trap + binary.LittleEndian.PutUint32(ill, 0xf8000000) // MOVD R0, (R0) case "mips", "mips64": - binary.BigEndian.PutUint32(ill, 0x00000034) // trap + binary.BigEndian.PutUint32(ill, 0xfc000000) // MOVV R0, (R0) case "mipsle", "mips64le": - binary.LittleEndian.PutUint32(ill, 0x00000034) // trap + binary.LittleEndian.PutUint32(ill, 0xfc000000) // MOVV R0, (R0) case "s390x": - binary.BigEndian.PutUint32(ill, 0) // undefined instruction + ill = append(ill, 0xa7, 0x09, 0x00, 0x00) // MOVD $0, R0 + ill = append(ill, 0xe3, 0x00, 0x00, 0x00, 0x00, 0x24) // MOVD R0, (R0) + case "riscv64": + binary.LittleEndian.PutUint32(ill, 0x00003023) // MOV X0, (X0) default: // Just leave it as 0 and hope for the best. } From 7c8f5356abd7aadf32b028ce76a8a76cd5438258 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 21 Dec 2020 01:55:44 -0500 Subject: [PATCH 20/27] [dev.regabi] cmd/compile: separate dowidth better Having a global MaxWidth lets us avoid needing to refer to thearch from split-out packages when all they need is thearch.MAXWIDTH. And make a couple interface changes to let dowidth avoid importing package ir directly. Then it can move into package types. Change-Id: I2c12e8e22252597530e648848320e19bdd490a01 Reviewed-on: https://go-review.googlesource.com/c/go/+/279302 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/abiutils_test.go | 1 + src/cmd/compile/internal/gc/align.go | 29 +++++++++++--------- src/cmd/compile/internal/gc/main.go | 1 + src/cmd/compile/internal/gc/pgen.go | 2 +- src/cmd/compile/internal/gc/reflect.go | 3 +- src/cmd/compile/internal/gc/sinit.go | 2 +- src/cmd/compile/internal/ir/name.go | 18 ++++++++++++ src/cmd/compile/internal/types/type.go | 12 ++++++++ 8 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/cmd/compile/internal/gc/abiutils_test.go b/src/cmd/compile/internal/gc/abiutils_test.go index 16bd787bea..14bd7ff097 100644 --- a/src/cmd/compile/internal/gc/abiutils_test.go +++ b/src/cmd/compile/internal/gc/abiutils_test.go @@ -29,6 +29,7 @@ func TestMain(m *testing.M) { thearch.LinkArch = &x86.Linkamd64 thearch.REGSP = x86.REGSP thearch.MAXWIDTH = 1 << 50 + MaxWidth = thearch.MAXWIDTH base.Ctxt = obj.Linknew(thearch.LinkArch) base.Ctxt.DiagFunc = base.Errorf base.Ctxt.DiagFlush = base.FlushErrors diff --git a/src/cmd/compile/internal/gc/align.go b/src/cmd/compile/internal/gc/align.go index 95a5dbef29..a9cf7fb50a 100644 --- a/src/cmd/compile/internal/gc/align.go +++ b/src/cmd/compile/internal/gc/align.go @@ -7,12 +7,14 @@ package gc import ( "bytes" "cmd/compile/internal/base" - "cmd/compile/internal/ir" "cmd/compile/internal/types" "fmt" "sort" ) +// MaxWidth is the maximum size of a value on the target architecture. +var MaxWidth int64 + // sizeCalculationDisabled indicates whether it is safe // to calculate Types' widths and alignments. See dowidth. var sizeCalculationDisabled bool @@ -84,7 +86,7 @@ func expandiface(t *types.Type) { sort.Sort(methcmp(methods)) - if int64(len(methods)) >= thearch.MAXWIDTH/int64(Widthptr) { + if int64(len(methods)) >= MaxWidth/int64(Widthptr) { base.ErrorfAt(typePos(t), "interface too large") } for i, m := range methods { @@ -118,8 +120,7 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 { o = Rnd(o, int64(f.Type.Align)) } f.Offset = o - if n := ir.AsNode(f.Nname); n != nil { - n := n.Name() + if f.Nname != nil { // addrescapes has similar code to update these offsets. // Usually addrescapes runs after widstruct, // in which case we could drop this, @@ -127,12 +128,7 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 { // NOTE(rsc): This comment may be stale. // It's possible the ordering has changed and this is // now the common case. I'm not sure. - if n.Name().Stackcopy != nil { - n.Name().Stackcopy.SetFrameOffset(o) - n.SetFrameOffset(0) - } else { - n.SetFrameOffset(o) - } + f.Nname.(types.VarObject).RecordFrameOffset(o) } w := f.Type.Width @@ -143,7 +139,7 @@ func widstruct(errtype *types.Type, t *types.Type, o int64, flag int) int64 { lastzero = o } o += w - maxwidth := thearch.MAXWIDTH + maxwidth := MaxWidth // On 32-bit systems, reflect tables impose an additional constraint // that each field start offset must fit in 31 bits. if maxwidth < 1<<32 { @@ -206,7 +202,7 @@ func findTypeLoop(t *types.Type, path *[]*types.Type) bool { } *path = append(*path, t) - if findTypeLoop(t.Obj().(*ir.Name).Ntype.Type(), path) { + if findTypeLoop(t.Obj().(types.TypeObject).TypeDefn(), path) { return true } *path = (*path)[:len(*path)-1] @@ -419,7 +415,7 @@ func dowidth(t *types.Type) { dowidth(t.Elem()) if t.Elem().Width != 0 { - cap := (uint64(thearch.MAXWIDTH) - 1) / uint64(t.Elem().Width) + cap := (uint64(MaxWidth) - 1) / uint64(t.Elem().Width) if uint64(t.NumElem()) > cap { base.ErrorfAt(typePos(t), "type %L larger than address space", t) } @@ -479,6 +475,13 @@ func dowidth(t *types.Type) { resumecheckwidth() } +// CalcStructSize calculates the size of s, +// filling in s.Width and s.Align, +// even if size calculation is otherwise disabled. +func CalcStructSize(s *types.Type) { + s.Width = widstruct(s, s, 0, 1) // sets align +} + // when a type's width should be known, we call checkwidth // to compute it. during a declaration like // diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index de2b3db36a..343ad9d1d9 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -208,6 +208,7 @@ func Main(archInit func(*Arch)) { Widthptr = thearch.LinkArch.PtrSize Widthreg = thearch.LinkArch.RegSize + MaxWidth = thearch.MAXWIDTH Target = new(ir.Package) diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index dae9d79147..8f7aa8e4e7 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -164,7 +164,7 @@ func (s *ssafn) AllocFrame(f *ssa.Func) { dowidth(n.Type()) w := n.Type().Width - if w >= thearch.MAXWIDTH || w < 0 { + if w >= MaxWidth || w < 0 { base.Fatalf("bad width") } if w == 0 && lastHasPtr { diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 615b8bdbf1..8e2c6f62e1 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -331,8 +331,7 @@ func deferstruct(stksize int64) *types.Type { // build struct holding the above fields s := types.NewStruct(types.NoPkg, fields) s.SetNoalg(true) - s.Width = widstruct(s, s, 0, 1) - s.Align = uint8(Widthptr) + CalcStructSize(s) return s } diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go index a845bc5d75..9ef2bd56eb 100644 --- a/src/cmd/compile/internal/gc/sinit.go +++ b/src/cmd/compile/internal/gc/sinit.go @@ -1019,7 +1019,7 @@ func stataddr(n ir.Node) (name *ir.Name, offset int64, ok bool) { } // Check for overflow. - if n.Type().Width != 0 && thearch.MAXWIDTH/n.Type().Width <= int64(l) { + if n.Type().Width != 0 && MaxWidth/n.Type().Width <= int64(l) { break } offset += int64(l) * n.Type().Width diff --git a/src/cmd/compile/internal/ir/name.go b/src/cmd/compile/internal/ir/name.go index b0b33cccfa..64c60b93d8 100644 --- a/src/cmd/compile/internal/ir/name.go +++ b/src/cmd/compile/internal/ir/name.go @@ -147,6 +147,24 @@ func (n *Name) isExpr() {} // Callers must use n.CloneName to make clear they intend to create a separate name. func (n *Name) CloneName() *Name { c := *n; return &c } +// TypeDefn returns the type definition for a named OTYPE. +// That is, given "type T Defn", it returns Defn. +// It is used by package types. +func (n *Name) TypeDefn() *types.Type { + return n.Ntype.Type() +} + +// RecordFrameOffset records the frame offset for the name. +// It is used by package types when laying out function arguments. +func (n *Name) RecordFrameOffset(offset int64) { + if n.Stackcopy != nil { + n.Stackcopy.SetFrameOffset(offset) + n.SetFrameOffset(0) + } else { + n.SetFrameOffset(offset) + } +} + // NewNameAt returns a new ONAME Node associated with symbol s at position pos. // The caller is responsible for setting Curfn. func NewNameAt(pos src.XPos, sym *types.Sym) *Name { diff --git a/src/cmd/compile/internal/types/type.go b/src/cmd/compile/internal/types/type.go index 4d1d30133c..752c268fa2 100644 --- a/src/cmd/compile/internal/types/type.go +++ b/src/cmd/compile/internal/types/type.go @@ -20,6 +20,18 @@ type Object interface { Type() *Type } +// A TypeObject is an Object representing a named type. +type TypeObject interface { + Object + TypeDefn() *Type // for "type T Defn", returns Defn +} + +// A VarObject is an Object representing a function argument, variable, or struct field. +type VarObject interface { + Object + RecordFrameOffset(int64) // save frame offset +} + //go:generate stringer -type EType -trimprefix T // EType describes a kind of type. From 3b12c6dc089f63d0fe2eeda27e65feb51c5e36d4 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 21 Dec 2020 02:22:42 -0500 Subject: [PATCH 21/27] [dev.regabi] cmd/compile: separate typecheck more cleanly Abstract the typecheck API a bit more so that it is easier to move into a new package. Change-Id: Ia0a0146151fa7f6073113e68a2c3f6e42a5d0ad8 Reviewed-on: https://go-review.googlesource.com/c/go/+/279303 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/alg.go | 4 +-- src/cmd/compile/internal/gc/main.go | 4 +++ src/cmd/compile/internal/gc/subr.go | 6 ++-- src/cmd/compile/internal/gc/typecheck.go | 37 ++++++++++++++++++++++-- src/cmd/compile/internal/gc/walk.go | 13 +++------ 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/cmd/compile/internal/gc/alg.go b/src/cmd/compile/internal/gc/alg.go index 036a1e7491..46ae76d58d 100644 --- a/src/cmd/compile/internal/gc/alg.go +++ b/src/cmd/compile/internal/gc/alg.go @@ -816,7 +816,7 @@ func eqstring(s, t ir.Node) (eqlen *ir.BinaryExpr, eqmem *ir.CallExpr) { fn := syslook("memequal") fn = substArgTypes(fn, types.Types[types.TUINT8], types.Types[types.TUINT8]) call := ir.NewCallExpr(base.Pos, ir.OCALL, fn, []ir.Node{sptr, tptr, ir.Copy(slen)}) - call = typecheck(call, ctxExpr|ctxMultiOK).(*ir.CallExpr) + TypecheckCall(call) cmp := ir.NewBinaryExpr(base.Pos, ir.OEQ, slen, tlen) cmp = typecheck(cmp, ctxExpr).(*ir.BinaryExpr) @@ -853,7 +853,7 @@ func eqinterface(s, t ir.Node) (eqtab *ir.BinaryExpr, eqdata *ir.CallExpr) { tdata.SetTypecheck(1) call := ir.NewCallExpr(base.Pos, ir.OCALL, fn, []ir.Node{stab, sdata, tdata}) - call = typecheck(call, ctxExpr|ctxMultiOK).(*ir.CallExpr) + TypecheckCall(call) cmp := ir.NewBinaryExpr(base.Pos, ir.OEQ, stab, ttab) cmp = typecheck(cmp, ctxExpr).(*ir.BinaryExpr) diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 343ad9d1d9..2a5ff3f5fd 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -212,6 +212,10 @@ func Main(archInit func(*Arch)) { Target = new(ir.Package) + NeedFuncSym = makefuncsym + NeedITab = func(t, iface *types.Type) { itabname(t, iface) } + NeedRuntimeType = addsignat // TODO(rsc): typenamesym for lock? + // initialize types package // (we need to do this to break dependencies that otherwise // would lead to import cycles) diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index 48cbd2505e..0f6c7023f2 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -309,7 +309,7 @@ func assignop(src, dst *types.Type) (ir.Op, string) { // us to de-virtualize calls through this // type/interface pair later. See peekitabs in reflect.go if isdirectiface(src) && !dst.IsEmptyInterface() { - itabname(src, dst) + NeedITab(src, dst) } return ir.OCONVIFACE, "" @@ -1011,6 +1011,7 @@ func adddot(n *ir.SelectorExpr) *ir.SelectorExpr { for c := len(path) - 1; c >= 0; c-- { dot := nodSym(ir.ODOT, n.Left(), path[c].field.Sym) dot.SetImplicit(true) + dot.SetType(path[c].field.Type) n.SetLeft(dot) } case ambig: @@ -1240,8 +1241,7 @@ func genwrapper(rcvr *types.Type, method *types.Field, newnam *types.Sym) { if !instrumenting && rcvr.IsPtr() && methodrcvr.IsPtr() && method.Embedded != 0 && !isifacemethod(method.Type) && !(thearch.LinkArch.Name == "ppc64le" && base.Ctxt.Flag_dynlink) { // generate tail call: adjust pointer receiver and jump to embedded method. left := dot.Left() // skip final .M - // TODO(mdempsky): Remove dependency on dotlist. - if !dotlist[0].field.Type.IsPtr() { + if !left.Type().IsPtr() { left = nodAddr(left) } as := ir.Nod(ir.OAS, nthis, convnop(left, rcvr)) diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 2d383ab49e..1aaa93fc3d 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -14,6 +14,37 @@ import ( "strings" ) +var ( + NeedFuncSym = func(*types.Sym) {} + NeedITab = func(t, itype *types.Type) {} + NeedRuntimeType = func(*types.Type) {} +) + +func TypecheckAssignExpr(n ir.Node) ir.Node { return typecheck(n, ctxExpr|ctxAssign) } +func TypecheckExpr(n ir.Node) ir.Node { return typecheck(n, ctxExpr) } +func TypecheckStmt(n ir.Node) ir.Node { return typecheck(n, ctxStmt) } + +func TypecheckExprs(exprs []ir.Node) { typecheckslice(exprs, ctxExpr) } +func TypecheckStmts(stmts []ir.Node) { typecheckslice(stmts, ctxStmt) } + +func TypecheckCall(call *ir.CallExpr) { + t := call.X.Type() + if t == nil { + panic("misuse of Call") + } + ctx := ctxStmt + if t.NumResults() > 0 { + ctx = ctxExpr | ctxMultiOK + } + if typecheck(call, ctx) != call { + panic("bad typecheck") + } +} + +func TypecheckCallee(n ir.Node) ir.Node { + return typecheck(n, ctxExpr|ctxCallee) +} + // To enable tracing support (-t flag), set enableTrace to true. const enableTrace = false @@ -2384,7 +2415,7 @@ func typecheckMethodExpr(n *ir.SelectorExpr) (res ir.Node) { // to make sure to generate wrappers for anonymous // receiver types too. if mt.Sym() == nil { - addsignat(t) + NeedRuntimeType(t) } } @@ -2417,7 +2448,7 @@ func typecheckMethodExpr(n *ir.SelectorExpr) (res ir.Node) { // Issue 25065. Make sure that we emit the symbol for a local method. if base.Ctxt.Flag_dynlink && !inimport && (t.Sym() == nil || t.Sym().Pkg == types.LocalPkg) { - makefuncsym(me.FuncName_.Sym()) + NeedFuncSym(me.FuncName_.Sym()) } return me @@ -3451,7 +3482,7 @@ func typecheckfunc(n *ir.Func) { } if base.Ctxt.Flag_dynlink && !inimport && n.Nname != nil { - makefuncsym(n.Sym()) + NeedFuncSym(n.Sym()) } } diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 7651bbca10..410155b3ea 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -2520,15 +2520,10 @@ func vmkcall(fn ir.Node, t *types.Type, init *ir.Nodes, va []ir.Node) *ir.CallEx base.Fatalf("vmkcall %v needs %v args got %v", fn, n, len(va)) } - call := ir.Nod(ir.OCALL, fn, nil) - call.PtrList().Set(va) - ctx := ctxStmt - if fn.Type().NumResults() > 0 { - ctx = ctxExpr | ctxMultiOK - } - r1 := typecheck(call, ctx) - r1.SetType(t) - return walkexpr(r1, init).(*ir.CallExpr) + call := ir.NewCallExpr(base.Pos, ir.OCALL, fn, va) + TypecheckCall(call) + call.SetType(t) + return walkexpr(call, init).(*ir.CallExpr) } func mkcall(name string, t *types.Type, init *ir.Nodes, args ...ir.Node) *ir.CallExpr { From 572f168ed26bb32e83562cffb336f2df3a651d9c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 21 Dec 2020 02:08:34 -0500 Subject: [PATCH 22/27] [dev.regabi] cmd/compile: separate various from Main Move various code out of Main itself and into helper functions that can be moved into other packages as package gc splits up. Similarly, move order and instrument inside walk to reduce the amount of API surface needed from the eventual package walk. Change-Id: I7849258038c6e39625a0385af9c0edd6a3b654a1 Reviewed-on: https://go-review.googlesource.com/c/go/+/279304 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/abiutils_test.go | 8 +- src/cmd/compile/internal/gc/dcl.go | 6 + src/cmd/compile/internal/gc/go.go | 2 - src/cmd/compile/internal/gc/inl.go | 20 ++ src/cmd/compile/internal/gc/main.go | 203 ++++--------------- src/cmd/compile/internal/gc/pgen.go | 10 +- src/cmd/compile/internal/gc/ssa.go | 19 +- src/cmd/compile/internal/gc/typecheck.go | 114 +++++++++++ src/cmd/compile/internal/gc/walk.go | 8 + 9 files changed, 211 insertions(+), 179 deletions(-) diff --git a/src/cmd/compile/internal/gc/abiutils_test.go b/src/cmd/compile/internal/gc/abiutils_test.go index 14bd7ff097..6ed27d794f 100644 --- a/src/cmd/compile/internal/gc/abiutils_test.go +++ b/src/cmd/compile/internal/gc/abiutils_test.go @@ -36,7 +36,13 @@ func TestMain(m *testing.M) { base.Ctxt.Bso = bufio.NewWriter(os.Stdout) Widthptr = thearch.LinkArch.PtrSize Widthreg = thearch.LinkArch.RegSize - initializeTypesPackage() + types.TypeLinkSym = func(t *types.Type) *obj.LSym { + return typenamesym(t).Linksym() + } + types.TypeLinkSym = func(t *types.Type) *obj.LSym { + return typenamesym(t).Linksym() + } + TypecheckInit() os.Exit(m.Run()) } diff --git a/src/cmd/compile/internal/gc/dcl.go b/src/cmd/compile/internal/gc/dcl.go index 09d2e7d8b7..bcd127b5f1 100644 --- a/src/cmd/compile/internal/gc/dcl.go +++ b/src/cmd/compile/internal/gc/dcl.go @@ -442,6 +442,12 @@ type funcStackEnt struct { dclcontext ir.Class } +func CheckFuncStack() { + if len(funcStack) != 0 { + base.Fatalf("funcStack is non-empty: %v", len(funcStack)) + } +} + // finish the body. // called in auto-declaration context. // returns in extern-declaration context. diff --git a/src/cmd/compile/internal/gc/go.go b/src/cmd/compile/internal/gc/go.go index 1707e6a11b..df91f6f530 100644 --- a/src/cmd/compile/internal/gc/go.go +++ b/src/cmd/compile/internal/gc/go.go @@ -129,8 +129,6 @@ var ( iscmp [ir.OEND]bool ) -var importlist []*ir.Func // imported functions and methods with inlinable bodies - var ( funcsymsmu sync.Mutex // protects funcsyms and associated package lookups (see func funcsym) funcsyms []*types.Sym diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index 5ada83b715..fde4d6910a 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -55,6 +55,26 @@ const ( inlineBigFunctionMaxCost = 20 // Max cost of inlinee when inlining into a "big" function. ) +func InlinePackage() { + // Find functions that can be inlined and clone them before walk expands them. + visitBottomUp(Target.Decls, func(list []*ir.Func, recursive bool) { + numfns := numNonClosures(list) + for _, n := range list { + if !recursive || numfns > 1 { + // We allow inlining if there is no + // recursion, or the recursion cycle is + // across more than one function. + caninl(n) + } else { + if base.Flag.LowerM > 1 { + fmt.Printf("%v: cannot inline %v: recursive\n", ir.Line(n), n.Nname) + } + } + inlcalls(n) + } + }) +} + // Get the function's package. For ordinary functions it's on the ->sym, but for imported methods // the ->sym can be re-used in the local package, so peel it off the receiver's type. func fnpkg(fn *ir.Name) *types.Pkg { diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 2a5ff3f5fd..4aa2a2ca47 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -191,24 +191,15 @@ func Main(archInit func(*Arch)) { IsIntrinsicCall = isIntrinsicCall SSADumpInline = ssaDumpInline - - ssaDump = os.Getenv("GOSSAFUNC") - ssaDir = os.Getenv("GOSSADIR") - if ssaDump != "" { - if strings.HasSuffix(ssaDump, "+") { - ssaDump = ssaDump[:len(ssaDump)-1] - ssaDumpStdout = true - } - spl := strings.Split(ssaDump, ":") - if len(spl) > 1 { - ssaDump = spl[0] - ssaDumpCFG = spl[1] - } - } + initSSAEnv() + initSSATables() Widthptr = thearch.LinkArch.PtrSize Widthreg = thearch.LinkArch.RegSize MaxWidth = thearch.MAXWIDTH + types.TypeLinkSym = func(t *types.Type) *obj.LSym { + return typenamesym(t).Linksym() + } Target = new(ir.Package) @@ -216,152 +207,40 @@ func Main(archInit func(*Arch)) { NeedITab = func(t, iface *types.Type) { itabname(t, iface) } NeedRuntimeType = addsignat // TODO(rsc): typenamesym for lock? - // initialize types package - // (we need to do this to break dependencies that otherwise - // would lead to import cycles) - initializeTypesPackage() - - dclcontext = ir.PEXTERN - autogeneratedPos = makePos(src.NewFileBase("", ""), 1, 0) - timings.Start("fe", "loadsys") - loadsys() + types.TypeLinkSym = func(t *types.Type) *obj.LSym { + return typenamesym(t).Linksym() + } + TypecheckInit() + // Parse input. timings.Start("fe", "parse") lines := parseFiles(flag.Args()) cgoSymABIs() timings.Stop() timings.AddEvent(int64(lines), "lines") - - finishUniverse() - recordPackageName() - typecheckok = true + // Typecheck. + TypecheckPackage() - // Process top-level declarations in phases. - - // Phase 1: const, type, and names and types of funcs. - // This will gather all the information about types - // and methods but doesn't depend on any of it. - // - // We also defer type alias declarations until phase 2 - // to avoid cycles like #18640. - // TODO(gri) Remove this again once we have a fix for #25838. - - // Don't use range--typecheck can add closures to Target.Decls. - timings.Start("fe", "typecheck", "top1") - for i := 0; i < len(Target.Decls); i++ { - n := Target.Decls[i] - if op := n.Op(); op != ir.ODCL && op != ir.OAS && op != ir.OAS2 && (op != ir.ODCLTYPE || !n.(*ir.Decl).Left().Name().Alias()) { - Target.Decls[i] = typecheck(n, ctxStmt) - } - } - - // Phase 2: Variable assignments. - // To check interface assignments, depends on phase 1. - - // Don't use range--typecheck can add closures to Target.Decls. - timings.Start("fe", "typecheck", "top2") - for i := 0; i < len(Target.Decls); i++ { - n := Target.Decls[i] - if op := n.Op(); op == ir.ODCL || op == ir.OAS || op == ir.OAS2 || op == ir.ODCLTYPE && n.(*ir.Decl).Left().Name().Alias() { - Target.Decls[i] = typecheck(n, ctxStmt) - } - } - - // Phase 3: Type check function bodies. - // Don't use range--typecheck can add closures to Target.Decls. - timings.Start("fe", "typecheck", "func") - var fcount int64 - for i := 0; i < len(Target.Decls); i++ { - n := Target.Decls[i] - if n.Op() == ir.ODCLFUNC { - Curfn = n.(*ir.Func) - decldepth = 1 - errorsBefore := base.Errors() - typecheckslice(Curfn.Body().Slice(), ctxStmt) - checkreturn(Curfn) - if base.Errors() > errorsBefore { - Curfn.PtrBody().Set(nil) // type errors; do not compile - } - // Now that we've checked whether n terminates, - // we can eliminate some obviously dead code. - deadcode(Curfn) - fcount++ - } - } - - // Phase 3.11: Check external declarations. - // TODO(mdempsky): This should be handled when type checking their - // corresponding ODCL nodes. - timings.Start("fe", "typecheck", "externdcls") - for i, n := range Target.Externs { - if n.Op() == ir.ONAME { - Target.Externs[i] = typecheck(Target.Externs[i], ctxExpr) - } - } - - // Phase 3.14: With all user code type-checked, it's now safe to verify map keys - // and unused dot imports. - checkMapKeys() + // With all user code typechecked, it's now safe to verify unused dot imports. checkDotImports() base.ExitIfErrors() - timings.AddEvent(fcount, "funcs") - + // Build init task. if initTask := fninit(); initTask != nil { exportsym(initTask) } - // Phase 4: Decide how to capture closed variables. - // This needs to run before escape analysis, - // because variables captured by value do not escape. - timings.Start("fe", "capturevars") - for _, n := range Target.Decls { - if n.Op() == ir.ODCLFUNC && n.Func().OClosure != nil { - Curfn = n.(*ir.Func) - capturevars(Curfn) - } - } - capturevarscomplete = true - Curfn = nil - base.ExitIfErrors() - - // Phase 5: Inlining + // Inlining timings.Start("fe", "inlining") - if base.Debug.TypecheckInl != 0 { - // Typecheck imported function bodies if Debug.l > 1, - // otherwise lazily when used or re-exported. - for _, n := range importlist { - if n.Inl != nil { - typecheckinl(n) - } - } - base.ExitIfErrors() - } - if base.Flag.LowerL != 0 { - // Find functions that can be inlined and clone them before walk expands them. - visitBottomUp(Target.Decls, func(list []*ir.Func, recursive bool) { - numfns := numNonClosures(list) - for _, n := range list { - if !recursive || numfns > 1 { - // We allow inlining if there is no - // recursion, or the recursion cycle is - // across more than one function. - caninl(n) - } else { - if base.Flag.LowerM > 1 { - fmt.Printf("%v: cannot inline %v: recursive\n", ir.Line(n), n.Nname) - } - } - inlcalls(n) - } - }) + InlinePackage() } + // Devirtualize. for _, n := range Target.Decls { if n.Op() == ir.ODCLFUNC { devirtualize(n.(*ir.Func)) @@ -369,7 +248,7 @@ func Main(archInit func(*Arch)) { } Curfn = nil - // Phase 6: Escape analysis. + // Escape analysis. // Required for moving heap allocations onto stack, // which in turn is required by the closure implementation, // which stores the addresses of stack variables into the closure. @@ -388,7 +267,7 @@ func Main(archInit func(*Arch)) { EnableNoWriteBarrierRecCheck() } - // Phase 7: Transform closure bodies to properly reference captured variables. + // Transform closure bodies to properly reference captured variables. // This needs to happen before walk, because closures must be transformed // before walk reaches a call of a closure. timings.Start("fe", "xclosures") @@ -410,10 +289,10 @@ func Main(archInit func(*Arch)) { Curfn = nil peekitabs() - // Phase 8: Compile top level functions. + // Compile top level functions. // Don't use range--walk can add functions to Target.Decls. timings.Start("be", "compilefuncs") - fcount = 0 + fcount := int64(0) for i := 0; i < len(Target.Decls); i++ { n := Target.Decls[i] if n.Op() == ir.ODCLFUNC { @@ -448,21 +327,9 @@ func Main(archInit func(*Arch)) { dumpasmhdr() } - // Check whether any of the functions we have compiled have gigantic stack frames. - sort.Slice(largeStackFrames, func(i, j int) bool { - return largeStackFrames[i].pos.Before(largeStackFrames[j].pos) - }) - for _, large := range largeStackFrames { - if large.callee != 0 { - base.ErrorfAt(large.pos, "stack frame too large (>1GB): %d MB locals + %d MB args + %d MB callee", large.locals>>20, large.args>>20, large.callee>>20) - } else { - base.ErrorfAt(large.pos, "stack frame too large (>1GB): %d MB locals + %d MB args", large.locals>>20, large.args>>20) - } - } + CheckLargeStacks() + CheckFuncStack() - if len(funcStack) != 0 { - base.Fatalf("funcStack is non-empty: %v", len(funcStack)) - } if len(compilequeue) != 0 { base.Fatalf("%d uncompiled functions", len(compilequeue)) } @@ -480,6 +347,20 @@ func Main(archInit func(*Arch)) { } } +func CheckLargeStacks() { + // Check whether any of the functions we have compiled have gigantic stack frames. + sort.Slice(largeStackFrames, func(i, j int) bool { + return largeStackFrames[i].pos.Before(largeStackFrames[j].pos) + }) + for _, large := range largeStackFrames { + if large.callee != 0 { + base.ErrorfAt(large.pos, "stack frame too large (>1GB): %d MB locals + %d MB args + %d MB callee", large.locals>>20, large.args>>20, large.callee>>20) + } else { + base.ErrorfAt(large.pos, "stack frame too large (>1GB): %d MB locals + %d MB args", large.locals>>20, large.args>>20) + } + } +} + func cgoSymABIs() { // The linker expects an ABI0 wrapper for all cgo-exported // functions. @@ -1140,16 +1021,6 @@ func parseLang(s string) (lang, error) { return lang{major: major, minor: minor}, nil } -func initializeTypesPackage() { - types.Widthptr = Widthptr - types.Dowidth = dowidth - types.TypeLinkSym = func(t *types.Type) *obj.LSym { - return typenamesym(t).Linksym() - } - - initUniverse() -} - // useNewABIWrapGen returns TRUE if the compiler should generate an // ABI wrapper for the function 'f'. func useABIWrapGen(f *ir.Func) bool { diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index 8f7aa8e4e7..e43471dbca 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -222,24 +222,16 @@ func funccompile(fn *ir.Func) { } func compile(fn *ir.Func) { - errorsBefore := base.Errors() - order(fn) - if base.Errors() > errorsBefore { - return - } - // Set up the function's LSym early to avoid data races with the assemblers. // Do this before walk, as walk needs the LSym to set attributes/relocations // (e.g. in markTypeUsedInInterface). initLSym(fn, true) + errorsBefore := base.Errors() walk(fn) if base.Errors() > errorsBefore { return } - if instrumenting { - instrument(fn) - } // From this point, there should be no uses of Curfn. Enforce that. Curfn = nil diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index b4cf8b6dc7..1fc1feae67 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "sort" + "strings" "bufio" "bytes" @@ -48,6 +49,22 @@ func ssaDumpInline(fn *ir.Func) { } } +func initSSAEnv() { + ssaDump = os.Getenv("GOSSAFUNC") + ssaDir = os.Getenv("GOSSADIR") + if ssaDump != "" { + if strings.HasSuffix(ssaDump, "+") { + ssaDump = ssaDump[:len(ssaDump)-1] + ssaDumpStdout = true + } + spl := strings.Split(ssaDump, ":") + if len(spl) > 1 { + ssaDump = spl[0] + ssaDumpCFG = spl[1] + } + } +} + func initssaconfig() { types_ := ssa.NewTypes() @@ -3357,7 +3374,7 @@ type intrinsicKey struct { fn string } -func init() { +func initSSATables() { intrinsics = map[intrinsicKey]intrinsicBuilder{} var all []*sys.Arch diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 1aaa93fc3d..cc5df3ebae 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -20,6 +20,96 @@ var ( NeedRuntimeType = func(*types.Type) {} ) +func TypecheckInit() { + types.Widthptr = Widthptr + types.Dowidth = dowidth + initUniverse() + dclcontext = ir.PEXTERN + timings.Start("fe", "loadsys") + loadsys() +} + +func TypecheckPackage() { + finishUniverse() + + typecheckok = true + + // Process top-level declarations in phases. + + // Phase 1: const, type, and names and types of funcs. + // This will gather all the information about types + // and methods but doesn't depend on any of it. + // + // We also defer type alias declarations until phase 2 + // to avoid cycles like #18640. + // TODO(gri) Remove this again once we have a fix for #25838. + + // Don't use range--typecheck can add closures to Target.Decls. + timings.Start("fe", "typecheck", "top1") + for i := 0; i < len(Target.Decls); i++ { + n := Target.Decls[i] + if op := n.Op(); op != ir.ODCL && op != ir.OAS && op != ir.OAS2 && (op != ir.ODCLTYPE || !n.(*ir.Decl).Left().Name().Alias()) { + Target.Decls[i] = typecheck(n, ctxStmt) + } + } + + // Phase 2: Variable assignments. + // To check interface assignments, depends on phase 1. + + // Don't use range--typecheck can add closures to Target.Decls. + timings.Start("fe", "typecheck", "top2") + for i := 0; i < len(Target.Decls); i++ { + n := Target.Decls[i] + if op := n.Op(); op == ir.ODCL || op == ir.OAS || op == ir.OAS2 || op == ir.ODCLTYPE && n.(*ir.Decl).Left().Name().Alias() { + Target.Decls[i] = typecheck(n, ctxStmt) + } + } + + // Phase 3: Type check function bodies. + // Don't use range--typecheck can add closures to Target.Decls. + timings.Start("fe", "typecheck", "func") + var fcount int64 + for i := 0; i < len(Target.Decls); i++ { + n := Target.Decls[i] + if n.Op() == ir.ODCLFUNC { + TypecheckFuncBody(n.(*ir.Func)) + fcount++ + } + } + + // Phase 4: Check external declarations. + // TODO(mdempsky): This should be handled when type checking their + // corresponding ODCL nodes. + timings.Start("fe", "typecheck", "externdcls") + for i, n := range Target.Externs { + if n.Op() == ir.ONAME { + Target.Externs[i] = typecheck(Target.Externs[i], ctxExpr) + } + } + + // Phase 5: With all user code type-checked, it's now safe to verify map keys. + checkMapKeys() + + // Phase 6: Decide how to capture closed variables. + // This needs to run before escape analysis, + // because variables captured by value do not escape. + timings.Start("fe", "capturevars") + for _, n := range Target.Decls { + if n.Op() == ir.ODCLFUNC && n.Func().OClosure != nil { + Curfn = n.(*ir.Func) + capturevars(Curfn) + } + } + capturevarscomplete = true + Curfn = nil + + if base.Debug.TypecheckInl != 0 { + // Typecheck imported function bodies if Debug.l > 1, + // otherwise lazily when used or re-exported. + TypecheckImports() + } +} + func TypecheckAssignExpr(n ir.Node) ir.Node { return typecheck(n, ctxExpr|ctxAssign) } func TypecheckExpr(n ir.Node) ir.Node { return typecheck(n, ctxExpr) } func TypecheckStmt(n ir.Node) ir.Node { return typecheck(n, ctxStmt) } @@ -45,6 +135,30 @@ func TypecheckCallee(n ir.Node) ir.Node { return typecheck(n, ctxExpr|ctxCallee) } +func TypecheckFuncBody(n *ir.Func) { + Curfn = n + decldepth = 1 + errorsBefore := base.Errors() + typecheckslice(n.Body(), ctxStmt) + checkreturn(n) + if base.Errors() > errorsBefore { + n.PtrBody().Set(nil) // type errors; do not compile + } + // Now that we've checked whether n terminates, + // we can eliminate some obviously dead code. + deadcode(n) +} + +var importlist []*ir.Func + +func TypecheckImports() { + for _, n := range importlist { + if n.Inl != nil { + typecheckinl(n) + } + } +} + // To enable tracing support (-t flag), set enableTrace to true. const enableTrace = false diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 410155b3ea..5545dcb345 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -26,6 +26,10 @@ const zeroValSize = 1024 // must match value of runtime/map.go:maxZero func walk(fn *ir.Func) { Curfn = fn errorsBefore := base.Errors() + order(fn) + if base.Errors() > errorsBefore { + return + } if base.Flag.W != 0 { s := fmt.Sprintf("\nbefore walk %v", Curfn.Sym()) @@ -80,6 +84,10 @@ func walk(fn *ir.Func) { s := fmt.Sprintf("enter %v", Curfn.Sym()) ir.DumpList(s, Curfn.Enter) } + + if instrumenting { + instrument(fn) + } } func walkstmtlist(s []ir.Node) { From 51ba53f5c2d58dd0c02b5ee1f4ef1db2577c4d3a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 21 Dec 2020 01:20:20 -0500 Subject: [PATCH 23/27] [dev.regabi] cmd/compile: separate misc for gc split Misc cleanup for splitting package gc: API tweaks and boundary adjustments. The change in ir.NewBlockStmt makes it a drop-in replacement for liststmt. Change-Id: I9455fe8ccae7d71fe8ccf390ac96672389bf4f3d Reviewed-on: https://go-review.googlesource.com/c/go/+/279305 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/escape.go | 4 ---- src/cmd/compile/internal/gc/iimport.go | 15 +++++++++++++++ src/cmd/compile/internal/gc/main.go | 17 ++++++++++------- src/cmd/compile/internal/gc/obj.go | 8 ++++---- src/cmd/compile/internal/gc/reflect.go | 12 ++++++------ src/cmd/compile/internal/gc/timings.go | 2 ++ src/cmd/compile/internal/ir/stmt.go | 7 +++++++ 7 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/cmd/compile/internal/gc/escape.go b/src/cmd/compile/internal/gc/escape.go index 235cef47ea..3351cfe968 100644 --- a/src/cmd/compile/internal/gc/escape.go +++ b/src/cmd/compile/internal/gc/escape.go @@ -143,10 +143,6 @@ type EscEdge struct { notes *EscNote } -func init() { - ir.EscFmt = escFmt -} - // escFmt is called from node printing to print information about escape analysis results. func escFmt(n ir.Node) string { text := "" diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index cd66d39b66..358fdef294 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -685,6 +685,21 @@ func (r *importReader) typeExt(t *types.Type) { // so we can use index to reference the symbol. var typeSymIdx = make(map[*types.Type][2]int64) +func BaseTypeIndex(t *types.Type) int64 { + tbase := t + if t.IsPtr() && t.Sym() == nil && t.Elem().Sym() != nil { + tbase = t.Elem() + } + i, ok := typeSymIdx[tbase] + if !ok { + return -1 + } + if t != tbase { + return i[1] + } + return i[0] +} + func (r *importReader) doInline(fn *ir.Func) { if len(fn.Inl.Body) != 0 { base.Fatalf("%v already has inline body", fn) diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 4aa2a2ca47..80b17ebbf8 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -54,9 +54,6 @@ func hidePanic() { // Target is the package being compiled. var Target *ir.Package -// timing data for compiler phases -var timings Timings - // Main parses flags and Go source files specified in the command-line // arguments, type-checks the parsed Go package, compiles functions to machine // code, and finally writes the compiled package definition to disk. @@ -189,6 +186,7 @@ func Main(archInit func(*Arch)) { logopt.LogJsonOption(base.Flag.JSON) } + ir.EscFmt = escFmt IsIntrinsicCall = isIntrinsicCall SSADumpInline = ssaDumpInline initSSAEnv() @@ -962,9 +960,11 @@ type lang struct { // any language version is supported. var langWant lang -// langSupported reports whether language version major.minor is -// supported in a particular package. -func langSupported(major, minor int, pkg *types.Pkg) bool { +// AllowsGoVersion reports whether a particular package +// is allowed to use Go version major.minor. +// We assume the imported packages have all been checked, +// so we only have to check the local package against the -lang flag. +func AllowsGoVersion(pkg *types.Pkg, major, minor int) bool { if pkg == nil { // TODO(mdempsky): Set Pkg for local types earlier. pkg = types.LocalPkg @@ -973,13 +973,16 @@ func langSupported(major, minor int, pkg *types.Pkg) bool { // Assume imported packages passed type-checking. return true } - if langWant.major == 0 && langWant.minor == 0 { return true } return langWant.major > major || (langWant.major == major && langWant.minor >= minor) } +func langSupported(major, minor int, pkg *types.Pkg) bool { + return AllowsGoVersion(pkg, major, minor) +} + // checkLang verifies that the -lang flag holds a valid value, and // exits if not. It initializes data used by langSupported. func checkLang() { diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index 094c386218..c6625da1da 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -127,8 +127,7 @@ func dumpdata() { addsignats(Target.Externs) dumpsignats() dumptabs() - ptabsLen := len(ptabs) - itabsLen := len(itabs) + numPTabs, numITabs := CountTabs() dumpimportstrings() dumpbasictypes() dumpembeds() @@ -168,10 +167,11 @@ func dumpdata() { if numExports != len(Target.Exports) { base.Fatalf("Target.Exports changed after compile functions loop") } - if ptabsLen != len(ptabs) { + newNumPTabs, newNumITabs := CountTabs() + if newNumPTabs != numPTabs { base.Fatalf("ptabs changed after compile functions loop") } - if itabsLen != len(itabs) { + if newNumITabs != numITabs { base.Fatalf("itabs changed after compile functions loop") } } diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 8e2c6f62e1..92b04f20d5 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -34,6 +34,10 @@ type ptabEntry struct { t *types.Type } +func CountTabs() (numPTabs, numITabs int) { + return len(ptabs), len(itabs) +} + // runtime interface and reflection data structures var ( signatmu sync.Mutex // protects signatset and signatslice @@ -1158,13 +1162,9 @@ func dtypesym(t *types.Type) *obj.LSym { if base.Ctxt.Pkgpath != "runtime" || (tbase != types.Types[tbase.Kind()] && tbase != types.ByteType && tbase != types.RuneType && tbase != types.ErrorType) { // int, float, etc // named types from other files are defined only by those files if tbase.Sym() != nil && tbase.Sym().Pkg != types.LocalPkg { - if i, ok := typeSymIdx[tbase]; ok { + if i := BaseTypeIndex(t); i >= 0 { lsym.Pkg = tbase.Sym().Pkg.Prefix - if t != tbase { - lsym.SymIdx = int32(i[1]) - } else { - lsym.SymIdx = int32(i[0]) - } + lsym.SymIdx = int32(i) lsym.Set(obj.AttrIndexed, true) } return lsym diff --git a/src/cmd/compile/internal/gc/timings.go b/src/cmd/compile/internal/gc/timings.go index 56b3899e2f..ac12d78d1e 100644 --- a/src/cmd/compile/internal/gc/timings.go +++ b/src/cmd/compile/internal/gc/timings.go @@ -11,6 +11,8 @@ import ( "time" ) +var timings Timings + // Timings collects the execution times of labeled phases // which are added trough a sequence of Start/Stop calls. // Events may be associated with each phase via AddEvent. diff --git a/src/cmd/compile/internal/ir/stmt.go b/src/cmd/compile/internal/ir/stmt.go index 12811821ad..e2543a5541 100644 --- a/src/cmd/compile/internal/ir/stmt.go +++ b/src/cmd/compile/internal/ir/stmt.go @@ -5,6 +5,7 @@ package ir import ( + "cmd/compile/internal/base" "cmd/compile/internal/types" "cmd/internal/src" ) @@ -164,6 +165,12 @@ type BlockStmt struct { func NewBlockStmt(pos src.XPos, list []Node) *BlockStmt { n := &BlockStmt{} n.pos = pos + if !pos.IsKnown() { + n.pos = base.Pos + if len(list) > 0 { + n.pos = list[0].Pos() + } + } n.op = OBLOCK n.List_.Set(list) return n From 280e7fd1ee47ad92b0031bbc0fa103ac25552950 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 21 Dec 2020 15:10:26 -0500 Subject: [PATCH 24/27] [dev.regabi] cmd/compile: only access Func method on concrete types Sets up for removing Func from Node interface. That means that once the Name reorg is done, which will let us remove Name, Sym, and Val, Node will be basically a minimal interface. Passes buildall w/ toolstash -cmp. Change-Id: I6e87897572debd7f8e29b4f5167763dc2792b408 Reviewed-on: https://go-review.googlesource.com/c/go/+/279484 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/closure.go | 8 +++---- src/cmd/compile/internal/gc/dcl.go | 1 + src/cmd/compile/internal/gc/escape.go | 5 +++-- src/cmd/compile/internal/gc/iimport.go | 4 ++-- src/cmd/compile/internal/gc/initorder.go | 1 + src/cmd/compile/internal/gc/inl.go | 3 ++- src/cmd/compile/internal/gc/main.go | 9 +++++--- src/cmd/compile/internal/gc/scc.go | 8 +++++-- src/cmd/compile/internal/gc/scope.go | 2 +- src/cmd/compile/internal/gc/sinit.go | 1 + src/cmd/compile/internal/gc/typecheck.go | 10 ++++++--- src/cmd/compile/internal/gc/walk.go | 7 +++--- src/cmd/compile/internal/ir/fmt.go | 1 + src/cmd/compile/internal/ir/func.go | 28 ++++++++++++++++++++---- 14 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go index e07ed4cd24..1f4bf969ad 100644 --- a/src/cmd/compile/internal/gc/closure.go +++ b/src/cmd/compile/internal/gc/closure.go @@ -76,7 +76,7 @@ func (p *noder) funcLit(expr *syntax.FuncLit) ir.Node { // function associated with the closure. // TODO: This creation of the named function should probably really be done in a // separate pass from type-checking. -func typecheckclosure(clo ir.Node, top int) { +func typecheckclosure(clo *ir.ClosureExpr, top int) { fn := clo.Func() // Set current associated iota value, so iota can be used inside // function in ConstSpec, see issue #22344 @@ -327,13 +327,13 @@ func transformclosure(fn *ir.Func) { // hasemptycvars reports whether closure clo has an // empty list of captured vars. -func hasemptycvars(clo ir.Node) bool { +func hasemptycvars(clo *ir.ClosureExpr) bool { return len(clo.Func().ClosureVars) == 0 } // closuredebugruntimecheck applies boilerplate checks for debug flags // and compiling runtime -func closuredebugruntimecheck(clo ir.Node) { +func closuredebugruntimecheck(clo *ir.ClosureExpr) { if base.Debug.Closure > 0 { if clo.Esc() == EscHeap { base.WarnfAt(clo.Pos(), "heap closure, captured vars = %v", clo.Func().ClosureVars) @@ -349,7 +349,7 @@ func closuredebugruntimecheck(clo ir.Node) { // closureType returns the struct type used to hold all the information // needed in the closure for clo (clo must be a OCLOSURE node). // The address of a variable of the returned type can be cast to a func. -func closureType(clo ir.Node) *types.Type { +func closureType(clo *ir.ClosureExpr) *types.Type { // Create closure in the form of a composite literal. // supposing the closure captures an int i and a string s // and has one float64 argument and no results, diff --git a/src/cmd/compile/internal/gc/dcl.go b/src/cmd/compile/internal/gc/dcl.go index bcd127b5f1..558bdbef92 100644 --- a/src/cmd/compile/internal/gc/dcl.go +++ b/src/cmd/compile/internal/gc/dcl.go @@ -892,6 +892,7 @@ func (c *nowritebarrierrecChecker) findExtraCalls(nn ir.Node) { case ir.ONAME: callee = arg.Name().Defn.(*ir.Func) case ir.OCLOSURE: + arg := arg.(*ir.ClosureExpr) callee = arg.Func() default: base.Fatalf("expected ONAME or OCLOSURE node, got %+v", arg) diff --git a/src/cmd/compile/internal/gc/escape.go b/src/cmd/compile/internal/gc/escape.go index 3351cfe968..6510dfc4b3 100644 --- a/src/cmd/compile/internal/gc/escape.go +++ b/src/cmd/compile/internal/gc/escape.go @@ -678,6 +678,7 @@ func (e *Escape) exprSkipInit(k EscHole, n ir.Node) { } case ir.OCLOSURE: + n := n.(*ir.ClosureExpr) k = e.spill(k, n) // Link addresses of captured variables to closure. @@ -879,7 +880,7 @@ func (e *Escape) call(ks []EscHole, call, where ir.Node) { case v.Op() == ir.ONAME && v.(*ir.Name).Class() == ir.PFUNC: fn = v.(*ir.Name) case v.Op() == ir.OCLOSURE: - fn = v.Func().Nname + fn = v.(*ir.ClosureExpr).Func().Nname } case ir.OCALLMETH: fn = methodExprName(call.Left()) @@ -1883,7 +1884,7 @@ func heapAllocReason(n ir.Node) string { return "too large for stack" } - if n.Op() == ir.OCLOSURE && closureType(n).Size() >= maxImplicitStackVarSize { + if n.Op() == ir.OCLOSURE && closureType(n.(*ir.ClosureExpr)).Size() >= maxImplicitStackVarSize { return "too large for stack" } if n.Op() == ir.OCALLPART && partialCallType(n.(*ir.CallPartExpr)).Size() >= maxImplicitStackVarSize { diff --git a/src/cmd/compile/internal/gc/iimport.go b/src/cmd/compile/internal/gc/iimport.go index 358fdef294..5f72cedb66 100644 --- a/src/cmd/compile/internal/gc/iimport.go +++ b/src/cmd/compile/internal/gc/iimport.go @@ -630,7 +630,7 @@ func (r *importReader) varExt(n ir.Node) { r.symIdx(n.Sym()) } -func (r *importReader) funcExt(n ir.Node) { +func (r *importReader) funcExt(n *ir.Name) { r.linkname(n.Sym()) r.symIdx(n.Sym()) @@ -654,7 +654,7 @@ func (r *importReader) methExt(m *types.Field) { if r.bool() { m.SetNointerface(true) } - r.funcExt(ir.AsNode(m.Nname)) + r.funcExt(m.Nname.(*ir.Name)) } func (r *importReader) linkname(s *types.Sym) { diff --git a/src/cmd/compile/internal/gc/initorder.go b/src/cmd/compile/internal/gc/initorder.go index 1b21d92f4b..c9c3361d3c 100644 --- a/src/cmd/compile/internal/gc/initorder.go +++ b/src/cmd/compile/internal/gc/initorder.go @@ -296,6 +296,7 @@ func (d *initDeps) visit(n ir.Node) { } case ir.OCLOSURE: + n := n.(*ir.ClosureExpr) d.inspectList(n.Func().Body()) case ir.ODOTMETH, ir.OCALLPART: diff --git a/src/cmd/compile/internal/gc/inl.go b/src/cmd/compile/internal/gc/inl.go index fde4d6910a..fc020000c7 100644 --- a/src/cmd/compile/internal/gc/inl.go +++ b/src/cmd/compile/internal/gc/inl.go @@ -237,7 +237,7 @@ func caninl(fn *ir.Func) { n.Func().Inl = &ir.Inline{ Cost: inlineMaxBudget - visitor.budget, - Dcl: pruneUnusedAutos(n.Defn.Func().Dcl, &visitor), + Dcl: pruneUnusedAutos(n.Defn.(*ir.Func).Func().Dcl, &visitor), Body: ir.DeepCopyList(src.NoXPos, fn.Body().Slice()), } @@ -677,6 +677,7 @@ func inlCallee(fn ir.Node) *ir.Func { return fn.Func() } case ir.OCLOSURE: + fn := fn.(*ir.ClosureExpr) c := fn.Func() caninl(c) return c diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go index 80b17ebbf8..94b4e0e674 100644 --- a/src/cmd/compile/internal/gc/main.go +++ b/src/cmd/compile/internal/gc/main.go @@ -270,9 +270,12 @@ func Main(archInit func(*Arch)) { // before walk reaches a call of a closure. timings.Start("fe", "xclosures") for _, n := range Target.Decls { - if n.Op() == ir.ODCLFUNC && n.Func().OClosure != nil { - Curfn = n.(*ir.Func) - transformclosure(Curfn) + if n.Op() == ir.ODCLFUNC { + n := n.(*ir.Func) + if n.Func().OClosure != nil { + Curfn = n + transformclosure(n) + } } } diff --git a/src/cmd/compile/internal/gc/scc.go b/src/cmd/compile/internal/gc/scc.go index 6e63d5287a..8fe20a80fd 100644 --- a/src/cmd/compile/internal/gc/scc.go +++ b/src/cmd/compile/internal/gc/scc.go @@ -56,8 +56,11 @@ func visitBottomUp(list []ir.Node, analyze func(list []*ir.Func, recursive bool) v.analyze = analyze v.nodeID = make(map[*ir.Func]uint32) for _, n := range list { - if n.Op() == ir.ODCLFUNC && !n.Func().IsHiddenClosure() { - v.visit(n.(*ir.Func)) + if n.Op() == ir.ODCLFUNC { + n := n.(*ir.Func) + if !n.Func().IsHiddenClosure() { + v.visit(n) + } } } } @@ -109,6 +112,7 @@ func (v *bottomUpVisitor) visit(n *ir.Func) uint32 { } } case ir.OCLOSURE: + n := n.(*ir.ClosureExpr) if m := v.visit(n.Func()); m < min { min = m } diff --git a/src/cmd/compile/internal/gc/scope.go b/src/cmd/compile/internal/gc/scope.go index fe4e1d185a..8dd44b1dd4 100644 --- a/src/cmd/compile/internal/gc/scope.go +++ b/src/cmd/compile/internal/gc/scope.go @@ -28,7 +28,7 @@ func findScope(marks []ir.Mark, pos src.XPos) ir.ScopeID { return marks[i-1].Scope } -func assembleScopes(fnsym *obj.LSym, fn ir.Node, dwarfVars []*dwarf.Var, varScopes []ir.ScopeID) []dwarf.Scope { +func assembleScopes(fnsym *obj.LSym, fn *ir.Func, dwarfVars []*dwarf.Var, varScopes []ir.ScopeID) []dwarf.Scope { // Initialize the DWARF scope tree based on lexical scopes. dwarfScopes := make([]dwarf.Scope, 1+len(fn.Func().Parents)) for i, parent := range fn.Func().Parents { diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go index 9ef2bd56eb..79c7215d4d 100644 --- a/src/cmd/compile/internal/gc/sinit.go +++ b/src/cmd/compile/internal/gc/sinit.go @@ -269,6 +269,7 @@ func (s *InitSchedule) staticassign(l *ir.Name, loff int64, r ir.Node, typ *type break case ir.OCLOSURE: + r := r.(*ir.ClosureExpr) if hasemptycvars(r) { if base.Debug.Closure > 0 { base.WarnfAt(r.Pos(), "closure converted to global") diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index cc5df3ebae..bb658999e5 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -95,9 +95,12 @@ func TypecheckPackage() { // because variables captured by value do not escape. timings.Start("fe", "capturevars") for _, n := range Target.Decls { - if n.Op() == ir.ODCLFUNC && n.Func().OClosure != nil { - Curfn = n.(*ir.Func) - capturevars(Curfn) + if n.Op() == ir.ODCLFUNC { + n := n.(*ir.Func) + if n.Func().OClosure != nil { + Curfn = n + capturevars(n) + } } } capturevarscomplete = true @@ -2078,6 +2081,7 @@ func typecheck1(n ir.Node, top int) (res ir.Node) { return n case ir.OCLOSURE: + n := n.(*ir.ClosureExpr) typecheckclosure(n, top) if n.Type() == nil { return n diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 5545dcb345..87f08f41c3 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -649,11 +649,12 @@ func walkexpr1(n ir.Node, init *ir.Nodes) ir.Node { // transformclosure already did all preparation work. // Prepend captured variables to argument list. - n.PtrList().Prepend(n.Left().Func().ClosureEnter.Slice()...) - n.Left().Func().ClosureEnter.Set(nil) + clo := n.Left().(*ir.ClosureExpr) + n.PtrList().Prepend(clo.Func().ClosureEnter.Slice()...) + clo.Func().ClosureEnter.Set(nil) // Replace OCLOSURE with ONAME/PFUNC. - n.SetLeft(n.Left().Func().Nname) + n.SetLeft(clo.Func().Nname) // Update type of OCALLFUNC node. // Output arguments had not changed, but their offsets could. diff --git a/src/cmd/compile/internal/ir/fmt.go b/src/cmd/compile/internal/ir/fmt.go index 6f15645813..76bb35f971 100644 --- a/src/cmd/compile/internal/ir/fmt.go +++ b/src/cmd/compile/internal/ir/fmt.go @@ -1189,6 +1189,7 @@ func dumpNode(w io.Writer, n Node, depth int) { case ODCLFUNC: // Func has many fields we don't want to print. // Bypass reflection and just print what we want. + n := n.(*Func) fmt.Fprintf(w, "%+v", n.Op()) dumpNodeHeader(w, n) fn := n.Func() diff --git a/src/cmd/compile/internal/ir/func.go b/src/cmd/compile/internal/ir/func.go index 8aa6daed6f..62ac5791d1 100644 --- a/src/cmd/compile/internal/ir/func.go +++ b/src/cmd/compile/internal/ir/func.go @@ -213,10 +213,21 @@ func (f *Func) SetWBPos(pos src.XPos) { // funcname returns the name (without the package) of the function n. func FuncName(n Node) string { - if n == nil || n.Func() == nil || n.Func().Nname == nil { + var f *Func + switch n := n.(type) { + case *Func: + f = n + case *Name: + f = n.Func() + case *CallPartExpr: + f = n.Func() + case *ClosureExpr: + f = n.Func() + } + if f == nil || f.Nname == nil { return "" } - return n.Func().Nname.Sym().Name + return f.Nname.Sym().Name } // pkgFuncName returns the name of the function referenced by n, with package prepended. @@ -231,10 +242,19 @@ func PkgFuncName(n Node) string { if n.Op() == ONAME { s = n.Sym() } else { - if n.Func() == nil || n.Func().Nname == nil { + var f *Func + switch n := n.(type) { + case *CallPartExpr: + f = n.Func() + case *ClosureExpr: + f = n.Func() + case *Func: + f = n + } + if f == nil || f.Nname == nil { return "" } - s = n.Func().Nname.Sym() + s = f.Nname.Sym() } pkg := s.Pkg From c40934b33d4d9f85ef5e891f8d26c3035ccce5bb Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 22 Dec 2020 00:07:40 -0500 Subject: [PATCH 25/27] [dev.regabi] cmd/compile: adjust one case in walkexpr The mid-case n := n.(*ir.AssignExpr) does not lend itself well to pulling the code into a new function, because n will be a function argument and will not be redeclarable. Change-Id: I673f2aa37eea64b083725326ed3fa36447bcc7af Reviewed-on: https://go-review.googlesource.com/c/go/+/279426 Trust: Russ Cox Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/walk.go | 38 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 87f08f41c3..d5d12453a7 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -702,38 +702,38 @@ func walkexpr1(n ir.Node, init *ir.Nodes) ir.Node { } else { n.(*ir.AssignStmt).SetLeft(left) } - n := n.(*ir.AssignStmt) + as := n.(*ir.AssignStmt) - if oaslit(n, init) { - return ir.NodAt(n.Pos(), ir.OBLOCK, nil, nil) + if oaslit(as, init) { + return ir.NodAt(as.Pos(), ir.OBLOCK, nil, nil) } - if n.Right() == nil { + if as.Right() == nil { // TODO(austin): Check all "implicit zeroing" - return n + return as } - if !instrumenting && isZero(n.Right()) { - return n + if !instrumenting && isZero(as.Right()) { + return as } - switch n.Right().Op() { + switch as.Right().Op() { default: - n.SetRight(walkexpr(n.Right(), init)) + as.SetRight(walkexpr(as.Right(), init)) case ir.ORECV: - // x = <-c; n.Left is x, n.Right.Left is c. + // x = <-c; as.Left is x, as.Right.Left is c. // order.stmt made sure x is addressable. - recv := n.Right().(*ir.UnaryExpr) + recv := as.Right().(*ir.UnaryExpr) recv.SetLeft(walkexpr(recv.Left(), init)) - n1 := nodAddr(n.Left()) + n1 := nodAddr(as.Left()) r := recv.Left() // the channel return mkcall1(chanfn("chanrecv1", 2, r.Type()), nil, init, r, n1) case ir.OAPPEND: // x = append(...) - call := n.Right().(*ir.CallExpr) + call := as.Right().(*ir.CallExpr) if call.Type().Elem().NotInHeap() { base.Errorf("%v can't be allocated in Go; it is incomplete (or unallocatable)", call.Type().Elem()) } @@ -745,24 +745,24 @@ func walkexpr1(n ir.Node, init *ir.Nodes) ir.Node { case call.IsDDD(): r = appendslice(call, init) // also works for append(slice, string). default: - r = walkappend(call, init, n) + r = walkappend(call, init, as) } - n.SetRight(r) + as.SetRight(r) if r.Op() == ir.OAPPEND { // Left in place for back end. // Do not add a new write barrier. // Set up address of type for back end. r.(*ir.CallExpr).SetLeft(typename(r.Type().Elem())) - return n + return as } // Otherwise, lowered for race detector. // Treat as ordinary assignment. } - if n.Left() != nil && n.Right() != nil { - return convas(n, init) + if as.Left() != nil && as.Right() != nil { + return convas(as, init) } - return n + return as case ir.OAS2: init.AppendNodes(n.PtrInit()) From c9fb4eb0a22131cc9922fa96afba01d4e21d4fd4 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 11:57:42 +0100 Subject: [PATCH 26/27] cmd/link: handle grouped resource sections The Go PE linker does not support enough generalized PE logic to properly handle .rsrc sections gracefully. Instead a few things are special cased for these. The linker also does not support PE's "grouped sections" features, in which input objects have several named sections that are sorted, merged, and renamed in the output file. In the past, more sophisticated support for resources or for PE features like grouped sections have not been necessary, as Go's own object formats are pretty vanilla, and GNU binutils also produces pretty vanilla objects where all sections are already merged. However, GNU binutils is lagging with arm support, and here LLVM has picked up the slack. In particular, LLVM has its own rc/cvtres combo, which are glued together in mingw LLVM distributions as windres, a command line compatible tool with binutils' windres, which supports arm and arm64. But there's a key difference between binutils' windres and LLVM's windres: the LLVM one uses proper grouped sections. So, this commit adds grouped sections support for resource sections to the linker. We don't attempt to plumb generic support for grouped sections, just as there isn't generic support already for what resources require. Instead we augment the resource handling logic to deal with standard two-section resource objects. We also add a test for this, akin to the current test for more vanilla binutils resource objects, and make sure that the rsrc tests are always performed. Fixes #42866. Fixes #43182. Change-Id: I059450021405cdf2ef1c195ddbab3960764ad711 Reviewed-on: https://go-review.googlesource.com/c/go/+/268337 Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Trust: Alex Brainman Trust: Jason A. Donenfeld --- src/cmd/link/internal/ld/lib.go | 2 +- src/cmd/link/internal/ld/pe.go | 60 ++++++++++-------- src/cmd/link/internal/loadpe/ldpe.go | 49 +++++++------- src/cmd/link/link_test.go | 19 ++++++ .../link/testdata/testPErsrc-complex/main.go | 43 +++++++++++++ .../testdata/testPErsrc-complex/rsrc.syso | Bin 0 -> 352 bytes 6 files changed, 124 insertions(+), 49 deletions(-) create mode 100644 src/cmd/link/testdata/testPErsrc-complex/main.go create mode 100644 src/cmd/link/testdata/testPErsrc-complex/rsrc.syso diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index 833b3eb9db..bf95745d8d 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -1820,7 +1820,7 @@ func ldobj(ctxt *Link, f *bio.Reader, lib *sym.Library, length int64, pn string, Errorf(nil, "%v", err) return } - if rsrc != 0 { + if len(rsrc) != 0 { setpersrc(ctxt, rsrc) } ctxt.Textp = append(ctxt.Textp, textp...) diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go index adbf516d5c..5edaf54dd2 100644 --- a/src/cmd/link/internal/ld/pe.go +++ b/src/cmd/link/internal/ld/pe.go @@ -253,7 +253,7 @@ type Dll struct { } var ( - rsrcsym loader.Sym + rsrcsyms []loader.Sym PESECTHEADR int32 PEFILEHEADR int32 pe64 int @@ -1508,46 +1508,56 @@ func (ctxt *Link) dope() { initdynexport(ctxt) } -func setpersrc(ctxt *Link, sym loader.Sym) { - if rsrcsym != 0 { +func setpersrc(ctxt *Link, syms []loader.Sym) { + if len(rsrcsyms) != 0 { Errorf(nil, "too many .rsrc sections") } - - rsrcsym = sym + rsrcsyms = syms } func addpersrc(ctxt *Link) { - if rsrcsym == 0 { + if len(rsrcsyms) == 0 { return } - data := ctxt.loader.Data(rsrcsym) - size := len(data) - h := pefile.addSection(".rsrc", size, size) + var size int64 + for _, rsrcsym := range rsrcsyms { + size += ctxt.loader.SymSize(rsrcsym) + } + h := pefile.addSection(".rsrc", int(size), int(size)) h.characteristics = IMAGE_SCN_MEM_READ | IMAGE_SCN_CNT_INITIALIZED_DATA h.checkOffset(ctxt.Out.Offset()) - // relocation - relocs := ctxt.loader.Relocs(rsrcsym) - for i := 0; i < relocs.Count(); i++ { - r := relocs.At(i) - p := data[r.Off():] - val := uint32(int64(h.virtualAddress) + r.Add()) - - // 32-bit little-endian - p[0] = byte(val) - - p[1] = byte(val >> 8) - p[2] = byte(val >> 16) - p[3] = byte(val >> 24) + for _, rsrcsym := range rsrcsyms { + // A split resource happens when the actual resource data and its relocations are + // split across multiple sections, denoted by a $01 or $02 at the end of the .rsrc + // section name. + splitResources := strings.Contains(ctxt.loader.SymName(rsrcsym), ".rsrc$") + relocs := ctxt.loader.Relocs(rsrcsym) + data := ctxt.loader.Data(rsrcsym) + for ri := 0; ri < relocs.Count(); ri++ { + r := relocs.At(ri) + p := data[r.Off():] + val := uint32(int64(h.virtualAddress) + r.Add()) + if splitResources { + // If we're a split resource section, and that section has relocation + // symbols, then the data that it points to doesn't actually begin at + // the virtual address listed in this current section, but rather + // begins at the section immediately after this one. So, in order to + // calculate the proper virtual address of the data it's pointing to, + // we have to add the length of this section to the virtual address. + // This works because .rsrc sections are divided into two (but not more) + // of these sections. + val += uint32(len(data)) + } + binary.LittleEndian.PutUint32(p, val) + } + ctxt.Out.Write(data) } - - ctxt.Out.Write(data) h.pad(ctxt.Out, uint32(size)) // update data directory pefile.dataDirectory[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE].VirtualAddress = h.virtualAddress - pefile.dataDirectory[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE].Size = h.virtualSize } diff --git a/src/cmd/link/internal/loadpe/ldpe.go b/src/cmd/link/internal/loadpe/ldpe.go index 1e6f978531..a5c025de8f 100644 --- a/src/cmd/link/internal/loadpe/ldpe.go +++ b/src/cmd/link/internal/loadpe/ldpe.go @@ -157,8 +157,9 @@ func makeUpdater(l *loader.Loader, bld *loader.SymbolBuilder, s loader.Sym) *loa // Load loads the PE file pn from input. // Symbols are written into syms, and a slice of the text symbols is returned. -// If an .rsrc section is found, its symbol is returned as rsrc. -func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (textp []loader.Sym, rsrc loader.Sym, err error) { +// If an .rsrc section or set of .rsrc$xx sections is found, its symbols are +// returned as rsrc. +func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Reader, pkg string, length int64, pn string) (textp []loader.Sym, rsrc []loader.Sym, err error) { lookup := func(name string, version int) (*loader.SymbolBuilder, loader.Sym) { s := l.LookupOrCreateSym(name, version) sb := l.MakeSymbolUpdater(s) @@ -176,7 +177,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read // TODO: replace pe.NewFile with pe.Load (grep for "add Load function" in debug/pe for details) f, err := pe.NewFile(sr) if err != nil { - return nil, 0, err + return nil, nil, err } defer f.Close() @@ -211,21 +212,21 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld.SetType(sym.STEXT) default: - return nil, 0, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name) + return nil, nil, fmt.Errorf("unexpected flags %#06x for PE section %s", sect.Characteristics, sect.Name) } if bld.Type() != sym.SNOPTRBSS { data, err := sect.Data() if err != nil { - return nil, 0, err + return nil, nil, err } sectdata[sect] = data bld.SetData(data) } bld.SetSize(int64(sect.Size)) sectsyms[sect] = s - if sect.Name == ".rsrc" { - rsrc = s + if sect.Name == ".rsrc" || strings.HasPrefix(sect.Name, ".rsrc$") { + rsrc = append(rsrc, s) } } @@ -246,22 +247,23 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read continue } + splitResources := strings.HasPrefix(rsect.Name, ".rsrc$") sb := l.MakeSymbolUpdater(sectsyms[rsect]) for j, r := range rsect.Relocs { if int(r.SymbolTableIndex) >= len(f.COFFSymbols) { - return nil, 0, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols)) + return nil, nil, fmt.Errorf("relocation number %d symbol index idx=%d cannot be large then number of symbols %d", j, r.SymbolTableIndex, len(f.COFFSymbols)) } pesym := &f.COFFSymbols[r.SymbolTableIndex] _, gosym, err := readpesym(l, arch, l.LookupOrCreateSym, f, pesym, sectsyms, localSymVersion) if err != nil { - return nil, 0, err + return nil, nil, err } if gosym == 0 { name, err := pesym.FullName(f.StringTable) if err != nil { name = string(pesym.Name[:]) } - return nil, 0, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type) + return nil, nil, fmt.Errorf("reloc of invalid sym %s idx=%d type=%d", name, r.SymbolTableIndex, pesym.Type) } rSym := gosym @@ -271,11 +273,11 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read var rType objabi.RelocType switch arch.Family { default: - return nil, 0, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family) + return nil, nil, fmt.Errorf("%s: unsupported arch %v", pn, arch.Family) case sys.I386, sys.AMD64: switch r.Type { default: - return nil, 0, fmt.Errorf("%s: %v: unknown relocation type %v", pn, sectsyms[rsect], r.Type) + return nil, nil, fmt.Errorf("%s: %v: unknown relocation type %v", pn, sectsyms[rsect], r.Type) case IMAGE_REL_I386_REL32, IMAGE_REL_AMD64_REL32, IMAGE_REL_AMD64_ADDR32, // R_X86_64_PC32 @@ -302,7 +304,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read case sys.ARM: switch r.Type { default: - return nil, 0, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, sectsyms[rsect], r.Type) + return nil, nil, fmt.Errorf("%s: %v: unknown ARM relocation type %v", pn, sectsyms[rsect], r.Type) case IMAGE_REL_ARM_SECREL: rType = objabi.R_PCREL @@ -323,8 +325,9 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read // ld -r could generate multiple section symbols for the // same section but with different values, we have to take - // that into account - if issect(pesym) { + // that into account, or in the case of split resources, + // the section and its symbols are split into two sections. + if issect(pesym) || splitResources { rAdd += int64(pesym.Value) } @@ -346,7 +349,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read name, err := pesym.FullName(f.StringTable) if err != nil { - return nil, 0, err + return nil, nil, err } if name == "" { continue @@ -384,7 +387,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld, s, err := readpesym(l, arch, l.LookupOrCreateSym, f, pesym, sectsyms, localSymVersion) if err != nil { - return nil, 0, err + return nil, nil, err } if pesym.SectionNumber == 0 { // extern @@ -402,14 +405,14 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read } else if pesym.SectionNumber > 0 && int(pesym.SectionNumber) <= len(f.Sections) { sect = f.Sections[pesym.SectionNumber-1] if _, found := sectsyms[sect]; !found { - return nil, 0, fmt.Errorf("%s: %v: missing sect.sym", pn, s) + return nil, nil, fmt.Errorf("%s: %v: missing sect.sym", pn, s) } } else { - return nil, 0, fmt.Errorf("%s: %v: sectnum < 0!", pn, s) + return nil, nil, fmt.Errorf("%s: %v: sectnum < 0!", pn, s) } if sect == nil { - return nil, 0, nil + return nil, nil, nil } if l.OuterSym(s) != 0 { @@ -418,7 +421,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read } outerName := l.SymName(l.OuterSym(s)) sectName := l.SymName(sectsyms[sect]) - return nil, 0, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName) + return nil, nil, fmt.Errorf("%s: duplicate symbol reference: %s in both %s and %s", pn, l.SymName(s), outerName, sectName) } bld = makeUpdater(l, bld, s) @@ -429,7 +432,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read bld.SetSize(4) if l.SymType(sectsym) == sym.STEXT { if bld.External() && !bld.DuplicateOK() { - return nil, 0, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s)) + return nil, nil, fmt.Errorf("%s: duplicate symbol definition", l.SymName(s)) } bld.SetExternal(true) } @@ -446,7 +449,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, input *bio.Read if l.SymType(s) == sym.STEXT { for ; s != 0; s = l.SubSym(s) { if l.AttrOnList(s) { - return nil, 0, fmt.Errorf("symbol %s listed multiple times", l.SymName(s)) + return nil, nil, fmt.Errorf("symbol %s listed multiple times", l.SymName(s)) } l.SetAttrOnList(s, true) textp = append(textp, s) diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go index 4eb02c9e8a..7eeb7ef568 100644 --- a/src/cmd/link/link_test.go +++ b/src/cmd/link/link_test.go @@ -786,6 +786,25 @@ func TestPErsrc(t *testing.T) { if !bytes.Contains(b, []byte("Hello Gophers!")) { t.Fatalf("binary does not contain expected content") } + + pkgdir = filepath.Join("testdata", "testPErsrc-complex") + exe = filepath.Join(tmpdir, "a.exe") + cmd = exec.Command(testenv.GoToolPath(t), "build", "-o", exe) + cmd.Dir = pkgdir + // cmd.Env = append(os.Environ(), "GOOS=windows", "GOARCH=amd64") // uncomment if debugging in a cross-compiling environment + out, err = cmd.CombinedOutput() + if err != nil { + t.Fatalf("building failed: %v, output:\n%s", err, out) + } + + // Check that the binary contains the rsrc data + b, err = ioutil.ReadFile(exe) + if err != nil { + t.Fatalf("reading output failed: %v", err) + } + if !bytes.Contains(b, []byte("resname RCDATA a.rc")) { + t.Fatalf("binary does not contain expected content") + } } func TestContentAddressableSymbols(t *testing.T) { diff --git a/src/cmd/link/testdata/testPErsrc-complex/main.go b/src/cmd/link/testdata/testPErsrc-complex/main.go new file mode 100644 index 0000000000..affd6eada2 --- /dev/null +++ b/src/cmd/link/testdata/testPErsrc-complex/main.go @@ -0,0 +1,43 @@ +// 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. + +// Test that a PE rsrc section is handled correctly, when the object files +// have been created by llvm-rc or msvc's rc.exe, which means there's the +// @feat.00 symbol as well as split .rsrc$00 and .rsrc$01 section to deal with. +// +// rsrc.syso is created with: +// windres -i a.rc -o rsrc.syso -O coff +// where this windres calls into llvm-rc and llvm-cvtres. The source file, +// a.rc, simply contains a reference to its own bytes: +// +// resname RCDATA a.rc +// +// Object dumping the resultant rsrc.syso, we can see the split sections and +// the @feat.00 SEH symbol: +// +// rsrc.syso: file format coff-x86-64 +// +// architecture: x86_64 +// start address: 0x0000000000000000 +// +// Export Table: +// Sections: +// Idx Name Size VMA Type +// 0 .rsrc$01 00000068 0000000000000000 DATA +// 1 .rsrc$02 00000018 0000000000000000 DATA +// +// SYMBOL TABLE: +// [ 0](sec -1)(fl 0x00)(ty 0)(scl 3) (nx 0) 0x00000011 @feat.00 +// [ 1](sec 1)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .rsrc$01 +// AUX scnlen 0x68 nreloc 1 nlnno 0 checksum 0x0 assoc 0 comdat 0 +// [ 3](sec 2)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .rsrc$02 +// AUX scnlen 0x18 nreloc 0 nlnno 0 checksum 0x0 assoc 0 comdat 0 +// [ 5](sec 2)(fl 0x00)(ty 0)(scl 3) (nx 0) 0x00000000 $R000000 +// RELOCATION RECORDS FOR [.rsrc$01]: +// OFFSET TYPE VALUE +// 0000000000000048 IMAGE_REL_AMD64_ADDR32NB $R000000 + +package main + +func main() {} diff --git a/src/cmd/link/testdata/testPErsrc-complex/rsrc.syso b/src/cmd/link/testdata/testPErsrc-complex/rsrc.syso new file mode 100644 index 0000000000000000000000000000000000000000..eff630b8a23de71fa27da5bf0b49965bf4cea69a GIT binary patch literal 352 zcmYdkV`8vbwl4ky0|Nsa5CZ|DUQuyTvWkHrgv-E?0c4~A@foN9Ban6gBB(kes4#;B zkZ}XXfzl3OrO1Me3|v4iP;CRMC<8+TP|N^L9OxVdP8J3R52#`hBu(rLK@6@8!3=&3 zjtss)7Dx>%5HkaDQEG8sVs5HJkh6 Date: Tue, 22 Dec 2020 15:59:09 -0500 Subject: [PATCH 27/27] [dev.regabi] codereview.cfg: add config for dev.regabi Change-Id: Ida5cae7475bc19388fa46ceca25d983f560fa4e8 Reviewed-on: https://go-review.googlesource.com/c/go/+/279524 Trust: Russ Cox Run-TryBot: Russ Cox Reviewed-by: Ian Lance Taylor --- codereview.cfg | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 codereview.cfg diff --git a/codereview.cfg b/codereview.cfg new file mode 100644 index 0000000000..a23b0a00d1 --- /dev/null +++ b/codereview.cfg @@ -0,0 +1,2 @@ +branch: dev.regabi +parent-branch: master