From 9d65578b83f0d52f0d2f526212dd3b4ee1a5f031 Mon Sep 17 00:00:00 2001 From: fanzha02 Date: Wed, 30 Jun 2021 10:51:54 +0800 Subject: [PATCH 01/11] cmd/compile: fix typos in document Correct "a2Spill" to "a3Spill" Change-Id: I6ac4c45973dfaeb16d3a90d835589b6af1aefe1d Reviewed-on: https://go-review.googlesource.com/c/go/+/331850 Trust: fannie zhang Reviewed-by: Cherry Mui --- src/cmd/compile/abi-internal.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/compile/abi-internal.md b/src/cmd/compile/abi-internal.md index 1ae3c2538ff..2bb40550832 100644 --- a/src/cmd/compile/abi-internal.md +++ b/src/cmd/compile/abi-internal.md @@ -233,7 +233,7 @@ stack frame is laid out in the following sequence: r1.x uintptr r1.y [2]uintptr a1Spill uint8 - a2Spill uint8 + a3Spill uint8 _ [6]uint8 // alignment padding In the stack frame, only the `a2` field is initialized on entry; the From eb437ba92cbb08a86ae064cbd7376c4a8e80485b Mon Sep 17 00:00:00 2001 From: go101 Date: Thu, 1 Jul 2021 14:25:45 +0000 Subject: [PATCH 02/11] cmd/compile: make stack value size threshold comparisons consistent Consistency is beautiful. Change-Id: Ib110dcff0ce2fa87b5576c79cd79c83aab385a7c GitHub-Last-Rev: b8758f8ae02cb025267aa87ebc5c2f9b4c32e742 GitHub-Pull-Request: golang/go#47011 Reviewed-on: https://go-review.googlesource.com/c/go/+/332230 Reviewed-by: Keith Randall Reviewed-by: Robert Griesemer Run-TryBot: Keith Randall TryBot-Result: Go Bot --- src/cmd/compile/internal/escape/escape.go | 8 ++++---- src/cmd/compile/internal/walk/builtin.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cmd/compile/internal/escape/escape.go b/src/cmd/compile/internal/escape/escape.go index 3ac7ff1ebe1..cd56f07b614 100644 --- a/src/cmd/compile/internal/escape/escape.go +++ b/src/cmd/compile/internal/escape/escape.go @@ -2013,14 +2013,14 @@ func HeapAllocReason(n ir.Node) string { return "too large for stack" } - if (n.Op() == ir.ONEW || n.Op() == ir.OPTRLIT) && n.Type().Elem().Width >= ir.MaxImplicitStackVarSize { + if (n.Op() == ir.ONEW || n.Op() == ir.OPTRLIT) && n.Type().Elem().Width > ir.MaxImplicitStackVarSize { return "too large for stack" } - if n.Op() == ir.OCLOSURE && typecheck.ClosureType(n.(*ir.ClosureExpr)).Size() >= ir.MaxImplicitStackVarSize { + if n.Op() == ir.OCLOSURE && typecheck.ClosureType(n.(*ir.ClosureExpr)).Size() > ir.MaxImplicitStackVarSize { return "too large for stack" } - if n.Op() == ir.OCALLPART && typecheck.PartialCallType(n.(*ir.SelectorExpr)).Size() >= ir.MaxImplicitStackVarSize { + if n.Op() == ir.OCALLPART && typecheck.PartialCallType(n.(*ir.SelectorExpr)).Size() > ir.MaxImplicitStackVarSize { return "too large for stack" } @@ -2033,7 +2033,7 @@ func HeapAllocReason(n ir.Node) string { if !ir.IsSmallIntConst(r) { return "non-constant size" } - if t := n.Type(); t.Elem().Width != 0 && ir.Int64Val(r) >= ir.MaxImplicitStackVarSize/t.Elem().Width { + if t := n.Type(); t.Elem().Width != 0 && ir.Int64Val(r) > ir.MaxImplicitStackVarSize/t.Elem().Width { return "too large for stack" } } diff --git a/src/cmd/compile/internal/walk/builtin.go b/src/cmd/compile/internal/walk/builtin.go index 1f08e4d3128..14efc05e327 100644 --- a/src/cmd/compile/internal/walk/builtin.go +++ b/src/cmd/compile/internal/walk/builtin.go @@ -489,7 +489,7 @@ func walkNew(n *ir.UnaryExpr, init *ir.Nodes) ir.Node { base.Errorf("%v can't be allocated in Go; it is incomplete (or unallocatable)", n.Type().Elem()) } if n.Esc() == ir.EscNone { - if t.Size() >= ir.MaxImplicitStackVarSize { + if t.Size() > ir.MaxImplicitStackVarSize { base.Fatalf("large ONEW with EscNone: %v", n) } return stackTempAddr(init, t) From 835d86a17ebf32a3cb081f66119c74363dbd8825 Mon Sep 17 00:00:00 2001 From: Yasuhiro Matsumoto Date: Wed, 23 Jun 2021 01:02:33 +0900 Subject: [PATCH 03/11] cmd/go: use path.Dir instead of filepath.Dir for package paths in 'go mod vendor' copyMetadata walk-up to parent directory until the pkg become modPath. But pkg should be slash-separated paths. It have to use path.Dir instead of filepath.Dir. Fixes #46867 Change-Id: I44cf1429fe52379a7415b94cc30ae3275cc430e8 Reviewed-on: https://go-review.googlesource.com/c/go/+/330149 Reviewed-by: Bryan C. Mills Trust: Bryan C. Mills Trust: Alexander Rakoczy Trust: Carlos Amedee Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot --- src/cmd/go/internal/modcmd/vendor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/internal/modcmd/vendor.go b/src/cmd/go/internal/modcmd/vendor.go index 8e1c0432f72..713d5f9f3fa 100644 --- a/src/cmd/go/internal/modcmd/vendor.go +++ b/src/cmd/go/internal/modcmd/vendor.go @@ -13,6 +13,7 @@ import ( "io" "io/fs" "os" + "path" "path/filepath" "sort" "strings" @@ -299,7 +300,7 @@ func copyMetadata(modPath, pkg, dst, src string, copiedFiles map[string]bool) { if modPath == pkg { break } - pkg = filepath.Dir(pkg) + pkg = path.Dir(pkg) dst = filepath.Dir(dst) src = filepath.Dir(src) } From 770899f7e11e32ee8500fc033a0aad369a6a5f7b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 1 Jul 2021 12:32:05 -0400 Subject: [PATCH 04/11] cmd/go: add a regression test for 'go mod vendor' path traversal For #46867 Change-Id: I1547ebf7b91e9ddd7b67fd2f20e91391d79fa35d Reviewed-on: https://go-review.googlesource.com/c/go/+/332250 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- .../testdata/script/mod_vendor_issue46867.txt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 src/cmd/go/testdata/script/mod_vendor_issue46867.txt diff --git a/src/cmd/go/testdata/script/mod_vendor_issue46867.txt b/src/cmd/go/testdata/script/mod_vendor_issue46867.txt new file mode 100644 index 00000000000..38ae87b44a9 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_vendor_issue46867.txt @@ -0,0 +1,31 @@ +# Regression test for golang.org/issue/46867: +# 'go mod vendor' on Windows attempted to open and copy +# files from directories outside of the module. + +cd subdir +go mod vendor +! exists vendor/example.net/NOTICE +exists vendor/example.net/m/NOTICE + +-- subdir/go.mod -- +module golang.org/issue46867 + +go 1.17 + +replace example.net/m v0.1.0 => ./m + +require example.net/m v0.1.0 +-- subdir/issue.go -- +package issue + +import _ "example.net/m/n" +-- subdir/m/go.mod -- +module example.net/m + +go 1.17 +-- subdir/m/n/n.go -- +package n +-- subdir/m/NOTICE -- +This notice is in module m and SHOULD be vendored. +-- subdir/NOTICE -- +This notice is outside of module m and SHOULD NOT be vendored. From ef8ae82b37657ab788f490bd757ad1b5592b952f Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 1 Jul 2021 11:22:02 -0400 Subject: [PATCH 05/11] cmd/compile: fix bug in dwarf-gen var location generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch fixes a bug in the SSA back end's DWARF generation code that determines variable locations / lifetimes. The code in question was written to handle sequences of initial pseudo-ops (zero width instructions such as OpPhi, OpArg, etc) in a basic block, detecting these ops at the start of a block and then treating the values specially when emitting ranges for the variables in those values. The logic in this code wasn't quite correct, meaning that a flag variable wasn't being set properly to record the presence of a block of zero-width value-bearing ops, leading to incorrect or missing DWARF locations for register params. Also in this patch is a tweak to some sanity-checking code intended to catch scheduling problems with OpArg/OpPhi etc. The checks need to allow for the possibility of an Arg op scheduled after a spill of an incoming register param inserted by the register allocator. Example: b1: v13 = ArgIntReg {p1+16} [2] : CX v14 = ArgIntReg {p2+16} [5] : R8 v38 = ArgIntReg {p3+16} [8] : R11 v35 = ArgIntReg {p1+0} [0] : AX v15 = StoreReg v35 : .autotmp_4[int] v40 = Arg {p4} [16] : p4+16[int] v1 = InitMem v3 = SB : SB v18 = CMPQ v14 v13 NE v18 → b3 b2 (unlikely) (18) Here the register allocator has decided to spill v35, meaning that the OpArg v40 is no longer going to be positioned prior to all other non-zero-width ops; this is a valid scenario and needs to be handled properly by the debug code. Fixes #46425. Change-Id: I239b3ad56a9c1b8ebf68af42e1f57308293ed7e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/332269 Trust: Than McIntosh Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh TryBot-Result: Go Bot --- src/cmd/compile/internal/ssa/debug.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/internal/ssa/debug.go b/src/cmd/compile/internal/ssa/debug.go index eaa94975ec2..8e2872363b6 100644 --- a/src/cmd/compile/internal/ssa/debug.go +++ b/src/cmd/compile/internal/ssa/debug.go @@ -1115,8 +1115,14 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { continue } + mustBeFirst := func(v *Value) bool { + return v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() || + v.Op == OpArgIntReg || v.Op == OpArgFloatReg + } + zeroWidthPending := false - apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr + blockPrologComplete := false // set to true at first non-zero-width op + apcChangedSize := 0 // size of changedVars for leading Args, Phi, ClosurePtr // expect to see values in pattern (apc)* (zerowidth|real)* for _, v := range b.Values { slots := state.valueNames[v.ID] @@ -1125,16 +1131,16 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { if opcodeTable[v.Op].zeroWidth { if changed { - if hasAnyArgOp(v) || v.Op == OpPhi || v.Op.isLoweredGetClosurePtr() { + if mustBeFirst(v) || v.Op == OpArg { // These ranges begin at true beginning of block, not after first instruction - if zeroWidthPending { - panic(fmt.Errorf("Unexpected op '%s' mixed with OpArg/OpPhi/OpLoweredGetClosurePtr at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) + if blockPrologComplete && mustBeFirst(v) { + panic(fmt.Errorf("Unexpected placement of op '%s' appearing after non-pseudo-op at beginning of block %s in %s\n%s", v.LongString(), b, b.Func.Name, b.Func)) } apcChangedSize = len(state.changedVars.contents()) + // Other zero-width ops must wait on a "real" op. + zeroWidthPending = true continue } - // Other zero-width ops must wait on a "real" op. - zeroWidthPending = true } continue } @@ -1145,6 +1151,7 @@ func (state *debugState) buildLocationLists(blockLocs []*BlockDebug) { // Not zero-width; i.e., a "real" instruction. zeroWidthPending = false + blockPrologComplete = true for i, varID := range state.changedVars.contents() { if i < apcChangedSize { // buffered true start-of-block changes state.updateVar(VarID(varID), v.Block, BlockStart) From 877688c838c35c48f8c658891ead6c381e21f6fa Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 10 Jun 2021 15:17:17 -0400 Subject: [PATCH 06/11] testing: add TB.Setenv For #41260 and #46688. Change-Id: I6f42742cc3234a90003136ae8798a6b0e1291788 Reviewed-on: https://go-review.googlesource.com/c/go/+/326790 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- api/go1.17.txt | 1 + src/testing/testing.go | 1 + 2 files changed, 2 insertions(+) diff --git a/api/go1.17.txt b/api/go1.17.txt index 8e4c0f5624b..3d0a464fec5 100644 --- a/api/go1.17.txt +++ b/api/go1.17.txt @@ -181,6 +181,7 @@ pkg syscall (windows-amd64), type SysProcAttr struct, AdditionalInheritedHandles pkg syscall (windows-amd64), type SysProcAttr struct, ParentProcess Handle pkg testing, method (*B) Setenv(string, string) pkg testing, method (*T) Setenv(string, string) +pkg testing, type TB interface, Setenv(string, string) pkg text/template/parse, const SkipFuncCheck = 2 pkg text/template/parse, const SkipFuncCheck Mode pkg time, const Layout = "01/02 03:04:05PM '06 -0700" diff --git a/src/testing/testing.go b/src/testing/testing.go index fdf57a39535..eeee0aac172 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -644,6 +644,7 @@ type TB interface { Log(args ...interface{}) Logf(format string, args ...interface{}) Name() string + Setenv(key, value string) Skip(args ...interface{}) SkipNow() Skipf(format string, args ...interface{}) From 03761ede028d811dd7d7cf8a2690d4bfa2771d85 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 30 Jun 2021 14:28:18 -0700 Subject: [PATCH 07/11] net: don't reject null mx records Bypass hostname validity checking when a null mx record is returned as, defined in RFC 7505. Updates #46979 Change-Id: Ibe683bd6b47333a8ff30909fb2680ec8e10696ef Reviewed-on: https://go-review.googlesource.com/c/go/+/332094 Trust: Roland Shoemaker Trust: Katie Hockman Run-TryBot: Roland Shoemaker TryBot-Result: Go Bot Reviewed-by: Katie Hockman --- src/net/dnsclient_unix_test.go | 40 ++++++++++++++++++++++++++++++++++ src/net/lookup.go | 4 +++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index d69107a2f23..59cdd2bf3e2 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -1957,3 +1957,43 @@ func TestCVE202133195(t *testing.T) { t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err, expected) } } + +func TestNullMX(t *testing.T) { + fake := fakeDNSServer{ + rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) { + r := dnsmessage.Message{ + Header: dnsmessage.Header{ + ID: q.Header.ID, + Response: true, + RCode: dnsmessage.RCodeSuccess, + }, + Questions: q.Questions, + Answers: []dnsmessage.Resource{ + { + Header: dnsmessage.ResourceHeader{ + Name: q.Questions[0].Name, + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName("."), + }, + }, + }, + } + return r, nil + }, + } + r := Resolver{PreferGo: true, Dial: fake.DialContext} + rrset, err := r.LookupMX(context.Background(), "golang.org") + if err != nil { + t.Fatalf("LookupMX: %v", err) + } + if want := []*MX{&MX{Host: "."}}; !reflect.DeepEqual(rrset, want) { + records := []string{} + for _, rr := range rrset { + records = append(records, fmt.Sprintf("%v", rr)) + } + t.Errorf("records = [%v]; want [%v]", strings.Join(records, " "), want[0]) + } +} diff --git a/src/net/lookup.go b/src/net/lookup.go index 02a4cdcd1ee..b5af3a0f867 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -500,7 +500,9 @@ func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) { if mx == nil { continue } - if !isDomainName(mx.Host) { + // Bypass the hostname validity check for targets which contain only a dot, + // as this is used to represent a 'Null' MX record. + if mx.Host != "." && !isDomainName(mx.Host) { return nil, &DNSError{Err: "MX target is invalid", Name: name} } } From 6125d0c4265067cdb67af1340bf689975dd128f4 Mon Sep 17 00:00:00 2001 From: komisan19 Date: Fri, 2 Jul 2021 05:41:10 +0000 Subject: [PATCH 08/11] cmd/dist: correct comment: SysProcAttri -> SysProcAttr Fixes #46982 Change-Id: I07a18507b7aad828714b187f296fa7268f32b1c4 GitHub-Last-Rev: f498febffdae0bc93ae1794d1ee62b2ef3ecf1bb GitHub-Pull-Request: golang/go#46983 Reviewed-on: https://go-review.googlesource.com/c/go/+/331869 Reviewed-by: Tobias Klauser Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot --- src/cmd/dist/test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 4acd357974c..f40fa926dfd 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -781,7 +781,7 @@ func (t *tester) registerTests() { t.registerTest("testasan", "../misc/cgo/testasan", "go", "run", ".") } if goos == "linux" && goarch != "ppc64le" { - // because syscall.SysProcAttri struct used in misc/cgo/testsanitizers is only built on linux. + // because syscall.SysProcAttr struct used in misc/cgo/testsanitizers is only built on linux. // Some inconsistent failures happen on ppc64le so disable for now. t.registerHostTest("testsanitizers", "../misc/cgo/testsanitizers", "misc/cgo/testsanitizers", ".") } From 743f03eeb0bdcb596b46fae51d23c0fcf0db0474 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 1 Jul 2021 16:52:59 -0700 Subject: [PATCH 09/11] spec, unsafe: clarify unsafe.Slice docs For #19367 Change-Id: If0ff8ddba3b6b48e2e198cf3653e73284c7572a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/332409 Trust: Ian Lance Taylor Reviewed-by: Matthew Dempsky Reviewed-by: Robert Griesemer Reviewed-by: Keith Randall --- doc/go_spec.html | 8 +++++--- src/unsafe/unsafe.go | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/go_spec.html b/doc/go_spec.html index e0602418e80..ad21ffb1b88 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ @@ -6782,7 +6782,8 @@ The rules for valid uses of Pointer The function Slice returns a slice whose underlying array starts at ptr -and whose length and capacity are len: +and whose length and capacity are len. +Slice(ptr, len) is equivalent to 

@@ -6790,7 +6791,8 @@ and whose length and capacity are len:

-As a special case, if ptr is nil and len is zero, +except that, as a special case, if ptr +is nil and len is zero, Slice returns nil.

diff --git a/src/unsafe/unsafe.go b/src/unsafe/unsafe.go index eaf72c96181..16e3890d0be 100644 --- a/src/unsafe/unsafe.go +++ b/src/unsafe/unsafe.go @@ -217,11 +217,13 @@ func Alignof(x ArbitraryType) uintptr func Add(ptr Pointer, len IntegerType) Pointer // The function Slice returns a slice whose underlying array starts at ptr -// and whose length and capacity are len: +// and whose length and capacity are len. +// Slice(ptr, len) is equivalent to // // (*[len]ArbitraryType)(unsafe.Pointer(ptr))[:] // -// As a special case, if ptr is nil and len is zero, Slice returns nil. +// except that, as a special case, if ptr is nil and len is zero, +// Slice returns nil. // // The len argument must be of integer type or an untyped constant. // A constant len argument must be non-negative and representable by a value of type int; From 287c5e8066396e953254d7980a80ec082edf11bd Mon Sep 17 00:00:00 2001 From: go101 Date: Fri, 2 Jul 2021 08:01:20 +0000 Subject: [PATCH 10/11] cmd/compile: fix stack growing algorithm The current stack growing implementation looks not right. Specially, the line runtime/stack.go#L1068 never gets executed, which causes many unnecessary copystack calls. This PR is trying to correct the implementation. As I'm not familiar with the code, the fix is just a guess. Change-Id: I0bea1148175fad34f74f19d455c240c94d3cb78b GitHub-Last-Rev: 57205f91fe6f7cecbf0b7aad0d90d2f81270b1e8 GitHub-Pull-Request: golang/go#47010 Reviewed-on: https://go-review.googlesource.com/c/go/+/332229 Reviewed-by: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Go Bot Trust: Dmitri Shuralyov --- src/runtime/stack.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/stack.go b/src/runtime/stack.go index b21c9c95182..6e0d157630b 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -1064,7 +1064,9 @@ func newstack() { // recheck the bounds on return.) if f := findfunc(gp.sched.pc); f.valid() { max := uintptr(funcMaxSPDelta(f)) - for newsize-gp.sched.sp < max+_StackGuard { + needed := max + _StackGuard + used := gp.stack.hi - gp.sched.sp + for newsize-used < needed { newsize *= 2 } } From 912f0750472dd4f674b69ca1616bfaf377af1805 Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Tue, 8 Jun 2021 20:13:23 +0200 Subject: [PATCH 11/11] net/http: mention socks5 support in proxy Change-Id: I55b6d5c77221569eeafea625379affd476a65772 Reviewed-on: https://go-review.googlesource.com/c/go/+/326011 Reviewed-by: Damien Neil Trust: Dmitri Shuralyov --- src/net/http/transport.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 47cb992a502..309194e8e52 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -427,6 +427,7 @@ func (t *Transport) onceSetNextProtoDefaults() { // // The environment values may be either a complete URL or a // "host[:port]", in which case the "http" scheme is assumed. +// The schemes "http", "https", and "socks5" are supported. // An error is returned if the value is a different form. // // A nil URL and nil error are returned if no proxy is defined in the