From 5c802c13e88b700b9acaf390d495a92101214e2b Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 14 May 2020 07:56:17 -0700 Subject: runtime: remove flaky "goroutine 2 bt" from gdb test This part of the test has been flaky despite repeated attempts to fix it, and it is unclear what exactly it is testing. Remove it. Fixes #24616. Change-Id: If7234f99dd3d3e92f15ccb94ee13e75c6da12537 Reviewed-on: https://go-review.googlesource.com/c/go/+/233942 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Than McIntosh --- src/runtime/runtime-gdb_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'src') diff --git a/src/runtime/runtime-gdb_test.go b/src/runtime/runtime-gdb_test.go index 2dfa473514..bb625aa406 100644 --- a/src/runtime/runtime-gdb_test.go +++ b/src/runtime/runtime-gdb_test.go @@ -108,7 +108,6 @@ import "fmt" import "runtime" var gslice []string func main() { - go func() { select{} }() // ensure a second goroutine is running mapvar := make(map[string]string, 13) mapvar["abc"] = "def" mapvar["ghi"] = "jkl" @@ -231,9 +230,6 @@ func testGdbPython(t *testing.T, cgo bool) { "-ex", "echo BEGIN goroutine 1 bt\n", "-ex", "goroutine 1 bt", "-ex", "echo END\n", - "-ex", "echo BEGIN goroutine 2 bt\n", - "-ex", "goroutine 2 bt", - "-ex", "echo END\n", "-ex", "echo BEGIN goroutine all bt\n", "-ex", "goroutine all bt", "-ex", "echo END\n", @@ -310,7 +306,6 @@ func testGdbPython(t *testing.T, cgo bool) { // Check that the backtraces are well formed. checkCleanBacktrace(t, blocks["goroutine 1 bt"]) - checkCleanBacktrace(t, blocks["goroutine 2 bt"]) checkCleanBacktrace(t, blocks["goroutine 1 bt at the end"]) btGoroutine1Re := regexp.MustCompile(`(?m)^#0\s+(0x[0-9a-f]+\s+in\s+)?main\.main.+at`) @@ -318,12 +313,7 @@ func testGdbPython(t *testing.T, cgo bool) { t.Fatalf("goroutine 1 bt failed: %s", bl) } - btGoroutine2Re := regexp.MustCompile(`(?m)^#0\s+(0x[0-9a-f]+\s+in\s+)?runtime.+at`) - if bl := blocks["goroutine 2 bt"]; !btGoroutine2Re.MatchString(bl) { - t.Fatalf("goroutine 2 bt failed: %s", bl) - } - - if bl := blocks["goroutine all bt"]; !btGoroutine1Re.MatchString(bl) || !btGoroutine2Re.MatchString(bl) { + if bl := blocks["goroutine all bt"]; !btGoroutine1Re.MatchString(bl) { t.Fatalf("goroutine all bt failed: %s", bl) } -- cgit v1.3-5-g9baa From f474e9e549b248653875c340b3c6d7ab495decfe Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Tue, 12 May 2020 15:44:47 -0700 Subject: runtime: add a lock partial order edge (assistQueue -> mspanSpecial) From interesting stack trace from GC assist through memory profiling to addspecial(). Fixes #39022 Change-Id: Ia0506b820fe29ae91490b61c4e9c2fffcad9f7d6 Reviewed-on: https://go-review.googlesource.com/c/go/+/233599 Run-TryBot: Dan Scales TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements --- src/runtime/lockrank.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/runtime/lockrank.go b/src/runtime/lockrank.go index 899c4e2e85..d97afac6a2 100644 --- a/src/runtime/lockrank.go +++ b/src/runtime/lockrank.go @@ -214,7 +214,7 @@ var lockPartialOrder [][]lockRank = [][]lockRank{ lockRankNotifyList: {}, lockRankTraceBuf: {lockRankScavenge}, lockRankTraceStrings: {lockRankTraceBuf}, - lockRankMspanSpecial: {lockRankScavenge, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings}, + lockRankMspanSpecial: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings}, lockRankProf: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, lockRankGcBitsArenas: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, lockRankRoot: {}, -- cgit v1.3-5-g9baa From 881d5405402d6e8c54f83eed6216a9ed29778006 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Thu, 14 May 2020 16:11:53 -0400 Subject: cmd/link: fix SLIBFUZZER_EXTRA_COUNTER symbol handling Found this while deleting the old code. This should be data2. Change-Id: I1232fac22ef63bb3a3f25a0558537cc371af3bd9 Reviewed-on: https://go-review.googlesource.com/c/go/+/234098 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Than McIntosh Reviewed-by: Jeremy Faller --- src/cmd/link/internal/ld/data.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go index 13ccb86a03..e29b6e8e3b 100644 --- a/src/cmd/link/internal/ld/data.go +++ b/src/cmd/link/internal/ld/data.go @@ -1730,7 +1730,7 @@ func (state *dodataState) allocateDataSections2(ctxt *Link) { ldr.SetSymSect(ldr.LookupOrCreateSym("runtime.end", 0), sect) // Coverage instrumentation counters for libfuzzer. - if len(state.data[sym.SLIBFUZZER_EXTRA_COUNTER]) > 0 { + if len(state.data2[sym.SLIBFUZZER_EXTRA_COUNTER]) > 0 { state.allocateNamedSectionAndAssignSyms2(&Segdata, "__libfuzzer_extra_counters", sym.SLIBFUZZER_EXTRA_COUNTER, sym.Sxxx, 06) } -- cgit v1.3-5-g9baa From 2b70ffe9307c0992e28513ba25081d767b5937b2 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Thu, 14 May 2020 19:22:59 -0400 Subject: cmd/link: detect trampoline of deferreturn call The runtime needs to find the PC of the deferreturn call in a few places. So for functions that have defer, we record the PC of deferreturn call in its funcdata. For very large binaries, the deferreturn call could be made through a trampoline. The current code of finding deferreturn PC fails in this case. This CL handles the trampoline as well. Fixes #39049. Change-Id: I929be54d6ae436f5294013793217dc2a35f080d4 Reviewed-on: https://go-review.googlesource.com/c/go/+/234105 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Jeremy Faller Reviewed-by: Than McIntosh --- src/cmd/link/internal/arm/asm.go | 6 +++++- src/cmd/link/internal/ld/pcln.go | 2 +- src/cmd/link/internal/loader/loader.go | 14 +++++++++++++- src/cmd/link/internal/ppc64/asm.go | 10 +++++++--- src/cmd/link/link_test.go | 19 ++++++++++++++++--- 5 files changed, 42 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/cmd/link/internal/arm/asm.go b/src/cmd/link/internal/arm/asm.go index a2024bcede..d28af163c7 100644 --- a/src/cmd/link/internal/arm/asm.go +++ b/src/cmd/link/internal/arm/asm.go @@ -383,12 +383,16 @@ func trampoline(ctxt *ld.Link, ldr *loader.Loader, ri int, rs, s loader.Sym) { offset := (signext24(r.Add()&0xffffff) + 2) * 4 var tramp loader.Sym for i := 0; ; i++ { - name := ldr.SymName(rs) + fmt.Sprintf("%+d-tramp%d", offset, i) + oName := ldr.SymName(rs) + name := oName + fmt.Sprintf("%+d-tramp%d", offset, i) tramp = ldr.LookupOrCreateSym(name, int(ldr.SymVersion(rs))) if ldr.SymType(tramp) == sym.SDYNIMPORT { // don't reuse trampoline defined in other module continue } + if oName == "runtime.deferreturn" { + ldr.SetIsDeferReturnTramp(tramp, true) + } if ldr.SymValue(tramp) == 0 { // either the trampoline does not exist -- we need to create one, // or found one the address which is not assigned -- this will be diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go index 00c29c63e0..5cbb7bbacc 100644 --- a/src/cmd/link/internal/ld/pcln.go +++ b/src/cmd/link/internal/ld/pcln.go @@ -161,7 +161,7 @@ func (state *pclnState) computeDeferReturn(target *Target, s loader.Sym) uint32 // set the resumption point to PC_B. lastWasmAddr = uint32(r.Add()) } - if r.Type().IsDirectCall() && r.Sym() == state.deferReturnSym { + if r.Type().IsDirectCall() && (r.Sym() == state.deferReturnSym || state.ldr.IsDeferReturnTramp(r.Sym())) { if target.IsWasm() { deferreturn = lastWasmAddr - 1 } else { diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 2627218ced..8e6451d270 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -236,7 +236,8 @@ type Loader struct { outdata [][]byte // symbol's data in the output buffer extRelocs [][]ExtReloc // symbol's external relocations - itablink map[Sym]struct{} // itablink[j] defined if j is go.itablink.* + itablink map[Sym]struct{} // itablink[j] defined if j is go.itablink.* + deferReturnTramp map[Sym]bool // whether the symbol is a trampoline of a deferreturn call objByPkg map[string]*oReader // map package path to its Go object reader @@ -362,6 +363,7 @@ func NewLoader(flags uint32, elfsetstring elfsetstringFunc, reporter *ErrorRepor attrCgoExportDynamic: make(map[Sym]struct{}), attrCgoExportStatic: make(map[Sym]struct{}), itablink: make(map[Sym]struct{}), + deferReturnTramp: make(map[Sym]bool), extStaticSyms: make(map[nameVer]Sym), builtinSyms: make([]Sym, nbuiltin), flags: flags, @@ -1062,6 +1064,16 @@ func (l *Loader) IsItabLink(i Sym) bool { return false } +// Return whether this is a trampoline of a deferreturn call. +func (l *Loader) IsDeferReturnTramp(i Sym) bool { + return l.deferReturnTramp[i] +} + +// Set that i is a trampoline of a deferreturn call. +func (l *Loader) SetIsDeferReturnTramp(i Sym, v bool) { + l.deferReturnTramp[i] = v +} + // growValues grows the slice used to store symbol values. func (l *Loader) growValues(reqLen int) { curLen := len(l.values) diff --git a/src/cmd/link/internal/ppc64/asm.go b/src/cmd/link/internal/ppc64/asm.go index bd4827ecb5..fc714c9dd1 100644 --- a/src/cmd/link/internal/ppc64/asm.go +++ b/src/cmd/link/internal/ppc64/asm.go @@ -686,16 +686,20 @@ func trampoline(ctxt *ld.Link, ldr *loader.Loader, ri int, rs, s loader.Sym) { // target is at some offset within the function. Calls to duff+8 and duff+256 must appear as // distinct trampolines. - name := ldr.SymName(rs) + oName := ldr.SymName(rs) + name := oName if r.Add() == 0 { - name = name + fmt.Sprintf("-tramp%d", i) + name += fmt.Sprintf("-tramp%d", i) } else { - name = name + fmt.Sprintf("%+x-tramp%d", r.Add(), i) + name += fmt.Sprintf("%+x-tramp%d", r.Add(), i) } // Look up the trampoline in case it already exists tramp = ldr.LookupOrCreateSym(name, int(ldr.SymVersion(rs))) + if oName == "runtime.deferreturn" { + ldr.SetIsDeferReturnTramp(tramp, true) + } if ldr.SymValue(tramp) == 0 { break } diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go index 1c9e177911..5ff9912a11 100644 --- a/src/cmd/link/link_test.go +++ b/src/cmd/link/link_test.go @@ -629,10 +629,23 @@ func TestFuncAlign(t *testing.T) { } } -const helloSrc = ` +const testTrampSrc = ` package main import "fmt" -func main() { fmt.Println("hello") } +func main() { + fmt.Println("hello") + + defer func(){ + if e := recover(); e == nil { + panic("did not panic") + } + }() + f1() +} + +// Test deferreturn trampolines. See issue #39049. +func f1() { defer f2() } +func f2() { panic("XXX") } ` func TestTrampoline(t *testing.T) { @@ -655,7 +668,7 @@ func TestTrampoline(t *testing.T) { defer os.RemoveAll(tmpdir) src := filepath.Join(tmpdir, "hello.go") - err = ioutil.WriteFile(src, []byte(helloSrc), 0666) + err = ioutil.WriteFile(src, []byte(testTrampSrc), 0666) if err != nil { t.Fatal(err) } -- cgit v1.3-5-g9baa From bb59a1360a9e6c2d32a59461da56e9bc3a5703ef Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Mon, 18 May 2020 09:34:17 +0100 Subject: runtime: don't enable notes (=signals) too early in Plan 9 The Plan 9 runtime startup was enabling notes (like Unix signals) before the gsignal stack was allocated. This left a small window of time where an interrupt (eg by the parent killing a subprocess quickly after exec) would cause a null pointer dereference in sigtramp. This would leave the interrupted process suspended in 'broken' state instead of exiting. We've observed this on the builders, where it can make a test time out waiting for the broken process to terminate. Updates #38772 Change-Id: I54584069fd3109595f06c78724c1f6419e028aab Reviewed-on: https://go-review.googlesource.com/c/go/+/234397 Run-TryBot: David du Colombier <0intro@gmail.com> TryBot-Result: Gobot Gobot Reviewed-by: David du Colombier <0intro@gmail.com> --- src/runtime/os_plan9.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/runtime/os_plan9.go b/src/runtime/os_plan9.go index b534cdba5d..2bea1058f2 100644 --- a/src/runtime/os_plan9.go +++ b/src/runtime/os_plan9.go @@ -293,7 +293,6 @@ func osinit() { ncpu = getproccount() physPageSize = getPageSize() getg().m.procid = getpid() - notify(unsafe.Pointer(funcPC(sigtramp))) } //go:nosplit @@ -311,6 +310,9 @@ func goenvs() { } func initsig(preinit bool) { + if !preinit { + notify(unsafe.Pointer(funcPC(sigtramp))) + } } //go:nosplit -- cgit v1.3-5-g9baa From afd477f2baca9c66ab7fda5fe565174db30933a7 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Thu, 14 May 2020 14:52:17 -0400 Subject: cmd: update golang.org/x/mod to v0.3.0 (same commit) v0.3.0 is a tag on 859b3ef565e2, the version that was already being used. This change is a no-op, except for letting us use a release version instead of a pseudo-version. For #36905 Change-Id: I70b8ce2a3f1451f5602c469501362d7a6a673b12 Reviewed-on: https://go-review.googlesource.com/c/go/+/234002 Run-TryBot: Jay Conrod TryBot-Result: Gobot Gobot Reviewed-by: Michael Matloob --- src/cmd/go.mod | 2 +- src/cmd/go.sum | 4 ++-- src/cmd/vendor/modules.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/cmd/go.mod b/src/cmd/go.mod index 9c78cd14e6..d56dde8a2a 100644 --- a/src/cmd/go.mod +++ b/src/cmd/go.mod @@ -7,7 +7,7 @@ require ( github.com/ianlancetaylor/demangle v0.0.0-20200414190113-039b1ae3a340 // indirect golang.org/x/arch v0.0.0-20200511175325-f7c78586839d golang.org/x/crypto v0.0.0-20200429183012-4b2356b1ed79 - golang.org/x/mod v0.2.1-0.20200429172858-859b3ef565e2 + golang.org/x/mod v0.3.0 golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 // indirect golang.org/x/tools v0.0.0-20200504152539-33427f1b0364 ) diff --git a/src/cmd/go.sum b/src/cmd/go.sum index f1b3754aad..922df777be 100644 --- a/src/cmd/go.sum +++ b/src/cmd/go.sum @@ -14,8 +14,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200429183012-4b2356b1ed79 h1:IaQbIIB2X/Mp/DKctl6ROxz1KyMlKp4uyvL6+kQ7C88= golang.org/x/crypto v0.0.0-20200429183012-4b2356b1ed79/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.2.1-0.20200429172858-859b3ef565e2 h1:VUsRDZIYpMs3R7PyYeN7BSbDfYjhxaX6HlWvM5iAEqs= -golang.org/x/mod v0.2.1-0.20200429172858-859b3ef565e2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= +golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/src/cmd/vendor/modules.txt b/src/cmd/vendor/modules.txt index 0a3ea66ffd..8a7976a4bf 100644 --- a/src/cmd/vendor/modules.txt +++ b/src/cmd/vendor/modules.txt @@ -29,7 +29,7 @@ golang.org/x/arch/x86/x86asm golang.org/x/crypto/ed25519 golang.org/x/crypto/ed25519/internal/edwards25519 golang.org/x/crypto/ssh/terminal -# golang.org/x/mod v0.2.1-0.20200429172858-859b3ef565e2 +# golang.org/x/mod v0.3.0 ## explicit golang.org/x/mod/internal/lazyregexp golang.org/x/mod/modfile -- cgit v1.3-5-g9baa From 3b0882e8387d63aeffab3929e2590655c61c1a8e Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Mon, 18 May 2020 15:50:03 -0400 Subject: crypto/tls: persist the createdAt time when re-wrapping session tickets Change-Id: I33fcde2d544943fb04c2599810cf7fb773aeba1f Reviewed-on: https://go-review.googlesource.com/c/go/+/234483 Run-TryBot: Katie Hockman Reviewed-by: Filippo Valsorda TryBot-Result: Gobot Gobot --- src/crypto/tls/handshake_client_test.go | 23 ++++++++++++++++++++++- src/crypto/tls/handshake_server.go | 21 +++++++++++++-------- 2 files changed, 35 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go index 313872ca76..de93e1b63f 100644 --- a/src/crypto/tls/handshake_client_test.go +++ b/src/crypto/tls/handshake_client_test.go @@ -980,7 +980,28 @@ func testResumption(t *testing.T, version uint16) { if bytes.Equal(ticket, getTicket()) { t.Fatal("new ticket wasn't provided after old ticket expired") } - testResumeState("FreshSessionTicket", true) + + // Age the session ticket a bit at a time, but don't expire it. + d := 0 * time.Hour + for i := 0; i < 13; i++ { + d += 12 * time.Hour + serverConfig.Time = func() time.Time { return time.Now().Add(d) } + testResumeState("OldSessionTicket", true) + } + // Expire it (now a little more than 7 days) and make sure a full + // handshake occurs for TLS 1.2. Resumption should still occur for + // TLS 1.3 since the client should be using a fresh ticket sent over + // by the server. + d += 12 * time.Hour + serverConfig.Time = func() time.Time { return time.Now().Add(d) } + if version == VersionTLS13 { + testResumeState("ExpiredSessionTicket", true) + } else { + testResumeState("ExpiredSessionTicket", false) + } + if bytes.Equal(ticket, getTicket()) { + t.Fatal("new ticket wasn't provided after old ticket expired") + } // Reset serverConfig to ensure that calling SetSessionTicketKeys // before the serverConfig is used works. diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 6aacfa1ff6..57fba108a7 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -75,13 +75,8 @@ func (hs *serverHandshakeState) handshake() error { if err := hs.establishKeys(); err != nil { return err } - // ticketSupported is set in a resumption handshake if the - // ticket from the client was encrypted with an old session - // ticket key and thus a refreshed ticket should be sent. - if hs.hello.ticketSupported { - if err := hs.sendSessionTicket(); err != nil { - return err - } + if err := hs.sendSessionTicket(); err != nil { + return err } if err := hs.sendFinished(c.serverFinished[:]); err != nil { return err @@ -688,6 +683,9 @@ func (hs *serverHandshakeState) readFinished(out []byte) error { } func (hs *serverHandshakeState) sendSessionTicket() error { + // ticketSupported is set in a resumption handshake if the + // ticket from the client was encrypted with an old session + // ticket key and thus a refreshed ticket should be sent. if !hs.hello.ticketSupported { return nil } @@ -695,6 +693,13 @@ func (hs *serverHandshakeState) sendSessionTicket() error { c := hs.c m := new(newSessionTicketMsg) + createdAt := uint64(c.config.time().Unix()) + if hs.sessionState != nil { + // If this is re-wrapping an old key, then keep + // the original time it was created. + createdAt = hs.sessionState.createdAt + } + var certsFromClient [][]byte for _, cert := range c.peerCertificates { certsFromClient = append(certsFromClient, cert.Raw) @@ -702,7 +707,7 @@ func (hs *serverHandshakeState) sendSessionTicket() error { state := sessionState{ vers: c.vers, cipherSuite: hs.suite.id, - createdAt: uint64(c.config.time().Unix()), + createdAt: createdAt, masterSecret: hs.masterSecret, certificates: certsFromClient, } -- cgit v1.3-5-g9baa From 185c3d46109fe067abf1d4ad4d4812006a55dbe2 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Mon, 18 May 2020 16:49:04 -0400 Subject: crypto/tls: remove version check when unmarshaling sessionState This was causing issues when fuzzing with TestMarshalUnmarshal since the test would occassionally set the version to VersionTLS13, which would fail when unmarshaling. The check doesn't add much in practice, and there is no harm in removing it to de-flake the test. Fixes #38902 Change-Id: I0906c570e9ed69c85fdd2c15f1b52f9e372c62e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/234486 Run-TryBot: Katie Hockman TryBot-Result: Gobot Gobot Reviewed-by: Filippo Valsorda --- src/crypto/tls/ticket.go | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/crypto/tls/ticket.go b/src/crypto/tls/ticket.go index 38b01fc25c..6c1d20da20 100644 --- a/src/crypto/tls/ticket.go +++ b/src/crypto/tls/ticket.go @@ -54,7 +54,6 @@ func (m *sessionState) unmarshal(data []byte) bool { *m = sessionState{usedOldKey: m.usedOldKey} s := cryptobyte.String(data) if ok := s.ReadUint16(&m.vers) && - m.vers != VersionTLS13 && s.ReadUint16(&m.cipherSuite) && readUint64(&s, &m.createdAt) && readUint16LengthPrefixed(&s, &m.masterSecret) && -- cgit v1.3-5-g9baa From 6d6e4827c0b8ce302f7815ab565617f4593c5b46 Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Mon, 4 May 2020 17:37:10 +0100 Subject: testing: return unique directory inside same base root for TempDir We use a single parent directory for all temporary directories created by a test so they're all kept together. Fixes #38850 Change-Id: If8edae10c5136efcbcf6fd632487d198b9e3a868 Reviewed-on: https://go-review.googlesource.com/c/go/+/231958 Reviewed-by: Russ Cox --- src/testing/testing.go | 10 +++++++++- src/testing/testing_test.go | 8 ++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/testing/testing.go b/src/testing/testing.go index 216e46ee81..aa1584f2d9 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -372,6 +372,7 @@ type common struct { tempDirOnce sync.Once tempDir string tempDirErr error + tempDirSeq int32 } // Short reports whether the -test.short flag is set. @@ -827,6 +828,8 @@ var tempDirReplacer struct { // The directory is automatically removed by Cleanup when the test and // all its subtests complete. func (c *common) TempDir() string { + // Use a single parent directory for all the temporary directories + // created by a test, each numbered sequentially. c.tempDirOnce.Do(func() { c.Helper() @@ -849,7 +852,12 @@ func (c *common) TempDir() string { if c.tempDirErr != nil { c.Fatalf("TempDir: %v", c.tempDirErr) } - return c.tempDir + seq := atomic.AddInt32(&c.tempDirSeq, 1) + dir := fmt.Sprintf("%s%c%03d", c.tempDir, os.PathSeparator, seq) + if err := os.Mkdir(dir, 0777); err != nil { + c.Fatalf("TempDir: %v", err) + } + return dir } // panicHanding is an argument to runCleanup. diff --git a/src/testing/testing_test.go b/src/testing/testing_test.go index 1340dae5c4..dbef7066e0 100644 --- a/src/testing/testing_test.go +++ b/src/testing/testing_test.go @@ -7,6 +7,7 @@ package testing_test import ( "io/ioutil" "os" + "path/filepath" "testing" ) @@ -55,8 +56,11 @@ func testTempDir(t *testing.T) { t.Fatal("expected dir") } dir2 := t.TempDir() - if dir != dir2 { - t.Fatal("directory changed between calls") + if dir == dir2 { + t.Fatal("subsequent calls to TempDir returned the same directory") + } + if filepath.Dir(dir) != filepath.Dir(dir2) { + t.Fatalf("calls to TempDir do not share a parent; got %q, %q", dir, dir2) } dirCh <- dir fi, err := os.Stat(dir) -- cgit v1.3-5-g9baa From 2dbbc867dbf44f72422d1827d18a2055f9b7b72f Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 19 May 2020 14:17:05 -0400 Subject: crypto/x509: save the temp dir in TestReadUniqueDirectoryEntries In CL 231958, TempDir was changed to create a new temp directory on each allocation, on the theory that it is easy to save in a variable for callers that want the same directory repeatedly. Apply that transformation here. Updates #38850 Change-Id: Ibb014095426c33038e0a2c95303579cf95d5c3ba Reviewed-on: https://go-review.googlesource.com/c/go/+/234582 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/crypto/x509/root_unix_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go index 39556ae60d..5a8015429c 100644 --- a/src/crypto/x509/root_unix_test.go +++ b/src/crypto/x509/root_unix_test.go @@ -204,7 +204,8 @@ func TestLoadSystemCertsLoadColonSeparatedDirs(t *testing.T) { } func TestReadUniqueDirectoryEntries(t *testing.T) { - temp := func(base string) string { return filepath.Join(t.TempDir(), base) } + tmp := t.TempDir() + temp := func(base string) string { return filepath.Join(tmp, base) } if f, err := os.Create(temp("file")); err != nil { t.Fatal(err) } else { @@ -216,7 +217,7 @@ func TestReadUniqueDirectoryEntries(t *testing.T) { if err := os.Symlink("../target-out", temp("link-out")); err != nil { t.Fatal(err) } - got, err := readUniqueDirectoryEntries(t.TempDir()) + got, err := readUniqueDirectoryEntries(tmp) if err != nil { t.Fatal(err) } -- cgit v1.3-5-g9baa From 13617380cab47a0cfba74650f1539fb2e72bb0fa Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 19 May 2020 15:04:48 -0400 Subject: testing: clean up remaining TempDir issues from CL 231958 Updates #38850 Change-Id: I33f48762f5520eb0c0a841d8ca1ccdd65ecc20c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/234583 Run-TryBot: Bryan C. Mills Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/math/big/link_test.go | 7 ++++--- src/os/readfrom_linux_test.go | 5 +++-- src/testing/testing.go | 5 ++--- 3 files changed, 9 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/math/big/link_test.go b/src/math/big/link_test.go index ad4359cee0..2212bd444f 100644 --- a/src/math/big/link_test.go +++ b/src/math/big/link_test.go @@ -20,8 +20,9 @@ func TestLinkerGC(t *testing.T) { t.Skip("skipping in short mode") } t.Parallel() + tmp := t.TempDir() goBin := testenv.GoToolPath(t) - goFile := filepath.Join(t.TempDir(), "x.go") + goFile := filepath.Join(tmp, "x.go") file := []byte(`package main import _ "math/big" func main() {} @@ -30,13 +31,13 @@ func main() {} t.Fatal(err) } cmd := exec.Command(goBin, "build", "-o", "x.exe", "x.go") - cmd.Dir = t.TempDir() + cmd.Dir = tmp if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("compile: %v, %s", err, out) } cmd = exec.Command(goBin, "tool", "nm", "x.exe") - cmd.Dir = t.TempDir() + cmd.Dir = tmp nm, err := cmd.CombinedOutput() if err != nil { t.Fatalf("nm: %v, %s", err, nm) diff --git a/src/os/readfrom_linux_test.go b/src/os/readfrom_linux_test.go index cecaed5214..b6f5cb7034 100644 --- a/src/os/readfrom_linux_test.go +++ b/src/os/readfrom_linux_test.go @@ -249,14 +249,15 @@ func newCopyFileRangeTest(t *testing.T, size int64) (dst, src *File, data []byte t.Helper() hook = hookCopyFileRange(t) + tmp := t.TempDir() - src, err := Create(filepath.Join(t.TempDir(), "src")) + src, err := Create(filepath.Join(tmp, "src")) if err != nil { t.Fatal(err) } t.Cleanup(func() { src.Close() }) - dst, err = Create(filepath.Join(t.TempDir(), "dst")) + dst, err = Create(filepath.Join(tmp, "dst")) if err != nil { t.Fatal(err) } diff --git a/src/testing/testing.go b/src/testing/testing.go index aa1584f2d9..608bb39671 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -822,11 +822,10 @@ var tempDirReplacer struct { } // TempDir returns a temporary directory for the test to use. -// It is lazily created on first access, and calls t.Fatal if the directory -// creation fails. -// Subsequent calls to t.TempDir return the same directory. // The directory is automatically removed by Cleanup when the test and // all its subtests complete. +// Each subsequent call to t.TempDir returns a unique directory; +// if the directory creation fails, TempDir terminates the test by calling Fatal. func (c *common) TempDir() string { // Use a single parent directory for all the temporary directories // created by a test, each numbered sequentially. -- cgit v1.3-5-g9baa From 4ec4a792f6e60c9c4f40a928650c66419ee0e446 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Wed, 20 May 2020 14:39:13 +1200 Subject: cmd/go: accept smart quotes when checking for missing gold in TestNoteReading Fixes #39157 Change-Id: Ia983f5e66698519cd19c1565cfb80e86d08fdfc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/234380 Reviewed-by: Ian Lance Taylor --- src/cmd/go/note_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/cmd/go/note_test.go b/src/cmd/go/note_test.go index 089e2f3376..659366dcf9 100644 --- a/src/cmd/go/note_test.go +++ b/src/cmd/go/note_test.go @@ -53,7 +53,7 @@ func TestNoteReading(t *testing.T) { // we've had trouble reading the notes generated by gold. err := tg.doRun([]string{"build", "-ldflags", "-buildid=" + buildID + " -linkmode=external -extldflags=-fuse-ld=gold", "-o", tg.path("hello3.exe"), tg.path("hello.go")}) if err != nil { - if tg.grepCountBoth("(invalid linker|gold|cannot find 'ld')") > 0 { + if tg.grepCountBoth("(invalid linker|gold|cannot find [‘']ld[’'])") > 0 { // It's not an error if gold isn't there. gcc claims it "cannot find 'ld'" if // ld.gold is missing, see issue #22340. t.Log("skipping gold test") -- cgit v1.3-5-g9baa From dfd613e0e4fd93ef945e9fbd6d42b79bcaf73905 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 18 May 2020 16:06:11 +0000 Subject: runtime: don't use (addrRange).subtract in removeGreaterEqual Currently in (*addrRanges).removeGreaterEqual we use (addrRange).subtract with a range from specified address to "infinity" which is supposed to be maxOffAddr. However, maxOffAddr is necessarily an inclusive bound on the address space, because on many platforms an exclusive bound would overflow back to 0. On some platforms like mips and mipsle, the address space is smaller than what's representable in a pointer, so if there's a range which hits the top of the address space (such as in the pageAlloc tests), the limit doesn't overflow, but maxOffAddr is inclusive, so any attempt to prune this range with (*addrRange).removeGreaterEqual causes a failure, since the range passed to subtract is contained within the address range which touches the top of the address space. Another problem with using subtract here is that addr and maxOffAddr.addr() may not be in the same segment which could cause makeAddrRange to panic. While this unlikely to happen, on some platforms such as Solaris it is possible. Fix these issues by not using subtract at all. Create a specific implementation of (addrRange).removeGreaterEqual which side-steps all of this by not having to worry about the top of the address space at all. Fixes #39128. Change-Id: Icd5b587b1a3d32a5681fb76cec4c001401f5756f Reviewed-on: https://go-review.googlesource.com/c/go/+/234457 Reviewed-by: Michael Pratt Reviewed-by: Austin Clements Reviewed-by: Cherry Zhang --- src/runtime/mranges.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/runtime/mranges.go b/src/runtime/mranges.go index c2b8e7161c..e23d0778eb 100644 --- a/src/runtime/mranges.go +++ b/src/runtime/mranges.go @@ -69,6 +69,18 @@ func (a addrRange) subtract(b addrRange) addrRange { return a } +// removeGreaterEqual removes all addresses in a greater than or equal +// to addr and returns the new range. +func (a addrRange) removeGreaterEqual(addr uintptr) addrRange { + if (offAddr{addr}).lessEqual(a.base) { + return addrRange{} + } + if a.limit.lessEqual(offAddr{addr}) { + return a + } + return makeAddrRange(a.base.addr(), addr) +} + var ( // minOffAddr is the minimum address in the offset space, and // it corresponds to the virtual address arenaBaseOffset. @@ -281,7 +293,7 @@ func (a *addrRanges) removeGreaterEqual(addr uintptr) { } if r := a.ranges[pivot-1]; r.contains(addr) { removed += r.size() - r = r.subtract(makeAddrRange(addr, maxOffAddr.addr())) + r = r.removeGreaterEqual(addr) if r.size() == 0 { pivot-- } else { -- cgit v1.3-5-g9baa From f7f9c8f2fb61fde1a109e277f27a30b610e67ed0 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 19 May 2020 01:29:11 -0400 Subject: runtime: allocate fewer bytes during TestEINTR This will hopefully address the occasional "runtime: out of memory" failures observed on the openbsd-arm-jsing builder: https://build.golang.org/log/c296d866e5d99ba401b18c1a2ff3e4d480e5238c Also make the "spin" and "winch" loops concurrent instead of sequential to cut down the test's running time. Finally, change Block to coordinate by closing stdin instead of sending SIGINT. The SIGINT handler wasn't necessarily registered by the time the signal was sent. Updates #20400 Updates #39043 Change-Id: Ie12fc75b87e33847dc25a12edb4126db27492da6 Reviewed-on: https://go-review.googlesource.com/c/go/+/234538 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/runtime/testdata/testprogcgo/eintr.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/runtime/testdata/testprogcgo/eintr.go b/src/runtime/testdata/testprogcgo/eintr.go index 58f0dd2ca3..9d9435d9a6 100644 --- a/src/runtime/testdata/testprogcgo/eintr.go +++ b/src/runtime/testdata/testprogcgo/eintr.go @@ -32,11 +32,11 @@ import ( "errors" "fmt" "io" + "io/ioutil" "log" "net" "os" "os/exec" - "os/signal" "sync" "syscall" "time" @@ -71,14 +71,15 @@ func EINTR() { // spin does CPU bound spinning and allocating for a millisecond, // to get a SIGURG. //go:noinline -func spin() (float64, [][]byte) { +func spin() (float64, []byte) { stop := time.Now().Add(time.Millisecond) r1 := 0.0 - var r2 [][]byte + r2 := make([]byte, 200) for time.Now().Before(stop) { for i := 1; i < 1e6; i++ { r1 += r1 / float64(i) - r2 = append(r2, bytes.Repeat([]byte{byte(i)}, 100)) + r2 = append(r2, bytes.Repeat([]byte{byte(i)}, 100)...) + r2 = r2[100:] } } return r1, r2 @@ -96,8 +97,13 @@ func winch() { // sendSomeSignals triggers a few SIGURG and SIGWINCH signals. func sendSomeSignals() { - spin() + done := make(chan struct{}) + go func() { + spin() + close(done) + }() winch() + <-done } // testPipe tests pipe operations. @@ -212,6 +218,10 @@ func testExec(wg *sync.WaitGroup) { go func() { defer wg.Done() cmd := exec.Command(os.Args[0], "Block") + stdin, err := cmd.StdinPipe() + if err != nil { + log.Fatal(err) + } cmd.Stderr = new(bytes.Buffer) cmd.Stdout = cmd.Stderr if err := cmd.Start(); err != nil { @@ -220,9 +230,7 @@ func testExec(wg *sync.WaitGroup) { go func() { sendSomeSignals() - if err := cmd.Process.Signal(os.Interrupt); err != nil { - panic(err) - } + stdin.Close() }() if err := cmd.Wait(); err != nil { @@ -231,10 +239,7 @@ func testExec(wg *sync.WaitGroup) { }() } -// Block blocks until the process receives os.Interrupt. +// Block blocks until stdin is closed. func Block() { - c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt) - defer signal.Stop(c) - <-c + io.Copy(ioutil.Discard, os.Stdin) } -- cgit v1.3-5-g9baa From daf70d6c1688a1ba1699c933b3c3f04d6f2f73d9 Mon Sep 17 00:00:00 2001 From: David Chase Date: Fri, 1 May 2020 17:30:33 -0400 Subject: cmd/go: remove GOAMD64 environment variable This removes the GOAMD64 environment variable and its documentation. The value is instead supplied by a compiled-in constant. Note that function alignment is also dependent on the value of the (removed) flag; it is 32 for aligned jumps, 16 if not. When the flag-dependent logic is removed, it will be 32. Updates #35881. Change-Id: Ic41c0b9833d2e8a31fa3ce8067d92aa2f165bf72 Reviewed-on: https://go-review.googlesource.com/c/go/+/231600 Reviewed-by: Austin Clements --- src/cmd/dist/build.go | 11 ----------- src/cmd/dist/buildruntime.go | 2 -- src/cmd/go/alldocs.go | 3 --- src/cmd/go/internal/cfg/cfg.go | 3 --- src/cmd/go/internal/help/helpdoc.go | 3 --- src/cmd/internal/objabi/util.go | 11 ++++------- src/internal/cfg/cfg.go | 1 - 7 files changed, 4 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index d22ee1d361..9e2b4f33b8 100644 --- a/src/cmd/dist/build.go +++ b/src/cmd/dist/build.go @@ -31,7 +31,6 @@ var ( goos string goarm string go386 string - goamd64 string gomips string gomips64 string goppc64 string @@ -152,12 +151,6 @@ func xinit() { } go386 = b - b = os.Getenv("GOAMD64") - if b == "" { - b = "alignedjumps" - } - goamd64 = b - b = os.Getenv("GOMIPS") if b == "" { b = "hardfloat" @@ -230,7 +223,6 @@ func xinit() { // For tools being invoked but also for os.ExpandEnv. os.Setenv("GO386", go386) - os.Setenv("GOAMD64", goamd64) os.Setenv("GOARCH", goarch) os.Setenv("GOARM", goarm) os.Setenv("GOHOSTARCH", gohostarch) @@ -1171,9 +1163,6 @@ func cmdenv() { if goarch == "386" { xprintf(format, "GO386", go386) } - if goarch == "amd64" { - xprintf(format, "GOAMD64", goamd64) - } if goarch == "mips" || goarch == "mipsle" { xprintf(format, "GOMIPS", gomips) } diff --git a/src/cmd/dist/buildruntime.go b/src/cmd/dist/buildruntime.go index f11933c925..2744951597 100644 --- a/src/cmd/dist/buildruntime.go +++ b/src/cmd/dist/buildruntime.go @@ -42,7 +42,6 @@ func mkzversion(dir, file string) { // // const defaultGOROOT = // const defaultGO386 = -// const defaultGOAMD64 = // const defaultGOARM = // const defaultGOMIPS = // const defaultGOMIPS64 = @@ -72,7 +71,6 @@ func mkzbootstrap(file string) { fmt.Fprintf(&buf, "import \"runtime\"\n") fmt.Fprintln(&buf) fmt.Fprintf(&buf, "const defaultGO386 = `%s`\n", go386) - fmt.Fprintf(&buf, "const defaultGOAMD64 = `%s`\n", goamd64) fmt.Fprintf(&buf, "const defaultGOARM = `%s`\n", goarm) fmt.Fprintf(&buf, "const defaultGOMIPS = `%s`\n", gomips) fmt.Fprintf(&buf, "const defaultGOMIPS64 = `%s`\n", gomips64) diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 5c1f7254bf..fdeef651c7 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1754,9 +1754,6 @@ // GO386 // For GOARCH=386, the floating point instruction set. // Valid values are 387, sse2. -// GOAMD64 -// For GOARCH=amd64, jumps can be optionally be aligned such that they do not end on -// or cross 32 byte boundaries. Valid values are alignedjumps (default), normaljumps. // GOMIPS // For GOARCH=mips{,le}, whether to use floating point instructions. // Valid values are hardfloat (default), softfloat. diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go index 21f55e852f..7f8f8e92be 100644 --- a/src/cmd/go/internal/cfg/cfg.go +++ b/src/cmd/go/internal/cfg/cfg.go @@ -241,7 +241,6 @@ var ( // Used in envcmd.MkEnv and build ID computations. GOARM = envOr("GOARM", fmt.Sprint(objabi.GOARM)) GO386 = envOr("GO386", objabi.GO386) - GOAMD64 = envOr("GOAMD64", objabi.GOAMD64) GOMIPS = envOr("GOMIPS", objabi.GOMIPS) GOMIPS64 = envOr("GOMIPS64", objabi.GOMIPS64) GOPPC64 = envOr("GOPPC64", fmt.Sprintf("%s%d", "power", objabi.GOPPC64)) @@ -267,8 +266,6 @@ func GetArchEnv() (key, val string) { return "GOARM", GOARM case "386": return "GO386", GO386 - case "amd64": - return "GOAMD64", GOAMD64 case "mips", "mipsle": return "GOMIPS", GOMIPS case "mips64", "mips64le": diff --git a/src/cmd/go/internal/help/helpdoc.go b/src/cmd/go/internal/help/helpdoc.go index 9583b3f327..693de8ff49 100644 --- a/src/cmd/go/internal/help/helpdoc.go +++ b/src/cmd/go/internal/help/helpdoc.go @@ -582,9 +582,6 @@ Architecture-specific environment variables: GO386 For GOARCH=386, the floating point instruction set. Valid values are 387, sse2. - GOAMD64 - For GOARCH=amd64, jumps can be optionally be aligned such that they do not end on - or cross 32 byte boundaries. Valid values are alignedjumps (default), normaljumps. GOMIPS For GOARCH=mips{,le}, whether to use floating point instructions. Valid values are hardfloat (default), softfloat. diff --git a/src/cmd/internal/objabi/util.go b/src/cmd/internal/objabi/util.go index 72dd5856f8..2f94ec6a67 100644 --- a/src/cmd/internal/objabi/util.go +++ b/src/cmd/internal/objabi/util.go @@ -37,16 +37,13 @@ var ( const ( ElfRelocOffset = 256 - MachoRelocOffset = 2048 // reserve enough space for ELF relocations + MachoRelocOffset = 2048 // reserve enough space for ELF relocations + Go115AMD64 = "alignedjumps" // Should be "alignedjumps" or "normaljumps"; this replaces environment variable introduced in CL 219357. ) +// TODO(1.16): assuming no issues in 1.15 release, remove this and related constant. func goamd64() string { - switch v := envOr("GOAMD64", defaultGOAMD64); v { - case "normaljumps", "alignedjumps": - return v - } - log.Fatalf("Invalid GOAMD64 value. Must be normaljumps or alignedjumps.") - panic("unreachable") + return Go115AMD64 } func goarm() int { diff --git a/src/internal/cfg/cfg.go b/src/internal/cfg/cfg.go index e40b7b4d1a..bdbe9df3e7 100644 --- a/src/internal/cfg/cfg.go +++ b/src/internal/cfg/cfg.go @@ -33,7 +33,6 @@ const KnownEnv = ` GCCGO GO111MODULE GO386 - GOAMD64 GOARCH GOARM GOBIN -- cgit v1.3-5-g9baa From aeab40317415468323ef45095f2090ff90c3ecd2 Mon Sep 17 00:00:00 2001 From: Yasuhiro Matsumoto Date: Thu, 14 May 2020 18:17:49 +0900 Subject: cmd/go: use temporary file for output of gcc command on Windows On Windows, some of gcc command (like msys2 native) output NUL as a file. Fixes #36000 Change-Id: I02c632fa2d710a011d79f24d5beee4bc57ad994e Reviewed-on: https://go-review.googlesource.com/c/go/+/233977 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/go/internal/work/exec.go | 14 +++++++++++++- src/cmd/go/testdata/script/issue36000.txt | 6 ++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 src/cmd/go/testdata/script/issue36000.txt (limited to 'src') diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 6ab3498c3e..071c9d2db9 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -2434,13 +2434,25 @@ func (b *Builder) gccSupportsFlag(compiler []string, flag string) bool { if b.flagCache == nil { b.flagCache = make(map[[2]string]bool) } + + tmp := os.DevNull + if runtime.GOOS == "windows" { + f, err := ioutil.TempFile(b.WorkDir, "") + if err != nil { + return false + } + f.Close() + tmp = f.Name() + defer os.Remove(tmp) + } + // We used to write an empty C file, but that gets complicated with // go build -n. We tried using a file that does not exist, but that // fails on systems with GCC version 4.2.1; that is the last GPLv2 // version of GCC, so some systems have frozen on it. // Now we pass an empty file on stdin, which should work at least for // GCC and clang. - cmdArgs := str.StringList(compiler, flag, "-c", "-x", "c", "-", "-o", os.DevNull) + cmdArgs := str.StringList(compiler, flag, "-c", "-x", "c", "-", "-o", tmp) if cfg.BuildN || cfg.BuildX { b.Showcmd(b.WorkDir, "%s || true", joinUnambiguously(cmdArgs)) if cfg.BuildN { diff --git a/src/cmd/go/testdata/script/issue36000.txt b/src/cmd/go/testdata/script/issue36000.txt new file mode 100644 index 0000000000..41732751ac --- /dev/null +++ b/src/cmd/go/testdata/script/issue36000.txt @@ -0,0 +1,6 @@ +# Tests golang.org/issue/36000 + +[!cgo] skip + +# go env with CGO flags should not make NUL file +go env CGO_CFLAGS -- cgit v1.3-5-g9baa From 567556d78657326c99b8fa84ec2a5ee511a0941b Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 19 May 2020 20:23:58 -0700 Subject: syscall: preserve Windows file permissions for O_CREAT|O_TRUNC On Windows, calling syscall.Open(file, O_CREAT|O_TRUNC, 0) for a file that already exists would change the file to be read-only. That is not how the Unix syscall.Open behaves, so avoid it on Windows by calling CreateFile twice if necessary. Fixes #38225 Change-Id: I70097fca8863df427cc8a97b9376a9ffc69c6318 Reviewed-on: https://go-review.googlesource.com/c/go/+/234534 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Alex Brainman --- src/os/os_test.go | 31 +++++++++++++++++++++++++++++++ src/syscall/syscall_windows.go | 20 ++++++++++++++++++++ 2 files changed, 51 insertions(+) (limited to 'src') diff --git a/src/os/os_test.go b/src/os/os_test.go index f86428b7b9..e8c64510f5 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -2539,3 +2539,34 @@ func isDeadlineExceeded(err error) bool { } return true } + +// Test that opening a file does not change its permissions. Issue 38225. +func TestOpenFileKeepsPermissions(t *testing.T) { + t.Parallel() + dir := t.TempDir() + name := filepath.Join(dir, "x") + f, err := Create(name) + if err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Error(err) + } + f, err = OpenFile(name, O_WRONLY|O_CREATE|O_TRUNC, 0) + if err != nil { + t.Fatal(err) + } + if fi, err := f.Stat(); err != nil { + t.Error(err) + } else if fi.Mode()&0222 == 0 { + t.Errorf("f.Stat.Mode after OpenFile is %v, should be writable", fi.Mode()) + } + if err := f.Close(); err != nil { + t.Error(err) + } + if fi, err := Stat(name); err != nil { + t.Error(err) + } else if fi.Mode()&0222 == 0 { + t.Errorf("Stat after OpenFile is %v, should be writable", fi.Mode()) + } +} diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index 89c0a930cb..f62c00d72f 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -339,6 +339,26 @@ func Open(path string, mode int, perm uint32) (fd Handle, err error) { var attrs uint32 = FILE_ATTRIBUTE_NORMAL if perm&S_IWRITE == 0 { attrs = FILE_ATTRIBUTE_READONLY + if createmode == CREATE_ALWAYS { + // We have been asked to create a read-only file. + // If the file already exists, the semantics of + // the Unix open system call is to preserve the + // existing permissions. If we pass CREATE_ALWAYS + // and FILE_ATTRIBUTE_READONLY to CreateFile, + // and the file already exists, CreateFile will + // change the file permissions. + // Avoid that to preserve the Unix semantics. + h, e := CreateFile(pathp, access, sharemode, sa, TRUNCATE_EXISTING, FILE_ATTRIBUTE_NORMAL, 0) + switch e { + case ERROR_FILE_NOT_FOUND, _ERROR_BAD_NETPATH, ERROR_PATH_NOT_FOUND: + // File does not exist. These are the same + // errors as Errno.Is checks for ErrNotExist. + // Carry on to create the file. + default: + // Success or some different error. + return h, e + } + } } h, e := CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0) return h, e -- cgit v1.3-5-g9baa From c53b2bdb35c5339df35b53c8fbf34e5cbede081f Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 18 May 2020 14:14:11 -0400 Subject: runtime: add a barrier after a new span is allocated When copying a stack, we 1. allocate a new stack, 2. adjust pointers pointing to the old stack to pointing to the new stack. If the GC is running on another thread concurrently, on a machine with weak memory model, the GC could observe the adjusted pointer (e.g. through gp._defer which could be a special heap-to-stack pointer), but not observe the publish of the new stack span. In this case, the GC will see the adjusted pointer pointing to an unallocated span, and throw. Fixing this by adding a publication barrier between the allocation of the span and adjusting pointers. One testcase for this is TestDeferHeapAndStack in long mode. It fails reliably on linux-mips64le-mengzhuo builder without the fix, and passes reliably after the fix. Fixes #35541. Change-Id: I82b09b824fdf14be7336a9ee853f56dec1b13b90 Reviewed-on: https://go-review.googlesource.com/c/go/+/234478 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements Reviewed-by: Michael Knyszek --- src/runtime/mheap.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 6f7dc6eaa6..2c7bfd8a59 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1283,9 +1283,10 @@ HaveSpan: // Publish the span in various locations. // This is safe to call without the lock held because the slots - // related to this span will only every be read or modified by - // this thread until pointers into the span are published or - // pageInUse is updated. + // related to this span will only ever be read or modified by + // this thread until pointers into the span are published (and + // we execute a publication barrier at the end of this function + // before that happens) or pageInUse is updated. h.setSpans(s.base(), npages, s) if !manual { @@ -1315,6 +1316,11 @@ HaveSpan: traceHeapAlloc() } } + + // Make sure the newly allocated span will be observed + // by the GC before pointers into the span are published. + publicationBarrier() + return s } -- cgit v1.3-5-g9baa From 0cfe1fb87815c4bee910f6f066f7872800dbce24 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Wed, 20 May 2020 17:54:55 -0400 Subject: cmd/go: rank errUseProxy lower when handling proxy errors modfetch.TryProxies ranks errors returned by GOPROXY entries by usefulness. It returns the error of the highest rank from the last proxy. Errors from "direct" and "noproxy" are most useful, followed by errors other than ErrNotExist, followed by ErrNotExist. This change ranks errUseProxy with ErrNotExist even though it's reported by "noproxy". There is almost always a more useful message than "path does not match GOPRIVATE/GONOPROXY". Fixes #39180 Change-Id: Ifa5b96462d7bf411e6d2d951888465c839d42471 Reviewed-on: https://go-review.googlesource.com/c/go/+/234687 Run-TryBot: Jay Conrod Reviewed-by: Bryan C. Mills TryBot-Result: Gobot Gobot --- src/cmd/go/internal/modfetch/proxy.go | 10 +++++++--- src/cmd/go/testdata/script/mod_gonoproxy.txt | 10 ++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/cmd/go/internal/modfetch/proxy.go b/src/cmd/go/internal/modfetch/proxy.go index 0ca43d4c4a..3971598733 100644 --- a/src/cmd/go/internal/modfetch/proxy.go +++ b/src/cmd/go/internal/modfetch/proxy.go @@ -196,8 +196,12 @@ func TryProxies(f func(proxy string) error) error { // We try to report the most helpful error to the user. "direct" and "noproxy" // errors are best, followed by proxy errors other than ErrNotExist, followed - // by ErrNotExist. Note that errProxyOff, errNoproxy, and errUseProxy are - // equivalent to ErrNotExist. + // by ErrNotExist. + // + // Note that errProxyOff, errNoproxy, and errUseProxy are equivalent to + // ErrNotExist. errUseProxy should only be returned if "noproxy" is the only + // proxy. errNoproxy should never be returned, since there should always be a + // more useful error from "noproxy" first. const ( notExistRank = iota proxyRank @@ -212,7 +216,7 @@ func TryProxies(f func(proxy string) error) error { } isNotExistErr := errors.Is(err, os.ErrNotExist) - if proxy.url == "direct" || proxy.url == "noproxy" { + if proxy.url == "direct" || (proxy.url == "noproxy" && err != errUseProxy) { bestErr = err bestErrRank = directRank } else if bestErrRank <= proxyRank && !isNotExistErr { diff --git a/src/cmd/go/testdata/script/mod_gonoproxy.txt b/src/cmd/go/testdata/script/mod_gonoproxy.txt index 2bd94cdee0..d7848c7d26 100644 --- a/src/cmd/go/testdata/script/mod_gonoproxy.txt +++ b/src/cmd/go/testdata/script/mod_gonoproxy.txt @@ -10,7 +10,7 @@ env GOSUMDB=$sumdb' '$proxy/sumdb-wrong ! go get rsc.io/quote stderr 'SECURITY ERROR' -# but GONOSUMDB bypasses sumdb, for rsc.io/quote, rsc.io/sampler, golang.org/x/text +# GONOSUMDB bypasses sumdb, for rsc.io/quote, rsc.io/sampler, golang.org/x/text env GONOSUMDB='*/quote,*/*mple*,golang.org/x' go get rsc.io/quote rm go.sum @@ -18,7 +18,13 @@ env GOPRIVATE='*/quote,*/*mple*,golang.org/x' env GONOPROXY=none # that is, proxy all despite GOPRIVATE go get rsc.io/quote -# and GONOPROXY bypasses proxy +# When GOPROXY=off, fetching modules not matched by GONOPROXY fails. +env GONOPROXY=*/fortune +env GOPROXY=off +! go get golang.org/x/text +stderr '^go get golang.org/x/text: module lookup disabled by GOPROXY=off$' + +# GONOPROXY bypasses proxy [!net] skip [!exec:git] skip env GOPRIVATE=none -- cgit v1.3-5-g9baa From fed33d76bcf5d378f0322b308768d156239b0bfc Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Fri, 15 May 2020 12:19:07 -0400 Subject: cmd/compile: delay inlinable method compilation for -c=1 When the concurrent back end is not enabled, it is possible to have a scenario where: we compile a specific inlinable non-pointer-receiver method T.M, then at some point later on in the compilation we visit a type that triggers generation of a pointer-receiver wrapper (*T).M, which then results in an inline of T.M into (*T).M. This introduces subtle differences in the DWARF as compared with when the concurrent back end is enabled (in the concurrent case, by the time we run the SSA back end on T.M is is marked as being inlined, whereas in the non-current case it is not marked inlined). As a fix, at the point where we would normally compile a given function in the xtop list right away, if the function is a method AND is inlinable AND hasn't been inlined, then delay its compilation until compileFunctions (so as to make sure that when we do compile it, all possible inlining has been complete). In addition, make sure that the abstract function symbol for the inlined function gets recorded correctly. Fixes #38068. Change-Id: I57410ab5658bd4ee5b4b80750518e9b20fd6ba52 Reviewed-on: https://go-review.googlesource.com/c/go/+/234178 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang Reviewed-by: Keith Randall --- src/cmd/compile/internal/gc/pgen.go | 25 +++++++++++++++++++++++-- src/cmd/internal/obj/objfile.go | 8 ++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index 6ccd0b8d94..74654c86bc 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -271,7 +271,7 @@ func compile(fn *Node) { } } - if compilenow() { + if compilenow(fn) { compileSSA(fn, 0) } else { compilequeue = append(compilequeue, fn) @@ -282,10 +282,31 @@ func compile(fn *Node) { // If functions are not compiled immediately, // they are enqueued in compilequeue, // which is drained by compileFunctions. -func compilenow() bool { +func compilenow(fn *Node) bool { + // Issue 38068: if this function is a method AND an inline + // candidate AND was not inlined (yet), put it onto the compile + // queue instead of compiling it immediately. This is in case we + // wind up inlining it into a method wrapper that is generated by + // compiling a function later on in the xtop list. + if fn.IsMethod() && isInlinableButNotInlined(fn) { + return false + } return nBackendWorkers == 1 && Debug_compilelater == 0 } +// isInlinableButNotInlined returns true if 'fn' was marked as an +// inline candidate but then never inlined (presumably because we +// found no call sites). +func isInlinableButNotInlined(fn *Node) bool { + if fn.Func.Nname.Func.Inl == nil { + return false + } + if fn.Sym == nil { + return true + } + return !fn.Sym.Linksym().WasInlined() +} + const maxStackSize = 1 << 30 // compileSSA builds an SSA backend function, diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go index 6d7f42ed0b..93c313862e 100644 --- a/src/cmd/internal/obj/objfile.go +++ b/src/cmd/internal/obj/objfile.go @@ -788,6 +788,14 @@ func (ft *DwarfFixupTable) SetPrecursorFunc(s *LSym, fn interface{}) { absfn.Type = objabi.SDWARFINFO ft.ctxt.Data = append(ft.ctxt.Data, absfn) + // In the case of "late" inlining (inlines that happen during + // wrapper generation as opposed to the main inlining phase) it's + // possible that we didn't cache the abstract function sym for the + // text symbol -- do so now if needed. See issue 38068. + if s.Func != nil && s.Func.dwarfAbsFnSym == nil { + s.Func.dwarfAbsFnSym = absfn + } + ft.precursor[s] = fnState{precursor: fn, absfn: absfn} } -- cgit v1.3-5-g9baa From 39ea0ea05dcfa281dc5977410b60458f2d2adb99 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 18 May 2020 15:23:46 -0400 Subject: cmd/link: fix size calculation for file space preallocation on darwin On darwin, we preallocate file storage space with fcntl F_ALLOCATEALL in F_PEOFPOSMODE mode. This is specified as allocating from the physical end of the file. So the size we give it should be the increment, instead of the total size. Fixes #39044. Change-Id: I10c7ee8d51f237b4a7604233ac7abc6f91dcd602 Reviewed-on: https://go-review.googlesource.com/c/go/+/234481 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements Reviewed-by: Jeremy Faller --- src/cmd/link/internal/ld/fallocate_test.go | 50 ++++++++++++++++++++++++++++++ src/cmd/link/internal/ld/outbuf_darwin.go | 17 +++++++--- 2 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 src/cmd/link/internal/ld/fallocate_test.go (limited to 'src') diff --git a/src/cmd/link/internal/ld/fallocate_test.go b/src/cmd/link/internal/ld/fallocate_test.go new file mode 100644 index 0000000000..a064bea23d --- /dev/null +++ b/src/cmd/link/internal/ld/fallocate_test.go @@ -0,0 +1,50 @@ +// 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 darwin linux + +package ld + +import ( + "io/ioutil" + "os" + "path/filepath" + "syscall" + "testing" +) + +func TestFallocate(t *testing.T) { + dir, err := ioutil.TempDir("", "TestFallocate") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + filename := filepath.Join(dir, "a.out") + out := NewOutBuf(nil) + err = out.Open(filename) + if err != nil { + t.Fatalf("Open file failed: %v", err) + } + defer out.Close() + + // Mmap 1 MiB initially, and grow to 2 and 3 MiB. + // Check if the file size and disk usage is expected. + for _, sz := range []int64{1 << 20, 2 << 20, 3 << 20} { + err = out.Mmap(uint64(sz)) + if err != nil { + t.Fatalf("Mmap failed: %v", err) + } + stat, err := os.Stat(filename) + if err != nil { + t.Fatalf("Stat failed: %v", err) + } + if got := stat.Size(); got != sz { + t.Errorf("unexpected file size: got %d, want %d", got, sz) + } + if got, want := stat.Sys().(*syscall.Stat_t).Blocks, (sz+511)/512; got != want { + t.Errorf("unexpected disk usage: got %d blocks, want %d", got, want) + } + out.munmap() + } +} diff --git a/src/cmd/link/internal/ld/outbuf_darwin.go b/src/cmd/link/internal/ld/outbuf_darwin.go index 299902ec62..9a74ba875e 100644 --- a/src/cmd/link/internal/ld/outbuf_darwin.go +++ b/src/cmd/link/internal/ld/outbuf_darwin.go @@ -10,16 +10,25 @@ import ( ) func (out *OutBuf) fallocate(size uint64) error { + stat, err := out.f.Stat() + if err != nil { + return err + } + cursize := uint64(stat.Size()) + if size <= cursize { + return nil + } + store := &syscall.Fstore_t{ Flags: syscall.F_ALLOCATEALL, Posmode: syscall.F_PEOFPOSMODE, Offset: 0, - Length: int64(size), + Length: int64(size - cursize), // F_PEOFPOSMODE allocates from the end of the file, so we want the size difference here } - _, _, err := syscall.Syscall(syscall.SYS_FCNTL, uintptr(out.f.Fd()), syscall.F_PREALLOCATE, uintptr(unsafe.Pointer(store))) - if err != 0 { - return err + _, _, errno := syscall.Syscall(syscall.SYS_FCNTL, uintptr(out.f.Fd()), syscall.F_PREALLOCATE, uintptr(unsafe.Pointer(store))) + if errno != 0 { + return errno } return nil -- cgit v1.3-5-g9baa From c847589ad06a1acfcceaac7b230c0d5a826caab8 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Tue, 19 May 2020 16:33:17 +0000 Subject: runtime: synchronize StartTrace and StopTrace with sysmon Currently sysmon is not stopped when the world is stopped, which is in general a difficult thing to do. The result of this is that when tracing starts and the value of trace.enabled changes, it's possible for sysmon to fail to emit an event when it really should. This leads to traces which the execution trace parser deems inconsistent. Fix this by putting all of sysmon's work behind a new lock sysmonlock. StartTrace and StopTrace both acquire this lock after stopping the world but before performing any work in order to ensure sysmon sees the required state change in tracing. This change is expected to slow down StartTrace and StopTrace, but will help ensure consistent traces are generated. Updates #29707. Fixes #38794. Change-Id: I64c58e7c3fd173cd5281ffc208d6db24ff6c0284 Reviewed-on: https://go-review.googlesource.com/c/go/+/234617 Run-TryBot: Michael Knyszek Run-TryBot: Ian Lance Taylor Reviewed-by: Ian Lance Taylor Reviewed-by: Hyang-Ah Hana Kim Reviewed-by: Michael Pratt --- src/runtime/lockrank.go | 41 ++++++++++++++++++++++------------------- src/runtime/proc.go | 14 ++++++++++++++ src/runtime/runtime2.go | 20 +++++++++++++------- src/runtime/trace.go | 12 ++++++++++++ 4 files changed, 61 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/runtime/lockrank.go b/src/runtime/lockrank.go index d97afac6a2..a4fe921557 100644 --- a/src/runtime/lockrank.go +++ b/src/runtime/lockrank.go @@ -33,6 +33,7 @@ const ( lockRankDummy lockRank = iota // Locks held above sched + lockRankSysmon lockRankScavenge lockRankForcegc lockRankSweepWaiters @@ -113,6 +114,7 @@ const lockRankLeafRank lockRank = 1000 var lockNames = []string{ lockRankDummy: "", + lockRankSysmon: "sysmon", lockRankScavenge: "scavenge", lockRankForcegc: "forcegc", lockRankSweepWaiters: "sweepWaiters", @@ -194,48 +196,49 @@ func (rank lockRank) String() string { // hchan lock can be held immediately above it when it is acquired. var lockPartialOrder [][]lockRank = [][]lockRank{ lockRankDummy: {}, - lockRankScavenge: {}, - lockRankForcegc: {}, + lockRankSysmon: {}, + lockRankScavenge: {lockRankSysmon}, + lockRankForcegc: {lockRankSysmon}, lockRankSweepWaiters: {}, lockRankAssistQueue: {}, lockRankCpuprof: {}, lockRankSweep: {}, - lockRankSched: {lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep}, + lockRankSched: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep}, lockRankDeadlock: {lockRankDeadlock}, lockRankPanic: {lockRankDeadlock}, - lockRankAllg: {lockRankSched, lockRankPanic}, - lockRankAllp: {lockRankSched}, + lockRankAllg: {lockRankSysmon, lockRankSched, lockRankPanic}, + lockRankAllp: {lockRankSysmon, lockRankSched}, lockRankPollDesc: {}, - lockRankTimers: {lockRankScavenge, lockRankSched, lockRankAllp, lockRankPollDesc, lockRankTimers}, + lockRankTimers: {lockRankSysmon, lockRankScavenge, lockRankSched, lockRankAllp, lockRankPollDesc, lockRankTimers}, lockRankItab: {}, lockRankReflectOffs: {lockRankItab}, lockRankHchan: {lockRankScavenge, lockRankSweep, lockRankHchan}, lockRankFin: {lockRankSched, lockRankAllg, lockRankTimers, lockRankHchan}, lockRankNotifyList: {}, - lockRankTraceBuf: {lockRankScavenge}, + lockRankTraceBuf: {lockRankSysmon, lockRankScavenge}, lockRankTraceStrings: {lockRankTraceBuf}, - lockRankMspanSpecial: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings}, - lockRankProf: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, - lockRankGcBitsArenas: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankMspanSpecial: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings}, + lockRankProf: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankGcBitsArenas: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, lockRankRoot: {}, - lockRankTrace: {lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankSched, lockRankHchan, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankSweep}, + lockRankTrace: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankSched, lockRankHchan, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankSweep}, lockRankTraceStackTab: {lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankSched, lockRankAllg, lockRankTimers, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankTrace}, lockRankNetpollInit: {lockRankTimers}, lockRankRwmutexW: {}, lockRankRwmutexR: {lockRankRwmutexW}, - lockRankMcentral: {lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, - lockRankSpine: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, - lockRankSpanSetSpine: {lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, - lockRankGscan: {lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankNotifyList, lockRankProf, lockRankGcBitsArenas, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankMcentral, lockRankSpine, lockRankSpanSetSpine}, - lockRankStackpool: {lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankRwmutexR, lockRankMcentral, lockRankSpine, lockRankSpanSetSpine, lockRankGscan}, - lockRankStackLarge: {lockRankAssistQueue, lockRankSched, lockRankItab, lockRankHchan, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankMcentral, lockRankSpanSetSpine, lockRankGscan}, + lockRankMcentral: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankSpine: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankSpanSetSpine: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankGscan: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankNotifyList, lockRankProf, lockRankGcBitsArenas, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankMcentral, lockRankSpine, lockRankSpanSetSpine}, + lockRankStackpool: {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankRwmutexR, lockRankMcentral, lockRankSpine, lockRankSpanSetSpine, lockRankGscan}, + lockRankStackLarge: {lockRankSysmon, lockRankAssistQueue, lockRankSched, lockRankItab, lockRankHchan, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankMcentral, lockRankSpanSetSpine, lockRankGscan}, lockRankDefer: {}, lockRankSudog: {lockRankNotifyList, lockRankHchan}, lockRankWbufSpans: {lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankSched, lockRankAllg, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceStrings, lockRankMspanSpecial, lockRankProf, lockRankRoot, lockRankGscan, lockRankDefer, lockRankSudog}, - lockRankMheap: {lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan, lockRankMspanSpecial, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankMcentral, lockRankGscan, lockRankStackpool, lockRankStackLarge, lockRankDefer, lockRankSudog, lockRankWbufSpans, lockRankSpanSetSpine}, - lockRankMheapSpecial: {lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, + lockRankMheap: {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan, lockRankMspanSpecial, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankMcentral, lockRankGscan, lockRankStackpool, lockRankStackLarge, lockRankDefer, lockRankSudog, lockRankWbufSpans, lockRankSpanSetSpine}, + lockRankMheapSpecial: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan}, lockRankGlobalAlloc: {lockRankProf, lockRankSpine, lockRankSpanSetSpine, lockRankMheap, lockRankMheapSpecial}, lockRankGFree: {lockRankSched}, diff --git a/src/runtime/proc.go b/src/runtime/proc.go index ca99870224..b423026c0e 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -533,6 +533,7 @@ func cpuinit() { // The new G calls runtime·main. func schedinit() { lockInit(&sched.lock, lockRankSched) + lockInit(&sched.sysmonlock, lockRankSysmon) lockInit(&sched.deferlock, lockRankDefer) lockInit(&sched.sudoglock, lockRankSudog) lockInit(&deadlock, lockRankDeadlock) @@ -4613,6 +4614,18 @@ func sysmon() { } unlock(&sched.lock) } + lock(&sched.sysmonlock) + { + // If we spent a long time blocked on sysmonlock + // then we want to update now and next since it's + // likely stale. + now1 := nanotime() + if now1-now > 50*1000 /* 50µs */ { + next, _ = timeSleepUntil() + } + now = now1 + } + // trigger libc interceptors if needed if *cgo_yield != nil { asmcgocall(*cgo_yield, nil) @@ -4665,6 +4678,7 @@ func sysmon() { lasttrace = now schedtrace(debug.scheddetail > 0) } + unlock(&sched.sysmonlock) } } diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 1fe41cf5b2..cffdb0bf27 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -349,9 +349,9 @@ type sudog struct { g *g - next *sudog - prev *sudog - elem unsafe.Pointer // data element (may point to stack) + next *sudog + prev *sudog + elem unsafe.Pointer // data element (may point to stack) // The following fields are never accessed concurrently. // For channels, waitlink is only accessed by g. @@ -366,10 +366,10 @@ type sudog struct { // g.selectDone must be CAS'd to win the wake-up race. isSelect bool - parent *sudog // semaRoot binary tree - waitlink *sudog // g.waiting list or semaRoot - waittail *sudog // semaRoot - c *hchan // channel + parent *sudog // semaRoot binary tree + waitlink *sudog // g.waiting list or semaRoot + waittail *sudog // semaRoot + c *hchan // channel } type libcall struct { @@ -768,6 +768,12 @@ type schedt struct { procresizetime int64 // nanotime() of last change to gomaxprocs totaltime int64 // ∫gomaxprocs dt up to procresizetime + + // sysmonlock protects sysmon's actions on the runtime. + // + // Acquire and hold this mutex to block sysmon from interacting + // with the rest of the runtime. + sysmonlock mutex } // Values for the flags field of a sigTabT. diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 33062daa46..169b650eb4 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -187,6 +187,9 @@ func StartTrace() error { // paired with an end). stopTheWorldGC("start tracing") + // Prevent sysmon from running any code that could generate events. + lock(&sched.sysmonlock) + // We are in stop-the-world, but syscalls can finish and write to trace concurrently. // Exitsyscall could check trace.enabled long before and then suddenly wake up // and decide to write to trace at a random point in time. @@ -196,6 +199,7 @@ func StartTrace() error { if trace.enabled || trace.shutdown { unlock(&trace.bufLock) + unlock(&sched.sysmonlock) startTheWorldGC() return errorString("tracing is already enabled") } @@ -267,6 +271,8 @@ func StartTrace() error { unlock(&trace.bufLock) + unlock(&sched.sysmonlock) + startTheWorldGC() return nil } @@ -278,11 +284,15 @@ func StopTrace() { // and also to avoid races with traceEvent. stopTheWorldGC("stop tracing") + // See the comment in StartTrace. + lock(&sched.sysmonlock) + // See the comment in StartTrace. lock(&trace.bufLock) if !trace.enabled { unlock(&trace.bufLock) + unlock(&sched.sysmonlock) startTheWorldGC() return } @@ -320,6 +330,8 @@ func StopTrace() { trace.shutdown = true unlock(&trace.bufLock) + unlock(&sched.sysmonlock) + startTheWorldGC() // The world is started but we've set trace.shutdown, so new tracing can't start. -- cgit v1.3-5-g9baa