--
cgit v1.3-5-g9baa
From 92bda33d2771a9b12868d9025f113538fa7a84de Mon Sep 17 00:00:00 2001
From: Austin Clements
Date: Fri, 31 Jul 2020 15:58:00 -0400
Subject: runtime: revert signal stack mlocking
Go 1.14 included a (rather awful) workaround for a Linux kernel bug
that corrupted vector registers on x86 CPUs during signal delivery
(https://bugzilla.kernel.org/show_bug.cgi?id=205663). This bug was
introduced in Linux 5.2 and fixed in 5.3.15, 5.4.2 and all 5.5 and
later kernels. The fix was also back-ported by major distros. This
workaround was necessary, but had unfortunate downsides, including
causing Go programs to exceed the mlock ulimit in many configurations
(#37436).
We're reasonably confident that by the Go 1.16 release, the number of
systems running affected kernels will be vanishingly small. Hence,
this CL removes this workaround.
This effectively reverts CLs 209597 (version parser), 209899 (mlock
top of signal stack), 210299 (better failure message), 223121 (soft
mlock failure handling), and 244059 (special-case patched Ubuntu
kernels). The one thing we keep is the osArchInit function. It's empty
everywhere now, but is a reasonable hook to have.
Updates #35326, #35777 (the original register corruption bugs).
Updates #40184 (request to revert in 1.15).
Fixes #35979.
Change-Id: Ie213270837095576f1f3ef46bf3de187dc486c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/246200
Run-TryBot: Austin Clements
TryBot-Result: Gobot Gobot
Reviewed-by: Ian Lance Taylor
---
src/runtime/defs_linux_386.go | 11 ----
src/runtime/defs_linux_amd64.go | 11 ----
src/runtime/export_test.go | 2 -
src/runtime/os_linux.go | 9 ---
src/runtime/os_linux_x86.go | 118 +---------------------------------------
src/runtime/panic.go | 10 ----
src/runtime/string.go | 34 ------------
src/runtime/string_test.go | 31 -----------
src/runtime/sys_linux_386.s | 19 -------
src/runtime/sys_linux_amd64.s | 19 -------
10 files changed, 1 insertion(+), 263 deletions(-)
diff --git a/src/runtime/defs_linux_386.go b/src/runtime/defs_linux_386.go
index f4db8cf927..64a0fbcaaa 100644
--- a/src/runtime/defs_linux_386.go
+++ b/src/runtime/defs_linux_386.go
@@ -226,14 +226,3 @@ type sockaddr_un struct {
family uint16
path [108]byte
}
-
-const __NEW_UTS_LEN = 64
-
-type new_utsname struct {
- sysname [__NEW_UTS_LEN + 1]byte
- nodename [__NEW_UTS_LEN + 1]byte
- release [__NEW_UTS_LEN + 1]byte
- version [__NEW_UTS_LEN + 1]byte
- machine [__NEW_UTS_LEN + 1]byte
- domainname [__NEW_UTS_LEN + 1]byte
-}
diff --git a/src/runtime/defs_linux_amd64.go b/src/runtime/defs_linux_amd64.go
index 8480d85219..1ae18a309b 100644
--- a/src/runtime/defs_linux_amd64.go
+++ b/src/runtime/defs_linux_amd64.go
@@ -262,14 +262,3 @@ type sockaddr_un struct {
family uint16
path [108]byte
}
-
-const __NEW_UTS_LEN = 64
-
-type new_utsname struct {
- sysname [__NEW_UTS_LEN + 1]byte
- nodename [__NEW_UTS_LEN + 1]byte
- release [__NEW_UTS_LEN + 1]byte
- version [__NEW_UTS_LEN + 1]byte
- machine [__NEW_UTS_LEN + 1]byte
- domainname [__NEW_UTS_LEN + 1]byte
-}
diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 5ab03f3f99..d591fdc4e9 100644
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -43,8 +43,6 @@ var PhysHugePageSize = physHugePageSize
var NetpollGenericInit = netpollGenericInit
-var ParseRelease = parseRelease
-
var Memmove = memmove
var MemclrNoHeapPointers = memclrNoHeapPointers
diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go
index 7b95ff2428..22931b4d5c 100644
--- a/src/runtime/os_linux.go
+++ b/src/runtime/os_linux.go
@@ -328,20 +328,11 @@ func libpreinit() {
initsig(true)
}
-// gsignalInitQuirk, if non-nil, is called for every allocated gsignal G.
-//
-// TODO(austin): Remove this after Go 1.15 when we remove the
-// mlockGsignal workaround.
-var gsignalInitQuirk func(gsignal *g)
-
// Called to initialize a new m (including the bootstrap m).
// Called on the parent thread (main thread in case of bootstrap), can allocate memory.
func mpreinit(mp *m) {
mp.gsignal = malg(32 * 1024) // Linux wants >= 2K
mp.gsignal.m = mp
- if gsignalInitQuirk != nil {
- gsignalInitQuirk(mp.gsignal)
- }
}
func gettid() uint32
diff --git a/src/runtime/os_linux_x86.go b/src/runtime/os_linux_x86.go
index 97f870707d..d91fa1a0d1 100644
--- a/src/runtime/os_linux_x86.go
+++ b/src/runtime/os_linux_x86.go
@@ -7,120 +7,4 @@
package runtime
-import (
- "runtime/internal/atomic"
- "unsafe"
-)
-
-//go:noescape
-func uname(utsname *new_utsname) int
-
-func mlock(addr, len uintptr) int
-
-func osArchInit() {
- // Linux 5.2 introduced a bug that can corrupt vector
- // registers on return from a signal if the signal stack isn't
- // faulted in:
- // https://bugzilla.kernel.org/show_bug.cgi?id=205663
- //
- // It was fixed in 5.3.15, 5.4.2, and all 5.5 and later
- // kernels.
- //
- // If we're on an affected kernel, work around this issue by
- // mlocking the top page of every signal stack. This doesn't
- // help for signal stacks created in C, but there's not much
- // we can do about that.
- //
- // TODO(austin): Remove this in Go 1.15, at which point it
- // will be unlikely to encounter any of the affected kernels
- // in the wild.
-
- var uts new_utsname
- if uname(&uts) < 0 {
- throw("uname failed")
- }
- // Check for null terminator to ensure gostringnocopy doesn't
- // walk off the end of the release string.
- found := false
- for _, b := range uts.release {
- if b == 0 {
- found = true
- break
- }
- }
- if !found {
- return
- }
- rel := gostringnocopy(&uts.release[0])
-
- major, minor, patch, ok := parseRelease(rel)
- if !ok {
- return
- }
-
- if major == 5 && minor == 4 && patch < 2 {
- // All 5.4 versions of Ubuntu are patched.
- procVersion := []byte("/proc/version\000")
- f := open(&procVersion[0], _O_RDONLY, 0)
- if f >= 0 {
- var buf [512]byte
- p := noescape(unsafe.Pointer(&buf[0]))
- n := read(f, p, int32(len(buf)))
- closefd(f)
-
- needle := []byte("Ubuntu")
- contains:
- for i, c := range buf[:n] {
- if c != needle[0] {
- continue
- }
- if int(n)-i < len(needle) {
- break
- }
- for j, c2 := range needle {
- if c2 != buf[i+j] {
- continue contains
- }
- }
- // This is an Ubuntu system.
- return
- }
- }
- }
-
- if major == 5 && (minor == 2 || minor == 3 && patch < 15 || minor == 4 && patch < 2) {
- gsignalInitQuirk = mlockGsignal
- if m0.gsignal != nil {
- throw("gsignal quirk too late")
- }
- throwReportQuirk = throwBadKernel
- }
-}
-
-func mlockGsignal(gsignal *g) {
- if atomic.Load(&touchStackBeforeSignal) != 0 {
- // mlock has already failed, don't try again.
- return
- }
-
- // This mlock call may fail, but we don't report the failure.
- // Instead, if something goes badly wrong, we rely on prepareSignalM
- // and throwBadKernel to do further mitigation and to report a problem
- // to the user if mitigation fails. This is because many
- // systems have a limit on the total mlock size, and many kernels
- // that appear to have bad versions are actually patched to avoid the
- // bug described above. We want Go 1.14 to run on those systems.
- // See #37436.
- if errno := mlock(gsignal.stack.hi-physPageSize, physPageSize); errno < 0 {
- atomic.Store(&touchStackBeforeSignal, uint32(-errno))
- }
-}
-
-// throwBadKernel is called, via throwReportQuirk, by throw.
-func throwBadKernel() {
- if errno := atomic.Load(&touchStackBeforeSignal); errno != 0 {
- println("runtime: note: your Linux kernel may be buggy")
- println("runtime: note: see https://golang.org/wiki/LinuxKernelSignalVectorBug")
- println("runtime: note: mlock workaround for kernel bug failed with errno", errno)
- }
-}
+func osArchInit() {}
diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index 615249f33c..127843b081 100644
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -1283,12 +1283,6 @@ func startpanic_m() bool {
}
}
-// throwReportQuirk, if non-nil, is called by throw after dumping the stacks.
-//
-// TODO(austin): Remove this after Go 1.15 when we remove the
-// mlockGsignal workaround.
-var throwReportQuirk func()
-
var didothers bool
var deadlock mutex
@@ -1335,10 +1329,6 @@ func dopanic_m(gp *g, pc, sp uintptr) bool {
printDebugLog()
- if throwReportQuirk != nil {
- throwReportQuirk()
- }
-
return docrash
}
diff --git a/src/runtime/string.go b/src/runtime/string.go
index 0515b56573..251044231e 100644
--- a/src/runtime/string.go
+++ b/src/runtime/string.go
@@ -499,37 +499,3 @@ func gostringw(strw *uint16) string {
b[n2] = 0 // for luck
return s[:n2]
}
-
-// parseRelease parses a dot-separated version number. It follows the
-// semver syntax, but allows the minor and patch versions to be
-// elided.
-func parseRelease(rel string) (major, minor, patch int, ok bool) {
- // Strip anything after a dash or plus.
- for i := 0; i < len(rel); i++ {
- if rel[i] == '-' || rel[i] == '+' {
- rel = rel[:i]
- break
- }
- }
-
- next := func() (int, bool) {
- for i := 0; i < len(rel); i++ {
- if rel[i] == '.' {
- ver, ok := atoi(rel[:i])
- rel = rel[i+1:]
- return ver, ok
- }
- }
- ver, ok := atoi(rel)
- rel = ""
- return ver, ok
- }
- if major, ok = next(); !ok || rel == "" {
- return
- }
- if minor, ok = next(); !ok || rel == "" {
- return
- }
- patch, ok = next()
- return
-}
diff --git a/src/runtime/string_test.go b/src/runtime/string_test.go
index b9ac667533..4eda12c35d 100644
--- a/src/runtime/string_test.go
+++ b/src/runtime/string_test.go
@@ -454,34 +454,3 @@ func TestAtoi32(t *testing.T) {
}
}
}
-
-type parseReleaseTest struct {
- in string
- major, minor, patch int
-}
-
-var parseReleaseTests = []parseReleaseTest{
- {"", -1, -1, -1},
- {"x", -1, -1, -1},
- {"5", 5, 0, 0},
- {"5.12", 5, 12, 0},
- {"5.12-x", 5, 12, 0},
- {"5.12.1", 5, 12, 1},
- {"5.12.1-x", 5, 12, 1},
- {"5.12.1.0", 5, 12, 1},
- {"5.20496382327982653440", -1, -1, -1},
-}
-
-func TestParseRelease(t *testing.T) {
- for _, test := range parseReleaseTests {
- major, minor, patch, ok := runtime.ParseRelease(test.in)
- if !ok {
- major, minor, patch = -1, -1, -1
- }
- if test.major != major || test.minor != minor || test.patch != patch {
- t.Errorf("parseRelease(%q) = (%v, %v, %v) want (%v, %v, %v)",
- test.in, major, minor, patch,
- test.major, test.minor, test.patch)
- }
- }
-}
diff --git a/src/runtime/sys_linux_386.s b/src/runtime/sys_linux_386.s
index 5b9b638ad7..1e3a834812 100644
--- a/src/runtime/sys_linux_386.s
+++ b/src/runtime/sys_linux_386.s
@@ -39,8 +39,6 @@
#define SYS_socketcall 102
#define SYS_setittimer 104
#define SYS_clone 120
-#define SYS_uname 122
-#define SYS_mlock 150
#define SYS_sched_yield 158
#define SYS_nanosleep 162
#define SYS_rt_sigreturn 173
@@ -808,20 +806,3 @@ TEXT runtime·sbrk0(SB),NOSPLIT,$0-4
INVOKE_SYSCALL
MOVL AX, ret+0(FP)
RET
-
-// func uname(utsname *new_utsname) int
-TEXT ·uname(SB),NOSPLIT,$0-8
- MOVL $SYS_uname, AX
- MOVL utsname+0(FP), BX
- INVOKE_SYSCALL
- MOVL AX, ret+4(FP)
- RET
-
-// func mlock(addr, len uintptr) int
-TEXT ·mlock(SB),NOSPLIT,$0-12
- MOVL $SYS_mlock, AX
- MOVL addr+0(FP), BX
- MOVL len+4(FP), CX
- INVOKE_SYSCALL
- MOVL AX, ret+8(FP)
- RET
diff --git a/src/runtime/sys_linux_amd64.s b/src/runtime/sys_linux_amd64.s
index fe9c6bce85..b60057ce83 100644
--- a/src/runtime/sys_linux_amd64.s
+++ b/src/runtime/sys_linux_amd64.s
@@ -33,10 +33,8 @@
#define SYS_clone 56
#define SYS_exit 60
#define SYS_kill 62
-#define SYS_uname 63
#define SYS_fcntl 72
#define SYS_sigaltstack 131
-#define SYS_mlock 149
#define SYS_arch_prctl 158
#define SYS_gettid 186
#define SYS_futex 202
@@ -789,20 +787,3 @@ TEXT runtime·sbrk0(SB),NOSPLIT,$0-8
SYSCALL
MOVQ AX, ret+0(FP)
RET
-
-// func uname(utsname *new_utsname) int
-TEXT ·uname(SB),NOSPLIT,$0-16
- MOVQ utsname+0(FP), DI
- MOVL $SYS_uname, AX
- SYSCALL
- MOVQ AX, ret+8(FP)
- RET
-
-// func mlock(addr, len uintptr) int
-TEXT ·mlock(SB),NOSPLIT,$0-24
- MOVQ addr+0(FP), DI
- MOVQ len+8(FP), SI
- MOVL $SYS_mlock, AX
- SYSCALL
- MOVQ AX, ret+16(FP)
- RET
--
cgit v1.3-5-g9baa
From 933ca0cfdc5b16a28661707b95485ce6d739bb97 Mon Sep 17 00:00:00 2001
From: Than McIntosh
Date: Wed, 12 Aug 2020 12:59:55 -0400
Subject: doc: add a release notes blurb on 1.16 linker improvements
Add a draft version of a blurb on improvements to the linker. This
will need to be finalized later in the release since there are still
some additional changes to be made to the linker in 1.16.
Updates #40703.
Change-Id: Id85c7e129071cc2faacb09c53a2968bd52b0a7b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/248238
Reviewed-by: Cherry Zhang
Reviewed-by: Austin Clements
---
doc/go1.16.html | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/doc/go1.16.html b/doc/go1.16.html
index f0e26a1033..1fea359436 100644
--- a/doc/go1.16.html
+++ b/doc/go1.16.html
@@ -62,7 +62,26 @@ Do not send CLs removing the interior tags from such phrases.
Linker
- TODO
+ This release includes additional improvements to the Go linker,
+ reducing linker resource usage (both time and memory) and improving
+ code robustness/maintainability. These changes form the second half
+ of a two-release project to
+ modernize the Go
+ linker.
+
+
+
+ The linker changes in 1.16 extend the 1.15 improvements to all
+ supported architecture/OS combinations (the 1.15 performance improvements
+ were primarily focused on ELF-based OSes and
+ amd64 architectures). For a representative set of
+ large Go programs, linking is 20-35% faster than 1.15 and requires
+ 5-15% less memory on average for linux/amd64, with larger
+ improvements for other architectures and OSes.
+
+
+
+ TODO: update with final numbers later in the release.
Core library
--
cgit v1.3-5-g9baa
From c2e73fb446bffd02c651e51c6641cc90fd065b70 Mon Sep 17 00:00:00 2001
From: Than McIntosh
Date: Tue, 23 Jun 2020 08:46:36 -0400
Subject: cmd/compile: remove AttrSeenGlobl (use AttrOnList instead)
Minor cleanup: remove the symbol attribute AttrSeenGlobal, since it is
redundant with the existing attribute AttrOnList (no need to have what
amounts to a separate flag for checking the same property).
Change-Id: Ia269b64de37c2bb4a2314bbecf3d2091c6d57424
Reviewed-on: https://go-review.googlesource.com/c/go/+/239477
Run-TryBot: Than McIntosh
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
src/cmd/compile/internal/gc/obj.go | 2 +-
src/cmd/internal/obj/link.go | 3 ---
src/cmd/internal/obj/plist.go | 4 ----
3 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go
index 0826b04e33..af5037c5a8 100644
--- a/src/cmd/compile/internal/gc/obj.go
+++ b/src/cmd/compile/internal/gc/obj.go
@@ -352,7 +352,7 @@ func stringsym(pos src.XPos, s string) (data *obj.LSym) {
symdata := Ctxt.Lookup(symdataname)
- if !symdata.SeenGlobl() {
+ if !symdata.OnList() {
// string data
off := dsname(symdata, 0, s, pos, "string")
ggloblsym(symdata, int32(off), obj.DUPOK|obj.RODATA|obj.LOCAL)
diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index dc47e51be9..311e5ae2e8 100644
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -480,7 +480,6 @@ const (
AttrWrapper
AttrNeedCtxt
AttrNoFrame
- AttrSeenGlobl
AttrOnList
AttrStatic
@@ -537,7 +536,6 @@ func (a Attribute) MakeTypelink() bool { return a&AttrMakeTypelink != 0 }
func (a Attribute) CFunc() bool { return a&AttrCFunc != 0 }
func (a Attribute) NoSplit() bool { return a&AttrNoSplit != 0 }
func (a Attribute) Leaf() bool { return a&AttrLeaf != 0 }
-func (a Attribute) SeenGlobl() bool { return a&AttrSeenGlobl != 0 }
func (a Attribute) OnList() bool { return a&AttrOnList != 0 }
func (a Attribute) ReflectMethod() bool { return a&AttrReflectMethod != 0 }
func (a Attribute) Local() bool { return a&AttrLocal != 0 }
@@ -574,7 +572,6 @@ var textAttrStrings = [...]struct {
{bit: AttrCFunc, s: "CFUNC"},
{bit: AttrNoSplit, s: "NOSPLIT"},
{bit: AttrLeaf, s: "LEAF"},
- {bit: AttrSeenGlobl, s: ""},
{bit: AttrOnList, s: ""},
{bit: AttrReflectMethod, s: "REFLECTMETHOD"},
{bit: AttrLocal, s: "LOCAL"},
diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go
index afe0ee4ee0..6e33f29959 100644
--- a/src/cmd/internal/obj/plist.go
+++ b/src/cmd/internal/obj/plist.go
@@ -145,10 +145,6 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) {
}
func (ctxt *Link) Globl(s *LSym, size int64, flag int) {
- if s.SeenGlobl() {
- fmt.Printf("duplicate %v\n", s)
- }
- s.Set(AttrSeenGlobl, true)
if s.OnList() {
ctxt.Diag("symbol %s listed multiple times", s.Name)
}
--
cgit v1.3-5-g9baa
From 7d7bd5abc7f7ac901830b79496f63ce86895e262 Mon Sep 17 00:00:00 2001
From: Lynn Boger
Date: Tue, 11 Aug 2020 12:04:25 -0400
Subject: cmd/internal/obj/ppc64: don't remove NOP in assembler
Previously, the assembler removed NOPs from the Prog list in
obj9.go. NOPs shouldn't be removed if they were added as
an inline mark, as described in the issue below.
Fixes #40689
Once the NOPs were left in the Prog list, some instructions
were flagged as invalid because they had an operand which was
not represented in optab. In order to preserve the previous
assembler behavior, entries were added to optab for those
operand cases. They were not flagged as errors before because
the NOP instructions were removed before the code to check the
valid opcode/operand combinations.
Change-Id: Iae5145f94459027cf458e914d7c5d6089807ccf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/247842
Run-TryBot: Lynn Boger
TryBot-Result: Gobot Gobot
Reviewed-by: Paul Murphy
Reviewed-by: Michael Munday
Reviewed-by: Keith Randall
---
src/cmd/internal/obj/ppc64/asm9.go | 3 +++
src/cmd/internal/obj/ppc64/obj9.go | 11 +++--------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/cmd/internal/obj/ppc64/asm9.go b/src/cmd/internal/obj/ppc64/asm9.go
index 0fd0744a42..238ca8f0b7 100644
--- a/src/cmd/internal/obj/ppc64/asm9.go
+++ b/src/cmd/internal/obj/ppc64/asm9.go
@@ -613,6 +613,9 @@ var optab = []Optab{
{obj.APCDATA, C_LCON, C_NONE, C_NONE, C_LCON, 0, 0, 0},
{obj.AFUNCDATA, C_SCON, C_NONE, C_NONE, C_ADDR, 0, 0, 0},
{obj.ANOP, C_NONE, C_NONE, C_NONE, C_NONE, 0, 0, 0},
+ {obj.ANOP, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // NOP operand variations added for #40689
+ {obj.ANOP, C_REG, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // to preserve previous behavior
+ {obj.ANOP, C_FREG, C_NONE, C_NONE, C_NONE, 0, 0, 0},
{obj.ADUFFZERO, C_NONE, C_NONE, C_NONE, C_LBRA, 11, 4, 0}, // same as ABR/ABL
{obj.ADUFFCOPY, C_NONE, C_NONE, C_NONE, C_LBRA, 11, 4, 0}, // same as ABR/ABL
{obj.APCALIGN, C_LCON, C_NONE, C_NONE, C_NONE, 0, 0, 0}, // align code
diff --git a/src/cmd/internal/obj/ppc64/obj9.go b/src/cmd/internal/obj/ppc64/obj9.go
index 16881c634b..749f7066de 100644
--- a/src/cmd/internal/obj/ppc64/obj9.go
+++ b/src/cmd/internal/obj/ppc64/obj9.go
@@ -429,7 +429,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
/*
* find leaf subroutines
- * strip NOPs
* expand RET
* expand BECOME pseudo
*/
@@ -559,10 +558,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
q = p
q1 = p.Pcond
if q1 != nil {
- for q1.As == obj.ANOP {
- q1 = q1.Link
- p.Pcond = q1
- }
+ // NOPs are not removed due to #40689.
if q1.Mark&LEAF == 0 {
q1.Mark |= LABEL
@@ -589,9 +585,8 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
continue
case obj.ANOP:
- q1 = p.Link
- q.Link = q1 /* q is non-nop */
- q1.Mark |= p.Mark
+ // NOPs are not removed due to
+ // #40689
continue
default:
--
cgit v1.3-5-g9baa
From cde5fd1c0f8c40804bfd942eec1e2d69bccf4e13 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder
Date: Thu, 13 Aug 2020 12:39:04 -0700
Subject: cmd/compile: correct type of CvtBoolToUint8 values
Fixes #40746
Change-Id: I539f07d1f958dacee87d846171a8889d03182d25
Reviewed-on: https://go-review.googlesource.com/c/go/+/248397
Run-TryBot: Josh Bleecher Snyder
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
src/cmd/compile/internal/ssa/phiopt.go | 2 +-
src/cmd/compile/internal/ssa/prove.go | 2 +-
test/fixedbugs/issue40746.go | 19 +++++++++++++++++++
3 files changed, 21 insertions(+), 2 deletions(-)
create mode 100644 test/fixedbugs/issue40746.go
diff --git a/src/cmd/compile/internal/ssa/phiopt.go b/src/cmd/compile/internal/ssa/phiopt.go
index 8643fa584c..db7b02275c 100644
--- a/src/cmd/compile/internal/ssa/phiopt.go
+++ b/src/cmd/compile/internal/ssa/phiopt.go
@@ -154,7 +154,7 @@ func phioptint(v *Value, b0 *Block, reverse int) {
}
v.AddArg(a)
- cvt := v.Block.NewValue1(v.Pos, OpCvtBoolToUint8, a.Type, a)
+ cvt := v.Block.NewValue1(v.Pos, OpCvtBoolToUint8, v.Block.Func.Config.Types.UInt8, a)
switch v.Type.Size() {
case 1:
v.reset(OpCopy)
diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go
index 6c6be39d34..ce7d689f93 100644
--- a/src/cmd/compile/internal/ssa/prove.go
+++ b/src/cmd/compile/internal/ssa/prove.go
@@ -1334,7 +1334,7 @@ func removeBranch(b *Block, branch branch) {
// isNonNegative reports whether v is known to be greater or equal to zero.
func isNonNegative(v *Value) bool {
if !v.Type.IsInteger() {
- panic("isNonNegative bad type")
+ v.Fatalf("isNonNegative bad type: %v", v.Type)
}
// TODO: return true if !v.Type.IsSigned()
// SSA isn't type-safe enough to do that now (issue 37753).
diff --git a/test/fixedbugs/issue40746.go b/test/fixedbugs/issue40746.go
new file mode 100644
index 0000000000..235282fd90
--- /dev/null
+++ b/test/fixedbugs/issue40746.go
@@ -0,0 +1,19 @@
+// compile
+
+// 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 p
+
+func f(x byte, b bool) byte {
+ var c byte
+ if b {
+ c = 1
+ }
+
+ if int8(c) < 0 {
+ x++
+ }
+ return x
+}
--
cgit v1.3-5-g9baa
From d0d6593d1d4e81acd073244f42b6893fa65c99d8 Mon Sep 17 00:00:00 2001
From: Michael Munday
Date: Mon, 10 Aug 2020 08:01:21 -0700
Subject: cmd/internal/obj: fix inline marker issue on s390x
The optimization that replaces inline markers with pre-existing
instructions assumes that 'Prog' values produced by the compiler are
still reachable after the assembler has run. This was not true on
s390x where the assembler was removing NOP instructions from the
linked list of 'Prog' values. This led to broken inlining data
which in turn caused an infinite loop in the runtime traceback code.
Fix this by stopping the s390x assembler backend removing NOP
values. It does not make any difference to the output of the
assembler because NOP instructions are 0 bytes long anyway.
Fixes #40473.
Change-Id: Ib4fabadd1de8adb80421f75950ee9aad2111147a
Reviewed-on: https://go-review.googlesource.com/c/go/+/247697
Run-TryBot: Michael Munday
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
src/cmd/internal/obj/pcln.go | 15 +++++++++++++++
src/cmd/internal/obj/s390x/objz.go | 11 -----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go
index 1f7ccf47ef..bffeda041d 100644
--- a/src/cmd/internal/obj/pcln.go
+++ b/src/cmd/internal/obj/pcln.go
@@ -278,6 +278,21 @@ func linkpcln(ctxt *Link, cursym *LSym) {
funcpctab(ctxt, &pcln.Pcfile, cursym, "pctofile", pctofileline, pcln)
funcpctab(ctxt, &pcln.Pcline, cursym, "pctoline", pctofileline, nil)
+ // Check that all the Progs used as inline markers are still reachable.
+ // See issue #40473.
+ inlMarkProgs := make(map[*Prog]struct{}, len(cursym.Func.InlMarks))
+ for _, inlMark := range cursym.Func.InlMarks {
+ inlMarkProgs[inlMark.p] = struct{}{}
+ }
+ for p := cursym.Func.Text; p != nil; p = p.Link {
+ if _, ok := inlMarkProgs[p]; ok {
+ delete(inlMarkProgs, p)
+ }
+ }
+ if len(inlMarkProgs) > 0 {
+ ctxt.Diag("one or more instructions used as inline markers are no longer reachable")
+ }
+
pcinlineState := new(pcinlineState)
funcpctab(ctxt, &pcln.Pcinline, cursym, "pctoinline", pcinlineState.pctoinline, nil)
for _, inlMark := range cursym.Func.InlMarks {
diff --git a/src/cmd/internal/obj/s390x/objz.go b/src/cmd/internal/obj/s390x/objz.go
index b14dc810fa..ef6335d849 100644
--- a/src/cmd/internal/obj/s390x/objz.go
+++ b/src/cmd/internal/obj/s390x/objz.go
@@ -283,17 +283,6 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
ACMPUBNE:
q = p
p.Mark |= BRANCH
- if p.Pcond != nil {
- q := p.Pcond
- for q.As == obj.ANOP {
- q = q.Link
- p.Pcond = q
- }
- }
-
- case obj.ANOP:
- q.Link = p.Link /* q is non-nop */
- p.Link.Mark |= p.Mark
default:
q = p
--
cgit v1.3-5-g9baa
From a20cb4ca5c14ff27bdf16989d450c83b22f156d8 Mon Sep 17 00:00:00 2001
From: Tim Möhlmann
Date: Thu, 13 Aug 2020 11:56:31 +0300
Subject: database/sql: make Rows.Scan properly wrap underlying errors
The prior implementation used the format verb %v which unfortunately
improperly wrapped any underlying scanner errors, and we couldn't use
errors.Is nor errors.As. This change fixes that by using the %w verb.
Added a unit to ensure that both error sub string matching works, but
also that errors.Is works as expected.
Fixes #38099
Change-Id: Iea667041dd8081d961246f77f2542330417292dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/248337
Reviewed-by: Emmanuel Odeke
Reviewed-by: Daniel Theophanes
Run-TryBot: Emmanuel Odeke
TryBot-Result: Gobot Gobot
---
src/database/sql/sql.go | 5 ++++-
src/database/sql/sql_test.go | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index b3d0653f5c..0b85db66b9 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -3110,6 +3110,9 @@ func rowsColumnInfoSetupConnLocked(rowsi driver.Rows) []*ColumnType {
// "select cursor(select * from my_table) from dual", into a
// *Rows value that can itself be scanned from. The parent
// select query will close any cursor *Rows if the parent *Rows is closed.
+//
+// If any of the first arguments implementing Scanner returns an error,
+// that error will be wrapped in the returned error
func (rs *Rows) Scan(dest ...interface{}) error {
rs.closemu.RLock()
@@ -3133,7 +3136,7 @@ func (rs *Rows) Scan(dest ...interface{}) error {
for i, sv := range rs.lastcols {
err := convertAssignRows(dest[i], sv, rs)
if err != nil {
- return fmt.Errorf(`sql: Scan error on column index %d, name %q: %v`, i, rs.rowsi.Columns()[i], err)
+ return fmt.Errorf(`sql: Scan error on column index %d, name %q: %w`, i, rs.rowsi.Columns()[i], err)
}
}
return nil
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 5727f0d8aa..762d42f54b 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -4149,6 +4149,41 @@ func TestQueryExecContextOnly(t *testing.T) {
}
}
+type alwaysErrScanner struct{}
+
+var errTestScanWrap = errors.New("errTestScanWrap")
+
+func (alwaysErrScanner) Scan(interface{}) error {
+ return errTestScanWrap
+}
+
+// Issue 38099: Ensure that Rows.Scan properly wraps underlying errors.
+func TestRowsScanProperlyWrapsErrors(t *testing.T) {
+ db := newTestDB(t, "people")
+ defer closeDB(t, db)
+
+ rows, err := db.Query("SELECT|people|age|")
+ if err != nil {
+ t.Fatalf("Query: %v", err)
+ }
+
+ var res alwaysErrScanner
+
+ for rows.Next() {
+ err = rows.Scan(&res)
+ if err == nil {
+ t.Fatal("expecting back an error")
+ }
+ if !errors.Is(err, errTestScanWrap) {
+ t.Fatalf("errors.Is mismatch\n%v\nWant: %v", err, errTestScanWrap)
+ }
+ // Ensure that error substring matching still correctly works.
+ if !strings.Contains(err.Error(), errTestScanWrap.Error()) {
+ t.Fatalf("Error %v does not contain %v", err, errTestScanWrap)
+ }
+ }
+}
+
// badConn implements a bad driver.Conn, for TestBadDriver.
// The Exec method panics.
type badConn struct{}
--
cgit v1.3-5-g9baa
From 5ae198087bd07e88009885ac96c864381f8d8272 Mon Sep 17 00:00:00 2001
From: Jay Conrod
Date: Thu, 4 Jun 2020 11:41:43 -0400
Subject: cmd/go: don't initialize Builder in envcmd.MkEnv
The Builder isn't needed by MkEnv, and Builder.Init doesn't have side
effects that change the environment. Builder.Init does currently call
CheckGOOSARCHPair, but that's being moved out in CL 234658.
Builder.Init creates the temporary work directory used by the
builder. For the builder created in MkEnv, this directory is never
used. Creating this directory can cause unnecessary errors for
commands that don't use a builder like 'go clean' and 'go list'.
Fixes #38395
Updates #24398
Change-Id: Ib93ae55afdf958000470657f4c4ff5bd92700e46
Reviewed-on: https://go-review.googlesource.com/c/go/+/236563
Run-TryBot: Jay Conrod
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Matloob
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/envcmd/env.go | 3 --
src/cmd/go/testdata/script/build_GOTMPDIR.txt | 49 +++++++++++++++++++++++----
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go
index 403e0f4a7b..7bd75f7305 100644
--- a/src/cmd/go/internal/envcmd/env.go
+++ b/src/cmd/go/internal/envcmd/env.go
@@ -63,9 +63,6 @@ var (
)
func MkEnv() []cfg.EnvVar {
- var b work.Builder
- b.Init()
-
envFile, _ := cfg.EnvFile()
env := []cfg.EnvVar{
{Name: "GO111MODULE", Value: cfg.Getenv("GO111MODULE")},
diff --git a/src/cmd/go/testdata/script/build_GOTMPDIR.txt b/src/cmd/go/testdata/script/build_GOTMPDIR.txt
index c93ca932ca..1073517c29 100644
--- a/src/cmd/go/testdata/script/build_GOTMPDIR.txt
+++ b/src/cmd/go/testdata/script/build_GOTMPDIR.txt
@@ -1,15 +1,50 @@
-env GO111MODULE=off
-[short] skip
-
# Set GOCACHE to a clean directory to ensure that 'go build' has work to report.
-env GOCACHE=$WORK/gocache
+[!windows] env GOCACHE=$WORK/gocache
+[windows] env GOCACHE=$WORK\gocache
-# Build should use GOTMPDIR if set.
-env GOTMPDIR=$WORK/my-favorite-tmpdir
+# 'go build' should use GOTMPDIR if set.
+[!windows] env GOTMPDIR=$WORK/my-favorite-tmpdir
+[windows] env GOTMPDIR=$WORK\my-favorite-tmpdir
mkdir $GOTMPDIR
-go build -work hello.go
+go build -x hello.go
stderr ^WORK=.*my-favorite-tmpdir
+# Make GOTMPDIR a regular file. This prevents the creation of work directories,
+# so we can check that certain commands don't create them.
+# This simulates running on a full disk or a read-only volume.
+rm $GOTMPDIR
+cp hello.go $GOTMPDIR # any file will do
+
+# 'go build' should fail if GOTMPDIR is read-only.
+! go build -x .
+stderr '^go: creating work dir: \w+ '$GOTMPDIR
+
+# 'go list' should only fail if it needs to build something.
+go list -x .
+! stderr 'creating work dir'
+stdout m
+go list -m all
+stdout m
+! go list -x -export .
+stderr '^go: creating work dir: \w+ '$GOTMPDIR
+
+# 'go clean -cache' and 'go clean -modcache' should not fail.
+go clean -x -cache
+! stderr 'creating work dir'
+go clean -x -modcache
+! stderr 'creating work dir'
+
+# 'go env' should not fail for specific variables.
+# Without arguments, it needs to initialize a builder to load cgo flags, and
+# that uses a temporary directory.
+! go env
+stderr '^go: creating work dir: \w+ '$GOTMPDIR
+go env GOROOT
+
+-- go.mod --
+module m
+
+go 1.15
-- hello.go --
package main
func main() { println("hello") }
--
cgit v1.3-5-g9baa
From 016e13df7475329c65524b2eabbc5207ceb4ee74 Mon Sep 17 00:00:00 2001
From: Jay Conrod
Date: Mon, 29 Jun 2020 16:28:18 -0400
Subject: cmd/go/internal/modfetch: stop migrating go.modverify to go.sum
go.modverify was renamed to go.sum before vgo was merged into
cmd/go. It's been long enough that we can safely drop support for it.
For #25525
Change-Id: If8da66280a0fb6a4d4db0b170700775523c18571
Reviewed-on: https://go-review.googlesource.com/c/go/+/240458
Run-TryBot: Jay Conrod
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/modfetch/fetch.go | 31 ++++---------------------------
src/cmd/go/testdata/script/mod_verify.txt | 8 +++-----
2 files changed, 7 insertions(+), 32 deletions(-)
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index fd7a5cef83..8df2289097 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -374,12 +374,11 @@ type modSum struct {
var goSum struct {
mu sync.Mutex
- m map[module.Version][]string // content of go.sum file (+ go.modverify if present)
+ m map[module.Version][]string // content of go.sum file
checked map[modSum]bool // sums actually checked during execution
dirty bool // whether we added any new sums to m
overwrite bool // if true, overwrite go.sum without incorporating its contents
enabled bool // whether to use go.sum at all
- modverify string // path to go.modverify, to be deleted
}
// initGoSum initializes the go.sum data.
@@ -403,19 +402,6 @@ func initGoSum() (bool, error) {
goSum.enabled = true
readGoSum(goSum.m, GoSumFile, data)
- // Add old go.modverify file.
- // We'll delete go.modverify in WriteGoSum.
- alt := strings.TrimSuffix(GoSumFile, ".sum") + ".modverify"
- if data, err := renameio.ReadFile(alt); err == nil {
- migrate := make(map[module.Version][]string)
- readGoSum(migrate, alt, data)
- for mod, sums := range migrate {
- for _, sum := range sums {
- addModSumLocked(mod, sum)
- }
- }
- goSum.modverify = alt
- }
return true, nil
}
@@ -616,14 +602,9 @@ func WriteGoSum() {
goSum.mu.Lock()
defer goSum.mu.Unlock()
- if !goSum.enabled {
- // If we haven't read the go.sum file yet, don't bother writing it: at best,
- // we could rename the go.modverify file if it isn't empty, but we haven't
- // needed to touch it so far — how important could it be?
- return
- }
- if !goSum.dirty {
- // Don't bother opening the go.sum file if we don't have anything to add.
+ if !goSum.enabled || !goSum.dirty {
+ // If we haven't read go.sum yet or if we don't have anything to add,
+ // don't bother opening it.
return
}
if cfg.BuildMod == "readonly" {
@@ -674,10 +655,6 @@ func WriteGoSum() {
goSum.checked = make(map[modSum]bool)
goSum.dirty = false
goSum.overwrite = false
-
- if goSum.modverify != "" {
- os.Remove(goSum.modverify) // best effort
- }
}
// TrimGoSum trims go.sum to contain only the modules for which keep[m] is true.
diff --git a/src/cmd/go/testdata/script/mod_verify.txt b/src/cmd/go/testdata/script/mod_verify.txt
index 646bc62bb7..3918400435 100644
--- a/src/cmd/go/testdata/script/mod_verify.txt
+++ b/src/cmd/go/testdata/script/mod_verify.txt
@@ -12,20 +12,18 @@ go mod verify
! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.zip
# With bad go.sum, sync (which must download) fails.
-# Even if the bad sum is in the old legacy go.modverify file.
rm go.sum
-cp go.sum.bad go.modverify
+cp go.sum.bad go.sum
! go mod tidy
stderr 'checksum mismatch'
! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.zip
-# With good go.sum, sync works (and moves go.modverify to go.sum).
+# With good go.sum, sync works.
rm go.sum
-cp go.sum.good go.modverify
+cp go.sum.good go.sum
go mod tidy
exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.zip
exists $GOPATH/pkg/mod/rsc.io/quote@v1.1.0/quote.go
-! exists go.modverify
# go.sum should have the new checksum for go.mod
grep '^rsc.io/quote v1.1.0/go.mod ' go.sum
--
cgit v1.3-5-g9baa
From 9a759593d7a71b4c061fd9bd053bd79584c632dc Mon Sep 17 00:00:00 2001
From: Jay Conrod
Date: Mon, 8 Jun 2020 18:06:11 -0400
Subject: cmd/go: don't save sums for modules loaded for import resolution
modfetch.WriteGoSum now accepts a map[module.Version]bool parameter.
This is used to prevent some new sums from being saved to go.sum when
they would be removed by the next 'go mod tidy'. Previusly, sums were
saved for modules looked up during import resolution.
A new function, modload.TrimGoSum, is also introduced, which marks
sums for deletion. 'go mod tidy' now uses this. The new logic
distinguishes between go.mod sums and content sums, which lets 'go mod
tidy' delete sums for modules in the build graph but not the build
list.
Fixes #31580
Fixes #36260
Fixes #33008
Change-Id: I06c4125704a8bbc9969de05265967ec1d2e6d3e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/237017
Run-TryBot: Jay Conrod
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Matloob
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/modcmd/init.go | 1 +
src/cmd/go/internal/modcmd/tidy.go | 38 +-----------
src/cmd/go/internal/modfetch/fetch.go | 83 +++++++++++++++++++--------
src/cmd/go/internal/modload/init.go | 69 ++++++++++++++++++++--
src/cmd/go/testdata/script/mod_sum_lookup.txt | 33 +++++++++++
src/cmd/go/testdata/script/mod_tidy_old.txt | 46 +++++++++++++++
6 files changed, 204 insertions(+), 66 deletions(-)
create mode 100644 src/cmd/go/testdata/script/mod_sum_lookup.txt
create mode 100644 src/cmd/go/testdata/script/mod_tidy_old.txt
diff --git a/src/cmd/go/internal/modcmd/init.go b/src/cmd/go/internal/modcmd/init.go
index ddb9aeebe9..95063e62f4 100644
--- a/src/cmd/go/internal/modcmd/init.go
+++ b/src/cmd/go/internal/modcmd/init.go
@@ -52,4 +52,5 @@ func runInit(ctx context.Context, cmd *base.Command, args []string) {
base.Fatalf("go mod init: module path must not contain '@'")
}
modload.InitMod() // does all the hard work
+ modload.WriteGoMod()
}
diff --git a/src/cmd/go/internal/modcmd/tidy.go b/src/cmd/go/internal/modcmd/tidy.go
index feb41a83b0..769cd11fe8 100644
--- a/src/cmd/go/internal/modcmd/tidy.go
+++ b/src/cmd/go/internal/modcmd/tidy.go
@@ -9,12 +9,9 @@ package modcmd
import (
"cmd/go/internal/base"
"cmd/go/internal/cfg"
- "cmd/go/internal/modfetch"
"cmd/go/internal/modload"
"cmd/go/internal/work"
"context"
-
- "golang.org/x/mod/module"
)
var cmdTidy = &base.Command{
@@ -45,39 +42,6 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) {
modload.LoadALL()
modload.TidyBuildList()
- modTidyGoSum() // updates memory copy; WriteGoMod on next line flushes it out
+ modload.TrimGoSum()
modload.WriteGoMod()
}
-
-// modTidyGoSum resets the go.sum file content
-// to be exactly what's needed for the current go.mod.
-func modTidyGoSum() {
- // Assuming go.sum already has at least enough from the successful load,
- // we only have to tell modfetch what needs keeping.
- reqs := modload.Reqs()
- keep := make(map[module.Version]bool)
- replaced := make(map[module.Version]bool)
- var walk func(module.Version)
- walk = func(m module.Version) {
- // If we build using a replacement module, keep the sum for the replacement,
- // since that's the code we'll actually use during a build.
- //
- // TODO(golang.org/issue/29182): Perhaps we should keep both sums, and the
- // sums for both sets of transitive requirements.
- r := modload.Replacement(m)
- if r.Path == "" {
- keep[m] = true
- } else {
- keep[r] = true
- replaced[m] = true
- }
- list, _ := reqs.Required(m)
- for _, r := range list {
- if !keep[r] && !replaced[r] {
- walk(r)
- }
- }
- }
- walk(modload.Target)
- modfetch.TrimGoSum(keep)
-}
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index 8df2289097..e40158b535 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -375,12 +375,15 @@ type modSum struct {
var goSum struct {
mu sync.Mutex
m map[module.Version][]string // content of go.sum file
- checked map[modSum]bool // sums actually checked during execution
- dirty bool // whether we added any new sums to m
+ status map[modSum]modSumStatus // state of sums in m
overwrite bool // if true, overwrite go.sum without incorporating its contents
enabled bool // whether to use go.sum at all
}
+type modSumStatus struct {
+ used, dirty bool
+}
+
// initGoSum initializes the go.sum data.
// The boolean it returns reports whether the
// use of go.sum is now enabled.
@@ -394,7 +397,7 @@ func initGoSum() (bool, error) {
}
goSum.m = make(map[module.Version][]string)
- goSum.checked = make(map[modSum]bool)
+ goSum.status = make(map[modSum]modSumStatus)
data, err := lockedfile.Read(GoSumFile)
if err != nil && !os.IsNotExist(err) {
return false, err
@@ -504,6 +507,11 @@ func checkModSum(mod module.Version, h string) error {
return err
}
done := inited && haveModSumLocked(mod, h)
+ if inited {
+ st := goSum.status[modSum{mod, h}]
+ st.used = true
+ goSum.status[modSum{mod, h}] = st
+ }
goSum.mu.Unlock()
if done {
@@ -523,6 +531,9 @@ func checkModSum(mod module.Version, h string) error {
if inited {
goSum.mu.Lock()
addModSumLocked(mod, h)
+ st := goSum.status[modSum{mod, h}]
+ st.dirty = true
+ goSum.status[modSum{mod, h}] = st
goSum.mu.Unlock()
}
return nil
@@ -532,7 +543,6 @@ func checkModSum(mod module.Version, h string) error {
// If it finds a conflicting pair instead, it calls base.Fatalf.
// goSum.mu must be locked.
func haveModSumLocked(mod module.Version, h string) bool {
- goSum.checked[modSum{mod, h}] = true
for _, vh := range goSum.m[mod] {
if h == vh {
return true
@@ -554,7 +564,6 @@ func addModSumLocked(mod module.Version, h string) {
fmt.Fprintf(os.Stderr, "warning: verifying %s@%s: unknown hashes in go.sum: %v; adding %v"+hashVersionMismatch, mod.Path, mod.Version, strings.Join(goSum.m[mod], ", "), h)
}
goSum.m[mod] = append(goSum.m[mod], h)
- goSum.dirty = true
}
// checkSumDB checks the mod, h pair against the Go checksum database.
@@ -598,13 +607,35 @@ func Sum(mod module.Version) string {
}
// WriteGoSum writes the go.sum file if it needs to be updated.
-func WriteGoSum() {
+//
+// keep is used to check whether a newly added sum should be saved in go.sum.
+// It should have entries for both module content sums and go.mod sums
+// (version ends with "/go.mod"). Existing sums will be preserved unless they
+// have been marked for deletion with TrimGoSum.
+func WriteGoSum(keep map[module.Version]bool) {
goSum.mu.Lock()
defer goSum.mu.Unlock()
- if !goSum.enabled || !goSum.dirty {
- // If we haven't read go.sum yet or if we don't have anything to add,
- // don't bother opening it.
+ // If we haven't read the go.sum file yet, don't bother writing it.
+ if !goSum.enabled {
+ return
+ }
+
+ // Check whether we need to add sums for which keep[m] is true or remove
+ // unused sums marked with TrimGoSum. If there are no changes to make,
+ // just return without opening go.sum.
+ dirty := false
+Outer:
+ for m, hs := range goSum.m {
+ for _, h := range hs {
+ st := goSum.status[modSum{m, h}]
+ if st.dirty && (!st.used || keep[m]) {
+ dirty = true
+ break Outer
+ }
+ }
+ }
+ if !dirty {
return
}
if cfg.BuildMod == "readonly" {
@@ -625,9 +656,10 @@ func WriteGoSum() {
// them without good reason.
goSum.m = make(map[module.Version][]string, len(goSum.m))
readGoSum(goSum.m, GoSumFile, data)
- for ms := range goSum.checked {
- addModSumLocked(ms.mod, ms.sum)
- goSum.dirty = true
+ for ms, st := range goSum.status {
+ if st.used {
+ addModSumLocked(ms.mod, ms.sum)
+ }
}
}
@@ -642,7 +674,10 @@ func WriteGoSum() {
list := goSum.m[m]
sort.Strings(list)
for _, h := range list {
- fmt.Fprintf(&buf, "%s %s %s\n", m.Path, m.Version, h)
+ st := goSum.status[modSum{m, h}]
+ if !st.dirty || (st.used && keep[m]) {
+ fmt.Fprintf(&buf, "%s %s %s\n", m.Path, m.Version, h)
+ }
}
}
return buf.Bytes(), nil
@@ -652,12 +687,16 @@ func WriteGoSum() {
base.Fatalf("go: updating go.sum: %v", err)
}
- goSum.checked = make(map[modSum]bool)
- goSum.dirty = false
+ goSum.status = make(map[modSum]modSumStatus)
goSum.overwrite = false
}
-// TrimGoSum trims go.sum to contain only the modules for which keep[m] is true.
+// TrimGoSum trims go.sum to contain only the modules needed for reproducible
+// builds.
+//
+// keep is used to check whether a sum should be retained in go.mod. It should
+// have entries for both module content sums and go.mod sums (version ends
+// with "/go.mod").
func TrimGoSum(keep map[module.Version]bool) {
goSum.mu.Lock()
defer goSum.mu.Unlock()
@@ -669,13 +708,11 @@ func TrimGoSum(keep map[module.Version]bool) {
return
}
- for m := range goSum.m {
- // If we're keeping x@v we also keep x@v/go.mod.
- // Map x@v/go.mod back to x@v for the keep lookup.
- noGoMod := module.Version{Path: m.Path, Version: strings.TrimSuffix(m.Version, "/go.mod")}
- if !keep[m] && !keep[noGoMod] {
- delete(goSum.m, m)
- goSum.dirty = true
+ for m, hs := range goSum.m {
+ if !keep[m] {
+ for _, h := range hs {
+ goSum.status[modSum{m, h}] = modSumStatus{used: false, dirty: true}
+ }
goSum.overwrite = true
}
}
diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go
index 664a2a1594..95334211ef 100644
--- a/src/cmd/go/internal/modload/init.go
+++ b/src/cmd/go/internal/modload/init.go
@@ -331,7 +331,11 @@ func die() {
}
// InitMod sets Target and, if there is a main module, parses the initial build
-// list from its go.mod file, creating and populating that file if needed.
+// list from its go.mod file. If InitMod is called by 'go mod init', InitMod
+// will populate go.mod in memory, possibly importing dependencies from a
+// legacy configuration file. For other commands, InitMod may make other
+// adjustments in memory, like adding a go directive. WriteGoMod should be
+// called later to write changes out to disk.
//
// As a side-effect, InitMod sets a default for cfg.BuildMod if it does not
// already have an explicit value.
@@ -352,7 +356,6 @@ func InitMod() {
// Running go mod init: do legacy module conversion
legacyModInit()
modFileToBuildList()
- WriteGoMod()
return
}
@@ -391,9 +394,6 @@ func InitMod() {
if cfg.BuildMod == "vendor" {
readVendorList()
checkVendorConsistency()
- } else {
- // TODO(golang.org/issue/33326): if cfg.BuildMod != "readonly"?
- WriteGoMod()
}
}
@@ -797,9 +797,10 @@ func WriteGoMod() {
base.Fatalf("go: updates to go.mod needed, disabled by -mod=readonly")
}
}
+
// Always update go.sum, even if we didn't change go.mod: we may have
// downloaded modules that we didn't have before.
- modfetch.WriteGoSum()
+ modfetch.WriteGoSum(keepSums())
if !dirty && cfg.CmdName != "mod tidy" {
// The go.mod file has the same semantic content that it had before
@@ -849,3 +850,59 @@ func WriteGoMod() {
base.Fatalf("go: updating go.mod: %v", err)
}
}
+
+// keepSums returns a set of module sums to preserve in go.sum. The set
+// includes entries for all modules used to load packages (according to
+// the last load function like ImportPaths, LoadALL, etc.). It also contains
+// entries for go.mod files needed for MVS (the version of these entries
+// ends with "/go.mod").
+func keepSums() map[module.Version]bool {
+ // Walk the module graph and keep sums needed by MVS.
+ modkey := func(m module.Version) module.Version {
+ return module.Version{Path: m.Path, Version: m.Version + "/go.mod"}
+ }
+ keep := make(map[module.Version]bool)
+ replaced := make(map[module.Version]bool)
+ reqs := Reqs()
+ var walk func(module.Version)
+ walk = func(m module.Version) {
+ // If we build using a replacement module, keep the sum for the replacement,
+ // since that's the code we'll actually use during a build.
+ //
+ // TODO(golang.org/issue/29182): Perhaps we should keep both sums, and the
+ // sums for both sets of transitive requirements.
+ r := Replacement(m)
+ if r.Path == "" {
+ keep[modkey(m)] = true
+ } else {
+ replaced[m] = true
+ keep[modkey(r)] = true
+ }
+ list, _ := reqs.Required(m)
+ for _, r := range list {
+ if !keep[modkey(r)] && !replaced[r] {
+ walk(r)
+ }
+ }
+ }
+ walk(Target)
+
+ // Add entries for modules that provided packages loaded with ImportPaths,
+ // LoadALL, or similar functions.
+ if loaded != nil {
+ for _, pkg := range loaded.pkgs {
+ m := pkg.mod
+ if r := Replacement(m); r.Path != "" {
+ keep[r] = true
+ } else {
+ keep[m] = true
+ }
+ }
+ }
+
+ return keep
+}
+
+func TrimGoSum() {
+ modfetch.TrimGoSum(keepSums())
+}
diff --git a/src/cmd/go/testdata/script/mod_sum_lookup.txt b/src/cmd/go/testdata/script/mod_sum_lookup.txt
new file mode 100644
index 0000000000..ed80a44984
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_sum_lookup.txt
@@ -0,0 +1,33 @@
+# When we attempt to resolve an import that doesn't exist, we should not save
+# hashes for downloaded modules.
+# Verifies golang.org/issue/36260.
+go list -e -tags=ignore ./noexist
+! exists go.sum
+
+# When an import is resolved successfully, we should only save hashes for
+# the module that provides the package, not for other modules looked up.
+# Verifies golang.org/issue/31580.
+go list ./exist
+grep '^example.com/join v1.1.0 h1:' go.sum
+! grep '^example.com/join/subpkg' go.sum
+cp go.sum go.list.sum
+go mod tidy
+cmp go.sum go.list.sum
+
+-- go.mod --
+module m
+
+go 1.15
+
+-- noexist/use.go --
+// ignore tags prevents errors in 'go mod tidy'
+// +build ignore
+
+package use
+
+import _ "example.com/join/subpkg/noexist"
+
+-- exist/use.go --
+package use
+
+import _ "example.com/join/subpkg"
diff --git a/src/cmd/go/testdata/script/mod_tidy_old.txt b/src/cmd/go/testdata/script/mod_tidy_old.txt
new file mode 100644
index 0000000000..7428f0ce8a
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_tidy_old.txt
@@ -0,0 +1,46 @@
+# 'go mod tidy' should remove content sums for module versions that aren't
+# in the build list. It should preserve go.mod sums for module versions that
+# are in the module graph though.
+# Verifies golang.org/issue/33008.
+go mod tidy
+! grep '^rsc.io/quote v1.5.0 h1:' go.sum
+grep '^rsc.io/quote v1.5.0/go.mod h1:' go.sum
+
+-- go.mod --
+module m
+
+go 1.15
+
+require (
+ rsc.io/quote v1.5.2
+ example.com/r v0.0.0
+)
+
+replace example.com/r => ./r
+
+-- go.sum --
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+rsc.io/quote v1.5.0 h1:6fJa6E+wGadANKkUMlZ0DhXFpoKlslOQDCo259XtdIE=
+rsc.io/quote v1.5.0/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
+rsc.io/quote v1.5.2 h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=
+rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
+rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=
+rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+rsc.io/testonly v1.0.0 h1:K/VWHdO+Jv7woUXG0GzVNx1czBXUt3Ib1deaMn+xk64=
+rsc.io/testonly v1.0.0/go.mod h1:OqmGbIFOcF+XrFReLOGZ6BhMM7uMBiQwZsyNmh74SzY=
+
+-- r/go.mod --
+module example.com/r
+
+require rsc.io/quote v1.5.0
+
+-- use.go --
+package use
+
+import _ "example.com/r"
+
+-- r/use.go --
+package use
+
+import _ "rsc.io/quote"
--
cgit v1.3-5-g9baa
From 8766f96dd72b5d124bf76bf5f88e260a88072683 Mon Sep 17 00:00:00 2001
From: Jay Conrod
Date: Thu, 25 Jun 2020 17:50:38 -0400
Subject: cmd/go: migrate to module.MatchPrefixPatterns
In CL 239797, str.GlobsMatchPath was copied to golang.org/x/mod/module
as MatchPrefixPatterns. This CL updates x/mod, switches calls to use
the new function, and deletes the old function.
For #38725
Change-Id: I7241032228b574aa539426a92d2f5aad9ee001e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/240061
Run-TryBot: Jay Conrod
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Matloob
---
src/cmd/go.mod | 2 +-
src/cmd/go.sum | 4 +-
src/cmd/go/internal/modfetch/insecure.go | 5 ++-
src/cmd/go/internal/modfetch/repo.go | 4 +-
src/cmd/go/internal/modfetch/sumdb.go | 3 +-
src/cmd/go/internal/str/path.go | 45 -----------------------
src/cmd/vendor/golang.org/x/mod/module/module.go | 47 ++++++++++++++++++++++++
src/cmd/vendor/modules.txt | 2 +-
8 files changed, 57 insertions(+), 55 deletions(-)
diff --git a/src/cmd/go.mod b/src/cmd/go.mod
index 6d57ceee79..21670b9996 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-20200622213623-75b288015ac9
- golang.org/x/mod v0.3.0
+ golang.org/x/mod v0.3.1-0.20200625141748-0b26df4a2231
golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 // indirect
golang.org/x/tools v0.0.0-20200616133436-c1934b75d054
golang.org/x/xerrors v0.0.0-20200806184451-1a77d5e9f316 // indirect
diff --git a/src/cmd/go.sum b/src/cmd/go.sum
index 3fc693e3bf..1b5ef515c2 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-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/mod v0.2.0/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/mod v0.3.1-0.20200625141748-0b26df4a2231 h1:R11LxkoUvECaAHdM5/ZOevSR7n+016EgTw8nbE1l+XM=
+golang.org/x/mod v0.3.1-0.20200625141748-0b26df4a2231/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/go/internal/modfetch/insecure.go b/src/cmd/go/internal/modfetch/insecure.go
index 8420432d6c..b692669cba 100644
--- a/src/cmd/go/internal/modfetch/insecure.go
+++ b/src/cmd/go/internal/modfetch/insecure.go
@@ -7,10 +7,11 @@ package modfetch
import (
"cmd/go/internal/cfg"
"cmd/go/internal/get"
- "cmd/go/internal/str"
+
+ "golang.org/x/mod/module"
)
// allowInsecure reports whether we are allowed to fetch this path in an insecure manner.
func allowInsecure(path string) bool {
- return get.Insecure || str.GlobsMatchPath(cfg.GOINSECURE, path)
+ return get.Insecure || module.MatchPrefixPatterns(cfg.GOINSECURE, path)
}
diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go
index f03bdd8d03..34f805d58a 100644
--- a/src/cmd/go/internal/modfetch/repo.go
+++ b/src/cmd/go/internal/modfetch/repo.go
@@ -16,9 +16,9 @@ import (
"cmd/go/internal/get"
"cmd/go/internal/modfetch/codehost"
"cmd/go/internal/par"
- "cmd/go/internal/str"
web "cmd/go/internal/web"
+ "golang.org/x/mod/module"
"golang.org/x/mod/semver"
)
@@ -217,7 +217,7 @@ func lookup(proxy, path string) (r Repo, err error) {
return nil, errLookupDisabled
}
- if str.GlobsMatchPath(cfg.GONOPROXY, path) {
+ if module.MatchPrefixPatterns(cfg.GONOPROXY, path) {
switch proxy {
case "noproxy", "direct":
return lookupDirect(path)
diff --git a/src/cmd/go/internal/modfetch/sumdb.go b/src/cmd/go/internal/modfetch/sumdb.go
index 7973f47426..783c4a433b 100644
--- a/src/cmd/go/internal/modfetch/sumdb.go
+++ b/src/cmd/go/internal/modfetch/sumdb.go
@@ -24,7 +24,6 @@ import (
"cmd/go/internal/cfg"
"cmd/go/internal/get"
"cmd/go/internal/lockedfile"
- "cmd/go/internal/str"
"cmd/go/internal/web"
"golang.org/x/mod/module"
@@ -34,7 +33,7 @@ import (
// useSumDB reports whether to use the Go checksum database for the given module.
func useSumDB(mod module.Version) bool {
- return cfg.GOSUMDB != "off" && !get.Insecure && !str.GlobsMatchPath(cfg.GONOSUMDB, mod.Path)
+ return cfg.GOSUMDB != "off" && !get.Insecure && !module.MatchPrefixPatterns(cfg.GONOSUMDB, mod.Path)
}
// lookupSumDB returns the Go checksum database's go.sum lines for the given module,
diff --git a/src/cmd/go/internal/str/path.go b/src/cmd/go/internal/str/path.go
index 95d91a3332..51ab2af82b 100644
--- a/src/cmd/go/internal/str/path.go
+++ b/src/cmd/go/internal/str/path.go
@@ -5,7 +5,6 @@
package str
import (
- "path"
"path/filepath"
"strings"
)
@@ -50,47 +49,3 @@ func HasFilePathPrefix(s, prefix string) bool {
return s[len(prefix)] == filepath.Separator && s[:len(prefix)] == prefix
}
}
-
-// GlobsMatchPath reports whether any path prefix of target
-// matches one of the glob patterns (as defined by path.Match)
-// in the comma-separated globs list.
-// It ignores any empty or malformed patterns in the list.
-func GlobsMatchPath(globs, target string) bool {
- for globs != "" {
- // Extract next non-empty glob in comma-separated list.
- var glob string
- if i := strings.Index(globs, ","); i >= 0 {
- glob, globs = globs[:i], globs[i+1:]
- } else {
- glob, globs = globs, ""
- }
- if glob == "" {
- continue
- }
-
- // A glob with N+1 path elements (N slashes) needs to be matched
- // against the first N+1 path elements of target,
- // which end just before the N+1'th slash.
- n := strings.Count(glob, "/")
- prefix := target
- // Walk target, counting slashes, truncating at the N+1'th slash.
- for i := 0; i < len(target); i++ {
- if target[i] == '/' {
- if n == 0 {
- prefix = target[:i]
- break
- }
- n--
- }
- }
- if n > 0 {
- // Not enough prefix elements.
- continue
- }
- matched, _ := path.Match(glob, prefix)
- if matched {
- return true
- }
- }
- return false
-}
diff --git a/src/cmd/vendor/golang.org/x/mod/module/module.go b/src/cmd/vendor/golang.org/x/mod/module/module.go
index 6cd37280a8..3a8b080c7b 100644
--- a/src/cmd/vendor/golang.org/x/mod/module/module.go
+++ b/src/cmd/vendor/golang.org/x/mod/module/module.go
@@ -97,6 +97,7 @@ package module
import (
"fmt"
+ "path"
"sort"
"strings"
"unicode"
@@ -716,3 +717,49 @@ func unescapeString(escaped string) (string, bool) {
}
return string(buf), true
}
+
+// MatchPrefixPatterns reports whether any path prefix of target matches one of
+// the glob patterns (as defined by path.Match) in the comma-separated globs
+// list. This implements the algorithm used when matching a module path to the
+// GOPRIVATE environment variable, as described by 'go help module-private'.
+//
+// It ignores any empty or malformed patterns in the list.
+func MatchPrefixPatterns(globs, target string) bool {
+ for globs != "" {
+ // Extract next non-empty glob in comma-separated list.
+ var glob string
+ if i := strings.Index(globs, ","); i >= 0 {
+ glob, globs = globs[:i], globs[i+1:]
+ } else {
+ glob, globs = globs, ""
+ }
+ if glob == "" {
+ continue
+ }
+
+ // A glob with N+1 path elements (N slashes) needs to be matched
+ // against the first N+1 path elements of target,
+ // which end just before the N+1'th slash.
+ n := strings.Count(glob, "/")
+ prefix := target
+ // Walk target, counting slashes, truncating at the N+1'th slash.
+ for i := 0; i < len(target); i++ {
+ if target[i] == '/' {
+ if n == 0 {
+ prefix = target[:i]
+ break
+ }
+ n--
+ }
+ }
+ if n > 0 {
+ // Not enough prefix elements.
+ continue
+ }
+ matched, _ := path.Match(glob, prefix)
+ if matched {
+ return true
+ }
+ }
+ return false
+}
diff --git a/src/cmd/vendor/modules.txt b/src/cmd/vendor/modules.txt
index 21fc78c237..7272f04ff3 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.3.0
+# golang.org/x/mod v0.3.1-0.20200625141748-0b26df4a2231
## explicit
golang.org/x/mod/internal/lazyregexp
golang.org/x/mod/modfile
--
cgit v1.3-5-g9baa
From 02a7b4b4a70d0574f82776309feaf28f109f5399 Mon Sep 17 00:00:00 2001
From: Jay Conrod
Date: Tue, 30 Jun 2020 13:48:15 -0400
Subject: cmd/go/internal/modload: don't initialize build cache
modload.Init initialized the build cache with the intent of providing
a better error message in Go 1.12, when the build cache became
mandatory (in module mode, packages aren't installed outside the build
cache). Unfortunately, this didn't provide a more descriptive error
(the cache calls base.Fatalf with its own message), and it caused
errors for commands that don't use the cache (like 'go mod edit').
This CL removes the cache initialization from modload.Init. The
builder will initialize it when it's needed.
For #39882
Change-Id: Ibc01ae4e59358dcd08a07ffc97bf556514d0366f
Reviewed-on: https://go-review.googlesource.com/c/go/+/240548
Run-TryBot: Jay Conrod
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
Reviewed-by: Michael Matloob
---
src/cmd/go/internal/modload/init.go | 7 ----
.../go/testdata/script/build_cache_disabled.txt | 46 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 7 deletions(-)
create mode 100644 src/cmd/go/testdata/script/build_cache_disabled.txt
diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go
index 95334211ef..fff060e665 100644
--- a/src/cmd/go/internal/modload/init.go
+++ b/src/cmd/go/internal/modload/init.go
@@ -20,7 +20,6 @@ import (
"strings"
"cmd/go/internal/base"
- "cmd/go/internal/cache"
"cmd/go/internal/cfg"
"cmd/go/internal/load"
"cmd/go/internal/lockedfile"
@@ -162,12 +161,6 @@ func Init() {
// We're in module mode. Install the hooks to make it work.
- if c := cache.Default(); c == nil {
- // With modules, there are no install locations for packages
- // other than the build cache.
- base.Fatalf("go: cannot use modules with build cache disabled")
- }
-
list := filepath.SplitList(cfg.BuildContext.GOPATH)
if len(list) == 0 || list[0] == "" {
base.Fatalf("missing $GOPATH")
diff --git a/src/cmd/go/testdata/script/build_cache_disabled.txt b/src/cmd/go/testdata/script/build_cache_disabled.txt
new file mode 100644
index 0000000000..2e1327880b
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_cache_disabled.txt
@@ -0,0 +1,46 @@
+# The build cache is required to build anything. It also may be needed to
+# initialize the build system, which is needed for commands like 'go env'.
+# However, there are lots of commands the cache is not needed for, and we
+# shouldn't require it when it won't be used.
+#
+# TODO(golang.org/issue/39882): commands below should work, too.
+# * go clean -modcache
+# * go env
+# * go fix
+# * go fmt
+# * go generate
+# * go get -d
+# * go list (without -export or -compiled)
+
+env GOCACHE=off
+
+# Commands that don't completely load packages should work.
+go doc fmt
+stdout Printf
+
+go fmt .
+
+! go tool compile -h
+stderr usage:
+
+go version
+stdout '^go version'
+
+
+# Module commands that don't load packages should work.
+go mod init m
+exists go.mod
+
+go mod edit -require rsc.io/quote@v1.5.2
+
+go mod download rsc.io/quote
+
+go mod graph
+stdout rsc.io/quote
+
+go mod verify
+
+-- main.go --
+package main
+
+func main() {}
--
cgit v1.3-5-g9baa
From 32a84c99e136ed5af0686dbedd31fd7dff40fb38 Mon Sep 17 00:00:00 2001
From: Keith Randall
Date: Sat, 8 Aug 2020 07:58:04 -0700
Subject: cmd/compile: fix live variable computation for deferreturn
Taking the live variable set from the last return point is problematic.
See #40629 for details, but there may not be a return point, or it may
be before the final defer.
Additionally, keeping track of the last call as a *Value doesn't quite
work. If it is dead-code eliminated, the storage for the Value is reused
for some other random instruction. Its live variable information,
if it is available at all, is wrong.
Instead, just mark all the open-defer argument slots as live
throughout the function. (They are already zero-initialized.)
Fixes #40629
Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13
Reviewed-on: https://go-review.googlesource.com/c/go/+/247522
Run-TryBot: Keith Randall
TryBot-Result: Gobot Gobot
Reviewed-by: Dan Scales
---
src/cmd/compile/internal/gc/plive.go | 118 ++++++++---------------------------
src/cmd/compile/internal/gc/ssa.go | 19 +-----
src/cmd/compile/internal/ssa/func.go | 11 +---
test/fixedbugs/issue40629.go | 69 ++++++++++++++++++++
4 files changed, 99 insertions(+), 118 deletions(-)
create mode 100644 test/fixedbugs/issue40629.go
diff --git a/src/cmd/compile/internal/gc/plive.go b/src/cmd/compile/internal/gc/plive.go
index b366c8a4a0..0cb2661997 100644
--- a/src/cmd/compile/internal/gc/plive.go
+++ b/src/cmd/compile/internal/gc/plive.go
@@ -140,24 +140,14 @@ type Liveness struct {
regMaps []liveRegMask
cache progeffectscache
-
- // These are only populated if open-coded defers are being used.
- // List of vars/stack slots storing defer args
- openDeferVars []openDeferVarInfo
- // Map from defer arg OpVarDef to the block where the OpVarDef occurs.
- openDeferVardefToBlockMap map[*Node]*ssa.Block
- // Map of blocks that cannot reach a return or exit (panic)
- nonReturnBlocks map[*ssa.Block]bool
-}
-
-type openDeferVarInfo struct {
- n *Node // Var/stack slot storing a defer arg
- varsIndex int // Index of variable in lv.vars
}
// LivenessMap maps from *ssa.Value to LivenessIndex.
type LivenessMap struct {
vals map[ssa.ID]LivenessIndex
+ // The set of live, pointer-containing variables at the deferreturn
+ // call (only set when open-coded defers are used).
+ deferreturn LivenessIndex
}
func (m *LivenessMap) reset() {
@@ -168,6 +158,7 @@ func (m *LivenessMap) reset() {
delete(m.vals, k)
}
}
+ m.deferreturn = LivenessInvalid
}
func (m *LivenessMap) set(v *ssa.Value, i LivenessIndex) {
@@ -542,7 +533,7 @@ func newliveness(fn *Node, f *ssa.Func, vars []*Node, idx map[*Node]int32, stkpt
if cap(lc.be) >= f.NumBlocks() {
lv.be = lc.be[:f.NumBlocks()]
}
- lv.livenessMap = LivenessMap{lc.livenessMap.vals}
+ lv.livenessMap = LivenessMap{vals: lc.livenessMap.vals, deferreturn: LivenessInvalid}
lc.livenessMap.vals = nil
}
if lv.be == nil {
@@ -893,58 +884,12 @@ func (lv *Liveness) hasStackMap(v *ssa.Value) bool {
func (lv *Liveness) prologue() {
lv.initcache()
- if lv.fn.Func.HasDefer() && !lv.fn.Func.OpenCodedDeferDisallowed() {
- lv.openDeferVardefToBlockMap = make(map[*Node]*ssa.Block)
- for i, n := range lv.vars {
- if n.Name.OpenDeferSlot() {
- lv.openDeferVars = append(lv.openDeferVars, openDeferVarInfo{n: n, varsIndex: i})
- }
- }
-
- // Find any blocks that cannot reach a return or a BlockExit
- // (panic) -- these must be because of an infinite loop.
- reachesRet := make(map[ssa.ID]bool)
- blockList := make([]*ssa.Block, 0, 256)
-
- for _, b := range lv.f.Blocks {
- if b.Kind == ssa.BlockRet || b.Kind == ssa.BlockRetJmp || b.Kind == ssa.BlockExit {
- blockList = append(blockList, b)
- }
- }
-
- for len(blockList) > 0 {
- b := blockList[0]
- blockList = blockList[1:]
- if reachesRet[b.ID] {
- continue
- }
- reachesRet[b.ID] = true
- for _, e := range b.Preds {
- blockList = append(blockList, e.Block())
- }
- }
-
- lv.nonReturnBlocks = make(map[*ssa.Block]bool)
- for _, b := range lv.f.Blocks {
- if !reachesRet[b.ID] {
- lv.nonReturnBlocks[b] = true
- //fmt.Println("No reach ret", lv.f.Name, b.ID, b.Kind)
- }
- }
- }
-
for _, b := range lv.f.Blocks {
be := lv.blockEffects(b)
// Walk the block instructions backward and update the block
// effects with the each prog effects.
for j := len(b.Values) - 1; j >= 0; j-- {
- if b.Values[j].Op == ssa.OpVarDef {
- n := b.Values[j].Aux.(*Node)
- if n.Name.OpenDeferSlot() {
- lv.openDeferVardefToBlockMap[n] = b
- }
- }
pos, e := lv.valueEffects(b.Values[j])
regUevar, regKill := lv.regEffects(b.Values[j])
if e&varkill != 0 {
@@ -961,20 +906,6 @@ func (lv *Liveness) prologue() {
}
}
-// markDeferVarsLive marks each variable storing an open-coded defer arg as
-// specially live in block b if the variable definition dominates block b.
-func (lv *Liveness) markDeferVarsLive(b *ssa.Block, newliveout *varRegVec) {
- // Only force computation of dominators if we have a block where we need
- // to specially mark defer args live.
- sdom := lv.f.Sdom()
- for _, info := range lv.openDeferVars {
- defB := lv.openDeferVardefToBlockMap[info.n]
- if sdom.IsAncestorEq(defB, b) {
- newliveout.vars.Set(int32(info.varsIndex))
- }
- }
-}
-
// Solve the liveness dataflow equations.
func (lv *Liveness) solve() {
// These temporary bitvectors exist to avoid successive allocations and
@@ -1018,23 +949,6 @@ func (lv *Liveness) solve() {
}
}
- if lv.fn.Func.HasDefer() && !lv.fn.Func.OpenCodedDeferDisallowed() &&
- (b.Kind == ssa.BlockExit || lv.nonReturnBlocks[b]) {
- // Open-coded defer args slots must be live
- // everywhere in a function, since a panic can
- // occur (almost) anywhere. Force all appropriate
- // defer arg slots to be live in BlockExit (panic)
- // blocks and in blocks that do not reach a return
- // (because of infinite loop).
- //
- // We are assuming that the defer exit code at
- // BlockReturn/BlockReturnJmp accesses all of the
- // defer args (with pointers), and so keeps them
- // live. This analysis may have to be adjusted if
- // that changes (because of optimizations).
- lv.markDeferVarsLive(b, &newliveout)
- }
-
if !be.liveout.Eq(newliveout) {
change = true
be.liveout.Copy(newliveout)
@@ -1087,6 +1001,17 @@ func (lv *Liveness) epilogue() {
n.Name.SetNeedzero(true)
livedefer.Set(int32(i))
}
+ if n.Name.OpenDeferSlot() {
+ // Open-coded defer args slots must be live
+ // everywhere in a function, since a panic can
+ // occur (almost) anywhere. Because it is live
+ // everywhere, it must be zeroed on entry.
+ livedefer.Set(int32(i))
+ // It was already marked as Needzero when created.
+ if !n.Name.Needzero() {
+ Fatalf("all pointer-containing defer arg slots should have Needzero set")
+ }
+ }
}
}
@@ -1188,6 +1113,17 @@ func (lv *Liveness) epilogue() {
lv.compact(b)
}
+ // If we have an open-coded deferreturn call, make a liveness map for it.
+ if lv.fn.Func.OpenCodedDeferDisallowed() {
+ lv.livenessMap.deferreturn = LivenessInvalid
+ } else {
+ lv.livenessMap.deferreturn = LivenessIndex{
+ stackMapIndex: lv.stackMapSet.add(livedefer),
+ regMapIndex: 0, // entry regMap, containing no live registers
+ isUnsafePoint: false,
+ }
+ }
+
// Done compacting. Throw out the stack map set.
lv.stackMaps = lv.stackMapSet.extractUniqe()
lv.stackMapSet = bvecSet{}
diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index d4d23a2956..5d0098b4e6 100644
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -4318,12 +4318,6 @@ func (s *state) openDeferExit() {
}
}
- if i == len(s.openDefers)-1 {
- // Record the call of the first defer. This will be used
- // to set liveness info for the deferreturn (which is also
- // used for any location that causes a runtime panic)
- s.f.LastDeferExit = call
- }
s.endBlock()
s.startBlock(bEnd)
}
@@ -5807,11 +5801,6 @@ type SSAGenState struct {
// wasm: The number of values on the WebAssembly stack. This is only used as a safeguard.
OnWasmStackSkipped int
-
- // Liveness index for the first function call in the final defer exit code
- // path that we generated. All defer functions and args should be live at
- // this point. This will be used to set the liveness for the deferreturn.
- lastDeferLiveness LivenessIndex
}
// Prog appends a new Prog.
@@ -6056,12 +6045,6 @@ func genssa(f *ssa.Func, pp *Progs) {
// instruction.
s.pp.nextLive = s.livenessMap.Get(v)
- // Remember the liveness index of the first defer call of
- // the last defer exit
- if v.Block.Func.LastDeferExit != nil && v == v.Block.Func.LastDeferExit {
- s.lastDeferLiveness = s.pp.nextLive
- }
-
// Special case for first line in function; move it to the start.
if firstPos != src.NoXPos {
s.SetPos(firstPos)
@@ -6122,7 +6105,7 @@ func genssa(f *ssa.Func, pp *Progs) {
// When doing open-coded defers, generate a disconnected call to
// deferreturn and a return. This will be used to during panic
// recovery to unwind the stack and return back to the runtime.
- s.pp.nextLive = s.lastDeferLiveness
+ s.pp.nextLive = s.livenessMap.deferreturn
gencallret(pp, Deferreturn)
}
diff --git a/src/cmd/compile/internal/ssa/func.go b/src/cmd/compile/internal/ssa/func.go
index 7cf72a8e37..4b9189fb3e 100644
--- a/src/cmd/compile/internal/ssa/func.go
+++ b/src/cmd/compile/internal/ssa/func.go
@@ -33,15 +33,8 @@ type Func struct {
Blocks []*Block // unordered set of all basic blocks (note: not indexable by ID)
Entry *Block // the entry basic block
- // If we are using open-coded defers, this is the first call to a deferred
- // function in the final defer exit sequence that we generated. This call
- // should be after all defer statements, and will have all args, etc. of
- // all defer calls as live. The liveness info of this call will be used
- // for the deferreturn/ret segment generated for functions with open-coded
- // defers.
- LastDeferExit *Value
- bid idAlloc // block ID allocator
- vid idAlloc // value ID allocator
+ bid idAlloc // block ID allocator
+ vid idAlloc // value ID allocator
// Given an environment variable used for debug hash match,
// what file (if any) receives the yes/no logging?
diff --git a/test/fixedbugs/issue40629.go b/test/fixedbugs/issue40629.go
new file mode 100644
index 0000000000..c6ef408f49
--- /dev/null
+++ b/test/fixedbugs/issue40629.go
@@ -0,0 +1,69 @@
+// run
+
+// 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 main
+
+import "fmt"
+
+const N = 40
+
+func main() {
+ var x [N]int // stack-allocated memory
+ for i := range x {
+ x[i] = 0x999
+ }
+
+ // This defer checks to see if x is uncorrupted.
+ defer func(p *[N]int) {
+ recover()
+ for i := range p {
+ if p[i] != 0x999 {
+ for j := range p {
+ fmt.Printf("p[%d]=0x%x\n", j, p[j])
+ }
+ panic("corrupted stack variable")
+ }
+ }
+ }(&x)
+
+ // This defer starts a new goroutine, which will (hopefully)
+ // overwrite x on the garbage stack.
+ defer func() {
+ c := make(chan bool)
+ go func() {
+ useStack(1000)
+ c <- true
+ }()
+ <-c
+
+ }()
+
+ // This defer causes a stack copy.
+ // The old stack is now garbage.
+ defer func() {
+ useStack(1000)
+ }()
+
+ // Trigger a segfault.
+ *g = 0
+
+ // Make the return statement unreachable.
+ // That makes the stack map at the deferreturn call empty.
+ // In particular, the argument to the first defer is not
+ // marked as a pointer, so it doesn't get adjusted
+ // during the stack copy.
+ for {
+ }
+}
+
+var g *int64
+
+func useStack(n int) {
+ if n == 0 {
+ return
+ }
+ useStack(n - 1)
+}
--
cgit v1.3-5-g9baa
From 407bf0ca67200463cdd451937623078e0240335e Mon Sep 17 00:00:00 2001
From: Alexander Klauer
Date: Mon, 29 Jun 2020 16:02:07 +0000
Subject: reflect: add parentheses to properly bind <- in ChanOf’s string
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Adds parentheses so as to properly bind <- to the right most
channel.
This meant that previously given:
ChanOf(<-chan T)
it would mistakenly try to look up the type as
chan <-chan T
instead of
chan (<-chan T)
Fixes #39897
Change-Id: I8564916055f5fadde3382e41fe8820a1071e5f13
GitHub-Last-Rev: f8f2abe8d4c9e3d1414c89cadca8a16ce5cdeab9
GitHub-Pull-Request: golang/go#39898
Reviewed-on: https://go-review.googlesource.com/c/go/+/240280
Reviewed-by: Emmanuel Odeke
Reviewed-by: Keith Randall
Run-TryBot: Emmanuel Odeke
TryBot-Result: Gobot Gobot
---
src/reflect/all_test.go | 16 ++++++++++++++++
src/reflect/type.go | 12 ++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 63f6a92157..6b31568bb9 100644
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -74,6 +74,10 @@ var typeTests = []pair{
{struct{ x ([]int8) }{}, "[]int8"},
{struct{ x (map[string]int32) }{}, "map[string]int32"},
{struct{ x (chan<- string) }{}, "chan<- string"},
+ {struct{ x (chan<- chan string) }{}, "chan<- chan string"},
+ {struct{ x (chan<- <-chan string) }{}, "chan<- <-chan string"},
+ {struct{ x (<-chan <-chan string) }{}, "<-chan <-chan string"},
+ {struct{ x (chan (<-chan string)) }{}, "chan (<-chan string)"},
{struct {
x struct {
c chan *int32
@@ -5491,6 +5495,18 @@ func TestChanOf(t *testing.T) {
// check that type already in binary is found
type T1 int
checkSameType(t, ChanOf(BothDir, TypeOf(T1(1))), (chan T1)(nil))
+
+ // Check arrow token association in undefined chan types.
+ var left chan<- chan T
+ var right chan (<-chan T)
+ tLeft := ChanOf(SendDir, ChanOf(BothDir, TypeOf(T(""))))
+ tRight := ChanOf(BothDir, ChanOf(RecvDir, TypeOf(T(""))))
+ if tLeft != TypeOf(left) {
+ t.Errorf("chan<-chan: have %s, want %T", tLeft, left)
+ }
+ if tRight != TypeOf(right) {
+ t.Errorf("chan<-chan: have %s, want %T", tRight, right)
+ }
}
func TestChanOfDir(t *testing.T) {
diff --git a/src/reflect/type.go b/src/reflect/type.go
index 38b1283d42..44c96fea82 100644
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -1789,7 +1789,6 @@ func ChanOf(dir ChanDir, t Type) Type {
}
// Look in known types.
- // TODO: Precedence when constructing string.
var s string
switch dir {
default:
@@ -1799,7 +1798,16 @@ func ChanOf(dir ChanDir, t Type) Type {
case RecvDir:
s = "<-chan " + typ.String()
case BothDir:
- s = "chan " + typ.String()
+ typeStr := typ.String()
+ if typeStr[0] == '<' {
+ // typ is recv chan, need parentheses as "<-" associates with leftmost
+ // chan possible, see:
+ // * https://golang.org/ref/spec#Channel_types
+ // * https://github.com/golang/go/issues/39897
+ s = "chan (" + typeStr + ")"
+ } else {
+ s = "chan " + typeStr
+ }
}
for _, tt := range typesByString(s) {
ch := (*chanType)(unsafe.Pointer(tt))
--
cgit v1.3-5-g9baa
From 24ff2af65e27eed1e8c7f09c21a5ca68fc2e07ab Mon Sep 17 00:00:00 2001
From: lufia
Date: Sun, 26 Jul 2020 19:04:37 +0900
Subject: cmd/dist: fix typo
Change-Id: Ib5d7f3eadff03070043d52659af4312ee293c586
Reviewed-on: https://go-review.googlesource.com/c/go/+/244817
Reviewed-by: Alberto Donizetti
Run-TryBot: Emmanuel Odeke
TryBot-Result: Gobot Gobot
---
src/cmd/dist/build.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go
index a817e6fcd7..397b3bb88f 100644
--- a/src/cmd/dist/build.go
+++ b/src/cmd/dist/build.go
@@ -1209,7 +1209,7 @@ func timelog(op, name string) {
}
i := strings.Index(s, " start")
if i < 0 {
- log.Fatalf("time log %s does not begin with start line", os.Getenv("GOBULDTIMELOGFILE"))
+ log.Fatalf("time log %s does not begin with start line", os.Getenv("GOBUILDTIMELOGFILE"))
}
t, err := time.Parse(time.UnixDate, s[:i])
if err != nil {
--
cgit v1.3-5-g9baa
From 5a18e0b58ca2d08f3988018a8759207cb64e651a Mon Sep 17 00:00:00 2001
From: Gaurav Singh
Date: Thu, 23 Jul 2020 23:27:05 +0000
Subject: sync: fix goroutine leak for when TestMutexFairness times out
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If the timeout triggers before writing to the done channel, the
goroutine will be blocked waiting for a corresponding read that’s
no longer existent, thus a goroutine leak. This change fixes that by
using a buffered channel instead.
Change-Id: I9cf4067a58bc5a729ab31e4426edd78bd359e8e0
GitHub-Last-Rev: a7d811a7be6d875175a894e53d474aa0034e7d2c
GitHub-Pull-Request: golang/go#40236
Reviewed-on: https://go-review.googlesource.com/c/go/+/242902
Reviewed-by: Emmanuel Odeke
Run-TryBot: Emmanuel Odeke
TryBot-Result: Gobot Gobot
---
src/sync/mutex_test.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sync/mutex_test.go b/src/sync/mutex_test.go
index e61a853642..98c1bf2a5f 100644
--- a/src/sync/mutex_test.go
+++ b/src/sync/mutex_test.go
@@ -194,7 +194,7 @@ func TestMutexFairness(t *testing.T) {
}
}
}()
- done := make(chan bool)
+ done := make(chan bool, 1)
go func() {
for i := 0; i < 10; i++ {
time.Sleep(100 * time.Microsecond)
--
cgit v1.3-5-g9baa
From f71444955a4c0962abb334a8f39438466c57a4db Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor
Date: Fri, 14 Aug 2020 14:39:19 -0700
Subject: test: add test case that caused gccgo undefined symbol reference
For #40252
Change-Id: Ie23d2789ca9b4b9081adb39ab64c80c412ad58ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/248637
Run-TryBot: Ian Lance Taylor
Reviewed-by: Cherry Zhang
TryBot-Result: Gobot Gobot
---
test/fixedbugs/issue40252.dir/a.go | 14 ++++++++++++++
test/fixedbugs/issue40252.dir/main.go | 16 ++++++++++++++++
test/fixedbugs/issue40252.go | 8 ++++++++
3 files changed, 38 insertions(+)
create mode 100644 test/fixedbugs/issue40252.dir/a.go
create mode 100644 test/fixedbugs/issue40252.dir/main.go
create mode 100644 test/fixedbugs/issue40252.go
diff --git a/test/fixedbugs/issue40252.dir/a.go b/test/fixedbugs/issue40252.dir/a.go
new file mode 100644
index 0000000000..5519e9331a
--- /dev/null
+++ b/test/fixedbugs/issue40252.dir/a.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.
+
+package a
+
+type I interface {
+ Func()
+}
+
+func Call() {
+ f := I.Func
+ f(nil)
+}
diff --git a/test/fixedbugs/issue40252.dir/main.go b/test/fixedbugs/issue40252.dir/main.go
new file mode 100644
index 0000000000..93f5b70624
--- /dev/null
+++ b/test/fixedbugs/issue40252.dir/main.go
@@ -0,0 +1,16 @@
+// 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 main
+
+import "./a"
+
+func main() {
+ defer func() {
+ if recover() == nil {
+ panic("expected nil pointer dereference")
+ }
+ }()
+ a.Call()
+}
diff --git a/test/fixedbugs/issue40252.go b/test/fixedbugs/issue40252.go
new file mode 100644
index 0000000000..9be4e665d2
--- /dev/null
+++ b/test/fixedbugs/issue40252.go
@@ -0,0 +1,8 @@
+// rundir
+
+// 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.
+
+// gccgo got an undefined symbol reference when inlining a method expression.
+package ignored
--
cgit v1.3-5-g9baa
From 6072d6ee3e173bb46370dabc158e2cb25a6f4877 Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor
Date: Fri, 10 Jul 2020 09:56:00 -0700
Subject: test: add a test case that gccgo fails to compile
Change-Id: If36394e059cdae49834d26ad4ffdd3092a72a0b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/241997
Run-TryBot: Ian Lance Taylor
TryBot-Result: Gobot Gobot
Reviewed-by: Cherry Zhang
---
test/fixedbugs/bug509.go | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 test/fixedbugs/bug509.go
diff --git a/test/fixedbugs/bug509.go b/test/fixedbugs/bug509.go
new file mode 100644
index 0000000000..df6ed61f89
--- /dev/null
+++ b/test/fixedbugs/bug509.go
@@ -0,0 +1,30 @@
+// compile
+
+// 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.
+
+// Gccgo mishandles a couple of alias cases.
+
+package p
+
+type S struct{}
+
+func (*S) M() {}
+
+type I interface {
+ M()
+}
+
+type A = *S
+
+var V1 I
+var _ = V1.(*S)
+var _ = V1.(A)
+
+func F() {
+ var v I
+ v = (*S)(nil)
+ v = A(nil)
+ _ = v
+}
--
cgit v1.3-5-g9baa
From 12d40adac46b5c771247a789205f7893bfd808b2 Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor
Date: Fri, 10 Jul 2020 12:21:41 -0700
Subject: test: add test for conversion of untyped bool to interface
gccgo miscompiled this case.
Updates #40152
Change-Id: I8448c155e802e39d8fc7cda4930ce62cb6363ce5
Reviewed-on: https://go-review.googlesource.com/c/go/+/242000
Run-TryBot: Ian Lance Taylor
TryBot-Result: Gobot Gobot
Reviewed-by: Emmanuel Odeke
Reviewed-by: Cherry Zhang
---
test/fixedbugs/issue40152.go | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 test/fixedbugs/issue40152.go
diff --git a/test/fixedbugs/issue40152.go b/test/fixedbugs/issue40152.go
new file mode 100644
index 0000000000..1cb68e9914
--- /dev/null
+++ b/test/fixedbugs/issue40152.go
@@ -0,0 +1,21 @@
+// run
+
+// 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.
+
+// Gccgo mishandles converting an untyped boolean to an interface type.
+
+package main
+
+func t(args ...interface{}) bool {
+ x := true
+ return x == args[0]
+}
+
+func main() {
+ r := t("x" == "x" && "y" == "y")
+ if !r {
+ panic(r)
+ }
+}
--
cgit v1.3-5-g9baa
From 441b52f5660ccde7848f034ba345d2f0088ea383 Mon Sep 17 00:00:00 2001
From: kakulisen
Date: Thu, 14 May 2020 16:20:58 +0800
Subject: math: simplify the code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Simplifying some code without compromising performance.
My CPU is Intel Xeon Gold 6161, 2.20GHz, 64-bit operating system.
The memory is 8GB. This is my test environment, I hope to help you judge.
Benchmark:
name old time/op new time/op delta
Log1p-4 21.8ns ± 5% 21.8ns ± 4% ~ (p=0.973 n=20+20)
Change-Id: Icd8f96f1325b00007602d114300b92d4c57de409
Reviewed-on: https://go-review.googlesource.com/c/go/+/233940
Reviewed-by: Robert Griesemer
---
src/math/log1p.go | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/math/log1p.go b/src/math/log1p.go
index c4ec61b225..e34e1ff4f2 100644
--- a/src/math/log1p.go
+++ b/src/math/log1p.go
@@ -122,10 +122,7 @@ func log1p(x float64) float64 {
return Inf(1)
}
- absx := x
- if absx < 0 {
- absx = -absx
- }
+ absx := Abs(x)
var f float64
var iu uint64
--
cgit v1.3-5-g9baa
From 0031fa80a3c6685e44e84533edbae0dad0eb0395 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le
Date: Thu, 7 May 2020 00:35:28 +0700
Subject: cmd/compile: another fix initializing blank fields in struct literal
CL 230121 fixed the bug that struct literal blank fields type array/struct
can not be initialized. But it still misses some cases when an expression
causes "candiscard(value)" return false. When these happen, we recursively
call fixedlit with "var_" set to "_", and hit the bug again.
To fix it, just making splitnode return "nblank" whenever "var_" is "nblank".
Fixes #38905
Change-Id: I281941b388acbd551a4d8ca1a235477f8d26fb6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/232617
Run-TryBot: Cuong Manh Le
TryBot-Result: Gobot Gobot
Reviewed-by: Matthew Dempsky
---
src/cmd/compile/internal/gc/sinit.go | 6 +++++-
test/fixedbugs/issue38905.go | 18 ++++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
create mode 100644 test/fixedbugs/issue38905.go
diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go
index 4a2edc7d21..71ed558461 100644
--- a/src/cmd/compile/internal/gc/sinit.go
+++ b/src/cmd/compile/internal/gc/sinit.go
@@ -506,6 +506,7 @@ const (
// fixedlit handles struct, array, and slice literals.
// TODO: expand documentation.
func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) {
+ isBlank := var_ == nblank
var splitnode func(*Node) (a *Node, value *Node)
switch n.Op {
case OARRAYLIT, OSLICELIT:
@@ -520,6 +521,9 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
}
a := nod(OINDEX, var_, nodintconst(k))
k++
+ if isBlank {
+ a = nblank
+ }
return a, r
}
case OSTRUCTLIT:
@@ -527,7 +531,7 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
if r.Op != OSTRUCTKEY {
Fatalf("fixedlit: rhs not OSTRUCTKEY: %v", r)
}
- if r.Sym.IsBlank() {
+ if r.Sym.IsBlank() || isBlank {
return nblank, r.Left
}
setlineno(r)
diff --git a/test/fixedbugs/issue38905.go b/test/fixedbugs/issue38905.go
new file mode 100644
index 0000000000..6f411b8605
--- /dev/null
+++ b/test/fixedbugs/issue38905.go
@@ -0,0 +1,18 @@
+// compile
+
+// 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.
+
+// Make sure that literal value can be passed to struct
+// blank field with expressions where candiscard(value)
+// returns false, see #38905.
+
+package p
+
+type t struct{ _ u }
+type u [10]struct{ f int }
+
+func f(x int) t { return t{u{{1 / x}, {1 % x}}} }
+func g(p *int) t { return t{u{{*p}}} }
+func h(s []int) t { return t{u{{s[0]}}} }
--
cgit v1.3-5-g9baa
From 82c45eb68187d7827bca392d528dbfa06607e3f0 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le
Date: Mon, 27 Jul 2020 12:08:56 +0700
Subject: cmd/compile: handle OCLOSURE/OCALLPART in mustHeapAlloc check
Currently, generated struct wrapper for closure is not handled in
mustHeapAlloc. That causes compiler crashes when the wrapper struct
is too large for stack, and must be heap allocated instead.
Fixes #39292
Change-Id: I14c1e591681d9d92317bb2396d6cf5207aa93e08
Reviewed-on: https://go-review.googlesource.com/c/go/+/244917
Run-TryBot: Cuong Manh Le
TryBot-Result: Gobot Gobot
Reviewed-by: Matthew Dempsky
---
src/cmd/compile/internal/gc/closure.go | 2 +-
src/cmd/compile/internal/gc/esc.go | 7 +++++++
test/fixedbugs/issue39292.go | 29 +++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 test/fixedbugs/issue39292.go
diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go
index 3bb7bb9834..04fb7d5495 100644
--- a/src/cmd/compile/internal/gc/closure.go
+++ b/src/cmd/compile/internal/gc/closure.go
@@ -526,7 +526,7 @@ func walkpartialcall(n *Node, init *Nodes) *Node {
// Create closure in the form of a composite literal.
// For x.M with receiver (x) type T, the generated code looks like:
//
- // clos = &struct{F uintptr; R T}{M.T·f, x}
+ // clos = &struct{F uintptr; R T}{T.M·f, x}
//
// Like walkclosure above.
diff --git a/src/cmd/compile/internal/gc/esc.go b/src/cmd/compile/internal/gc/esc.go
index f3e9ab78ef..628953741a 100644
--- a/src/cmd/compile/internal/gc/esc.go
+++ b/src/cmd/compile/internal/gc/esc.go
@@ -187,6 +187,13 @@ func mustHeapAlloc(n *Node) bool {
return true
}
+ if n.Op == OCLOSURE && closureType(n).Size() >= maxImplicitStackVarSize {
+ return true
+ }
+ if n.Op == OCALLPART && partialCallType(n).Size() >= maxImplicitStackVarSize {
+ return true
+ }
+
if n.Op == OMAKESLICE && !isSmallMakeSlice(n) {
return true
}
diff --git a/test/fixedbugs/issue39292.go b/test/fixedbugs/issue39292.go
new file mode 100644
index 0000000000..5d6595c234
--- /dev/null
+++ b/test/fixedbugs/issue39292.go
@@ -0,0 +1,29 @@
+// errorcheck -0 -m -l
+
+// 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 p
+
+type t [10000]*int
+
+func (t) f() {
+}
+
+func x() {
+ x := t{}.f // ERROR "t literal.f escapes to heap"
+ x()
+}
+
+func y() {
+ var i int // ERROR "moved to heap: i"
+ y := (&t{&i}).f // ERROR "\(&t literal\).f escapes to heap" "&t literal escapes to heap"
+ y()
+}
+
+func z() {
+ var i int // ERROR "moved to heap: i"
+ z := t{&i}.f // ERROR "t literal.f escapes to heap"
+ z()
+}
--
cgit v1.3-5-g9baa
From 948d324f7d8641a042da46c25417ebabd84e5e78 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le
Date: Wed, 12 Aug 2020 23:12:57 +0700
Subject: cmd/compile: add failing test case for #24305
Updates #24305
Change-Id: Ib0b093e33004a978467cdd1e77186798392d4eca
Reviewed-on: https://go-review.googlesource.com/c/go/+/248217
Run-TryBot: Cuong Manh Le
TryBot-Result: Gobot Gobot
Reviewed-by: Matthew Dempsky
---
test/escape5.go | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/test/escape5.go b/test/escape5.go
index 061e57a069..2ed2023cd2 100644
--- a/test/escape5.go
+++ b/test/escape5.go
@@ -179,6 +179,13 @@ func _() {
u.N()
}
+func fbad24305() {
+ // BAD u should not be heap allocated
+ var u U // ERROR "moved to heap: u"
+ (*U).M(&u)
+ (*U).N(&u)
+}
+
// Issue 24730: taking address in a loop causes unnecessary escape
type T24730 struct {
x [64]byte
--
cgit v1.3-5-g9baa
From 69d34e2c6965f70fe1ead3e7e8ab45ada3267ebc Mon Sep 17 00:00:00 2001
From: Matthew Dempsky
Date: Fri, 14 Aug 2020 21:30:04 -0700
Subject: test: bump array size in fixedbugs/issue39292.go
The previous array length was large enough to exceed
maxImplicitStackSize on 64-bit architectures, but not on 32-bit
architectures.
Fixes #40808.
Change-Id: I69e9abb447454b2e7875ba503a0cb772e965ae31
Reviewed-on: https://go-review.googlesource.com/c/go/+/248680
Run-TryBot: Matthew Dempsky
Reviewed-by: Cuong Manh Le
TryBot-Result: Gobot Gobot
---
test/fixedbugs/issue39292.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/fixedbugs/issue39292.go b/test/fixedbugs/issue39292.go
index 5d6595c234..1be88653e9 100644
--- a/test/fixedbugs/issue39292.go
+++ b/test/fixedbugs/issue39292.go
@@ -6,7 +6,7 @@
package p
-type t [10000]*int
+type t [20000]*int
func (t) f() {
}
--
cgit v1.3-5-g9baa
From c0dded04f7ded5048b44200078a1f723f5e1bcc1 Mon Sep 17 00:00:00 2001
From: "Jason A. Donenfeld"
Date: Tue, 14 Jul 2020 01:41:03 -0600
Subject: runtime: do not explicitly exit on ctrl handler
The default ctrl+c handler should process exits in situations where it
makes sense, like console apps, but not in situations where it doesn't,
like libraries or services. Therefore, we should remove the exit(2) so
that the default handler is used for this. This also uses the more
proper windows exit code of STATUS_CONTROL_C_EXIT, with the base case
handler installed by KernelBase.dll. In particular, this helps in the
case of services, which previously would terminate when receiving
shutdown signals, instead of passing them onward to the service program.
In this CL, contrary to CL 244959, we do not need to special case
services with expensive detection algorithms, or rely on hard-coded
library/archive flags.
Fixes #40167.
Fixes #40074.
Change-Id: I9bf6ed6f65cefeff754d270aa33fa4df8d0b451f
Reviewed-on: https://go-review.googlesource.com/c/go/+/243597
Run-TryBot: Jason A. Donenfeld
TryBot-Result: Gobot Gobot
Reviewed-by: Alex Brainman
Reviewed-by: Jason A. Donenfeld
---
src/runtime/os_windows.go | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index a584ada702..a62e941229 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -1010,11 +1010,6 @@ func ctrlhandler1(_type uint32) uint32 {
if sigsend(s) {
return 1
}
- if !islibrary && !isarchive {
- // Only exit the program if we don't have a DLL.
- // See https://golang.org/issues/35965.
- exit(2) // SIGINT, SIGTERM, etc
- }
return 0
}
--
cgit v1.3-5-g9baa
From ccc951637be806e6e7a3c2c922bf4746b60e7395 Mon Sep 17 00:00:00 2001
From: Alberto Donizetti
Date: Thu, 13 Aug 2020 12:56:57 +0200
Subject: cmd/link: move comma outside quotes
Change-Id: I2ecf8976a6289924ac7bfe7ace129a462537e11d
Reviewed-on: https://go-review.googlesource.com/c/go/+/248339
Reviewed-by: Emmanuel Odeke
Reviewed-by: Than McIntosh
---
src/cmd/link/doc.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cmd/link/doc.go b/src/cmd/link/doc.go
index 219499be0a..604675caec 100644
--- a/src/cmd/link/doc.go
+++ b/src/cmd/link/doc.go
@@ -3,7 +3,7 @@
// license that can be found in the LICENSE file.
/*
-Link, typically invoked as ``go tool link,'' reads the Go archive or object
+Link, typically invoked as ``go tool link'', reads the Go archive or object
for a package main, along with its dependencies, and combines them
into an executable binary.
--
cgit v1.3-5-g9baa
From b6ad2880323191713a5525bae5eb27d62c1d1c35 Mon Sep 17 00:00:00 2001
From: Agniva De Sarker
Date: Sat, 13 Jun 2020 10:37:22 +0530
Subject: net/http: avoid setting body when NoBody is set for js/wasm
When http.NoBody is set, it is equivalent to Body being zero bytes.
We therefore set the body only if it is of length greater than 0.
Manually verified with wasmbrowsertest.
Fixes #36339
Change-Id: I9c108c38f99409f72ea101819af572429505a8ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/237758
Run-TryBot: Agniva De Sarker
TryBot-Result: Gobot Gobot
Reviewed-by: Brad Fitzpatrick
Reviewed-by: Johan Brandhorst
Reviewed-by: Emmanuel Odeke
---
src/net/http/roundtrip_js.go | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/net/http/roundtrip_js.go b/src/net/http/roundtrip_js.go
index 509d229aad..b09923c386 100644
--- a/src/net/http/roundtrip_js.go
+++ b/src/net/http/roundtrip_js.go
@@ -98,9 +98,11 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
return nil, err
}
req.Body.Close()
- buf := uint8Array.New(len(body))
- js.CopyBytesToJS(buf, body)
- opt.Set("body", buf)
+ if len(body) != 0 {
+ buf := uint8Array.New(len(body))
+ js.CopyBytesToJS(buf, body)
+ opt.Set("body", buf)
+ }
}
fetchPromise := js.Global().Call("fetch", req.URL.String(), opt)
--
cgit v1.3-5-g9baa
From cf9b4f63a57b4360be700831781885fc6cf5a0b1 Mon Sep 17 00:00:00 2001
From: Joel Sing
Date: Tue, 5 May 2020 03:15:58 +1000
Subject: runtime: use riscv64 RDTIME instruction
Use the actual RDTIME instruction, rather than a WORD.
Generated code is the same.
Change-Id: I6f6f5a1836eae2d05af34d4a22db2ede4fdcb458
Reviewed-on: https://go-review.googlesource.com/c/go/+/231997
Reviewed-by: Cherry Zhang
Run-TryBot: Cherry Zhang
TryBot-Result: Gobot Gobot
---
src/runtime/asm_riscv64.s | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/runtime/asm_riscv64.s b/src/runtime/asm_riscv64.s
index d7c45a183d..8f6c8773eb 100644
--- a/src/runtime/asm_riscv64.s
+++ b/src/runtime/asm_riscv64.s
@@ -79,7 +79,7 @@ TEXT setg_gcc<>(SB),NOSPLIT,$0-0
// func cputicks() int64
TEXT runtime·cputicks(SB),NOSPLIT,$0-8
- WORD $0xc0102573 // rdtime a0
+ RDTIME A0
MOV A0, ret+0(FP)
RET
--
cgit v1.3-5-g9baa
From d30363062283dcdca4392ef61f13f9b332ca8bc3 Mon Sep 17 00:00:00 2001
From: Joel Sing
Date: Mon, 25 May 2020 03:23:30 +1000
Subject: syscall: support rawVforkSyscall on linux/riscv64
Updates #31936
Change-Id: I7dcb8987d4c306ccc97704b9c1b12313ba8bf242
Reviewed-on: https://go-review.googlesource.com/c/go/+/234960
Reviewed-by: Cherry Zhang
Run-TryBot: Cherry Zhang
TryBot-Result: Gobot Gobot
---
src/syscall/asm_linux_riscv64.s | 22 ++++++++++++++++++++++
src/syscall/exec_linux.go | 6 +++++-
src/syscall/syscall_linux_riscv64.go | 4 +---
3 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/syscall/asm_linux_riscv64.s b/src/syscall/asm_linux_riscv64.s
index ad0b6b17d9..f172dd3d9b 100644
--- a/src/syscall/asm_linux_riscv64.s
+++ b/src/syscall/asm_linux_riscv64.s
@@ -104,6 +104,28 @@ err:
MOV A0, err+72(FP) // errno
RET
+// func rawVforkSyscall(trap, a1 uintptr) (r1, err uintptr)
+TEXT ·rawVforkSyscall(SB),NOSPLIT|NOFRAME,$0-32
+ MOV a1+8(FP), A0
+ MOV ZERO, A1
+ MOV ZERO, A2
+ MOV ZERO, A3
+ MOV ZERO, A4
+ MOV ZERO, A5
+ MOV trap+0(FP), A7 // syscall entry
+ ECALL
+ MOV $-4096, T0
+ BLTU T0, A0, err
+ MOV A0, r1+16(FP) // r1
+ MOV ZERO, err+24(FP) // errno
+ RET
+err:
+ MOV $-1, T0
+ MOV T0, r1+16(FP) // r1
+ SUB A0, ZERO, A0
+ MOV A0, err+24(FP) // errno
+ RET
+
TEXT ·rawSyscallNoError(SB),NOSPLIT,$0-48
MOV a1+8(FP), A0
MOV a2+16(FP), A1
diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index b7351cda82..b6acad96ea 100644
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -207,7 +207,11 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
}
}
- hasRawVforkSyscall := runtime.GOARCH == "amd64" || runtime.GOARCH == "ppc64" || runtime.GOARCH == "s390x" || runtime.GOARCH == "arm64"
+ var hasRawVforkSyscall bool
+ switch runtime.GOARCH {
+ case "amd64", "arm64", "ppc64", "riscv64", "s390x":
+ hasRawVforkSyscall = true
+ }
// About to call fork.
// No more allocation or calls of non-assembly functions.
diff --git a/src/syscall/syscall_linux_riscv64.go b/src/syscall/syscall_linux_riscv64.go
index d54bd38510..088e23439f 100644
--- a/src/syscall/syscall_linux_riscv64.go
+++ b/src/syscall/syscall_linux_riscv64.go
@@ -199,6 +199,4 @@ func Pause() error {
return err
}
-func rawVforkSyscall(trap, a1 uintptr) (r1 uintptr, err Errno) {
- panic("not implemented")
-}
+func rawVforkSyscall(trap, a1 uintptr) (r1 uintptr, err Errno)
--
cgit v1.3-5-g9baa
From c6a11f0dd279f374602794af60c7cde4585a1e6f Mon Sep 17 00:00:00 2001
From: Keith Randall
Date: Tue, 11 Aug 2020 13:04:48 -0700
Subject: crypto,internal/bytealg: fix assembly that clobbers BP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
BP should be callee-save. It will be saved automatically if
there is a nonzero frame size. Otherwise, we need to avoid this register.
Change-Id: If3f551efa42d830c8793d9f0183cb8daad7a2ab5
Reviewed-on: https://go-review.googlesource.com/c/go/+/248260
Run-TryBot: Keith Randall
Reviewed-by: Michael Knyszek
Reviewed-by: Martin Möhrmann
TryBot-Result: Gobot Gobot
---
src/crypto/elliptic/p256_asm_amd64.s | 5 ++--
src/crypto/md5/md5block_amd64.s | 2 +-
src/internal/bytealg/index_amd64.s | 52 ++++++++++++++++++------------------
src/runtime/sys_linux_amd64.s | 8 +++---
4 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/src/crypto/elliptic/p256_asm_amd64.s b/src/crypto/elliptic/p256_asm_amd64.s
index 7afa54a58c..c77b11bcf2 100644
--- a/src/crypto/elliptic/p256_asm_amd64.s
+++ b/src/crypto/elliptic/p256_asm_amd64.s
@@ -1336,7 +1336,7 @@ TEXT p256SubInternal(SB),NOSPLIT,$0
RET
/* ---------------------------------------*/
-TEXT p256MulInternal(SB),NOSPLIT,$0
+TEXT p256MulInternal(SB),NOSPLIT,$8
MOVQ acc4, mul0
MULQ t0
MOVQ mul0, acc0
@@ -1519,7 +1519,7 @@ TEXT p256MulInternal(SB),NOSPLIT,$0
RET
/* ---------------------------------------*/
-TEXT p256SqrInternal(SB),NOSPLIT,$0
+TEXT p256SqrInternal(SB),NOSPLIT,$8
MOVQ acc4, mul0
MULQ acc5
@@ -2345,4 +2345,3 @@ TEXT ·p256PointDoubleAsm(SB),NOSPLIT,$256-48
RET
/* ---------------------------------------*/
-
diff --git a/src/crypto/md5/md5block_amd64.s b/src/crypto/md5/md5block_amd64.s
index 90d932b146..7c7d92d7e8 100644
--- a/src/crypto/md5/md5block_amd64.s
+++ b/src/crypto/md5/md5block_amd64.s
@@ -13,7 +13,7 @@
// Licence: I hereby disclaim the copyright on this code and place it
// in the public domain.
-TEXT ·block(SB),NOSPLIT,$0-32
+TEXT ·block(SB),NOSPLIT,$8-32
MOVQ dig+0(FP), BP
MOVQ p+8(FP), SI
MOVQ p_len+16(FP), DX
diff --git a/src/internal/bytealg/index_amd64.s b/src/internal/bytealg/index_amd64.s
index 4459820801..6193b57239 100644
--- a/src/internal/bytealg/index_amd64.s
+++ b/src/internal/bytealg/index_amd64.s
@@ -8,7 +8,7 @@
TEXT ·Index(SB),NOSPLIT,$0-56
MOVQ a_base+0(FP), DI
MOVQ a_len+8(FP), DX
- MOVQ b_base+24(FP), BP
+ MOVQ b_base+24(FP), R8
MOVQ b_len+32(FP), AX
MOVQ DI, R10
LEAQ ret+48(FP), R11
@@ -17,7 +17,7 @@ TEXT ·Index(SB),NOSPLIT,$0-56
TEXT ·IndexString(SB),NOSPLIT,$0-40
MOVQ a_base+0(FP), DI
MOVQ a_len+8(FP), DX
- MOVQ b_base+16(FP), BP
+ MOVQ b_base+16(FP), R8
MOVQ b_len+24(FP), AX
MOVQ DI, R10
LEAQ ret+32(FP), R11
@@ -26,7 +26,7 @@ TEXT ·IndexString(SB),NOSPLIT,$0-40
// AX: length of string, that we are searching for
// DX: length of string, in which we are searching
// DI: pointer to string, in which we are searching
-// BP: pointer to string, that we are searching for
+// R8: pointer to string, that we are searching for
// R11: address, where to put return value
// Note: We want len in DX and AX, because PCMPESTRI implicitly consumes them
TEXT indexbody<>(SB),NOSPLIT,$0
@@ -37,11 +37,11 @@ TEXT indexbody<>(SB),NOSPLIT,$0
no_sse42:
CMPQ AX, $2
JA _3_or_more
- MOVW (BP), BP
+ MOVW (R8), R8
LEAQ -1(DI)(DX*1), DX
loop2:
MOVW (DI), SI
- CMPW SI,BP
+ CMPW SI,R8
JZ success
ADDQ $1,DI
CMPQ DI,DX
@@ -50,12 +50,12 @@ loop2:
_3_or_more:
CMPQ AX, $3
JA _4_or_more
- MOVW 1(BP), BX
- MOVW (BP), BP
+ MOVW 1(R8), BX
+ MOVW (R8), R8
LEAQ -2(DI)(DX*1), DX
loop3:
MOVW (DI), SI
- CMPW SI,BP
+ CMPW SI,R8
JZ partial_success3
ADDQ $1,DI
CMPQ DI,DX
@@ -72,11 +72,11 @@ partial_success3:
_4_or_more:
CMPQ AX, $4
JA _5_or_more
- MOVL (BP), BP
+ MOVL (R8), R8
LEAQ -3(DI)(DX*1), DX
loop4:
MOVL (DI), SI
- CMPL SI,BP
+ CMPL SI,R8
JZ success
ADDQ $1,DI
CMPQ DI,DX
@@ -87,11 +87,11 @@ _5_or_more:
JA _8_or_more
LEAQ 1(DI)(DX*1), DX
SUBQ AX, DX
- MOVL -4(BP)(AX*1), BX
- MOVL (BP), BP
+ MOVL -4(R8)(AX*1), BX
+ MOVL (R8), R8
loop5to7:
MOVL (DI), SI
- CMPL SI,BP
+ CMPL SI,R8
JZ partial_success5to7
ADDQ $1,DI
CMPQ DI,DX
@@ -108,11 +108,11 @@ partial_success5to7:
_8_or_more:
CMPQ AX, $8
JA _9_or_more
- MOVQ (BP), BP
+ MOVQ (R8), R8
LEAQ -7(DI)(DX*1), DX
loop8:
MOVQ (DI), SI
- CMPQ SI,BP
+ CMPQ SI,R8
JZ success
ADDQ $1,DI
CMPQ DI,DX
@@ -123,11 +123,11 @@ _9_or_more:
JA _16_or_more
LEAQ 1(DI)(DX*1), DX
SUBQ AX, DX
- MOVQ -8(BP)(AX*1), BX
- MOVQ (BP), BP
+ MOVQ -8(R8)(AX*1), BX
+ MOVQ (R8), R8
loop9to15:
MOVQ (DI), SI
- CMPQ SI,BP
+ CMPQ SI,R8
JZ partial_success9to15
ADDQ $1,DI
CMPQ DI,DX
@@ -144,7 +144,7 @@ partial_success9to15:
_16_or_more:
CMPQ AX, $16
JA _17_or_more
- MOVOU (BP), X1
+ MOVOU (R8), X1
LEAQ -15(DI)(DX*1), DX
loop16:
MOVOU (DI), X2
@@ -161,8 +161,8 @@ _17_or_more:
JA _32_or_more
LEAQ 1(DI)(DX*1), DX
SUBQ AX, DX
- MOVOU -16(BP)(AX*1), X0
- MOVOU (BP), X1
+ MOVOU -16(R8)(AX*1), X0
+ MOVOU (R8), X1
loop17to31:
MOVOU (DI), X2
PCMPEQB X1,X2
@@ -188,7 +188,7 @@ partial_success17to31:
_32_or_more:
CMPQ AX, $32
JA _33_to_63
- VMOVDQU (BP), Y1
+ VMOVDQU (R8), Y1
LEAQ -31(DI)(DX*1), DX
loop32:
VMOVDQU (DI), Y2
@@ -203,8 +203,8 @@ loop32:
_33_to_63:
LEAQ 1(DI)(DX*1), DX
SUBQ AX, DX
- VMOVDQU -32(BP)(AX*1), Y0
- VMOVDQU (BP), Y1
+ VMOVDQU -32(R8)(AX*1), Y0
+ VMOVDQU (R8), Y1
loop33to63:
VMOVDQU (DI), Y2
VPCMPEQB Y1, Y2, Y3
@@ -241,10 +241,10 @@ sse42:
// This value was determined experimentally and is the ~same
// on Nehalem (first with SSE42) and Haswell.
JAE _9_or_more
- LEAQ 16(BP), SI
+ LEAQ 16(R8), SI
TESTW $0xff0, SI
JEQ no_sse42
- MOVOU (BP), X1
+ MOVOU (R8), X1
LEAQ -15(DI)(DX*1), SI
MOVQ $16, R9
SUBQ AX, R9 // We advance by 16-len(sep) each iteration, so precalculate it into R9
diff --git a/src/runtime/sys_linux_amd64.s b/src/runtime/sys_linux_amd64.s
index b60057ce83..621c01b365 100644
--- a/src/runtime/sys_linux_amd64.s
+++ b/src/runtime/sys_linux_amd64.s
@@ -212,7 +212,7 @@ TEXT runtime·walltime1(SB),NOSPLIT,$16-12
// due to stack probes inserted to avoid stack/heap collisions.
// See issue #20427.
- MOVQ SP, BP // Save old SP; BP unchanged by C code.
+ MOVQ SP, R12 // Save old SP; R12 unchanged by C code.
get_tls(CX)
MOVQ g(CX), AX
@@ -250,7 +250,7 @@ noswitch:
MOVQ 0(SP), AX // sec
MOVQ 8(SP), DX // nsec
ret:
- MOVQ BP, SP // Restore real SP
+ MOVQ R12, SP // Restore real SP
// Restore vdsoPC, vdsoSP
// We don't worry about being signaled between the two stores.
// If we are not in a signal handler, we'll restore vdsoSP to 0,
@@ -277,7 +277,7 @@ fallback:
TEXT runtime·nanotime1(SB),NOSPLIT,$16-8
// Switch to g0 stack. See comment above in runtime·walltime.
- MOVQ SP, BP // Save old SP; BP unchanged by C code.
+ MOVQ SP, R12 // Save old SP; R12 unchanged by C code.
get_tls(CX)
MOVQ g(CX), AX
@@ -315,7 +315,7 @@ noswitch:
MOVQ 0(SP), AX // sec
MOVQ 8(SP), DX // nsec
ret:
- MOVQ BP, SP // Restore real SP
+ MOVQ R12, SP // Restore real SP
// Restore vdsoPC, vdsoSP
// We don't worry about being signaled between the two stores.
// If we are not in a signal handler, we'll restore vdsoSP to 0,
--
cgit v1.3-5-g9baa
From 01f99b4e9540f34b44e13b25f6dd04b82ac952d9 Mon Sep 17 00:00:00 2001
From: Keith Randall
Date: Tue, 11 Aug 2020 13:07:35 -0700
Subject: cmd/compile: mark DUFFZERO/DUFFCOPY as async unsafe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
These operations are async unsafe on architectures that use
frame pointers.
The reason is they rely on data being safe when stored below the stack
pointer. They do:
45da69: 48 89 6c 24 f0 mov %rbp,-0x10(%rsp)
45da6e: 48 8d 6c 24 f0 lea -0x10(%rsp),%rbp
45da73: e8 7d d0 ff ff callq 45aaf5
45da78: 48 8b 6d 00 mov 0x0(%rbp),%rbp
This dance ensures that inside duffzero, it looks like there is a
proper frame pointer set up, so that stack walkbacks work correctly if
the kernel samples during duffzero.
However, this instruction sequence depends on data not being clobbered
even though it is below the stack pointer.
If there is an async interrupt at any of those last 3 instructions,
and the interrupt decides to insert a call to asyncPreempt, then the
saved frame pointer on the stack gets clobbered. The last instruction
above then restores junk to the frame pointer.
To prevent this, mark these instructions as async unsafe.
(The body of duffzero is already async unsafe, as it is in package runtime.)
Change-Id: I5562e82f9f5bd2fb543dcf2b6b9133d87ff83032
Reviewed-on: https://go-review.googlesource.com/c/go/+/248261
Run-TryBot: Keith Randall
TryBot-Result: Gobot Gobot
Reviewed-by: Martin Möhrmann
Reviewed-by: Cherry Zhang
---
src/cmd/compile/internal/ssa/gen/AMD64Ops.go | 2 ++
src/cmd/compile/internal/ssa/gen/ARM64Ops.go | 2 ++
src/cmd/compile/internal/ssa/opGen.go | 4 ++++
3 files changed, 8 insertions(+)
diff --git a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go
index a3b29049df..e6d66957dd 100644
--- a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go
+++ b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go
@@ -748,6 +748,7 @@ func init() {
clobbers: buildReg("DI"),
},
faultOnNilArg0: true,
+ unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
},
{name: "MOVOconst", reg: regInfo{nil, 0, []regMask{fp}}, typ: "Int128", aux: "Int128", rematerializeable: true},
@@ -786,6 +787,7 @@ func init() {
clobberFlags: true,
faultOnNilArg0: true,
faultOnNilArg1: true,
+ unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
},
// arg0 = destination pointer
diff --git a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go
index b402e35ea6..2424e67e20 100644
--- a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go
+++ b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go
@@ -507,6 +507,7 @@ func init() {
clobbers: buildReg("R20 R30"),
},
faultOnNilArg0: true,
+ unsafePoint: true, // FP maintenance around DUFFZERO can be clobbered by interrupts
},
// large zeroing
@@ -547,6 +548,7 @@ func init() {
},
faultOnNilArg0: true,
faultOnNilArg1: true,
+ unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
},
// large move
diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go
index 9efa1bfcc4..408c855dbd 100644
--- a/src/cmd/compile/internal/ssa/opGen.go
+++ b/src/cmd/compile/internal/ssa/opGen.go
@@ -13119,6 +13119,7 @@ var opcodeTable = [...]opInfo{
auxType: auxInt64,
argLen: 3,
faultOnNilArg0: true,
+ unsafePoint: true,
reg: regInfo{
inputs: []inputInfo{
{0, 128}, // DI
@@ -13196,6 +13197,7 @@ var opcodeTable = [...]opInfo{
clobberFlags: true,
faultOnNilArg0: true,
faultOnNilArg1: true,
+ unsafePoint: true,
reg: regInfo{
inputs: []inputInfo{
{0, 128}, // DI
@@ -20734,6 +20736,7 @@ var opcodeTable = [...]opInfo{
auxType: auxInt64,
argLen: 2,
faultOnNilArg0: true,
+ unsafePoint: true,
reg: regInfo{
inputs: []inputInfo{
{0, 1048576}, // R20
@@ -20760,6 +20763,7 @@ var opcodeTable = [...]opInfo{
argLen: 3,
faultOnNilArg0: true,
faultOnNilArg1: true,
+ unsafePoint: true,
reg: regInfo{
inputs: []inputInfo{
{0, 2097152}, // R21
--
cgit v1.3-5-g9baa
From 8c39bbf9c93f773ab351bbddb4c3dd93e4fddc76 Mon Sep 17 00:00:00 2001
From: Keith Randall
Date: Tue, 11 Aug 2020 13:19:57 -0700
Subject: cmd/compile: stop race instrumentation from clobbering frame pointer
There is an optimization rule that removes calls to racefuncenter and
racefuncexit, if there are no other race calls in the function. The
rule removes the call to racefuncenter, but it does *not* remove the
store of its argument to the outargs section of the frame. If the
outargs section is now size 0 (because the calls to racefuncenter/exit
were the only calls), then that argument store clobbers the frame
pointer instead.
The fix is to remove the argument store when removing the call to
racefuncenter. (Racefuncexit doesn't have an argument.)
Change-Id: I183ec4d92bbb4920200e1be27b7b8f66b89a2a0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/248262
Reviewed-by: Robert Griesemer
Run-TryBot: Keith Randall
TryBot-Result: Gobot Gobot
---
src/cmd/compile/internal/gc/racewalk.go | 2 +-
src/cmd/compile/internal/ssa/rewrite.go | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/cmd/compile/internal/gc/racewalk.go b/src/cmd/compile/internal/gc/racewalk.go
index 6f251377c9..3552617401 100644
--- a/src/cmd/compile/internal/gc/racewalk.go
+++ b/src/cmd/compile/internal/gc/racewalk.go
@@ -42,7 +42,7 @@ var omit_pkgs = []string{
"internal/cpu",
}
-// Only insert racefuncenterfp/racefuncexit into the following packages.
+// Don't insert racefuncenterfp/racefuncexit into the following packages.
// Memory accesses in the packages are either uninteresting or will cause false positives.
var norace_inst_pkgs = []string{"sync", "sync/atomic"}
diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go
index 2152b1675a..e082bb1dfa 100644
--- a/src/cmd/compile/internal/ssa/rewrite.go
+++ b/src/cmd/compile/internal/ssa/rewrite.go
@@ -1379,6 +1379,15 @@ func needRaceCleanup(sym Sym, v *Value) bool {
}
}
}
+ if symNamed(sym, "runtime.racefuncenter") {
+ // If we're removing racefuncenter, remove its argument as well.
+ if v.Args[0].Op != OpStore {
+ return false
+ }
+ mem := v.Args[0].Args[2]
+ v.Args[0].reset(OpCopy)
+ v.Args[0].AddArg(mem)
+ }
return true
}
--
cgit v1.3-5-g9baa
From c810c6db101b53154b06f9ef1ff7455aaff16c36 Mon Sep 17 00:00:00 2001
From: Chirag Sukhala
Date: Sun, 16 Aug 2020 21:49:53 +0000
Subject: doc/articles/wiki: add missing log import to net/http tutorial
The log package is used with the net/http but was not in the import clause.
Change-Id: Ic45b987633adf0ee15defd4d136b5d37027e22b0
GitHub-Last-Rev: e74aff53370569864b7ec8c18617a5d992d34bf2
GitHub-Pull-Request: golang/go#36674
Reviewed-on: https://go-review.googlesource.com/c/go/+/215618
Reviewed-by: Emmanuel Odeke
---
doc/articles/wiki/index.html | 1 +
1 file changed, 1 insertion(+)
diff --git a/doc/articles/wiki/index.html b/doc/articles/wiki/index.html
index 4e3a5deab5..a74a58e317 100644
--- a/doc/articles/wiki/index.html
+++ b/doc/articles/wiki/index.html
@@ -257,6 +257,7 @@ To use the net/http package, it must be imported:
import (
"fmt"
"io/ioutil"
+ "log"
"net/http"
)
--
cgit v1.3-5-g9baa
From f7fc25ed5a16ee7678680ffd0bcc3078cc249e0a Mon Sep 17 00:00:00 2001
From: Nigel Tao
Date: Fri, 14 Aug 2020 00:04:05 +1000
Subject: image/gif: add more writer benchmarks
The two existing benchmarks encode randomized pixels, which isn't very
representative. The two new benchmarks encode a PNG photo as a GIF.
Also rename the benchmarks for consistency.
Also fix the bytes-per-op measure for paletted images, which are 1 (not
4) bytes per pixel.
Also simplify BenchmarkEncodeRandomPaletted (formerly just called
BenchmarkEncode). It doesn't need to generate a random palette (and the
GIF encoder largely doesn't care about the palette's RGBA values).
Use palette.Plan9 instead, a pre-existing 256-element color palette.
Change-Id: I10a6ea4e9590bb0d9f76e8cc0f4a88d43b1d650d
Reviewed-on: https://go-review.googlesource.com/c/go/+/248218
Reviewed-by: David Symonds
---
src/image/gif/writer_test.go | 53 +++++++++++++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/src/image/gif/writer_test.go b/src/image/gif/writer_test.go
index 9b15c8d99d..5d1b2c439e 100644
--- a/src/image/gif/writer_test.go
+++ b/src/image/gif/writer_test.go
@@ -9,6 +9,7 @@ import (
"image"
"image/color"
"image/color/palette"
+ "image/draw"
_ "image/png"
"io/ioutil"
"math/rand"
@@ -656,25 +657,14 @@ func TestEncodeWrappedImage(t *testing.T) {
}
}
-func BenchmarkEncode(b *testing.B) {
+func BenchmarkEncodeRandomPaletted(b *testing.B) {
+ img := image.NewPaletted(image.Rect(0, 0, 640, 480), palette.Plan9)
rnd := rand.New(rand.NewSource(123))
-
- // Restrict to a 256-color paletted image to avoid quantization path.
- palette := make(color.Palette, 256)
- for i := range palette {
- palette[i] = color.RGBA{
- uint8(rnd.Intn(256)),
- uint8(rnd.Intn(256)),
- uint8(rnd.Intn(256)),
- 255,
- }
- }
- img := image.NewPaletted(image.Rect(0, 0, 640, 480), palette)
for i := range img.Pix {
img.Pix[i] = uint8(rnd.Intn(256))
}
- b.SetBytes(640 * 480 * 4)
+ b.SetBytes(640 * 480 * 1)
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
@@ -682,7 +672,7 @@ func BenchmarkEncode(b *testing.B) {
}
}
-func BenchmarkQuantizedEncode(b *testing.B) {
+func BenchmarkEncodeRandomRGBA(b *testing.B) {
img := image.NewRGBA(image.Rect(0, 0, 640, 480))
bo := img.Bounds()
rnd := rand.New(rand.NewSource(123))
@@ -696,6 +686,7 @@ func BenchmarkQuantizedEncode(b *testing.B) {
})
}
}
+
b.SetBytes(640 * 480 * 4)
b.ReportAllocs()
b.ResetTimer()
@@ -703,3 +694,35 @@ func BenchmarkQuantizedEncode(b *testing.B) {
Encode(ioutil.Discard, img, nil)
}
}
+
+func BenchmarkEncodeRealisticPaletted(b *testing.B) {
+ rgba, err := readImg("../testdata/video-001.png")
+ if err != nil {
+ b.Fatalf("readImg: %v", err)
+ }
+ bo := rgba.Bounds()
+ img := image.NewPaletted(bo, palette.Plan9)
+ draw.Draw(img, bo, rgba, bo.Min, draw.Src)
+
+ b.SetBytes(int64(bo.Dx() * bo.Dy() * 1))
+ b.ReportAllocs()
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ {
+ Encode(ioutil.Discard, img, nil)
+ }
+}
+
+func BenchmarkEncodeRealisticRGBA(b *testing.B) {
+ img, err := readImg("../testdata/video-001.png")
+ if err != nil {
+ b.Fatalf("readImg: %v", err)
+ }
+ bo := img.Bounds()
+
+ b.SetBytes(int64(bo.Dx() * bo.Dy() * 4))
+ b.ReportAllocs()
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ {
+ Encode(ioutil.Discard, img, nil)
+ }
+}
--
cgit v1.3-5-g9baa
From 6f99b33c18266a8858af96163de97173bdf6f081 Mon Sep 17 00:00:00 2001
From: Polina Osadcha
Date: Thu, 18 Jun 2020 16:17:13 +0300
Subject: all: replace Replace(..., -1) with ReplaceAll(...)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Change-Id: I8f7cff7a83a9c50bfa3331e8b40e4a6c2e1c0eee
Reviewed-on: https://go-review.googlesource.com/c/go/+/245198
Run-TryBot: Martin Möhrmann
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
doc/progs/run.go | 2 +-
src/cmd/cover/cover_test.go | 2 +-
src/cmd/go/internal/version/version.go | 2 +-
src/runtime/mkpreempt.go | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/doc/progs/run.go b/doc/progs/run.go
index baef3f79f9..8ac75cdcff 100644
--- a/doc/progs/run.go
+++ b/doc/progs/run.go
@@ -105,7 +105,7 @@ func test(tmpdir, file, want string) error {
// Canonicalize output.
out = bytes.TrimRight(out, "\n")
- out = bytes.Replace(out, []byte{'\n'}, []byte{' '}, -1)
+ out = bytes.ReplaceAll(out, []byte{'\n'}, []byte{' '})
// Check the result.
match, err := regexp.Match(want, out)
diff --git a/src/cmd/cover/cover_test.go b/src/cmd/cover/cover_test.go
index 8a56e39011..1c252e6e45 100644
--- a/src/cmd/cover/cover_test.go
+++ b/src/cmd/cover/cover_test.go
@@ -179,7 +179,7 @@ func TestCover(t *testing.T) {
}
lines := bytes.Split(file, []byte("\n"))
for i, line := range lines {
- lines[i] = bytes.Replace(line, []byte("LINE"), []byte(fmt.Sprint(i+1)), -1)
+ lines[i] = bytes.ReplaceAll(line, []byte("LINE"), []byte(fmt.Sprint(i+1)))
}
// Add a function that is not gofmt'ed. This used to cause a crash.
diff --git a/src/cmd/go/internal/version/version.go b/src/cmd/go/internal/version/version.go
index 056db7bf9e..c2de8d326d 100644
--- a/src/cmd/go/internal/version/version.go
+++ b/src/cmd/go/internal/version/version.go
@@ -138,7 +138,7 @@ func scanFile(file string, info os.FileInfo, mustPrint bool) {
fmt.Printf("%s: %s\n", file, vers)
if *versionM && mod != "" {
- fmt.Printf("\t%s\n", strings.Replace(mod[:len(mod)-1], "\n", "\n\t", -1))
+ fmt.Printf("\t%s\n", strings.ReplaceAll(mod[:len(mod)-1], "\n", "\n\t"))
}
}
diff --git a/src/runtime/mkpreempt.go b/src/runtime/mkpreempt.go
index 1fe77663b9..44dea22ef3 100644
--- a/src/runtime/mkpreempt.go
+++ b/src/runtime/mkpreempt.go
@@ -131,7 +131,7 @@ func header(arch string) {
func p(f string, args ...interface{}) {
fmted := fmt.Sprintf(f, args...)
- fmt.Fprintf(out, "\t%s\n", strings.Replace(fmted, "\n", "\n\t", -1))
+ fmt.Fprintf(out, "\t%s\n", strings.ReplaceAll(fmted, "\n", "\n\t"))
}
func label(l string) {
--
cgit v1.3-5-g9baa
From 51ac0f0f4cb432204dee3d434335fd1e61ca8446 Mon Sep 17 00:00:00 2001
From: Polina Osadcha
Date: Thu, 18 Jun 2020 16:17:13 +0300
Subject: strings: optimize Replace by using a strings.Builder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
name old time/op new time/op delta
ReplaceAll 162ns ±26% 134ns ±26% -17.44% (p=0.014 n=10+10)
name old alloc/op new alloc/op delta
ReplaceAll 32.0B ± 0% 16.0B ± 0% -50.00% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
ReplaceAll 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=10+10)
Change-Id: Ia8377141d3adb84c7bd94e511ac8f739915aeb40
Reviewed-on: https://go-review.googlesource.com/c/go/+/245197
Run-TryBot: Martin Möhrmann
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
src/strings/strings.go | 12 ++++++------
src/strings/strings_test.go | 9 +++++++++
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/strings/strings.go b/src/strings/strings.go
index d6f5cea6e6..b429735fea 100644
--- a/src/strings/strings.go
+++ b/src/strings/strings.go
@@ -934,8 +934,8 @@ func Replace(s, old, new string, n int) string {
}
// Apply replacements to buffer.
- t := make([]byte, len(s)+n*(len(new)-len(old)))
- w := 0
+ var b Builder
+ b.Grow(len(s) + n*(len(new)-len(old)))
start := 0
for i := 0; i < n; i++ {
j := start
@@ -947,12 +947,12 @@ func Replace(s, old, new string, n int) string {
} else {
j += Index(s[start:], old)
}
- w += copy(t[w:], s[start:j])
- w += copy(t[w:], new)
+ b.WriteString(s[start:j])
+ b.WriteString(new)
start = j + len(old)
}
- w += copy(t[w:], s[start:])
- return string(t[0:w])
+ b.WriteString(s[start:])
+ return b.String()
}
// ReplaceAll returns a copy of the string s with all
diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go
index c01c4dabc5..09e5b27cc3 100644
--- a/src/strings/strings_test.go
+++ b/src/strings/strings_test.go
@@ -1900,3 +1900,12 @@ func BenchmarkTrimSpace(b *testing.B) {
})
}
}
+
+var stringSink string
+
+func BenchmarkReplaceAll(b *testing.B) {
+ b.ReportAllocs()
+ for i := 0; i < b.N; i++ {
+ stringSink = ReplaceAll("banana", "a", "<>")
+ }
+}
--
cgit v1.3-5-g9baa
From 99f179f55a66f35dc7861fa411b42ed61bd0df31 Mon Sep 17 00:00:00 2001
From: Martin Möhrmann
Date: Mon, 20 Jul 2020 07:57:06 +0200
Subject: fmt: avoid badverb formatting for %q when used with integers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Instead of returning a bad verb error format for runes above
utf8.Maxrune return a quoted utf8.RuneError rune (\ufffd).
This makes the behaviour consistent with the "c" verb and
aligns behaviour to not return bad verb error format when
a verb is applied to the correct argument type.
Fixes #14569
Change-Id: I679485f6bb90ebe408423ab68af16cce38816cd0
Reviewed-on: https://go-review.googlesource.com/c/go/+/248759
Run-TryBot: Martin Möhrmann
TryBot-Result: Gobot Gobot
Reviewed-by: Rob Pike
---
src/fmt/fmt_test.go | 8 ++++----
src/fmt/print.go | 6 +-----
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go
index 6004061020..87fb323809 100644
--- a/src/fmt/fmt_test.go
+++ b/src/fmt/fmt_test.go
@@ -290,11 +290,11 @@ var fmtTests = []struct {
{"%q", '\U00000e00', `'\u0e00'`},
{"%q", '\U0010ffff', `'\U0010ffff'`},
// Runes that are not valid.
- {"%q", int32(-1), "%!q(int32=-1)"},
+ {"%q", int32(-1), `'�'`},
{"%q", 0xDC80, `'�'`},
- {"%q", rune(0x110000), "%!q(int32=1114112)"},
- {"%q", int64(0xFFFFFFFFF), "%!q(int64=68719476735)"},
- {"%q", uint64(0xFFFFFFFFF), "%!q(uint64=68719476735)"},
+ {"%q", rune(0x110000), `'�'`},
+ {"%q", int64(0xFFFFFFFFF), `'�'`},
+ {"%q", uint64(0xFFFFFFFFF), `'�'`},
// width
{"%5s", "abc", " abc"},
diff --git a/src/fmt/print.go b/src/fmt/print.go
index 595869140a..778b5b0938 100644
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -388,11 +388,7 @@ func (p *pp) fmtInteger(v uint64, isSigned bool, verb rune) {
case 'c':
p.fmt.fmtC(v)
case 'q':
- if v <= utf8.MaxRune {
- p.fmt.fmtQc(v)
- } else {
- p.badVerb(verb)
- }
+ p.fmt.fmtQc(v)
case 'U':
p.fmt.fmtUnicode(v)
default:
--
cgit v1.3-5-g9baa
From f979d072d339a24e4938d46588c153587d61af19 Mon Sep 17 00:00:00 2001
From: Martin Möhrmann
Date: Sun, 3 May 2020 16:26:05 +0200
Subject: runtime: avoid memclr call for keys in mapdelete_fast
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Replace memclrHasPointers calls for keys in mapdelete_fast*
functions with direct writes since the key sizes are known
at compile time.
name old time/op new time/op delta
MapDelete/Pointer/100 33.7ns ± 1% 23.7ns ± 2% -29.68% (p=0.000 n=7+9)
MapDelete/Pointer/1000 41.6ns ± 5% 34.9ns ± 4% -16.01% (p=0.000 n=9+10)
MapDelete/Pointer/10000 45.6ns ± 1% 38.2ns ± 2% -16.34% (p=0.000 n=8+10)
Change-Id: Icaac43b520b93c2cf9fd192b822fae7203a7bbf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/231737
Run-TryBot: Martin Möhrmann
TryBot-Result: Gobot Gobot
Reviewed-by: Keith Randall
---
src/runtime/map_fast32.go | 8 ++++++--
src/runtime/map_fast64.go | 8 +++++++-
src/runtime/map_test.go | 22 ++++++++++++++++++++++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/runtime/map_fast32.go b/src/runtime/map_fast32.go
index 534454f3ad..d035ed0386 100644
--- a/src/runtime/map_fast32.go
+++ b/src/runtime/map_fast32.go
@@ -299,8 +299,12 @@ search:
continue
}
// Only clear key if there are pointers in it.
- if t.key.ptrdata != 0 {
- memclrHasPointers(k, t.key.size)
+ // This can only happen if pointers are 32 bit
+ // wide as 64 bit pointers do not fit into a 32 bit key.
+ if sys.PtrSize == 4 && t.key.ptrdata != 0 {
+ // The key must be a pointer as we checked pointers are
+ // 32 bits wide and the key is 32 bits wide also.
+ *(*unsafe.Pointer)(k) = nil
}
e := add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.elemsize))
if t.elem.ptrdata != 0 {
diff --git a/src/runtime/map_fast64.go b/src/runtime/map_fast64.go
index 1669c7cfe9..f1f3927598 100644
--- a/src/runtime/map_fast64.go
+++ b/src/runtime/map_fast64.go
@@ -300,7 +300,13 @@ search:
}
// Only clear key if there are pointers in it.
if t.key.ptrdata != 0 {
- memclrHasPointers(k, t.key.size)
+ if sys.PtrSize == 8 {
+ *(*unsafe.Pointer)(k) = nil
+ } else {
+ // There are three ways to squeeze at one ore more 32 bit pointers into 64 bits.
+ // Just call memclrHasPointers instead of trying to handle all cases here.
+ memclrHasPointers(k, 8)
+ }
}
e := add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.elemsize))
if t.elem.ptrdata != 0 {
diff --git a/src/runtime/map_test.go b/src/runtime/map_test.go
index 1b7ccad6ed..302b3c23c1 100644
--- a/src/runtime/map_test.go
+++ b/src/runtime/map_test.go
@@ -993,6 +993,27 @@ func benchmarkMapDeleteStr(b *testing.B, n int) {
}
}
+func benchmarkMapDeletePointer(b *testing.B, n int) {
+ i2p := make([]*int, n)
+ for i := 0; i < n; i++ {
+ i2p[i] = new(int)
+ }
+ a := make(map[*int]int, n)
+ b.ResetTimer()
+ k := 0
+ for i := 0; i < b.N; i++ {
+ if len(a) == 0 {
+ b.StopTimer()
+ for j := 0; j < n; j++ {
+ a[i2p[j]] = j
+ }
+ k = i
+ b.StartTimer()
+ }
+ delete(a, i2p[i-k])
+ }
+}
+
func runWith(f func(*testing.B, int), v ...int) func(*testing.B) {
return func(b *testing.B) {
for _, n := range v {
@@ -1023,6 +1044,7 @@ func BenchmarkMapDelete(b *testing.B) {
b.Run("Int32", runWith(benchmarkMapDeleteInt32, 100, 1000, 10000))
b.Run("Int64", runWith(benchmarkMapDeleteInt64, 100, 1000, 10000))
b.Run("Str", runWith(benchmarkMapDeleteStr, 100, 1000, 10000))
+ b.Run("Pointer", runWith(benchmarkMapDeletePointer, 100, 1000, 10000))
}
func TestDeferDeleteSlow(t *testing.T) {
--
cgit v1.3-5-g9baa
From 681559e1f10f83a053b4ebab101de3d77ede8353 Mon Sep 17 00:00:00 2001
From: "zero.xu"
Date: Mon, 17 Aug 2020 07:06:32 +0000
Subject: runtime: update comment: modTimer is called by Timer.Reset
Change-Id: I97d0d1343d41b603a68388e496411fb040dc6d66
GitHub-Last-Rev: d11177ad249bd844dd9e7e355eea28596d0b1fa8
GitHub-Pull-Request: golang/go#38625
Reviewed-on: https://go-review.googlesource.com/c/go/+/229767
Reviewed-by: Emmanuel Odeke
---
src/runtime/time.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/runtime/time.go b/src/runtime/time.go
index fdb5066b24..f895bf8443 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -403,7 +403,7 @@ func dodeltimer0(pp *p) {
}
// modtimer modifies an existing timer.
-// This is called by the netpoll code or time.Ticker.Reset.
+// This is called by the netpoll code or time.Ticker.Reset or time.Timer.Reset.
// Reports whether the timer was modified before it was run.
func modtimer(t *timer, when, period int64, f func(interface{}, uintptr), arg interface{}, seq uintptr) bool {
if when < 0 {
--
cgit v1.3-5-g9baa
From 9138a2a67f7f29948f6f608bf904b3605f1b45d0 Mon Sep 17 00:00:00 2001
From: Joel Sing
Date: Tue, 26 May 2020 14:40:44 +1000
Subject: cmd/link: avoid duplicate DT_NEEDED entries
When adding a new library entry, ensure we record it as seen to avoid
adding duplicates of it.
Fixes #39256
Change-Id: Id309adf80c533d78fd485517c18bc9ab5f1d29fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/235257
Run-TryBot: Joel Sing
TryBot-Result: Gobot Gobot
Reviewed-by: Cherry Zhang
---
src/cmd/link/internal/ld/elf.go | 1 +
src/cmd/link/internal/ld/elf_test.go | 55 +++++++++++++++++++++++
src/cmd/link/internal/ld/testdata/issue39256/x.go | 20 +++++++++
src/cmd/link/internal/ld/testdata/issue39256/x.s | 10 +++++
4 files changed, 86 insertions(+)
create mode 100644 src/cmd/link/internal/ld/testdata/issue39256/x.go
create mode 100644 src/cmd/link/internal/ld/testdata/issue39256/x.s
diff --git a/src/cmd/link/internal/ld/elf.go b/src/cmd/link/internal/ld/elf.go
index 957f5081f6..2862f65f9f 100644
--- a/src/cmd/link/internal/ld/elf.go
+++ b/src/cmd/link/internal/ld/elf.go
@@ -2378,6 +2378,7 @@ func elfadddynsym(ldr *loader.Loader, target *Target, syms *ArchSyms, s loader.S
if target.Arch.Family == sys.AMD64 && !cgoeDynamic && dil != "" && !seenlib[dil] {
du := ldr.MakeSymbolUpdater(syms.Dynamic)
Elfwritedynent(target.Arch, du, DT_NEEDED, uint64(dstru.Addstring(dil)))
+ seenlib[dil] = true
}
} else {
diff --git a/src/cmd/link/internal/ld/elf_test.go b/src/cmd/link/internal/ld/elf_test.go
index 8e86beb1ec..37f0e77336 100644
--- a/src/cmd/link/internal/ld/elf_test.go
+++ b/src/cmd/link/internal/ld/elf_test.go
@@ -13,6 +13,7 @@ import (
"os"
"os/exec"
"path/filepath"
+ "runtime"
"testing"
)
@@ -77,3 +78,57 @@ func main() {
t.Fatalf("Unexpected sh info, want greater than 0, got: %d", section.Info)
}
}
+
+func TestNoDuplicateNeededEntries(t *testing.T) {
+ testenv.MustHaveGoBuild(t)
+ testenv.MustHaveCGO(t)
+
+ // run this test on just a small set of platforms (no need to test it
+ // across the board given the nature of the test).
+ pair := runtime.GOOS + "-" + runtime.GOARCH
+ switch pair {
+ case "linux-amd64", "freebsd-amd64", "openbsd-amd64":
+ default:
+ t.Skip("no need for test on " + pair)
+ }
+
+ t.Parallel()
+
+ dir, err := ioutil.TempDir("", "no-dup-needed")
+ if err != nil {
+ t.Fatalf("Failed to create temp dir: %v", err)
+ }
+ defer os.RemoveAll(dir)
+
+ wd, err := os.Getwd()
+ if err != nil {
+ t.Fatalf("Failed to get working directory: %v", err)
+ }
+
+ path := filepath.Join(dir, "x")
+ argv := []string{"build", "-o", path, filepath.Join(wd, "testdata", "issue39256")}
+ out, err := exec.Command(testenv.GoToolPath(t), argv...).CombinedOutput()
+ if err != nil {
+ t.Fatalf("Build failure: %s\n%s\n", err, string(out))
+ }
+
+ f, err := elf.Open(path)
+ if err != nil {
+ t.Fatalf("Failed to open ELF file: %v", err)
+ }
+ libs, err := f.ImportedLibraries()
+ if err != nil {
+ t.Fatalf("Failed to read imported libraries: %v", err)
+ }
+
+ var count int
+ for _, lib := range libs {
+ if lib == "libc.so" {
+ count++
+ }
+ }
+
+ if got, want := count, 1; got != want {
+ t.Errorf("Got %d entries for `libc.so`, want %d", got, want)
+ }
+}
diff --git a/src/cmd/link/internal/ld/testdata/issue39256/x.go b/src/cmd/link/internal/ld/testdata/issue39256/x.go
new file mode 100644
index 0000000000..d8562ad172
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/issue39256/x.go
@@ -0,0 +1,20 @@
+// 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 main
+
+import (
+ _ "unsafe"
+)
+
+//go:cgo_import_dynamic libc_getpid getpid "libc.so"
+//go:cgo_import_dynamic libc_kill kill "libc.so"
+//go:cgo_import_dynamic libc_close close "libc.so"
+//go:cgo_import_dynamic libc_open open "libc.so"
+
+func trampoline()
+
+func main() {
+ trampoline()
+}
diff --git a/src/cmd/link/internal/ld/testdata/issue39256/x.s b/src/cmd/link/internal/ld/testdata/issue39256/x.s
new file mode 100644
index 0000000000..41a54b2e04
--- /dev/null
+++ b/src/cmd/link/internal/ld/testdata/issue39256/x.s
@@ -0,0 +1,10 @@
+// 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.
+
+TEXT ·trampoline(SB),0,$0
+ CALL libc_getpid(SB)
+ CALL libc_kill(SB)
+ CALL libc_open(SB)
+ CALL libc_close(SB)
+ RET
--
cgit v1.3-5-g9baa
From d79350bac73670c04a91b6761d334b810201f6ee Mon Sep 17 00:00:00 2001
From: Tobias Klauser
Date: Mon, 4 May 2020 18:36:31 +0200
Subject: runtime: use hw.ncpuonline sysctl in getncpu on netbsd
Since NetBSD 7, hw.ncpuonline reports the number of CPUs online, while
hw.cpu reports the number of CPUs configured. Try hw.cpuonline first and
fall back to hw.ncpu in case it fails (which is the case on NetBSD
before 7.0).
This follows the behavior on OpenBSD (see CL 161757). Also, Go
in pkgsrc is patched to use hw.cpuonline, so this CL would allow said
patch to be dropped.
Updates #30824
Change-Id: Id1c19dff2c1e4401e6074179fae7c708ba0e3098
Reviewed-on: https://go-review.googlesource.com/c/go/+/231957
Run-TryBot: Tobias Klauser
TryBot-Result: Gobot Gobot
Reviewed-by: Ian Lance Taylor
Reviewed-by: Benny Siegert
---
src/runtime/os_netbsd.go | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/runtime/os_netbsd.go b/src/runtime/os_netbsd.go
index 97106c7b9d..f7f90cedc1 100644
--- a/src/runtime/os_netbsd.go
+++ b/src/runtime/os_netbsd.go
@@ -95,18 +95,28 @@ var sigset_all = sigset{[4]uint32{^uint32(0), ^uint32(0), ^uint32(0), ^uint32(0)
// From NetBSD's
const (
- _CTL_HW = 6
- _HW_NCPU = 3
- _HW_PAGESIZE = 7
+ _CTL_HW = 6
+ _HW_NCPU = 3
+ _HW_PAGESIZE = 7
+ _HW_NCPUONLINE = 16
)
-func getncpu() int32 {
- mib := [2]uint32{_CTL_HW, _HW_NCPU}
- out := uint32(0)
+func sysctlInt(mib []uint32) (int32, bool) {
+ var out int32
nout := unsafe.Sizeof(out)
- ret := sysctl(&mib[0], 2, (*byte)(unsafe.Pointer(&out)), &nout, nil, 0)
- if ret >= 0 {
- return int32(out)
+ ret := sysctl(&mib[0], uint32(len(mib)), (*byte)(unsafe.Pointer(&out)), &nout, nil, 0)
+ if ret < 0 {
+ return 0, false
+ }
+ return out, true
+}
+
+func getncpu() int32 {
+ if n, ok := sysctlInt([]uint32{_CTL_HW, _HW_NCPUONLINE}); ok {
+ return int32(n)
+ }
+ if n, ok := sysctlInt([]uint32{_CTL_HW, _HW_NCPU}); ok {
+ return int32(n)
}
return 1
}
--
cgit v1.3-5-g9baa
From bf512685fee6282f1a50069ef444412bdf59611b Mon Sep 17 00:00:00 2001
From: Cholerae Hu
Date: Wed, 5 Aug 2020 13:52:32 +0800
Subject: syscall: cap RLIMIT_NOFILE soft limit in TestRlimit on darwin
On some machines, kern.maxfilesperproc is 4096. If Rlimit.Cur is larger
than that, Setrlimit will get an errEINVAL.
Fixes #40564.
Change-Id: Ib94303c790a489ff0559c88d41a021e514d18f8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246658
Reviewed-by: Tobias Klauser
Run-TryBot: Tobias Klauser
TryBot-Result: Gobot Gobot
---
src/syscall/syscall_unix_test.go | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/syscall/syscall_unix_test.go b/src/syscall/syscall_unix_test.go
index 13b79ca8d8..3c7982eefe 100644
--- a/src/syscall/syscall_unix_test.go
+++ b/src/syscall/syscall_unix_test.go
@@ -336,11 +336,11 @@ func TestRlimit(t *testing.T) {
}
set := rlimit
set.Cur = set.Max - 1
- if runtime.GOOS == "darwin" && set.Cur > 10240 {
- // The max file limit is 10240, even though
- // the max returned by Getrlimit is 1<<63-1.
- // This is OPEN_MAX in sys/syslimits.h.
- set.Cur = 10240
+ if runtime.GOOS == "darwin" && set.Cur > 4096 {
+ // rlim_min for RLIMIT_NOFILE should be equal to
+ // or lower than kern.maxfilesperproc, which on
+ // some machines are 4096. See #40564.
+ set.Cur = 4096
}
err = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &set)
if err != nil {
@@ -353,8 +353,8 @@ func TestRlimit(t *testing.T) {
}
set = rlimit
set.Cur = set.Max - 1
- if runtime.GOOS == "darwin" && set.Cur > 10240 {
- set.Cur = 10240
+ if runtime.GOOS == "darwin" && set.Cur > 4096 {
+ set.Cur = 4096
}
if set != get {
t.Fatalf("Rlimit: change failed: wanted %#v got %#v", set, get)
--
cgit v1.3-5-g9baa
From dc12d5b0f5e9c1cfec2a8eb6dd7ff3473c36d45c Mon Sep 17 00:00:00 2001
From: Tobias Klauser
Date: Mon, 17 Aug 2020 11:28:26 +0200
Subject: all: add empty line between copyright header and package clause
Makes sure the copyright notice is not interpreted as the package level
godoc.
Change-Id: I2afce7c9d620f19d51ec1438b1d0db1774b57146
Reviewed-on: https://go-review.googlesource.com/c/go/+/248760
Run-TryBot: Tobias Klauser
TryBot-Result: Gobot Gobot
Reviewed-by: Dave Cheney
---
src/cmd/compile/internal/ssa/debug.go | 1 +
src/cmd/compile/internal/ssa/passbm_test.go | 1 +
src/cmd/go/internal/trace/trace.go | 1 +
src/cmd/link/internal/benchmark/bench_test.go | 1 +
src/cmd/link/internal/ld/errors.go | 1 +
src/runtime/closure_test.go | 1 +
src/runtime/map_benchmark_test.go | 1 +
src/runtime/slice_test.go | 1 +
src/sync/cond_test.go | 1 +
test/fixedbugs/issue15281.go | 1 +
10 files changed, 10 insertions(+)
diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go
index 13fe67cbca..6353f72897 100644
--- a/src/cmd/compile/internal/ssa/debug.go
+++ b/src/cmd/compile/internal/ssa/debug.go
@@ -1,6 +1,7 @@
// Copyright 2017 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 ssa
import (
diff --git a/src/cmd/compile/internal/ssa/passbm_test.go b/src/cmd/compile/internal/ssa/passbm_test.go
index eefdbb8722..3fd3eb579b 100644
--- a/src/cmd/compile/internal/ssa/passbm_test.go
+++ b/src/cmd/compile/internal/ssa/passbm_test.go
@@ -1,6 +1,7 @@
// Copyright 2015 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 ssa
import (
diff --git a/src/cmd/go/internal/trace/trace.go b/src/cmd/go/internal/trace/trace.go
index 7cb7636a34..c8fac92c9f 100644
--- a/src/cmd/go/internal/trace/trace.go
+++ b/src/cmd/go/internal/trace/trace.go
@@ -1,6 +1,7 @@
// 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 trace
import (
diff --git a/src/cmd/link/internal/benchmark/bench_test.go b/src/cmd/link/internal/benchmark/bench_test.go
index d8ec717c7c..419dc55724 100644
--- a/src/cmd/link/internal/benchmark/bench_test.go
+++ b/src/cmd/link/internal/benchmark/bench_test.go
@@ -1,6 +1,7 @@
// 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 benchmark
import (
diff --git a/src/cmd/link/internal/ld/errors.go b/src/cmd/link/internal/ld/errors.go
index c5ce097fde..d6e8ff236d 100644
--- a/src/cmd/link/internal/ld/errors.go
+++ b/src/cmd/link/internal/ld/errors.go
@@ -1,6 +1,7 @@
// 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 ld
import (
diff --git a/src/runtime/closure_test.go b/src/runtime/closure_test.go
index ea65fbd5f5..741c932eab 100644
--- a/src/runtime/closure_test.go
+++ b/src/runtime/closure_test.go
@@ -1,6 +1,7 @@
// Copyright 2011 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 runtime_test
import "testing"
diff --git a/src/runtime/map_benchmark_test.go b/src/runtime/map_benchmark_test.go
index 893cb6c5b6..d0becc9ddb 100644
--- a/src/runtime/map_benchmark_test.go
+++ b/src/runtime/map_benchmark_test.go
@@ -1,6 +1,7 @@
// Copyright 2013 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 runtime_test
import (
diff --git a/src/runtime/slice_test.go b/src/runtime/slice_test.go
index e963a43dd3..cd2bc26d1e 100644
--- a/src/runtime/slice_test.go
+++ b/src/runtime/slice_test.go
@@ -1,6 +1,7 @@
// Copyright 2011 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 runtime_test
import (
diff --git a/src/sync/cond_test.go b/src/sync/cond_test.go
index 9d0d9adc74..859cae59bc 100644
--- a/src/sync/cond_test.go
+++ b/src/sync/cond_test.go
@@ -1,6 +1,7 @@
// Copyright 2011 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 sync_test
import (
diff --git a/test/fixedbugs/issue15281.go b/test/fixedbugs/issue15281.go
index 187c96f218..390867c848 100644
--- a/test/fixedbugs/issue15281.go
+++ b/test/fixedbugs/issue15281.go
@@ -3,6 +3,7 @@
// Copyright 2016 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
+
package main
import "runtime"
--
cgit v1.3-5-g9baa
From 7bbd5ca5a6a94f58d33de6b1244248a32dc8cd9c Mon Sep 17 00:00:00 2001
From: Austin Clements
Date: Wed, 22 Jul 2020 11:21:36 -0400
Subject: runtime: replace index and contains with bytealg calls
The runtime has its own implementation of string indexing. To reduce
code duplication and cognitive load, replace this with calls to the
internal/bytealg package. We can't do this on Plan 9 because it needs
string indexing in a note handler (which isn't allowed to use the
optimized bytealg version because it uses SSE), so we can't just
eliminate the index function, but this CL does down-scope it so make
it clear it's only for note handlers on Plan 9.
Change-Id: Ie1a142678262048515c481e8c26313b80c5875df
Reviewed-on: https://go-review.googlesource.com/c/go/+/244537
Run-TryBot: Austin Clements
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Knyszek
Reviewed-by: Michael Pratt
---
src/runtime/os_plan9.go | 18 ++++++++++++++++--
src/runtime/proc.go | 3 ++-
src/runtime/runtime1.go | 5 +++--
src/runtime/string.go | 16 ----------------
src/runtime/traceback.go | 3 ++-
5 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/runtime/os_plan9.go b/src/runtime/os_plan9.go
index 9e187d2220..128c30adeb 100644
--- a/src/runtime/os_plan9.go
+++ b/src/runtime/os_plan9.go
@@ -82,10 +82,10 @@ func sigpanic() {
note := gostringnocopy((*byte)(unsafe.Pointer(g.m.notesig)))
switch g.sig {
case _SIGRFAULT, _SIGWFAULT:
- i := index(note, "addr=")
+ i := indexNoFloat(note, "addr=")
if i >= 0 {
i += 5
- } else if i = index(note, "va="); i >= 0 {
+ } else if i = indexNoFloat(note, "va="); i >= 0 {
i += 3
} else {
panicmem()
@@ -111,6 +111,20 @@ func sigpanic() {
}
}
+// indexNoFloat is bytealg.IndexString but safe to use in a note
+// handler.
+func indexNoFloat(s, t string) int {
+ if len(t) == 0 {
+ return 0
+ }
+ for i := 0; i < len(s); i++ {
+ if s[i] == t[0] && hasPrefix(s[i:], t) {
+ return i
+ }
+ }
+ return -1
+}
+
func atolwhex(p string) int64 {
for hasPrefix(p, " ") || hasPrefix(p, "\t") {
p = p[1:]
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 035822216d..ed7e2128ae 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -5,6 +5,7 @@
package runtime
import (
+ "internal/bytealg"
"internal/cpu"
"runtime/internal/atomic"
"runtime/internal/sys"
@@ -5460,7 +5461,7 @@ func haveexperiment(name string) bool {
x := sys.Goexperiment
for x != "" {
xname := ""
- i := index(x, ",")
+ i := bytealg.IndexByteString(x, ',')
if i < 0 {
xname, x = x, ""
} else {
diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go
index c65a534ef6..7c893aa25c 100644
--- a/src/runtime/runtime1.go
+++ b/src/runtime/runtime1.go
@@ -5,6 +5,7 @@
package runtime
import (
+ "internal/bytealg"
"runtime/internal/atomic"
"runtime/internal/sys"
"unsafe"
@@ -347,13 +348,13 @@ func parsedebugvars() {
for p := gogetenv("GODEBUG"); p != ""; {
field := ""
- i := index(p, ",")
+ i := bytealg.IndexByteString(p, ',')
if i < 0 {
field, p = p, ""
} else {
field, p = p[:i], p[i+1:]
}
- i = index(field, "=")
+ i = bytealg.IndexByteString(field, '=')
if i < 0 {
continue
}
diff --git a/src/runtime/string.go b/src/runtime/string.go
index 251044231e..9a601f0094 100644
--- a/src/runtime/string.go
+++ b/src/runtime/string.go
@@ -335,22 +335,6 @@ func gostringn(p *byte, l int) string {
return s
}
-func index(s, t string) int {
- if len(t) == 0 {
- return 0
- }
- for i := 0; i < len(s); i++ {
- if s[i] == t[0] && hasPrefix(s[i:], t) {
- return i
- }
- }
- return -1
-}
-
-func contains(s, t string) bool {
- return index(s, t) >= 0
-}
-
func hasPrefix(s, prefix string) bool {
return len(s) >= len(prefix) && s[:len(prefix)] == prefix
}
diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 944c8473d2..96e552524e 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -5,6 +5,7 @@
package runtime
import (
+ "internal/bytealg"
"runtime/internal/atomic"
"runtime/internal/sys"
"unsafe"
@@ -848,7 +849,7 @@ func showfuncinfo(f funcInfo, firstFrame bool, funcID, childID funcID) bool {
return true
}
- return contains(name, ".") && (!hasPrefix(name, "runtime.") || isExportedRuntime(name))
+ return bytealg.IndexByteString(name, '.') >= 0 && (!hasPrefix(name, "runtime.") || isExportedRuntime(name))
}
// isExportedRuntime reports whether name is an exported runtime function.
--
cgit v1.3-5-g9baa
From 7148abc1b900555199998aac25af11783a9eb41c Mon Sep 17 00:00:00 2001
From: Austin Clements
Date: Fri, 5 Jun 2020 16:44:29 -0400
Subject: runtime: simplify heapBitsSetType doubleCheck
The heapBitsSetType function has a slow doubleCheck debugging mode
that checks the bitmap written out by the rest of the function using
far more obvious logic. But even this has some surprisingly complex
logic in it. Simplify it a bit. This also happens to fix the logic on
32-bit.
Fixes #40335.
Change-Id: I5cee482ad8adbd01cf5b98e35a270fe941ba4940
Reviewed-on: https://go-review.googlesource.com/c/go/+/244538
Run-TryBot: Austin Clements
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Knyszek
---
src/runtime/mbitmap.go | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index 35332c91c4..cad6f56404 100644
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -1403,17 +1403,20 @@ Phase4:
// Double check the whole bitmap.
if doubleCheck {
// x+size may not point to the heap, so back up one
- // word and then call next().
- end := heapBitsForAddr(x + size - sys.PtrSize).next()
- endAI := arenaIdx(end.arena)
- if !outOfPlace && (end.bitp == nil || (end.shift == 0 && end.bitp == &mheap_.arenas[endAI.l1()][endAI.l2()].bitmap[0])) {
- // The unrolling code above walks hbitp just
- // past the bitmap without moving to the next
- // arena. Synthesize this for end.bitp.
- end.arena--
- endAI = arenaIdx(end.arena)
- end.bitp = addb(&mheap_.arenas[endAI.l1()][endAI.l2()].bitmap[0], heapArenaBitmapBytes)
- end.last = nil
+ // word and then advance it the way we do above.
+ end := heapBitsForAddr(x + size - sys.PtrSize)
+ if outOfPlace {
+ // In out-of-place copying, we just advance
+ // using next.
+ end = end.next()
+ } else {
+ // Don't use next because that may advance to
+ // the next arena and the in-place logic
+ // doesn't do that.
+ end.shift += heapBitsShift
+ if end.shift == 4*heapBitsShift {
+ end.bitp, end.shift = add1(end.bitp), 0
+ }
}
if typ.kind&kindGCProg == 0 && (hbitp != end.bitp || (w == nw+2) != (end.shift == 2)) {
println("ended at wrong bitmap byte for", typ.string(), "x", dataSize/typ.size)
@@ -1437,8 +1440,9 @@ Phase4:
var have, want uint8
have = (*h.bitp >> h.shift) & (bitPointer | bitScan)
if i >= totalptr {
- want = 0 // deadmarker
if typ.kind&kindGCProg != 0 && i < (totalptr+3)/4*4 {
+ // heapBitsSetTypeGCProg always fills
+ // in full nibbles of bitScan.
want = bitScan
}
} else {
--
cgit v1.3-5-g9baa
From d19fedd180fceb6a60961e19387893ddb047e4e6 Mon Sep 17 00:00:00 2001
From: Austin Clements
Date: Fri, 5 Jun 2020 16:48:03 -0400
Subject: runtime: move checkmarks to a separate bitmap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Currently, the GC stores the object marks for checkmarks mode in the
heap bitmap using a rather complex encoding: for one word objects, the
checkmark is stored in the pointer/scalar bit since one word objects
must be pointers; for larger objects, the checkmark is stored in what
would be the scan/dead bit for the second word of the object. This
encoding made more sense when the runtime used the first scan/dead bit
as the regular mark bit, but we moved away from that long ago.
This encoding and overloading of the heap bitmap bits causes a great
deal of complexity in many parts of the allocator and garbage
collector and leads to some subtle bugs like #15903.
This CL moves the checkmarks mark bits into their own per-arena bitmap
and reclaims the second scan/dead bit as a regular scan/dead bit.
I tested this by enabling doubleCheck mode in heapBitsSetType and
running in both regular and GODEBUG=gccheckmark=1 mode.
Fixes #15903.
No performance degradation. (Very slight improvement on a few
benchmarks, but it's probably just noise.)
name old time/op new time/op delta
BiogoIgor 16.6s ± 1% 16.4s ± 1% -0.94% (p=0.000 n=25+24)
BiogoKrishna 19.2s ± 3% 19.2s ± 3% ~ (p=0.638 n=23+25)
BleveIndexBatch100 6.12s ± 5% 6.17s ± 4% ~ (p=0.170 n=25+25)
CompileTemplate 206ms ± 1% 205ms ± 1% -0.43% (p=0.005 n=24+24)
CompileUnicode 82.2ms ± 2% 81.5ms ± 2% -0.95% (p=0.001 n=22+22)
CompileGoTypes 755ms ± 3% 754ms ± 4% ~ (p=0.715 n=25+25)
CompileCompiler 3.73s ± 1% 3.73s ± 1% ~ (p=0.445 n=25+24)
CompileSSA 8.67s ± 1% 8.66s ± 1% ~ (p=0.836 n=24+22)
CompileFlate 134ms ± 2% 133ms ± 1% -0.66% (p=0.001 n=24+23)
CompileGoParser 164ms ± 1% 163ms ± 1% -0.85% (p=0.000 n=24+24)
CompileReflect 466ms ± 5% 466ms ± 3% ~ (p=0.863 n=25+25)
CompileTar 182ms ± 1% 182ms ± 1% -0.31% (p=0.048 n=24+24)
CompileXML 249ms ± 1% 248ms ± 1% -0.32% (p=0.031 n=21+25)
CompileStdCmd 10.3s ± 1% 10.3s ± 1% ~ (p=0.459 n=23+23)
FoglemanFauxGLRenderRotateBoat 8.66s ± 1% 8.62s ± 1% -0.47% (p=0.000 n=23+24)
FoglemanPathTraceRenderGopherIter1 20.3s ± 3% 20.2s ± 2% ~ (p=0.893 n=25+25)
GopherLuaKNucleotide 29.7s ± 1% 29.8s ± 2% ~ (p=0.421 n=24+25)
MarkdownRenderXHTML 246ms ± 1% 247ms ± 1% ~ (p=0.558 n=25+24)
Tile38WithinCircle100kmRequest 779µs ± 4% 779µs ± 3% ~ (p=0.954 n=25+25)
Tile38IntersectsCircle100kmRequest 1.02ms ± 3% 1.01ms ± 4% ~ (p=0.658 n=25+25)
Tile38KNearestLimit100Request 984µs ± 4% 986µs ± 4% ~ (p=0.627 n=24+25)
[Geo mean] 552ms 551ms -0.19%
https://perf.golang.org/search?q=upload:20200723.6
Change-Id: Ic703f26a83fb034941dc6f4788fc997d56890dec
Reviewed-on: https://go-review.googlesource.com/c/go/+/244539
Run-TryBot: Austin Clements
TryBot-Result: Gobot Gobot
Reviewed-by: Michael Knyszek
Reviewed-by: Martin Möhrmann
---
src/reflect/all_test.go | 5 +-
src/runtime/cgocall.go | 2 +-
src/runtime/gcinfo_test.go | 19 +----
src/runtime/heapdump.go | 2 +-
src/runtime/mbitmap.go | 173 +++++++++------------------------------------
src/runtime/mcheckmark.go | 100 ++++++++++++++++++++++++++
src/runtime/mgc.go | 4 +-
src/runtime/mgcmark.go | 70 +-----------------
src/runtime/mheap.go | 4 ++
9 files changed, 148 insertions(+), 231 deletions(-)
create mode 100644 src/runtime/mcheckmark.go
diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 6b31568bb9..ed2f225077 100644
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -6467,12 +6467,9 @@ func verifyGCBitsSlice(t *testing.T, typ Type, cap int, bits []byte) {
// Repeat the bitmap for the slice size, trimming scalars in
// the last element.
bits = rep(cap, bits)
- for len(bits) > 2 && bits[len(bits)-1] == 0 {
+ for len(bits) > 0 && bits[len(bits)-1] == 0 {
bits = bits[:len(bits)-1]
}
- if len(bits) == 2 && bits[0] == 0 && bits[1] == 0 {
- bits = bits[:0]
- }
if !bytes.Equal(heapBits, bits) {
t.Errorf("heapBits incorrect for make(%v, 0, %v)\nhave %v\nwant %v", typ, cap, heapBits, bits)
}
diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index a4e64b00cc..099aa540e0 100644
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -605,7 +605,7 @@ func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) {
hbits := heapBitsForAddr(base)
n := span.elemsize
for i = uintptr(0); i < n; i += sys.PtrSize {
- if i != 1*sys.PtrSize && !hbits.morePointers() {
+ if !hbits.morePointers() {
// No more possible pointers.
break
}
diff --git a/src/runtime/gcinfo_test.go b/src/runtime/gcinfo_test.go
index ec1ba90c2e..0808b416f0 100644
--- a/src/runtime/gcinfo_test.go
+++ b/src/runtime/gcinfo_test.go
@@ -77,7 +77,7 @@ func TestGCInfo(t *testing.T) {
}
for i := 0; i < 10; i++ {
- verifyGCInfo(t, "heap Ptr", escape(new(Ptr)), trimDead(padDead(infoPtr)))
+ verifyGCInfo(t, "heap Ptr", escape(new(Ptr)), trimDead(infoPtr))
verifyGCInfo(t, "heap PtrSlice", escape(&make([]*byte, 10)[0]), trimDead(infoPtr10))
verifyGCInfo(t, "heap ScalarPtr", escape(new(ScalarPtr)), trimDead(infoScalarPtr))
verifyGCInfo(t, "heap ScalarPtrSlice", escape(&make([]ScalarPtr, 4)[0]), trimDead(infoScalarPtr4))
@@ -97,25 +97,10 @@ func verifyGCInfo(t *testing.T, name string, p interface{}, mask0 []byte) {
}
}
-func padDead(mask []byte) []byte {
- // Because the dead bit isn't encoded in the second word,
- // and because on 32-bit systems a one-word allocation
- // uses a two-word block, the pointer info for a one-word
- // object needs to be expanded to include an extra scalar
- // on 32-bit systems to match the heap bitmap.
- if runtime.PtrSize == 4 && len(mask) == 1 {
- return []byte{mask[0], 0}
- }
- return mask
-}
-
func trimDead(mask []byte) []byte {
- for len(mask) > 2 && mask[len(mask)-1] == typeScalar {
+ for len(mask) > 0 && mask[len(mask)-1] == typeScalar {
mask = mask[:len(mask)-1]
}
- if len(mask) == 2 && mask[0] == typeScalar && mask[1] == typeScalar {
- mask = mask[:0]
- }
return mask
}
diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
index cfd5c251b4..4c35309211 100644
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -713,7 +713,7 @@ func makeheapobjbv(p uintptr, size uintptr) bitvector {
i := uintptr(0)
hbits := heapBitsForAddr(p)
for ; i < nptr; i++ {
- if i != 1 && !hbits.morePointers() {
+ if !hbits.morePointers() {
break // end of object
}
if hbits.isPointer() {
diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index cad6f56404..8de44c14b9 100644
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -6,10 +6,11 @@
//
// Stack, data, and bss bitmaps
//
-// Stack frames and global variables in the data and bss sections are described
-// by 1-bit bitmaps in which 0 means uninteresting and 1 means live pointer
-// to be visited during GC. The bits in each byte are consumed starting with
-// the low bit: 1<<0, 1<<1, and so on.
+// Stack frames and global variables in the data and bss sections are
+// described by bitmaps with 1 bit per pointer-sized word. A "1" bit
+// means the word is a live pointer to be visited by the GC (referred to
+// as "pointer"). A "0" bit means the word should be ignored by GC
+// (referred to as "scalar", though it could be a dead pointer value).
//
// Heap bitmap
//
@@ -20,18 +21,13 @@
// through start+3*ptrSize, ha.bitmap[1] holds the entries for
// start+4*ptrSize through start+7*ptrSize, and so on.
//
-// In each 2-bit entry, the lower bit holds the same information as in the 1-bit
-// bitmaps: 0 means uninteresting and 1 means live pointer to be visited during GC.
-// The meaning of the high bit depends on the position of the word being described
-// in its allocated object. In all words *except* the second word, the
-// high bit indicates that the object is still being described. In
-// these words, if a bit pair with a high bit 0 is encountered, the
-// low bit can also be assumed to be 0, and the object description is
-// over. This 00 is called the ``dead'' encoding: it signals that the
-// rest of the words in the object are uninteresting to the garbage
-// collector.
-//
-// In the second word, the high bit is the GC ``checkmarked'' bit (see below).
+// In each 2-bit entry, the lower bit is a pointer/scalar bit, just
+// like in the stack/data bitmaps described above. The upper bit
+// indicates scan/dead: a "1" value ("scan") indicates that there may
+// be pointers in later words of the allocation, and a "0" value
+// ("dead") indicates there are no more pointers in the allocation. If
+// the upper bit is 0, the lower bit must also be 0, and this
+// indicates scanning can ignore the rest of the allocation.
//
// The 2-bit entries are split when written into the byte, so that the top half
// of the byte contains 4 high bits and the bottom half contains 4 low (pointer)
@@ -39,38 +35,14 @@
// This form allows a copy from the 1-bit to the 4-bit form to keep the
// pointer bits contiguous, instead of having to space them out.
//
-// The code makes use of the fact that the zero value for a heap bitmap
-// has no live pointer bit set and is (depending on position), not used,
-// not checkmarked, and is the dead encoding.
-// These properties must be preserved when modifying the encoding.
+// The code makes use of the fact that the zero value for a heap
+// bitmap means scalar/dead. This property must be preserved when
+// modifying the encoding.
//
// The bitmap for noscan spans is not maintained. Code must ensure
// that an object is scannable before consulting its bitmap by
// checking either the noscan bit in the span or by consulting its
// type's information.
-//
-// Checkmarks
-//
-// In a concurrent garbage collector, one worries about failing to mark
-// a live object due to mutations without write barriers or bugs in the
-// collector implementation. As a sanity check, the GC has a 'checkmark'
-// mode that retraverses the object graph with the world stopped, to make
-// sure that everything that should be marked is marked.
-// In checkmark mode, in the heap bitmap, the high bit of the 2-bit entry
-// for the second word of the object holds the checkmark bit.
-// When not in checkmark mode, this bit is set to 1.
-//
-// The smallest possible allocation is 8 bytes. On a 32-bit machine, that
-// means every allocated object has two words, so there is room for the
-// checkmark bit. On a 64-bit machine, however, the 8-byte allocation is
-// just one word, so the second bit pair is not available for encoding the
-// checkmark. However, because non-pointer allocations are combined
-// into larger 16-byte (maxTinySize) allocations, a plain 8-byte allocation
-// must be a pointer, so the type bit in the first word is not actually needed.
-// It is still used in general, except in checkmark the type bit is repurposed
-// as the checkmark bit and then reinitialized (to 1) as the type bit when
-// finished.
-//
package runtime
@@ -551,33 +523,6 @@ func (h heapBits) isPointer() bool {
return h.bits()&bitPointer != 0
}
-// isCheckmarked reports whether the heap bits have the checkmarked bit set.
-// It must be told how large the object at h is, because the encoding of the
-// checkmark bit varies by size.
-// h must describe the initial word of the object.
-func (h heapBits) isCheckmarked(size uintptr) bool {
- if size == sys.PtrSize {
- return (*h.bitp>>h.shift)&bitPointer != 0
- }
- // All multiword objects are 2-word aligned,
- // so we know that the initial word's 2-bit pair
- // and the second word's 2-bit pair are in the
- // same heap bitmap byte, *h.bitp.
- return (*h.bitp>>(heapBitsShift+h.shift))&bitScan != 0
-}
-
-// setCheckmarked sets the checkmarked bit.
-// It must be told how large the object at h is, because the encoding of the
-// checkmark bit varies by size.
-// h must describe the initial word of the object.
-func (h heapBits) setCheckmarked(size uintptr) {
- if size == sys.PtrSize {
- atomic.Or8(h.bitp, bitPointer<= nw {
goto Phase3
}
@@ -1203,14 +1103,13 @@ func heapBitsSetType(x, size, dataSize uintptr, typ *_type) {
// We took care of 1-word and 2-word objects above,
// so this is at least a 6-word object.
hb = (b & (bitPointer | bitPointer< 1 {
+ hb |= bitScan << (3 * heapBitsShift)
+ }
b >>= 2
nb -= 2
- // Note: no bitScan for second word because that's
- // the checkmark.
- *hbitp &^= uint8((bitPointer | bitScan | (bitPointer << heapBitsShift)) << (2 * heapBitsShift))
+ *hbitp &^= uint8((bitPointer | bitScan | ((bitPointer | bitScan) << heapBitsShift)) << (2 * heapBitsShift))
*hbitp |= uint8(hb)
hbitp = add1(hbitp)
if w += 2; w >= nw {
@@ -1449,11 +1348,7 @@ Phase4:
if j < nptr && (*addb(ptrmask, j/8)>>(j%8))&1 != 0 {
want |= bitPointer
}
- if i != 1 {
- want |= bitScan
- } else {
- have &^= bitScan
- }
+ want |= bitScan
}
if have != want {
println("mismatch writing bits for", typ.string(), "x", dataSize/typ.size)
@@ -2013,7 +1908,7 @@ func getgcmask(ep interface{}) (mask []byte) {
if hbits.isPointer() {
mask[i/sys.PtrSize] = 1
}
- if i != 1*sys.PtrSize && !hbits.morePointers() {
+ if !hbits.morePointers() {
mask = mask[:i/sys.PtrSize]
break
}
diff --git a/src/runtime/mcheckmark.go b/src/runtime/mcheckmark.go
new file mode 100644
index 0000000000..1fd8e4e78f
--- /dev/null
+++ b/src/runtime/mcheckmark.go
@@ -0,0 +1,100 @@
+// 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.
+
+// GC checkmarks
+//
+// In a concurrent garbage collector, one worries about failing to mark
+// a live object due to mutations without write barriers or bugs in the
+// collector implementation. As a sanity check, the GC has a 'checkmark'
+// mode that retraverses the object graph with the world stopped, to make
+// sure that everything that should be marked is marked.
+
+package runtime
+
+import (
+ "runtime/internal/atomic"
+ "runtime/internal/sys"
+ "unsafe"
+)
+
+// A checkmarksMap stores the GC marks in "checkmarks" mode. It is a
+// per-arena bitmap with a bit for every word in the arena. The mark
+// is stored on the bit corresponding to the first word of the marked
+// allocation.
+//
+//go:notinheap
+type checkmarksMap [heapArenaBytes / sys.PtrSize / 8]uint8
+
+// If useCheckmark is true, marking of an object uses the checkmark
+// bits instead of the standard mark bits.
+var useCheckmark = false
+
+// startCheckmarks prepares for the checkmarks phase.
+//
+// The world must be stopped.
+func startCheckmarks() {
+ // Clear all checkmarks.
+ for _, ai := range mheap_.allArenas {
+ arena := mheap_.arenas[ai.l1()][ai.l2()]
+ bitmap := arena.checkmarks
+
+ if bitmap == nil {
+ // Allocate bitmap on first use.
+ bitmap = (*checkmarksMap)(persistentalloc(unsafe.Sizeof(*bitmap), 0, &memstats.gc_sys))
+ if bitmap == nil {
+ throw("out of memory allocating checkmarks bitmap")
+ }
+ arena.checkmarks = bitmap
+ } else {
+ // Otherwise clear the existing bitmap.
+ for i := range bitmap {
+ bitmap[i] = 0
+ }
+ }
+ }
+ // Enable checkmarking.
+ useCheckmark = true
+}
+
+// endCheckmarks ends the checkmarks phase.
+func endCheckmarks() {
+ if gcMarkWorkAvailable(nil) {
+ throw("GC work not flushed")
+ }
+ useCheckmark = false
+}
+
+// setCheckmark throws if marking object is a checkmarks violation,
+// and otherwise sets obj's checkmark. It returns true if obj was
+// already checkmarked.
+func setCheckmark(obj, base, off uintptr, mbits markBits) bool {
+ if !mbits.isMarked() {
+ printlock()
+ print("runtime: checkmarks found unexpected unmarked object obj=", hex(obj), "\n")
+ print("runtime: found obj at *(", hex(base), "+", hex(off), ")\n")
+
+ // Dump the source (base) object
+ gcDumpObject("base", base, off)
+
+ // Dump the object
+ gcDumpObject("obj", obj, ^uintptr(0))
+
+ getg().m.traceback = 2
+ throw("checkmark found unmarked object")
+ }
+
+ ai := arenaIndex(obj)
+ arena := mheap_.arenas[ai.l1()][ai.l2()]
+ arenaWord := (obj / heapArenaBytes / 8) % uintptr(len(arena.checkmarks))
+ mask := byte(1 << ((obj / heapArenaBytes) % 8))
+ bytep := &arena.checkmarks[arenaWord]
+
+ if atomic.Load8(bytep)&mask != 0 {
+ // Already checkmarked.
+ return true
+ }
+
+ atomic.Or8(bytep, mask)
+ return false
+}
diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index b3499516f6..c8c4a4c758 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -1670,13 +1670,13 @@ func gcMarkTermination(nextTriggerRatio float64) {
// mark using checkmark bits, to check that we
// didn't forget to mark anything during the
// concurrent mark process.
+ startCheckmarks()
gcResetMarkState()
- initCheckmarks()
gcw := &getg().m.p.ptr().gcw
gcDrain(gcw, 0)
wbBufFlush1(getg().m.p.ptr())
gcw.dispose()
- clearCheckmarks()
+ endCheckmarks()
}
// marking is complete so we can turn the write barrier off
diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index fe988c46d9..96910ff729 100644
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -1354,11 +1354,7 @@ func scanobject(b uintptr, gcw *gcWork) {
}
// Load bits once. See CL 22712 and issue 16973 for discussion.
bits := hbits.bits()
- // During checkmarking, 1-word objects store the checkmark
- // in the type bit for the one word. The only one-word objects
- // are pointers, or else they'd be merged with other non-pointer
- // data into larger allocations.
- if i != 1*sys.PtrSize && bits&bitScan == 0 {
+ if bits&bitScan == 0 {
break // no more pointers in this object
}
if bits&bitPointer == 0 {
@@ -1511,28 +1507,10 @@ func greyobject(obj, base, off uintptr, span *mspan, gcw *gcWork, objIndex uintp
mbits := span.markBitsForIndex(objIndex)
if useCheckmark {
- if !mbits.isMarked() {
- printlock()
- print("runtime:greyobject: checkmarks finds unexpected unmarked object obj=", hex(obj), "\n")
- print("runtime: found obj at *(", hex(base), "+", hex(off), ")\n")
-
- // Dump the source (base) object
- gcDumpObject("base", base, off)
-
- // Dump the object
- gcDumpObject("obj", obj, ^uintptr(0))
-
- getg().m.traceback = 2
- throw("checkmark found unmarked object")
- }
- hbits := heapBitsForAddr(obj)
- if hbits.isCheckmarked(span.elemsize) {
+ if setCheckmark(obj, base, off, mbits) {
+ // Already marked.
return
}
- hbits.setCheckmarked(span.elemsize)
- if !hbits.isCheckmarked(span.elemsize) {
- throw("setCheckmarked and isCheckmarked disagree")
- }
} else {
if debug.gccheckmark > 0 && span.isFree(objIndex) {
print("runtime: marking free object ", hex(obj), " found at *(", hex(base), "+", hex(off), ")\n")
@@ -1661,45 +1639,3 @@ func gcMarkTinyAllocs() {
greyobject(c.tiny, 0, 0, span, gcw, objIndex)
}
}
-
-// Checkmarking
-
-// To help debug the concurrent GC we remark with the world
-// stopped ensuring that any object encountered has their normal
-// mark bit set. To do this we use an orthogonal bit
-// pattern to indicate the object is marked. The following pattern
-// uses the upper two bits in the object's boundary nibble.
-// 01: scalar not marked
-// 10: pointer not marked
-// 11: pointer marked
-// 00: scalar marked
-// Xoring with 01 will flip the pattern from marked to unmarked and vica versa.
-// The higher bit is 1 for pointers and 0 for scalars, whether the object
-// is marked or not.
-// The first nibble no longer holds the typeDead pattern indicating that the
-// there are no more pointers in the object. This information is held
-// in the second nibble.
-
-// If useCheckmark is true, marking of an object uses the
-// checkmark bits (encoding above) instead of the standard
-// mark bits.
-var useCheckmark = false
-
-//go:nowritebarrier
-func initCheckmarks() {
- useCheckmark = true
- for _, s := range mheap_.allspans {
- if s.state.get() == mSpanInUse {
- heapBitsForAddr(s.base()).initCheckmarkSpan(s.layout())
- }
- }
-}
-
-func clearCheckmarks() {
- useCheckmark = false
- for _, s := range mheap_.allspans {
- if s.state.get() == mSpanInUse {
- heapBitsForAddr(s.base()).clearCheckmarkSpan(s.layout())
- }
- }
-}
diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index 2c7bfd8a59..6341375160 100644
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -300,6 +300,10 @@ type heapArena struct {
// during marking.
pageSpecials [pagesPerArena / 8]uint8
+ // checkmarks stores the debug.gccheckmark state. It is only
+ // used if debug.gccheckmark > 0.
+ checkmarks *checkmarksMap
+
// zeroedBase marks the first byte of the first page in this
// arena which hasn't been used yet and is therefore already
// zero. zeroedBase is relative to the arena base.
--
cgit v1.3-5-g9baa
From a22ec6e650669f5101c7e0955d82e29d644eef4e Mon Sep 17 00:00:00 2001
From: Austin Clements
Date: Mon, 17 Aug 2020 13:17:26 +0000
Subject: Revert "cmd/internal/obj: fix inline marker issue on s390x"
This reverts CL 247697.
Reason for revert: This change broke the linux-arm builder.
Change-Id: I8ca0d5b3b2ea0109ffbfadeab1406a1b60e7d18d
Reviewed-on: https://go-review.googlesource.com/c/go/+/248718
Reviewed-by: Michael Munday
Run-TryBot: Michael Munday
TryBot-Result: Gobot Gobot
---
src/cmd/internal/obj/pcln.go | 15 ---------------
src/cmd/internal/obj/s390x/objz.go | 11 +++++++++++
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go
index bffeda041d..1f7ccf47ef 100644
--- a/src/cmd/internal/obj/pcln.go
+++ b/src/cmd/internal/obj/pcln.go
@@ -278,21 +278,6 @@ func linkpcln(ctxt *Link, cursym *LSym) {
funcpctab(ctxt, &pcln.Pcfile, cursym, "pctofile", pctofileline, pcln)
funcpctab(ctxt, &pcln.Pcline, cursym, "pctoline", pctofileline, nil)
- // Check that all the Progs used as inline markers are still reachable.
- // See issue #40473.
- inlMarkProgs := make(map[*Prog]struct{}, len(cursym.Func.InlMarks))
- for _, inlMark := range cursym.Func.InlMarks {
- inlMarkProgs[inlMark.p] = struct{}{}
- }
- for p := cursym.Func.Text; p != nil; p = p.Link {
- if _, ok := inlMarkProgs[p]; ok {
- delete(inlMarkProgs, p)
- }
- }
- if len(inlMarkProgs) > 0 {
- ctxt.Diag("one or more instructions used as inline markers are no longer reachable")
- }
-
pcinlineState := new(pcinlineState)
funcpctab(ctxt, &pcln.Pcinline, cursym, "pctoinline", pcinlineState.pctoinline, nil)
for _, inlMark := range cursym.Func.InlMarks {
diff --git a/src/cmd/internal/obj/s390x/objz.go b/src/cmd/internal/obj/s390x/objz.go
index ef6335d849..b14dc810fa 100644
--- a/src/cmd/internal/obj/s390x/objz.go
+++ b/src/cmd/internal/obj/s390x/objz.go
@@ -283,6 +283,17 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
ACMPUBNE:
q = p
p.Mark |= BRANCH
+ if p.Pcond != nil {
+ q := p.Pcond
+ for q.As == obj.ANOP {
+ q = q.Link
+ p.Pcond = q
+ }
+ }
+
+ case obj.ANOP:
+ q.Link = p.Link /* q is non-nop */
+ p.Link.Mark |= p.Mark
default:
q = p
--
cgit v1.3-5-g9baa
From 7ee26224436d80dca3f7e98c8fcf21185522d8e6 Mon Sep 17 00:00:00 2001
From: Cherry Zhang
Date: Wed, 12 Aug 2020 20:27:57 -0400
Subject: cmd/link: link dynamic library automatically
cgo_import_dynamic pragma indicates a symbol is imported from a
dynamic library. Currently, the linker does not actually link
against the dynamic library, so we have to "force" it by using
//go:cgo_import_dynamic _ _ "dylib"
syntax, which links in the library unconditionally.
This CL changes it to link in the library automatically when a
symbol is imported from the library, without using the "force"
syntax. (The "force" syntax is still supported.)
Remove the unconditional imports in the runtime. Now,
Security.framework and CoreFoundation.framework are only linked
when the x509 package is imported (or otherwise specified).
Fixes #40727.
Change-Id: Ied36b1f621cdcc5dc4a8f497cdf1c554a182d0e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/248333
Run-TryBot: Cherry Zhang
Reviewed-by: Filippo Valsorda
Reviewed-by: Than McIntosh
Reviewed-by: Keith Randall
TryBot-Result: Gobot Gobot
---
src/cmd/link/internal/ld/go.go | 3 +++
src/runtime/sys_darwin.go | 6 ------
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/cmd/link/internal/ld/go.go b/src/cmd/link/internal/ld/go.go
index bf5c9ca1ba..b3541c46c0 100644
--- a/src/cmd/link/internal/ld/go.go
+++ b/src/cmd/link/internal/ld/go.go
@@ -183,6 +183,9 @@ func setCgoAttr(ctxt *Link, lookup func(string, int) loader.Sym, file string, pk
hostObjSyms[s] = struct{}{}
}
havedynamic = 1
+ if lib != "" && ctxt.IsDarwin() {
+ machoadddynlib(lib, ctxt.LinkMode)
+ }
}
continue
diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go
index 06474434c9..e4f19bbf41 100644
--- a/src/runtime/sys_darwin.go
+++ b/src/runtime/sys_darwin.go
@@ -489,9 +489,3 @@ func setNonblock(fd int32) {
//go:cgo_import_dynamic libc_pthread_cond_wait pthread_cond_wait "/usr/lib/libSystem.B.dylib"
//go:cgo_import_dynamic libc_pthread_cond_timedwait_relative_np pthread_cond_timedwait_relative_np "/usr/lib/libSystem.B.dylib"
//go:cgo_import_dynamic libc_pthread_cond_signal pthread_cond_signal "/usr/lib/libSystem.B.dylib"
-
-// Magic incantation to get libSystem and friends actually dynamically linked.
-// TODO: Why does the code require this? See cmd/link/internal/ld/go.go
-//go:cgo_import_dynamic _ _ "/usr/lib/libSystem.B.dylib"
-//go:cgo_import_dynamic _ _ "/System/Library/Frameworks/Security.framework/Versions/A/Security"
-//go:cgo_import_dynamic _ _ "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation"
--
cgit v1.3-5-g9baa
From a2a2237ae02016dd9ce16388963cfceece6744f3 Mon Sep 17 00:00:00 2001
From: Cherry Zhang
Date: Thu, 13 Aug 2020 16:59:52 -0400
Subject: cmd/link: emit correct jump instruction on ARM for DYNIMPORT
On ARM, for a JMP/CALL relocation, the instruction bytes is
encoded in Reloc.Add (issue #19811). I really hate it, but before
it is fixed we have to follow the rule and emit the right bits
from r.Add.
Fixes #40769.
Change-Id: I862e105408d344c5cc58ca9140d2e552e4364453
Reviewed-on: https://go-review.googlesource.com/c/go/+/248399
Reviewed-by: Jeremy Faller
Reviewed-by: Joel Sing
Reviewed-by: Than McIntosh
---
src/cmd/link/internal/arm/asm.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cmd/link/internal/arm/asm.go b/src/cmd/link/internal/arm/asm.go
index 22bcb518df..611c96ce35 100644
--- a/src/cmd/link/internal/arm/asm.go
+++ b/src/cmd/link/internal/arm/asm.go
@@ -220,7 +220,7 @@ func adddynrel(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s loade
addpltsym(target, ldr, syms, targ)
su := ldr.MakeSymbolUpdater(s)
su.SetRelocSym(rIdx, syms.PLT)
- su.SetRelocAdd(rIdx, int64(ldr.SymPlt(targ)))
+ su.SetRelocAdd(rIdx, int64(braddoff(int32(r.Add()), ldr.SymPlt(targ)/4))) // TODO: don't use r.Add for instruction bytes (issue 19811)
return true
case objabi.R_ADDR:
--
cgit v1.3-5-g9baa
From abfeec5eb0356d1ac91a097d2124a6b7c8cfccd4 Mon Sep 17 00:00:00 2001
From: Carlos Alexandro Becker
Date: Sun, 16 Aug 2020 21:58:40 +0000
Subject: testing/iotest: add ErrReader
Adds an io.Reader that always returns 0 and a non-nil error.
Fixes #38781
Change-Id: I56bd124de07bc8809e77c6cfaab33a1e32cfe2ee
GitHub-Last-Rev: 4e232b17e9120405d4ea4743350ee361a3505043
GitHub-Pull-Request: golang/go#34741
Reviewed-on: https://go-review.googlesource.com/c/go/+/199501
Run-TryBot: Emmanuel Odeke
TryBot-Result: Gobot Gobot
Reviewed-by: Emmanuel Odeke
---
src/testing/iotest/logger_test.go | 12 ++----------
src/testing/iotest/reader.go | 15 +++++++++++++++
src/testing/iotest/reader_test.go | 10 ++++++++++
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/src/testing/iotest/logger_test.go b/src/testing/iotest/logger_test.go
index c121bf48f7..575f37e05c 100644
--- a/src/testing/iotest/logger_test.go
+++ b/src/testing/iotest/logger_test.go
@@ -81,14 +81,6 @@ func TestWriteLogger_errorOnWrite(t *testing.T) {
}
}
-type errReader struct {
- err error
-}
-
-func (r errReader) Read([]byte) (int, error) {
- return 0, r.err
-}
-
func TestReadLogger(t *testing.T) {
olw := log.Writer()
olf := log.Flags()
@@ -146,14 +138,14 @@ func TestReadLogger_errorOnRead(t *testing.T) {
data := []byte("Hello, World!")
p := make([]byte, len(data))
- lr := errReader{err: errors.New("Read Error!")}
+ lr := ErrReader()
rl := NewReadLogger("read", lr)
n, err := rl.Read(p)
if err == nil {
t.Fatalf("Unexpectedly succeeded to read: %v", err)
}
- wantLogWithHex := fmt.Sprintf("lr: read %x: %v\n", p[:n], "Read Error!")
+ wantLogWithHex := fmt.Sprintf("lr: read %x: %v\n", p[:n], "io")
if g, w := lOut.String(), wantLogWithHex; g != w {
t.Errorf("ReadLogger mismatch\n\tgot: %q\n\twant: %q", g, w)
}
diff --git a/src/testing/iotest/reader.go b/src/testing/iotest/reader.go
index 8d82018fd6..b18e912f27 100644
--- a/src/testing/iotest/reader.go
+++ b/src/testing/iotest/reader.go
@@ -68,6 +68,7 @@ func (r *dataErrReader) Read(p []byte) (n int, err error) {
return
}
+// ErrTimeout is a fake timeout error.
var ErrTimeout = errors.New("timeout")
// TimeoutReader returns ErrTimeout on the second read
@@ -86,3 +87,17 @@ func (r *timeoutReader) Read(p []byte) (int, error) {
}
return r.r.Read(p)
}
+
+// ErrIO is a fake IO error.
+var ErrIO = errors.New("io")
+
+// ErrReader returns a fake error every time it is read from.
+func ErrReader() io.Reader {
+ return errReader(0)
+}
+
+type errReader int
+
+func (r errReader) Read(p []byte) (int, error) {
+ return 0, ErrIO
+}
diff --git a/src/testing/iotest/reader_test.go b/src/testing/iotest/reader_test.go
index 9397837e08..ccba22ee29 100644
--- a/src/testing/iotest/reader_test.go
+++ b/src/testing/iotest/reader_test.go
@@ -224,3 +224,13 @@ func TestDataErrReader_emptyReader(t *testing.T) {
t.Errorf("Unexpectedly read %d bytes, wanted %d", g, w)
}
}
+
+func TestErrReader(t *testing.T) {
+ n, err := ErrReader().Read([]byte{})
+ if err != ErrIO {
+ t.Errorf("ErrReader.Read(any) should have returned ErrIO, returned %v", err)
+ }
+ if n != 0 {
+ t.Errorf("ErrReader.Read(any) should have read 0 bytes, read %v", n)
+ }
+}
--
cgit v1.3-5-g9baa
From 49003da6d437ef1a4e1e55cf86240480f17dc8ab Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Wed, 17 Jun 2020 17:40:35 -0400
Subject: cmd/go/internal/trace: add function to distinguish goroutines
trace.StartGoroutine will associate the trace information on the context
with a new chrome profiler thread id. The chrome profiler doesn't
expect multiple trace events to have the same thread id, so this
will allow us to display concurrent events on the trace.
Updates #38714
Change-Id: I888b0cce15a5a01db66366716fdd85bf86c832cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/248319
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/trace/trace.go | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/cmd/go/internal/trace/trace.go b/src/cmd/go/internal/trace/trace.go
index c8fac92c9f..32ce0408f5 100644
--- a/src/cmd/go/internal/trace/trace.go
+++ b/src/cmd/go/internal/trace/trace.go
@@ -34,20 +34,33 @@ func StartSpan(ctx context.Context, name string) (context.Context, *Span) {
if !ok {
return ctx, nil
}
- childSpan := &Span{t: tc.t, name: name, start: time.Now()}
+ childSpan := &Span{t: tc.t, name: name, tid: tc.tid, start: time.Now()}
tc.t.writeEvent(&traceviewer.Event{
Name: childSpan.name,
Time: float64(childSpan.start.UnixNano()) / float64(time.Microsecond),
+ TID: childSpan.tid,
Phase: "B",
})
- ctx = context.WithValue(ctx, traceKey{}, traceContext{tc.t})
+ ctx = context.WithValue(ctx, traceKey{}, traceContext{tc.t, tc.tid})
return ctx, childSpan
}
+// Goroutine associates the context with a new Thread ID. The Chrome trace viewer associates each
+// trace event with a thread, and doesn't expect events with the same thread id to happen at the
+// same time.
+func Goroutine(ctx context.Context) context.Context {
+ tc, ok := getTraceContext(ctx)
+ if !ok {
+ return ctx
+ }
+ return context.WithValue(ctx, traceKey{}, traceContext{tc.t, tc.t.getNextTID()})
+}
+
type Span struct {
t *tracer
name string
+ tid uint64
start time.Time
end time.Time
}
@@ -60,12 +73,15 @@ func (s *Span) Done() {
s.t.writeEvent(&traceviewer.Event{
Name: s.name,
Time: float64(s.end.UnixNano()) / float64(time.Microsecond),
+ TID: s.tid,
Phase: "E",
})
}
type tracer struct {
file chan traceFile // 1-buffered
+
+ nextTID uint64
}
func (t *tracer) writeEvent(ev *traceviewer.Event) error {
@@ -103,12 +119,17 @@ func (t *tracer) Close() error {
return firstErr
}
+func (t *tracer) getNextTID() uint64 {
+ return atomic.AddUint64(&t.nextTID, 1)
+}
+
// traceKey is the context key for tracing information. It is unexported to prevent collisions with context keys defined in
// other packages.
type traceKey struct{}
type traceContext struct {
- t *tracer
+ t *tracer
+ tid uint64
}
// Start starts a trace which writes to the given file.
--
cgit v1.3-5-g9baa
From 023d4973851a25e2a47b1ebaf96833c9209efd7c Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Wed, 17 Jun 2020 18:05:16 -0400
Subject: cmd/go: add trace events for each action
This change adds a trace event for each action and also
annotates each of the action execution goroutines with trace.Goroutine
so that the actions eaxecuted by each goroutine appear on different threads in
the chrome trace viewer.
Updates #38714
Change-Id: I2e58dc5606b2e3f7f87076a61e1cc6a2014255c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/248320
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/trace/trace.go | 4 ++--
src/cmd/go/internal/work/exec.go | 13 +++++++++++--
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/cmd/go/internal/trace/trace.go b/src/cmd/go/internal/trace/trace.go
index 32ce0408f5..24130d9d72 100644
--- a/src/cmd/go/internal/trace/trace.go
+++ b/src/cmd/go/internal/trace/trace.go
@@ -45,10 +45,10 @@ func StartSpan(ctx context.Context, name string) (context.Context, *Span) {
return ctx, childSpan
}
-// Goroutine associates the context with a new Thread ID. The Chrome trace viewer associates each
+// StartGoroutine associates the context with a new Thread ID. The Chrome trace viewer associates each
// trace event with a thread, and doesn't expect events with the same thread id to happen at the
// same time.
-func Goroutine(ctx context.Context) context.Context {
+func StartGoroutine(ctx context.Context) context.Context {
tc, ok := getTraceContext(ctx)
if !ok {
return ctx
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 3ea3293ae1..56a127f36f 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -115,13 +115,21 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
// Handle runs a single action and takes care of triggering
// any actions that are runnable as a result.
- handle := func(a *Action) {
+ handle := func(ctx context.Context, a *Action) {
if a.json != nil {
a.json.TimeStart = time.Now()
}
var err error
if a.Func != nil && (!a.Failed || a.IgnoreFail) {
+ // TODO(matloob): Better action descriptions
+ desc := "Executing action "
+ if a.Package != nil {
+ desc += "(" + a.Mode + " " + a.Package.Desc() + ")"
+ }
+ ctx, span := trace.StartSpan(ctx, desc)
+ _ = ctx
err = a.Func(b, a)
+ span.Done()
}
if a.json != nil {
a.json.TimeDone = time.Now()
@@ -169,6 +177,7 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
for i := 0; i < par; i++ {
wg.Add(1)
go func() {
+ ctx := trace.StartGoroutine(ctx)
defer wg.Done()
for {
select {
@@ -181,7 +190,7 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
b.exec.Lock()
a := b.ready.pop()
b.exec.Unlock()
- handle(a)
+ handle(ctx, a)
case <-base.Interrupted:
base.SetExitStatus(1)
return
--
cgit v1.3-5-g9baa
From a26d687ebb23fa14b777ef5bf69b56556124ff3b Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Wed, 17 Jun 2020 18:18:23 -0400
Subject: cmd/go: propagate context into Action.Func calls
Action.Func is now a func(*Builder, context.Context, *Action), so that
contexts can be propagated into the action funcs. While context
is traditionally the first parameter of a function, it's the second
parameter of Action.Func's type to continue to allow for methods
on Builder to be used as functions taking a *Builder as the first
parameter. context.Context is instead the first parameter on
those functions.
Change-Id: I5f058d6a99a1e96fe2025f2e8ce30a033d12e935
Reviewed-on: https://go-review.googlesource.com/c/go/+/248321
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/run/run.go | 2 +-
src/cmd/go/internal/test/test.go | 10 +++++-----
src/cmd/go/internal/work/action.go | 15 ++++++++-------
src/cmd/go/internal/work/exec.go | 17 ++++++++---------
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/cmd/go/internal/run/run.go b/src/cmd/go/internal/run/run.go
index 3630f68c54..deec5106ff 100644
--- a/src/cmd/go/internal/run/run.go
+++ b/src/cmd/go/internal/run/run.go
@@ -146,7 +146,7 @@ func runRun(ctx context.Context, cmd *base.Command, args []string) {
// buildRunProgram is the action for running a binary that has already
// been compiled. We ignore exit status.
-func buildRunProgram(b *work.Builder, a *work.Action) error {
+func buildRunProgram(b *work.Builder, ctx context.Context, a *work.Action) error {
cmdline := str.StringList(work.FindExecCmd(), a.Deps[0].Target, a.Args)
if cfg.BuildN || cfg.BuildX {
b.Showcmd("", "%s", strings.Join(cmdline, " "))
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index d71d339828..9788590938 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1069,7 +1069,7 @@ func (lockedStdout) Write(b []byte) (int, error) {
}
// builderRunTest is the action for running a test binary.
-func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
+func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work.Action) error {
if a.Failed {
// We were unable to build the binary.
a.Failed = false
@@ -1642,7 +1642,7 @@ func coveragePercentage(out []byte) string {
}
// builderCleanTest is the action for cleaning up after a test.
-func builderCleanTest(b *work.Builder, a *work.Action) error {
+func builderCleanTest(b *work.Builder, ctx context.Context, a *work.Action) error {
if cfg.BuildWork {
return nil
}
@@ -1654,7 +1654,7 @@ func builderCleanTest(b *work.Builder, a *work.Action) error {
}
// builderPrintTest is the action for printing a test result.
-func builderPrintTest(b *work.Builder, a *work.Action) error {
+func builderPrintTest(b *work.Builder, ctx context.Context, a *work.Action) error {
clean := a.Deps[0]
run := clean.Deps[0]
if run.TestOutput != nil {
@@ -1665,7 +1665,7 @@ func builderPrintTest(b *work.Builder, a *work.Action) error {
}
// builderNoTest is the action for testing a package with no test files.
-func builderNoTest(b *work.Builder, a *work.Action) error {
+func builderNoTest(b *work.Builder, ctx context.Context, a *work.Action) error {
var stdout io.Writer = os.Stdout
if testJSON {
json := test2json.NewConverter(lockedStdout{}, a.Package.ImportPath, test2json.Timestamp)
@@ -1677,7 +1677,7 @@ func builderNoTest(b *work.Builder, a *work.Action) error {
}
// printExitStatus is the action for printing the exit status
-func printExitStatus(b *work.Builder, a *work.Action) error {
+func printExitStatus(b *work.Builder, ctx context.Context, a *work.Action) error {
if !testJSON && len(pkgArgs) != 0 {
if base.GetExitStatus() != 0 {
fmt.Println("FAIL")
diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go
index 6b5f9e4807..a37a5e618d 100644
--- a/src/cmd/go/internal/work/action.go
+++ b/src/cmd/go/internal/work/action.go
@@ -10,6 +10,7 @@ import (
"bufio"
"bytes"
"container/heap"
+ "context"
"debug/elf"
"encoding/json"
"fmt"
@@ -63,13 +64,13 @@ type Builder struct {
// An Action represents a single action in the action graph.
type Action struct {
- Mode string // description of action operation
- Package *load.Package // the package this action works on
- Deps []*Action // actions that must happen before this one
- Func func(*Builder, *Action) error // the action itself (nil = no-op)
- IgnoreFail bool // whether to run f even if dependencies fail
- TestOutput *bytes.Buffer // test output buffer
- Args []string // additional args for runProgram
+ Mode string // description of action operation
+ Package *load.Package // the package this action works on
+ Deps []*Action // actions that must happen before this one
+ Func func(*Builder, context.Context, *Action) error // the action itself (nil = no-op)
+ IgnoreFail bool // whether to run f even if dependencies fail
+ TestOutput *bytes.Buffer // test output buffer
+ Args []string // additional args for runProgram
triggers []*Action // inverse of deps
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 56a127f36f..3903502a67 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -127,8 +127,7 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
desc += "(" + a.Mode + " " + a.Package.Desc() + ")"
}
ctx, span := trace.StartSpan(ctx, desc)
- _ = ctx
- err = a.Func(b, a)
+ err = a.Func(b, ctx, a)
span.Done()
}
if a.json != nil {
@@ -400,7 +399,7 @@ const (
// build is the action for building a single package.
// Note that any new influence on this logic must be reported in b.buildActionID above as well.
-func (b *Builder) build(a *Action) (err error) {
+func (b *Builder) build(ctx context.Context, a *Action) (err error) {
p := a.Package
bit := func(x uint32, b bool) uint32 {
@@ -1005,7 +1004,7 @@ var VetFlags []string
// VetExplicit records whether the vet flags were set explicitly on the command line.
var VetExplicit bool
-func (b *Builder) vet(a *Action) error {
+func (b *Builder) vet(ctx context.Context, a *Action) error {
// a.Deps[0] is the build of the package being vetted.
// a.Deps[1] is the build of the "fmt" package.
@@ -1196,7 +1195,7 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
// link is the action for linking a single command.
// Note that any new influence on this logic must be reported in b.linkActionID above as well.
-func (b *Builder) link(a *Action) (err error) {
+func (b *Builder) link(ctx context.Context, a *Action) (err error) {
if b.useCache(a, b.linkActionID(a), a.Package.Target) || b.IsCmdList {
return nil
}
@@ -1388,7 +1387,7 @@ func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string,
return
}
-func (b *Builder) installShlibname(a *Action) error {
+func (b *Builder) installShlibname(ctx context.Context, a *Action) error {
if err := allowInstall(a); err != nil {
return err
}
@@ -1437,7 +1436,7 @@ func (b *Builder) linkSharedActionID(a *Action) cache.ActionID {
return h.Sum()
}
-func (b *Builder) linkShared(a *Action) (err error) {
+func (b *Builder) linkShared(ctx context.Context, a *Action) (err error) {
if b.useCache(a, b.linkSharedActionID(a), a.Target) || b.IsCmdList {
return nil
}
@@ -1463,7 +1462,7 @@ func (b *Builder) linkShared(a *Action) (err error) {
}
// BuildInstallFunc is the action for installing a single package or executable.
-func BuildInstallFunc(b *Builder, a *Action) (err error) {
+func BuildInstallFunc(b *Builder, ctx context.Context, a *Action) (err error) {
defer func() {
if err != nil && err != errPrintedOutput {
// a.Package == nil is possible for the go install -buildmode=shared
@@ -1716,7 +1715,7 @@ func (b *Builder) writeFile(file string, text []byte) error {
}
// Install the cgo export header file, if there is one.
-func (b *Builder) installHeader(a *Action) error {
+func (b *Builder) installHeader(ctx context.Context, a *Action) error {
src := a.Objdir + "_cgo_install.h"
if _, err := os.Stat(src); os.IsNotExist(err) {
// If the file does not exist, there are no exported
--
cgit v1.3-5-g9baa
From 15b98e55d195bd876203506d5f513546dd4e3b36 Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Thu, 18 Jun 2020 11:58:46 -0400
Subject: cmd/go: mark trace flows between actions
This could help make it easier to identify blocking
dependencies when examining traces. Flows can be turned
off when viewing traces to remove potential distractions.
Updates #38714
Change-Id: Ibfd3f1a1861e3cac31addb053a2fca7ee796c4d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/248322
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/trace/trace.go | 51 +++++++++++++++++++++++++++++++++++---
src/cmd/go/internal/work/action.go | 10 +++++---
src/cmd/go/internal/work/exec.go | 4 +++
3 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/src/cmd/go/internal/trace/trace.go b/src/cmd/go/internal/trace/trace.go
index 24130d9d72..f108a2b6ca 100644
--- a/src/cmd/go/internal/trace/trace.go
+++ b/src/cmd/go/internal/trace/trace.go
@@ -15,6 +15,18 @@ import (
"time"
)
+// Constants used in event fields.
+// See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU
+// for more details.
+const (
+ phaseDurationBegin = "B"
+ phaseDurationEnd = "E"
+ phaseFlowStart = "s"
+ phaseFlowEnd = "f"
+
+ bindEnclosingSlice = "e"
+)
+
var traceStarted int32
func getTraceContext(ctx context.Context) (traceContext, bool) {
@@ -39,7 +51,7 @@ func StartSpan(ctx context.Context, name string) (context.Context, *Span) {
Name: childSpan.name,
Time: float64(childSpan.start.UnixNano()) / float64(time.Microsecond),
TID: childSpan.tid,
- Phase: "B",
+ Phase: phaseDurationBegin,
})
ctx = context.WithValue(ctx, traceKey{}, traceContext{tc.t, tc.tid})
return ctx, childSpan
@@ -56,6 +68,34 @@ func StartGoroutine(ctx context.Context) context.Context {
return context.WithValue(ctx, traceKey{}, traceContext{tc.t, tc.t.getNextTID()})
}
+// Flow marks a flow indicating that the 'to' span depends on the 'from' span.
+// Flow should be called while the 'to' span is in progress.
+func Flow(ctx context.Context, from *Span, to *Span) {
+ tc, ok := getTraceContext(ctx)
+ if !ok || from == nil || to == nil {
+ return
+ }
+
+ id := tc.t.getNextFlowID()
+ tc.t.writeEvent(&traceviewer.Event{
+ Name: from.name + " -> " + to.name,
+ Category: "flow",
+ ID: id,
+ Time: float64(from.end.UnixNano()) / float64(time.Microsecond),
+ Phase: phaseFlowStart,
+ TID: from.tid,
+ })
+ tc.t.writeEvent(&traceviewer.Event{
+ Name: from.name + " -> " + to.name,
+ Category: "flow", // TODO(matloob): Add Category to Flow?
+ ID: id,
+ Time: float64(to.start.UnixNano()) / float64(time.Microsecond),
+ Phase: phaseFlowEnd,
+ TID: to.tid,
+ BindPoint: bindEnclosingSlice,
+ })
+}
+
type Span struct {
t *tracer
@@ -74,14 +114,15 @@ func (s *Span) Done() {
Name: s.name,
Time: float64(s.end.UnixNano()) / float64(time.Microsecond),
TID: s.tid,
- Phase: "E",
+ Phase: phaseDurationEnd,
})
}
type tracer struct {
file chan traceFile // 1-buffered
- nextTID uint64
+ nextTID uint64
+ nextFlowID uint64
}
func (t *tracer) writeEvent(ev *traceviewer.Event) error {
@@ -123,6 +164,10 @@ func (t *tracer) getNextTID() uint64 {
return atomic.AddUint64(&t.nextTID, 1)
}
+func (t *tracer) getNextFlowID() uint64 {
+ return atomic.AddUint64(&t.nextFlowID, 1)
+}
+
// traceKey is the context key for tracing information. It is unexported to prevent collisions with context keys defined in
// other packages.
type traceKey struct{}
diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go
index a37a5e618d..825e763c03 100644
--- a/src/cmd/go/internal/work/action.go
+++ b/src/cmd/go/internal/work/action.go
@@ -26,6 +26,7 @@ import (
"cmd/go/internal/cache"
"cmd/go/internal/cfg"
"cmd/go/internal/load"
+ "cmd/go/internal/trace"
"cmd/internal/buildid"
)
@@ -92,10 +93,11 @@ type Action struct {
output []byte // output redirect buffer (nil means use b.Print)
// Execution state.
- pending int // number of deps yet to complete
- priority int // relative execution priority
- Failed bool // whether the action failed
- json *actionJSON // action graph information
+ pending int // number of deps yet to complete
+ priority int // relative execution priority
+ Failed bool // whether the action failed
+ json *actionJSON // action graph information
+ traceSpan *trace.Span
}
// BuildActionID returns the action ID section of a's build ID.
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 3903502a67..681ecd7646 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -127,6 +127,10 @@ func (b *Builder) Do(ctx context.Context, root *Action) {
desc += "(" + a.Mode + " " + a.Package.Desc() + ")"
}
ctx, span := trace.StartSpan(ctx, desc)
+ a.traceSpan = span
+ for _, d := range a.Deps {
+ trace.Flow(ctx, d.traceSpan, a.traceSpan)
+ }
err = a.Func(b, ctx, a)
span.Done()
}
--
cgit v1.3-5-g9baa
From 38fea3a4ec97fbcfad1f2d329f3a12c53cc36301 Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Mon, 22 Jun 2020 18:39:10 -0400
Subject: cmd/go: add tracing instrumentation to load.TestPackagesFor
This change adds tracing instrumentation into load.TestPackagesFor,
propagating context through its callers.
Updates #38714
Change-Id: I80fefaf3116ccccffaa8bb7613a656bda867394c
Reviewed-on: https://go-review.googlesource.com/c/go/+/248323
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/list/list.go | 4 ++--
src/cmd/go/internal/load/test.go | 14 ++++++++++----
src/cmd/go/internal/test/test.go | 6 +++---
src/cmd/go/internal/vet/vet.go | 2 +-
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go
index 3ec243a759..7747e730ae 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -477,9 +477,9 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
var pmain, ptest, pxtest *load.Package
var err error
if *listE {
- pmain, ptest, pxtest = load.TestPackagesAndErrors(p, nil)
+ pmain, ptest, pxtest = load.TestPackagesAndErrors(ctx, p, nil)
} else {
- pmain, ptest, pxtest, err = load.TestPackagesFor(p, nil)
+ pmain, ptest, pxtest, err = load.TestPackagesFor(ctx, p, nil)
if err != nil {
base.Errorf("can't load test package: %s", err)
}
diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go
index 6d251e8358..6db8a00245 100644
--- a/src/cmd/go/internal/load/test.go
+++ b/src/cmd/go/internal/load/test.go
@@ -6,7 +6,7 @@ package load
import (
"bytes"
- "cmd/go/internal/str"
+ "context"
"errors"
"fmt"
"go/ast"
@@ -20,6 +20,9 @@ import (
"strings"
"unicode"
"unicode/utf8"
+
+ "cmd/go/internal/str"
+ "cmd/go/internal/trace"
)
var TestMainDeps = []string{
@@ -42,8 +45,8 @@ type TestCover struct {
// TestPackagesFor is like TestPackagesAndErrors but it returns
// an error if the test packages or their dependencies have errors.
// Only test packages without errors are returned.
-func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Package, err error) {
- pmain, ptest, pxtest = TestPackagesAndErrors(p, cover)
+func TestPackagesFor(ctx context.Context, p *Package, cover *TestCover) (pmain, ptest, pxtest *Package, err error) {
+ pmain, ptest, pxtest = TestPackagesAndErrors(ctx, p, cover)
for _, p1 := range []*Package{ptest, pxtest, pmain} {
if p1 == nil {
// pxtest may be nil
@@ -89,7 +92,10 @@ func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Packag
//
// The caller is expected to have checked that len(p.TestGoFiles)+len(p.XTestGoFiles) > 0,
// or else there's no point in any of this.
-func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest *Package) {
+func TestPackagesAndErrors(ctx context.Context, p *Package, cover *TestCover) (pmain, ptest, pxtest *Package) {
+ ctx, span := trace.StartSpan(ctx, "load.TestPackagesAndErrors")
+ defer span.Done()
+
pre := newPreload()
defer pre.flush()
allImports := append([]string{}, p.TestImports...)
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 9788590938..cda51053fb 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -746,7 +746,7 @@ func runTest(ctx context.Context, cmd *base.Command, args []string) {
ensureImport(p, "sync/atomic")
}
- buildTest, runTest, printTest, err := builderTest(&b, p)
+ buildTest, runTest, printTest, err := builderTest(&b, ctx, p)
if err != nil {
str := err.Error()
str = strings.TrimPrefix(str, "\n")
@@ -813,7 +813,7 @@ var windowsBadWords = []string{
"update",
}
-func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, printAction *work.Action, err error) {
+func builderTest(b *work.Builder, ctx context.Context, p *load.Package) (buildAction, runAction, printAction *work.Action, err error) {
if len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 {
build := b.CompileAction(work.ModeBuild, work.ModeBuild, p)
run := &work.Action{Mode: "test run", Package: p, Deps: []*work.Action{build}}
@@ -836,7 +836,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
DeclVars: declareCoverVars,
}
}
- pmain, ptest, pxtest, err := load.TestPackagesFor(p, cover)
+ pmain, ptest, pxtest, err := load.TestPackagesFor(ctx, p, cover)
if err != nil {
return nil, nil, nil, err
}
diff --git a/src/cmd/go/internal/vet/vet.go b/src/cmd/go/internal/vet/vet.go
index 58f392eb96..b306572281 100644
--- a/src/cmd/go/internal/vet/vet.go
+++ b/src/cmd/go/internal/vet/vet.go
@@ -77,7 +77,7 @@ func runVet(ctx context.Context, cmd *base.Command, args []string) {
root := &work.Action{Mode: "go vet"}
for _, p := range pkgs {
- _, ptest, pxtest, err := load.TestPackagesFor(p, nil)
+ _, ptest, pxtest, err := load.TestPackagesFor(ctx, p, nil)
if err != nil {
base.Errorf("%v", err)
continue
--
cgit v1.3-5-g9baa
From ebccba7954fe9507df993dda7ba78fa34e030390 Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Mon, 22 Jun 2020 19:02:00 -0400
Subject: cmd/go: process -debug-trace flag for cmd/test and cmd/vet
These commands are build-like commands that do their own flag
processing, so the value of debug-trace isn't available until
the command starts running. Start tracing in the cmd's run
function.
Updates #38714
Change-Id: I4d633e6ee907bf09feac52c2aff3daceb9b20e12
Reviewed-on: https://go-review.googlesource.com/c/go/+/248324
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/test/test.go | 18 ++++++++++++++++++
src/cmd/go/internal/vet/vet.go | 25 +++++++++++++++++++++++--
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index cda51053fb..9c120e08dc 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -31,6 +31,7 @@ import (
"cmd/go/internal/lockedfile"
"cmd/go/internal/modload"
"cmd/go/internal/str"
+ "cmd/go/internal/trace"
"cmd/go/internal/work"
"cmd/internal/test2json"
)
@@ -571,6 +572,23 @@ func runTest(ctx context.Context, cmd *base.Command, args []string) {
pkgArgs, testArgs = testFlags(args)
+ if cfg.DebugTrace != "" {
+ var close func() error
+ var err error
+ ctx, close, err = trace.Start(ctx, cfg.DebugTrace)
+ if err != nil {
+ base.Fatalf("failed to start trace: %v", err)
+ }
+ defer func() {
+ if err := close(); err != nil {
+ base.Fatalf("failed to stop trace: %v", err)
+ }
+ }()
+ }
+
+ ctx, span := trace.StartSpan(ctx, fmt.Sprint("Running ", cmd.Name(), " command"))
+ defer span.Done()
+
work.FindExecCmd() // initialize cached result
work.BuildInit()
diff --git a/src/cmd/go/internal/vet/vet.go b/src/cmd/go/internal/vet/vet.go
index b306572281..cf2c8d59e8 100644
--- a/src/cmd/go/internal/vet/vet.go
+++ b/src/cmd/go/internal/vet/vet.go
@@ -6,12 +6,16 @@
package vet
import (
+ "context"
+ "fmt"
+ "path/filepath"
+
"cmd/go/internal/base"
+ "cmd/go/internal/cfg"
"cmd/go/internal/load"
"cmd/go/internal/modload"
+ "cmd/go/internal/trace"
"cmd/go/internal/work"
- "context"
- "path/filepath"
)
// Break init loop.
@@ -54,6 +58,23 @@ func runVet(ctx context.Context, cmd *base.Command, args []string) {
vetFlags, pkgArgs := vetFlags(args)
+ if cfg.DebugTrace != "" {
+ var close func() error
+ var err error
+ ctx, close, err = trace.Start(ctx, cfg.DebugTrace)
+ if err != nil {
+ base.Fatalf("failed to start trace: %v", err)
+ }
+ defer func() {
+ if err := close(); err != nil {
+ base.Fatalf("failed to stop trace: %v", err)
+ }
+ }()
+ }
+
+ ctx, span := trace.StartSpan(ctx, fmt.Sprint("Running ", cmd.Name(), " command"))
+ defer span.Done()
+
work.BuildInit()
work.VetFlags = vetFlags
if len(vetFlags) > 0 {
--
cgit v1.3-5-g9baa
From 2ac4bf3802f0786a0afb09488173507f40d5d885 Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Wed, 24 Jun 2020 15:58:47 -0400
Subject: cmd/go: add span for modload.LoadBuildList
This change adds context, and a span to modload.LoadBuildList and
propagates context into modload.BuildList. It's the start
of a run of CLs to add trace spans for module operations.
Updates #38714
Change-Id: I0d58dd394051526338092dc9a5ec29a9e087e4e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/248325
Run-TryBot: Michael Matloob
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/list/list.go | 4 ++--
src/cmd/go/internal/modcmd/download.go | 2 +-
src/cmd/go/internal/modcmd/graph.go | 2 +-
src/cmd/go/internal/modcmd/verify.go | 2 +-
src/cmd/go/internal/modcmd/why.go | 2 +-
src/cmd/go/internal/modget/get.go | 8 ++++----
src/cmd/go/internal/modload/list.go | 9 +++++----
src/cmd/go/internal/modload/load.go | 23 ++++++++++++++---------
8 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go
index 7747e730ae..7303e6c866 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -413,9 +413,9 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
}
}
- modload.LoadBuildList()
+ modload.LoadBuildList(ctx)
- mods := modload.ListModules(args, *listU, *listVersions)
+ mods := modload.ListModules(ctx, args, *listU, *listVersions)
if !*listE {
for _, m := range mods {
if m.Error != nil {
diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go
index b43c32be5a..946e8ed3cf 100644
--- a/src/cmd/go/internal/modcmd/download.go
+++ b/src/cmd/go/internal/modcmd/download.go
@@ -106,7 +106,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
var work par.Work
listU := false
listVersions := false
- for _, info := range modload.ListModules(args, listU, listVersions) {
+ for _, info := range modload.ListModules(ctx, args, listU, listVersions) {
if info.Replace != nil {
info = info.Replace
}
diff --git a/src/cmd/go/internal/modcmd/graph.go b/src/cmd/go/internal/modcmd/graph.go
index fff5b02626..4853503fd4 100644
--- a/src/cmd/go/internal/modcmd/graph.go
+++ b/src/cmd/go/internal/modcmd/graph.go
@@ -49,7 +49,7 @@ func runGraph(ctx context.Context, cmd *base.Command, args []string) {
base.Fatalf("go: cannot find main module; see 'go help modules'")
}
}
- modload.LoadBuildList()
+ modload.LoadBuildList(ctx)
reqs := modload.MinReqs()
format := func(m module.Version) string {
diff --git a/src/cmd/go/internal/modcmd/verify.go b/src/cmd/go/internal/modcmd/verify.go
index 570e571049..73ab714d10 100644
--- a/src/cmd/go/internal/modcmd/verify.go
+++ b/src/cmd/go/internal/modcmd/verify.go
@@ -60,7 +60,7 @@ func runVerify(ctx context.Context, cmd *base.Command, args []string) {
sem := make(chan token, runtime.GOMAXPROCS(0))
// Use a slice of result channels, so that the output is deterministic.
- mods := modload.LoadBuildList()[1:]
+ mods := modload.LoadBuildList(ctx)[1:]
errsChans := make([]<-chan []error, len(mods))
for i, mod := range mods {
diff --git a/src/cmd/go/internal/modcmd/why.go b/src/cmd/go/internal/modcmd/why.go
index 3f9cf0f120..f400339b25 100644
--- a/src/cmd/go/internal/modcmd/why.go
+++ b/src/cmd/go/internal/modcmd/why.go
@@ -74,7 +74,7 @@ func runWhy(ctx context.Context, cmd *base.Command, args []string) {
base.Fatalf("go mod why: module query not allowed")
}
}
- mods := modload.ListModules(args, listU, listVersions)
+ mods := modload.ListModules(ctx, args, listU, listVersions)
byModule := make(map[module.Version][]string)
for _, path := range loadALL() {
m := modload.PackageModule(path)
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index b217196931..93a6bb54d5 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -278,7 +278,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
}
modload.LoadTests = *getT
- buildList := modload.LoadBuildList()
+ buildList := modload.LoadBuildList(ctx)
buildList = buildList[:len(buildList):len(buildList)] // copy on append
versionByPath := make(map[string]string)
for _, m := range buildList {
@@ -444,7 +444,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
// packages in unknown modules can't be expanded. This also avoids looking
// up new modules while loading packages, only to downgrade later.
queryCache := make(map[querySpec]*query)
- byPath := runQueries(queryCache, queries, nil)
+ byPath := runQueries(ctx, queryCache, queries, nil)
// Add missing modules to the build list.
// We call SetBuildList here and elsewhere, since newUpgrader,
@@ -586,7 +586,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
// Query target versions for modules providing packages matched by
// command line arguments.
- byPath = runQueries(queryCache, queries, modOnly)
+ byPath = runQueries(ctx, queryCache, queries, modOnly)
// Handle upgrades. This is needed for arguments that didn't match
// modules or matched different modules from a previous iteration. It
@@ -724,7 +724,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
// versions (including earlier queries in the modOnly map), an error will be
// reported. A map from module paths to queries is returned, which includes
// queries and modOnly.
-func runQueries(cache map[querySpec]*query, queries []*query, modOnly map[string]*query) map[string]*query {
+func runQueries(ctx context.Context, cache map[querySpec]*query, queries []*query, modOnly map[string]*query) map[string]*query {
var lookup par.Work
for _, q := range queries {
if cached := cache[q.querySpec]; cached != nil {
diff --git a/src/cmd/go/internal/modload/list.go b/src/cmd/go/internal/modload/list.go
index 9400793bcb..4768516e90 100644
--- a/src/cmd/go/internal/modload/list.go
+++ b/src/cmd/go/internal/modload/list.go
@@ -5,6 +5,7 @@
package modload
import (
+ "context"
"errors"
"fmt"
"os"
@@ -19,8 +20,8 @@ import (
"golang.org/x/mod/module"
)
-func ListModules(args []string, listU, listVersions bool) []*modinfo.ModulePublic {
- mods := listModules(args, listVersions)
+func ListModules(ctx context.Context, args []string, listU, listVersions bool) []*modinfo.ModulePublic {
+ mods := listModules(ctx, args, listVersions)
if listU || listVersions {
var work par.Work
for _, m := range mods {
@@ -42,8 +43,8 @@ func ListModules(args []string, listU, listVersions bool) []*modinfo.ModulePubli
return mods
}
-func listModules(args []string, listVersions bool) []*modinfo.ModulePublic {
- LoadBuildList()
+func listModules(ctx context.Context, args []string, listVersions bool) []*modinfo.ModulePublic {
+ LoadBuildList(ctx)
if len(args) == 0 {
return []*modinfo.ModulePublic{moduleInfo(buildList[0], true)}
}
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 30992e0cc2..8190009b23 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -6,14 +6,7 @@ package modload
import (
"bytes"
- "cmd/go/internal/base"
- "cmd/go/internal/cfg"
- "cmd/go/internal/imports"
- "cmd/go/internal/modfetch"
- "cmd/go/internal/mvs"
- "cmd/go/internal/par"
- "cmd/go/internal/search"
- "cmd/go/internal/str"
+ "context"
"errors"
"fmt"
"go/build"
@@ -24,6 +17,16 @@ import (
"sort"
"strings"
+ "cmd/go/internal/base"
+ "cmd/go/internal/cfg"
+ "cmd/go/internal/imports"
+ "cmd/go/internal/modfetch"
+ "cmd/go/internal/mvs"
+ "cmd/go/internal/par"
+ "cmd/go/internal/search"
+ "cmd/go/internal/str"
+ "cmd/go/internal/trace"
+
"golang.org/x/mod/module"
)
@@ -385,7 +388,9 @@ func DirImportPath(dir string) string {
// LoadBuildList need only be called if ImportPaths is not
// (typically in commands that care about the module but
// no particular package).
-func LoadBuildList() []module.Version {
+func LoadBuildList(ctx context.Context) []module.Version {
+ ctx, span := trace.StartSpan(ctx, "LoadBuildList")
+ defer span.Done()
InitMod()
ReloadBuildList()
WriteGoMod()
--
cgit v1.3-5-g9baa
From f30044a03bc7cf107dbec03c02fb6d0072878252 Mon Sep 17 00:00:00 2001
From: Michael Matloob
Date: Thu, 25 Jun 2020 19:11:28 -0400
Subject: cmd/go/internal: remove some users of par.Work
par.Work is used in a number of places as a parallel
work queue. This change replaces it with goroutines
and channels in a number of simpler places where it's
used.
Change-Id: I0620eda46ec7b2c0599a8b9361639af7bb73a05a
Reviewed-on: https://go-review.googlesource.com/c/go/+/248326
Run-TryBot: Michael Matloob
TryBot-Result: Gobot Gobot
Reviewed-by: Bryan C. Mills
---
src/cmd/go/internal/modcmd/download.go | 69 +++++++++++++++++++---------------
src/cmd/go/internal/modcmd/graph.go | 19 +++++-----
src/cmd/go/internal/modconv/convert.go | 59 ++++++++++++++++-------------
src/cmd/go/internal/modget/get.go | 41 +++++++++++++-------
src/cmd/go/internal/modload/list.go | 37 +++++++++++-------
5 files changed, 133 insertions(+), 92 deletions(-)
diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go
index 946e8ed3cf..857362a72e 100644
--- a/src/cmd/go/internal/modcmd/download.go
+++ b/src/cmd/go/internal/modcmd/download.go
@@ -5,15 +5,15 @@
package modcmd
import (
+ "cmd/go/internal/modfetch"
"context"
"encoding/json"
"os"
+ "runtime"
"cmd/go/internal/base"
"cmd/go/internal/cfg"
- "cmd/go/internal/modfetch"
"cmd/go/internal/modload"
- "cmd/go/internal/par"
"cmd/go/internal/work"
"golang.org/x/mod/module"
@@ -102,33 +102,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
}
}
- var mods []*moduleJSON
- var work par.Work
- listU := false
- listVersions := false
- for _, info := range modload.ListModules(ctx, args, listU, listVersions) {
- if info.Replace != nil {
- info = info.Replace
- }
- if info.Version == "" && info.Error == nil {
- // main module or module replaced with file path.
- // Nothing to download.
- continue
- }
- m := &moduleJSON{
- Path: info.Path,
- Version: info.Version,
- }
- mods = append(mods, m)
- if info.Error != nil {
- m.Error = info.Error.Err
- continue
- }
- work.Add(m)
- }
-
- work.Do(10, func(item interface{}) {
- m := item.(*moduleJSON)
+ downloadModule := func(m *moduleJSON) {
var err error
m.Info, err = modfetch.InfoFile(m.Path, m.Version)
if err != nil {
@@ -157,7 +131,42 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
m.Error = err.Error()
return
}
- })
+ }
+
+ var mods []*moduleJSON
+ listU := false
+ listVersions := false
+ type token struct{}
+ sem := make(chan token, runtime.GOMAXPROCS(0))
+ for _, info := range modload.ListModules(ctx, args, listU, listVersions) {
+ if info.Replace != nil {
+ info = info.Replace
+ }
+ if info.Version == "" && info.Error == nil {
+ // main module or module replaced with file path.
+ // Nothing to download.
+ continue
+ }
+ m := &moduleJSON{
+ Path: info.Path,
+ Version: info.Version,
+ }
+ mods = append(mods, m)
+ if info.Error != nil {
+ m.Error = info.Error.Err
+ continue
+ }
+ sem <- token{}
+ go func() {
+ downloadModule(m)
+ <-sem
+ }()
+ }
+
+ // Fill semaphore channel to wait for goroutines to finish.
+ for n := cap(sem); n > 0; n-- {
+ sem <- token{}
+ }
if *downloadJSON {
for _, m := range mods {
diff --git a/src/cmd/go/internal/modcmd/graph.go b/src/cmd/go/internal/modcmd/graph.go
index 4853503fd4..6da12b9cab 100644
--- a/src/cmd/go/internal/modcmd/graph.go
+++ b/src/cmd/go/internal/modcmd/graph.go
@@ -15,7 +15,6 @@ import (
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modload"
- "cmd/go/internal/par"
"cmd/go/internal/work"
"golang.org/x/mod/module"
@@ -59,23 +58,25 @@ func runGraph(ctx context.Context, cmd *base.Command, args []string) {
return m.Path + "@" + m.Version
}
- // Note: using par.Work only to manage work queue.
- // No parallelism here, so no locking.
var out []string
var deps int // index in out where deps start
- var work par.Work
- work.Add(modload.Target)
- work.Do(1, func(item interface{}) {
- m := item.(module.Version)
+ seen := map[module.Version]bool{modload.Target: true}
+ queue := []module.Version{modload.Target}
+ for len(queue) > 0 {
+ var m module.Version
+ m, queue = queue[0], queue[1:]
list, _ := reqs.Required(m)
for _, r := range list {
- work.Add(r)
+ if !seen[r] {
+ queue = append(queue, r)
+ seen[r] = true
+ }
out = append(out, format(m)+" "+format(r)+"\n")
}
if m == modload.Target {
deps = len(out)
}
- })
+ }
sort.Slice(out[deps:], func(i, j int) bool {
return out[deps+i][0] < out[deps+j][0]
diff --git a/src/cmd/go/internal/modconv/convert.go b/src/cmd/go/internal/modconv/convert.go
index f465a9f395..d5a0bc21e9 100644
--- a/src/cmd/go/internal/modconv/convert.go
+++ b/src/cmd/go/internal/modconv/convert.go
@@ -7,13 +7,12 @@ package modconv
import (
"fmt"
"os"
+ "runtime"
"sort"
"strings"
- "sync"
"cmd/go/internal/base"
"cmd/go/internal/modfetch"
- "cmd/go/internal/par"
"golang.org/x/mod/modfile"
"golang.org/x/mod/module"
@@ -42,46 +41,52 @@ func ConvertLegacyConfig(f *modfile.File, file string, data []byte) error {
// Convert requirements block, which may use raw SHA1 hashes as versions,
// to valid semver requirement list, respecting major versions.
- var (
- work par.Work
- mu sync.Mutex
- need = make(map[string]string)
- replace = make(map[string]*modfile.Replace)
- )
+ versions := make([]*module.Version, len(mf.Require))
+ replace := make(map[string]*modfile.Replace)
for _, r := range mf.Replace {
replace[r.New.Path] = r
replace[r.Old.Path] = r
}
- for _, r := range mf.Require {
+
+ type token struct{}
+ sem := make(chan token, runtime.GOMAXPROCS(0))
+ for i, r := range mf.Require {
m := r.Mod
if m.Path == "" {
continue
}
if re, ok := replace[m.Path]; ok {
- work.Add(re.New)
- continue
+ m = re.New
}
- work.Add(r.Mod)
+ sem <- token{}
+ go func(i int, m module.Version) {
+ repo, info, err := modfetch.ImportRepoRev(m.Path, m.Version)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "go: converting %s: stat %s@%s: %v\n", base.ShortPath(file), m.Path, m.Version, err)
+ return
+ }
+
+ path := repo.ModulePath()
+ versions[i].Path = path
+ versions[i].Version = info.Version
+
+ <-sem
+ }(i, m)
+ }
+ // Fill semaphore channel to wait for all tasks to finish.
+ for n := cap(sem); n > 0; n-- {
+ sem <- token{}
}
- work.Do(10, func(item interface{}) {
- r := item.(module.Version)
- repo, info, err := modfetch.ImportRepoRev(r.Path, r.Version)
- if err != nil {
- fmt.Fprintf(os.Stderr, "go: converting %s: stat %s@%s: %v\n", base.ShortPath(file), r.Path, r.Version, err)
- return
- }
- mu.Lock()
- path := repo.ModulePath()
+ need := map[string]string{}
+ for _, v := range versions {
// Don't use semver.Max here; need to preserve +incompatible suffix.
- if v, ok := need[path]; !ok || semver.Compare(v, info.Version) < 0 {
- need[path] = info.Version
+ if needv, ok := need[v.Path]; !ok || semver.Compare(needv, v.Version) < 0 {
+ need[v.Path] = v.Version
}
- mu.Unlock()
- })
-
- var paths []string
+ }
+ paths := make([]string, 0, len(need))
for path := range need {
paths = append(paths, path)
}
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index 93a6bb54d5..d02c9a8da5 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -11,6 +11,7 @@ import (
"fmt"
"os"
"path/filepath"
+ "runtime"
"sort"
"strings"
"sync"
@@ -21,7 +22,6 @@ import (
"cmd/go/internal/load"
"cmd/go/internal/modload"
"cmd/go/internal/mvs"
- "cmd/go/internal/par"
"cmd/go/internal/search"
"cmd/go/internal/work"
@@ -725,18 +725,8 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
// reported. A map from module paths to queries is returned, which includes
// queries and modOnly.
func runQueries(ctx context.Context, cache map[querySpec]*query, queries []*query, modOnly map[string]*query) map[string]*query {
- var lookup par.Work
- for _, q := range queries {
- if cached := cache[q.querySpec]; cached != nil {
- *q = *cached
- } else {
- cache[q.querySpec] = q
- lookup.Add(q)
- }
- }
- lookup.Do(10, func(item interface{}) {
- q := item.(*query)
+ runQuery := func(q *query) {
if q.vers == "none" {
// Wait for downgrade step.
q.m = module.Version{Path: q.path, Version: "none"}
@@ -747,7 +737,32 @@ func runQueries(ctx context.Context, cache map[querySpec]*query, queries []*quer
base.Errorf("go get %s: %v", q.arg, err)
}
q.m = m
- })
+ }
+
+ type token struct{}
+ sem := make(chan token, runtime.GOMAXPROCS(0))
+ for _, q := range queries {
+ if cached := cache[q.querySpec]; cached != nil {
+ *q = *cached
+ } else {
+ sem <- token{}
+ go func(q *query) {
+ runQuery(q)
+ <-sem
+ }(q)
+ }
+ }
+
+ // Fill semaphore channel to wait for goroutines to finish.
+ for n := cap(sem); n > 0; n-- {
+ sem <- token{}
+ }
+
+ // Add to cache after concurrent section to avoid races...
+ for _, q := range queries {
+ cache[q.querySpec] = q
+ }
+
base.ExitIfErrors()
byPath := make(map[string]*query)
diff --git a/src/cmd/go/internal/modload/list.go b/src/cmd/go/internal/modload/list.go
index 4768516e90..8db4d64706 100644
--- a/src/cmd/go/internal/modload/list.go
+++ b/src/cmd/go/internal/modload/list.go
@@ -9,12 +9,12 @@ import (
"errors"
"fmt"
"os"
+ "runtime"
"strings"
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modinfo"
- "cmd/go/internal/par"
"cmd/go/internal/search"
"golang.org/x/mod/module"
@@ -22,24 +22,35 @@ import (
func ListModules(ctx context.Context, args []string, listU, listVersions bool) []*modinfo.ModulePublic {
mods := listModules(ctx, args, listVersions)
+
+ type token struct{}
+ sem := make(chan token, runtime.GOMAXPROCS(0))
if listU || listVersions {
- var work par.Work
for _, m := range mods {
- work.Add(m)
+ add := func(m *modinfo.ModulePublic) {
+ sem <- token{}
+ go func() {
+ if listU {
+ addUpdate(m)
+ }
+ if listVersions {
+ addVersions(m)
+ }
+ <-sem
+ }()
+ }
+
+ add(m)
if m.Replace != nil {
- work.Add(m.Replace)
+ add(m.Replace)
}
}
- work.Do(10, func(item interface{}) {
- m := item.(*modinfo.ModulePublic)
- if listU {
- addUpdate(m)
- }
- if listVersions {
- addVersions(m)
- }
- })
}
+ // Fill semaphore channel to wait for all tasks to finish.
+ for n := cap(sem); n > 0; n-- {
+ sem <- token{}
+ }
+
return mods
}
--
cgit v1.3-5-g9baa
From 1b86bdbdc3991c13c6ed156100a5f4918fdd9c6b Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills"
Date: Fri, 14 Aug 2020 17:44:22 -0400
Subject: cmd/test2json: do not emit a final Action if the result is not known
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If we are parsing a test output, and the test does not end in the
usual PASS or FAIL line (say, because it panicked), then we need the
exit status of the test binary in order to determine whether the test
passed or failed. If we don't have that status available, we shouldn't
guess arbitrarily — instead, we should omit the final "pass" or "fail"
action entirely.
(In practice, we nearly always DO have the final status, such as when
running 'go test' or 'go tool test2json some.exe'.)
Fixes #40132
Change-Id: Iae482577361a6033395fe4a05d746b980e18c3de
Reviewed-on: https://go-review.googlesource.com/c/go/+/248624
Run-TryBot: Bryan C. Mills
TryBot-Result: Gobot Gobot
Reviewed-by: Jay Conrod
---
src/cmd/go/internal/test/test.go | 8 +-
src/cmd/go/testdata/script/test_json_exit.txt | 102 +++++++++++++++++++++
src/cmd/internal/test2json/test2json.go | 44 +++++----
.../internal/test2json/testdata/benchshort.json | 1 -
src/cmd/internal/test2json/testdata/empty.json | 1 -
src/cmd/test2json/main.go | 6 +-
6 files changed, 139 insertions(+), 23 deletions(-)
create mode 100644 src/cmd/go/testdata/script/test_json_exit.txt
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 9c120e08dc..9cef8cf89c 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1098,9 +1098,13 @@ func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work.
}
var stdout io.Writer = os.Stdout
+ var err error
if testJSON {
json := test2json.NewConverter(lockedStdout{}, a.Package.ImportPath, test2json.Timestamp)
- defer json.Close()
+ defer func() {
+ json.Exited(err)
+ json.Close()
+ }()
stdout = json
}
@@ -1204,7 +1208,7 @@ func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work.
}
t0 := time.Now()
- err := cmd.Start()
+ err = cmd.Start()
// This is a last-ditch deadline to detect and
// stop wedged test binaries, to keep the builders
diff --git a/src/cmd/go/testdata/script/test_json_exit.txt b/src/cmd/go/testdata/script/test_json_exit.txt
new file mode 100644
index 0000000000..dc7ffb06cf
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_json_exit.txt
@@ -0,0 +1,102 @@
+[short] skip
+
+go test -c -o mainpanic.exe ./mainpanic &
+go test -c -o mainexit0.exe ./mainexit0 &
+go test -c -o testpanic.exe ./testpanic &
+go test -c -o testbgpanic.exe ./testbgpanic &
+wait
+
+# Test binaries that panic in TestMain should be marked as failing.
+
+! go test -json ./mainpanic
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+! go tool test2json ./mainpanic.exe
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+# Test binaries that exit with status 0 should be marked as passing.
+
+go test -json ./mainexit0
+stdout '"Action":"pass"'
+! stdout '"Action":"fail"'
+
+go tool test2json ./mainexit0.exe
+stdout '"Action":"pass"'
+! stdout '"Action":"fail"'
+
+# Test functions that panic should never be marked as passing
+# (https://golang.org/issue/40132).
+
+! go test -json ./testpanic
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+! go tool test2json ./testpanic.exe -test.v
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+! go tool test2json ./testpanic.exe
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+# Tests that panic in a background goroutine should be marked as failing.
+
+! go test -json ./testbgpanic
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+! go tool test2json ./testbgpanic.exe -test.v
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+! go tool test2json ./testbgpanic.exe
+stdout '"Action":"fail"'
+! stdout '"Action":"pass"'
+
+-- go.mod --
+module m
+go 1.14
+-- mainpanic/mainpanic_test.go --
+package mainpanic_test
+
+import "testing"
+
+func TestMain(m *testing.M) {
+ panic("haha no")
+}
+-- mainexit0/mainexit0_test.go --
+package mainexit0_test
+
+import (
+ "fmt"
+ "os"
+ "testing"
+)
+
+func TestMain(m *testing.M) {
+ fmt.Println("nothing to do")
+ os.Exit(0)
+}
+-- testpanic/testpanic_test.go --
+package testpanic_test
+
+import "testing"
+
+func TestPanic(*testing.T) {
+ panic("haha no")
+}
+-- testbgpanic/testbgpanic_test.go --
+package testbgpanic_test
+
+import "testing"
+
+func TestPanicInBackground(*testing.T) {
+ c := make(chan struct{})
+ go func() {
+ panic("haha no")
+ close(c)
+ }()
+ <-c
+}
diff --git a/src/cmd/internal/test2json/test2json.go b/src/cmd/internal/test2json/test2json.go
index a01a8900e8..4eb6dd4838 100644
--- a/src/cmd/internal/test2json/test2json.go
+++ b/src/cmd/internal/test2json/test2json.go
@@ -45,10 +45,10 @@ type textBytes []byte
func (b textBytes) MarshalText() ([]byte, error) { return b, nil }
-// A converter holds the state of a test-to-JSON conversion.
+// A Converter holds the state of a test-to-JSON conversion.
// It implements io.WriteCloser; the caller writes test output in,
// and the converter writes JSON output to w.
-type converter struct {
+type Converter struct {
w io.Writer // JSON output stream
pkg string // package to name in events
mode Mode // mode bits
@@ -100,9 +100,9 @@ var (
//
// The pkg string, if present, specifies the import path to
// report in the JSON stream.
-func NewConverter(w io.Writer, pkg string, mode Mode) io.WriteCloser {
- c := new(converter)
- *c = converter{
+func NewConverter(w io.Writer, pkg string, mode Mode) *Converter {
+ c := new(Converter)
+ *c = Converter{
w: w,
pkg: pkg,
mode: mode,
@@ -122,11 +122,20 @@ func NewConverter(w io.Writer, pkg string, mode Mode) io.WriteCloser {
}
// Write writes the test input to the converter.
-func (c *converter) Write(b []byte) (int, error) {
+func (c *Converter) Write(b []byte) (int, error) {
c.input.write(b)
return len(b), nil
}
+// Exited marks the test process as having exited with the given error.
+func (c *Converter) Exited(err error) {
+ if err == nil {
+ c.result = "pass"
+ } else {
+ c.result = "fail"
+ }
+}
+
var (
// printed by test on successful run.
bigPass = []byte("PASS\n")
@@ -160,7 +169,7 @@ var (
// handleInputLine handles a single whole test output line.
// It must write the line to c.output but may choose to do so
// before or after emitting other events.
-func (c *converter) handleInputLine(line []byte) {
+func (c *Converter) handleInputLine(line []byte) {
// Final PASS or FAIL.
if bytes.Equal(line, bigPass) || bytes.Equal(line, bigFail) || bytes.HasPrefix(line, bigFailErrorPrefix) {
c.flushReport(0)
@@ -286,7 +295,7 @@ func (c *converter) handleInputLine(line []byte) {
}
// flushReport flushes all pending PASS/FAIL reports at levels >= depth.
-func (c *converter) flushReport(depth int) {
+func (c *Converter) flushReport(depth int) {
c.testName = ""
for len(c.report) > depth {
e := c.report[len(c.report)-1]
@@ -298,23 +307,22 @@ func (c *converter) flushReport(depth int) {
// Close marks the end of the go test output.
// It flushes any pending input and then output (only partial lines at this point)
// and then emits the final overall package-level pass/fail event.
-func (c *converter) Close() error {
+func (c *Converter) Close() error {
c.input.flush()
c.output.flush()
- e := &event{Action: "pass"}
if c.result != "" {
- e.Action = c.result
- }
- if c.mode&Timestamp != 0 {
- dt := time.Since(c.start).Round(1 * time.Millisecond).Seconds()
- e.Elapsed = &dt
+ e := &event{Action: c.result}
+ if c.mode&Timestamp != 0 {
+ dt := time.Since(c.start).Round(1 * time.Millisecond).Seconds()
+ e.Elapsed = &dt
+ }
+ c.writeEvent(e)
}
- c.writeEvent(e)
return nil
}
// writeOutputEvent writes a single output event with the given bytes.
-func (c *converter) writeOutputEvent(out []byte) {
+func (c *Converter) writeOutputEvent(out []byte) {
c.writeEvent(&event{
Action: "output",
Output: (*textBytes)(&out),
@@ -323,7 +331,7 @@ func (c *converter) writeOutputEvent(out []byte) {
// writeEvent writes a single event.
// It adds the package, time (if requested), and test name (if needed).
-func (c *converter) writeEvent(e *event) {
+func (c *Converter) writeEvent(e *event) {
e.Package = c.pkg
if c.mode&Timestamp != 0 {
t := time.Now()
diff --git a/src/cmd/internal/test2json/testdata/benchshort.json b/src/cmd/internal/test2json/testdata/benchshort.json
index 28e287c848..34b03b9362 100644
--- a/src/cmd/internal/test2json/testdata/benchshort.json
+++ b/src/cmd/internal/test2json/testdata/benchshort.json
@@ -4,4 +4,3 @@
{"Action":"output","Output":"# but to avoid questions of timing, we just use a file with no \\n at all.\n"}
{"Action":"output","Output":"BenchmarkFoo \t"}
{"Action":"output","Output":"10000 early EOF"}
-{"Action":"pass"}
diff --git a/src/cmd/internal/test2json/testdata/empty.json b/src/cmd/internal/test2json/testdata/empty.json
index 80b5217501..e69de29bb2 100644
--- a/src/cmd/internal/test2json/testdata/empty.json
+++ b/src/cmd/internal/test2json/testdata/empty.json
@@ -1 +0,0 @@
-{"Action":"pass"}
diff --git a/src/cmd/test2json/main.go b/src/cmd/test2json/main.go
index 0385d8f246..57a874193e 100644
--- a/src/cmd/test2json/main.go
+++ b/src/cmd/test2json/main.go
@@ -118,12 +118,16 @@ func main() {
w := &countWriter{0, c}
cmd.Stdout = w
cmd.Stderr = w
- if err := cmd.Run(); err != nil {
+ err := cmd.Run()
+ if err != nil {
if w.n > 0 {
// Assume command printed why it failed.
} else {
fmt.Fprintf(c, "test2json: %v\n", err)
}
+ }
+ c.Exited(err)
+ if err != nil {
c.Close()
os.Exit(1)
}
--
cgit v1.3-5-g9baa
From 797124f5ff4bb80957007adbf3115287a4e90870 Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills"
Date: Fri, 14 Aug 2020 15:47:49 -0400
Subject: cmd/go/internal/test: keep looking for go command flags after
ambiguous test flag
Fixes #40763
Change-Id: I275970d1f8561414571a5b93e368d68fa052c60f
Reviewed-on: https://go-review.googlesource.com/c/go/+/248618
Run-TryBot: Bryan C. Mills