From 9c7eb68fc6d4bd8ab9a33bc5c1d89d263be1ab1b Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 14 Oct 2020 16:03:48 -0700 Subject: [release-branch.go1.15] runtime: stop preemption during syscall.Exec on Darwin On current macOS versions a program that receives a signal during an execve can fail with a SIGILL signal. This appears to be a macOS kernel bug. It has been reported to Apple. This CL partially works around the problem by using execLock to not send preemption signals during execve. Of course some other stray signal could occur, but at least we can avoid exacerbating the problem. We can't simply disable signals, as that would mean that the exec'ed process would start with all signals blocked, which it likely does not expect. For #41702 Fixes #41704 Change-Id: I91b0add967b315671ddcf73269c4d30136e579b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/262438 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang (cherry picked from commit 64fb6ae95f1c322486cbfb758552bb8439a8e6e8) Reviewed-on: https://go-review.googlesource.com/c/go/+/262717 --- src/runtime/signal_unix.go | 11 +++++++++++ src/syscall/exec_unix_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index dd6d79f8ec..f9784b84a7 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -357,6 +357,13 @@ func preemptM(mp *m) { // required). return } + + // On Darwin, don't try to preempt threads during exec. + // Issue #41702. + if GOOS == "darwin" { + execLock.rlock() + } + if atomic.Cas(&mp.signalPending, 0, 1) { // If multiple threads are preempting the same M, it may send many // signals to the same M such that it hardly make progress, causing @@ -365,6 +372,10 @@ func preemptM(mp *m) { // Only send a signal if there isn't already one pending. signalM(mp, sigPreempt) } + + if GOOS == "darwin" { + execLock.runlock() + } } // sigFetchG fetches the value of G safely when running in a signal handler. diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go index 4eb3c5c6c8..d005bba610 100644 --- a/src/syscall/exec_unix_test.go +++ b/src/syscall/exec_unix_test.go @@ -9,11 +9,14 @@ package syscall_test import ( "internal/testenv" "io" + "math/rand" "os" "os/exec" "os/signal" + "runtime" "syscall" "testing" + "time" "unsafe" ) @@ -241,3 +244,45 @@ func TestInvalidExec(t *testing.T) { } }) } + +// TestExec is for issue #41702. +func TestExec(t *testing.T) { + cmd := exec.Command(os.Args[0], "-test.run=TestExecHelper") + cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=2") + o, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("%s\n%v", o, err) + } +} + +// TestExecHelper is used by TestExec. It does nothing by itself. +// In testing on macOS 10.14, this used to fail with +// "signal: illegal instruction" more than half the time. +func TestExecHelper(t *testing.T) { + if os.Getenv("GO_WANT_HELPER_PROCESS") != "2" { + return + } + + // We don't have to worry about restoring these values. + // We are in a child process that only runs this test, + // and we are going to call syscall.Exec anyhow. + runtime.GOMAXPROCS(50) + os.Setenv("GO_WANT_HELPER_PROCESS", "3") + + stop := time.Now().Add(time.Second) + for i := 0; i < 100; i++ { + go func(i int) { + r := rand.New(rand.NewSource(int64(i))) + for time.Now().Before(stop) { + r.Uint64() + } + }(i) + } + + time.Sleep(10 * time.Millisecond) + + argv := []string{os.Args[0], "-test.run=TestExecHelper"} + syscall.Exec(os.Args[0], argv, os.Environ()) + + t.Error("syscall.Exec returned") +} -- cgit v1.2.3-54-g00ecf From 0d489b80dc9e87013a8c3a650dfc05a0c4dde04a Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Oct 2020 14:19:10 -0700 Subject: [release-branch.go1.15] syscall: use MustHaveExec in TestExec For #41702 For #41704 Change-Id: Ib2b15e52aa1fef2f5e644b316c726150252fa9f8 Reviewed-on: https://go-review.googlesource.com/c/go/+/262738 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang (cherry picked from commit 11cfb48df192c14d185c1cfcaad1ba3e7b84c807) Reviewed-on: https://go-review.googlesource.com/c/go/+/264020 --- src/syscall/exec_unix_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go index d005bba610..4431f7fc90 100644 --- a/src/syscall/exec_unix_test.go +++ b/src/syscall/exec_unix_test.go @@ -247,6 +247,7 @@ func TestInvalidExec(t *testing.T) { // TestExec is for issue #41702. func TestExec(t *testing.T) { + testenv.MustHaveExec(t) cmd := exec.Command(os.Args[0], "-test.run=TestExecHelper") cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=2") o, err := cmd.CombinedOutput() -- cgit v1.2.3-54-g00ecf From 3b1f07fff774f86f13316f7bec6552566568fc10 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Oct 2020 14:39:12 -0700 Subject: [release-branch.go1.15] runtime: wait for preemption signals before syscall.Exec For #41702 For #41704 For #42023 Change-Id: If07f40b1d73b8f276ee28ffb8b7214175e56c24d Reviewed-on: https://go-review.googlesource.com/c/go/+/262817 Trust: Ian Lance Taylor Trust: Bryan C. Mills Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang (cherry picked from commit 05739d6f17c57f09264272621b88725a463234d0) Reviewed-on: https://go-review.googlesource.com/c/go/+/264022 --- src/runtime/proc.go | 21 +++++++++++++++++++++ src/runtime/signal_unix.go | 11 +++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 035822216d..2f1272d310 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1281,6 +1281,14 @@ found: checkdead() unlock(&sched.lock) + if GOOS == "darwin" { + // Make sure pendingPreemptSignals is correct when an M exits. + // For #41702. + if atomic.Load(&m.signalPending) != 0 { + atomic.Xadd(&pendingPreemptSignals, -1) + } + } + if osStack { // Return from mstart and let the system thread // library free the g0 stack and terminate the thread. @@ -3475,11 +3483,24 @@ func syscall_runtime_AfterForkInChild() { inForkedChild = false } +// pendingPreemptSignals is the number of preemption signals +// that have been sent but not received. This is only used on Darwin. +// For #41702. +var pendingPreemptSignals uint32 + // Called from syscall package before Exec. //go:linkname syscall_runtime_BeforeExec syscall.runtime_BeforeExec func syscall_runtime_BeforeExec() { // Prevent thread creation during exec. execLock.lock() + + // On Darwin, wait for all pending preemption signals to + // be received. See issue #41702. + if GOOS == "darwin" { + for int32(atomic.Load(&pendingPreemptSignals)) > 0 { + osyield() + } + } } // Called from syscall package after Exec. diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index f9784b84a7..80fd2d6604 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -336,6 +336,10 @@ func doSigPreempt(gp *g, ctxt *sigctxt) { // Acknowledge the preemption. atomic.Xadd(&gp.m.preemptGen, 1) atomic.Store(&gp.m.signalPending, 0) + + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, -1) + } } const preemptMSupported = true @@ -365,6 +369,10 @@ func preemptM(mp *m) { } if atomic.Cas(&mp.signalPending, 0, 1) { + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, 1) + } + // If multiple threads are preempting the same M, it may send many // signals to the same M such that it hardly make progress, causing // live-lock problem. Apparently this could happen on darwin. See @@ -436,6 +444,9 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) { // no non-Go signal handler for sigPreempt. // The default behavior for sigPreempt is to ignore // the signal, so badsignal will be a no-op anyway. + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, -1) + } return } c.fixsigcode(sig) -- cgit v1.2.3-54-g00ecf From f68af196a36e740a56121dc759d1f0f3ab7749c6 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 30 Sep 2020 14:51:45 -0700 Subject: [release-branch.go1.15] src, net/http: update vendor, regenerate h2_bundle.go Features CL: net/http2: send WINDOW_UPDATE on a body's write failure https://golang.org/cl/258478 (updates #41387) Created by: go mod edit -replace=golang.org/x/net=golang.org/x/net@release-branch.go1.15-bundle GOFLAGS='-mod=mod' go generate -run=bundle std go mod edit -dropreplace=golang.org/x/net go get -d golang.org/x/net@release-branch.go1.15 go mod tidy go mod vendor Updates #40423 Fixes #41387 Change-Id: I052037d6b6ed38b9d9782e19b8ce283875354c92 Reviewed-on: https://go-review.googlesource.com/c/go/+/258540 Run-TryBot: Emmanuel Odeke Reviewed-by: Dmitri Shuralyov TryBot-Result: Go Bot Trust: Emmanuel Odeke --- src/go.mod | 2 +- src/go.sum | 4 ++-- src/net/http/h2_bundle.go | 1 + src/vendor/modules.txt | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/go.mod b/src/go.mod index b002f8e516..6b97366bbe 100644 --- a/src/go.mod +++ b/src/go.mod @@ -4,7 +4,7 @@ go 1.15 require ( golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 - golang.org/x/net v0.0.0-20200707034311-ab3426394381 + golang.org/x/net v0.0.0-20201008223702-a5fa9d4b7c91 golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 // indirect golang.org/x/text v0.3.3-0.20200430171850-afb9336c4530 // indirect ) diff --git a/src/go.sum b/src/go.sum index 528f7e460e..fbd3279aad 100644 --- a/src/go.sum +++ b/src/go.sum @@ -2,8 +2,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk 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/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU= -golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= +golang.org/x/net v0.0.0-20201008223702-a5fa9d4b7c91 h1:zd7kl5i5PDM0OnFbRWVM6B8mXojzv8LOkHN9LsOrRf4= +golang.org/x/net v0.0.0-20201008223702-a5fa9d4b7c91/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 81c3671f85..29296991ef 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -5265,6 +5265,7 @@ func (sc *http2serverConn) processData(f *http2DataFrame) error { if len(data) > 0 { wrote, err := st.body.Write(data) if err != nil { + sc.sendWindowUpdate(nil, int(f.Length)-wrote) return http2streamError(id, http2ErrCodeStreamClosed) } if wrote != len(data) { diff --git a/src/vendor/modules.txt b/src/vendor/modules.txt index e687d77b4d..03ca3c3ae4 100644 --- a/src/vendor/modules.txt +++ b/src/vendor/modules.txt @@ -8,7 +8,7 @@ golang.org/x/crypto/curve25519 golang.org/x/crypto/hkdf golang.org/x/crypto/internal/subtle golang.org/x/crypto/poly1305 -# golang.org/x/net v0.0.0-20200707034311-ab3426394381 +# golang.org/x/net v0.0.0-20201008223702-a5fa9d4b7c91 ## explicit golang.org/x/net/dns/dnsmessage golang.org/x/net/http/httpguts -- cgit v1.2.3-54-g00ecf From 1a05c910fbf62f6f06930394853e9cf7ca6087c6 Mon Sep 17 00:00:00 2001 From: dqu123 Date: Sat, 10 Oct 2020 16:25:07 -0400 Subject: [release-branch.go1.15] net/http: deep copy Request.TransferEncoding The existing implementation in Request.Clone() assigns the wrong pointer to r2.TransferEncoding. Updates #41907. Fixes #41914. Change-Id: I7f220a41b1b46a55d1a1005e47c6dd69478cb025 Reviewed-on: https://go-review.googlesource.com/c/go/+/261378 Reviewed-by: Emmanuel Odeke Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Trust: Carlos Amedee Trust: Emmanuel Odeke --- src/net/http/request.go | 2 +- src/net/http/request_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/net/http/request.go b/src/net/http/request.go index fe6b60982c..54ec1c5593 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -382,7 +382,7 @@ func (r *Request) Clone(ctx context.Context) *Request { if s := r.TransferEncoding; s != nil { s2 := make([]string, len(s)) copy(s2, s) - r2.TransferEncoding = s + r2.TransferEncoding = s2 } r2.Form = cloneURLValues(r.Form) r2.PostForm = cloneURLValues(r.PostForm) diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 42c16d00ea..461d66e05d 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -828,6 +828,27 @@ func TestWithContextDeepCopiesURL(t *testing.T) { } } +// Ensure that Request.Clone creates a deep copy of TransferEncoding. +// See issue 41907. +func TestRequestCloneTransferEncoding(t *testing.T) { + body := strings.NewReader("body") + req, _ := NewRequest("POST", "https://example.org/", body) + req.TransferEncoding = []string{ + "encoding1", + } + + clonedReq := req.Clone(context.Background()) + // modify original after deep copy + req.TransferEncoding[0] = "encoding2" + + if req.TransferEncoding[0] != "encoding2" { + t.Error("expected req.TransferEncoding to be changed") + } + if clonedReq.TransferEncoding[0] != "encoding1" { + t.Error("expected clonedReq.TransferEncoding to be unchanged") + } +} + func TestNoPanicOnRoundTripWithBasicAuth_h1(t *testing.T) { testNoPanicWithBasicAuth(t, h1Mode) } -- cgit v1.2.3-54-g00ecf From 8f14c1840d15233b7f3d08f0acf0b0559d465a56 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Wed, 30 Sep 2020 12:52:47 -0400 Subject: [release-branch.go1.15] cmd/{compile,link}: backport fix for issue 39757 During Go 1.15 development, a fix was added to the toolchain for issue information. The 1.15 line tables were slightly malformed in the way that they used the DWARF "end sequence" operator, resulting in incorrect line table info for the final instruction in the final function of a compilation unit. This problem was fixed in https://golang.org/cl/235739, which made it into Go 1.15. It now appears that while the fix works OK for linux, in certain cases it causes issues with the Darwin linker (the "address not in any section" ld64 error reported in issue #40974). During Go 1.16 development, the fix in https://golang.org/cl/235739 was revised so as to fix another related problem (described in issue #39757); the newer fix does not trigger the problem in the Darwin linker however. This CL back-ports the changes in https://golang.org/cl/239286 to the 1.15 release branch, so as to fix the Darwin linker error. Updates #38192. Updates #39757. Fixes #40974. Change-Id: I9350fec4503cd3a76b97aaea0d8aed1511662e29 Reviewed-on: https://go-review.googlesource.com/c/go/+/258422 Run-TryBot: Than McIntosh TryBot-Result: Go Bot Reviewed-by: Austin Clements Reviewed-by: Jeremy Faller Reviewed-by: Cherry Zhang Trust: Than McIntosh --- src/cmd/internal/obj/dwarf.go | 46 ++++---- src/cmd/link/internal/ld/dwarf.go | 16 --- src/cmd/link/internal/ld/dwarf_test.go | 119 +++++++++++++++++++++ .../ld/testdata/issue39757/issue39757main.go | 15 +++ 4 files changed, 157 insertions(+), 39 deletions(-) create mode 100644 src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go diff --git a/src/cmd/internal/obj/dwarf.go b/src/cmd/internal/obj/dwarf.go index 4118c6442c..cd1d5b8687 100644 --- a/src/cmd/internal/obj/dwarf.go +++ b/src/cmd/internal/obj/dwarf.go @@ -44,14 +44,12 @@ func (ctxt *Link) generateDebugLinesSymbol(s, lines *LSym) { } } - // Set up the debug_lines state machine. - // NB: This state machine is reset to this state when we've finished - // generating the line table. See below. - // TODO: Once delve can support multiple DW_LNS_end_statements, we don't have - // to do this. + // Set up the debug_lines state machine to the default values + // we expect at the start of a new sequence. stmt := true line := int64(1) pc := s.Func.Text.Pc + var lastpc int64 // last PC written to line table, not last PC in func name := "" prologue, wrotePrologue := false, false // Walk the progs, generating the DWARF table. @@ -86,30 +84,32 @@ func (ctxt *Link) generateDebugLinesSymbol(s, lines *LSym) { if line != int64(newLine) || wrote { pcdelta := p.Pc - pc + lastpc = p.Pc putpclcdelta(ctxt, dctxt, lines, uint64(pcdelta), int64(newLine)-line) line, pc = int64(newLine), p.Pc } } - // Because these symbols will be concatenated together by the linker, we need - // to reset the state machine that controls the debug symbols. The fields in - // the state machine that need to be reset are: - // file = 1 - // line = 1 - // column = 0 - // stmt = set in header, we assume true - // basic_block = false - // Careful readers of the DWARF specification will note that we don't reset - // the address of the state machine -- but this will happen at the beginning - // of the NEXT block of opcodes. - dctxt.AddUint8(lines, dwarf.DW_LNS_set_file) + // Because these symbols will be concatenated together by the + // linker, we need to reset the state machine that controls the + // debug symbols. Do this using an end-of-sequence operator. + // + // Note: at one point in time, Delve did not support multiple end + // sequence ops within a compilation unit (bug for this: + // https://github.com/go-delve/delve/issues/1694), however the bug + // has since been fixed (Oct 2019). + // + // Issue 38192: the DWARF standard specifies that when you issue + // an end-sequence op, the PC value should be one past the last + // text address in the translation unit, so apply a delta to the + // text address before the end sequence op. If this isn't done, + // GDB will assign a line number of zero the last row in the line + // table, which we don't want. + lastlen := uint64(s.Size - (lastpc - s.Func.Text.Pc)) + putpclcdelta(ctxt, dctxt, lines, lastlen, 0) + dctxt.AddUint8(lines, 0) // start extended opcode dwarf.Uleb128put(dctxt, lines, 1) - dctxt.AddUint8(lines, dwarf.DW_LNS_advance_line) - dwarf.Sleb128put(dctxt, lines, int64(1-line)) - if !stmt { - dctxt.AddUint8(lines, dwarf.DW_LNS_negate_stmt) - } - dctxt.AddUint8(lines, dwarf.DW_LNS_copy) + dctxt.AddUint8(lines, dwarf.DW_LNE_end_sequence) } func putpclcdelta(linkctxt *Link, dctxt dwCtxt, s *LSym, deltaPC uint64, deltaLC int64) { diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go index 8df03d74f1..6d330061ab 100644 --- a/src/cmd/link/internal/ld/dwarf.go +++ b/src/cmd/link/internal/ld/dwarf.go @@ -1264,22 +1264,6 @@ func (d *dwctxt2) writelines(unit *sym.CompilationUnit, ls loader.Sym) { } } - // Issue 38192: the DWARF standard specifies that when you issue - // an end-sequence op, the PC value should be one past the last - // text address in the translation unit, so apply a delta to the - // text address before the end sequence op. If this isn't done, - // GDB will assign a line number of zero the last row in the line - // table, which we don't want. The 1 + ptrsize amount is somewhat - // arbitrary, this is chosen to be consistent with the way LLVM - // emits its end sequence ops. - lsu.AddUint8(dwarf.DW_LNS_advance_pc) - dwarf.Uleb128put(d, lsDwsym, int64(1+d.arch.PtrSize)) - - // Emit an end-sequence at the end of the unit. - lsu.AddUint8(0) // start extended opcode - dwarf.Uleb128put(d, lsDwsym, 1) - lsu.AddUint8(dwarf.DW_LNE_end_sequence) - if d.linkctxt.HeadType == objabi.Haix { saveDwsectCUSize(".debug_line", unit.Lib.Pkg, uint64(lsu.Size()-unitLengthOffset)) } diff --git a/src/cmd/link/internal/ld/dwarf_test.go b/src/cmd/link/internal/ld/dwarf_test.go index fb9c45b07d..f3dd53792a 100644 --- a/src/cmd/link/internal/ld/dwarf_test.go +++ b/src/cmd/link/internal/ld/dwarf_test.go @@ -1479,3 +1479,122 @@ func TestIssue38192(t *testing.T) { t.Logf("row %d: A=%x F=%s L=%d\n", i, r.Address, r.File.Name, r.Line) } } + +func TestIssue39757(t *testing.T) { + testenv.MustHaveGoBuild(t) + + if runtime.GOOS == "plan9" { + t.Skip("skipping on plan9; no DWARF symbol table in executables") + } + + // In this bug the DWARF line table contents for the last couple of + // instructions in a function were incorrect (bad file/line). This + // test verifies that all of the line table rows for a function + // of interest have the same file (no "autogenerated"). + // + // Note: the function in this test was written with an eye towards + // ensuring that there are no inlined routines from other packages + // (which could introduce other source files into the DWARF); it's + // possible that at some point things could evolve in the + // compiler/runtime in ways that aren't happening now, so this + // might be something to check for if it does start failing. + + tmpdir, err := ioutil.TempDir("", "TestIssue38192") + if err != nil { + t.Fatalf("could not create directory: %v", err) + } + defer os.RemoveAll(tmpdir) + wd, err := os.Getwd() + if err != nil { + t.Fatalf("where am I? %v", err) + } + pdir := filepath.Join(wd, "testdata", "issue39757") + f := gobuildTestdata(t, tmpdir, pdir, DefaultOpt) + + syms, err := f.Symbols() + if err != nil { + t.Fatal(err) + } + + var addr uint64 + for _, sym := range syms { + if sym.Name == "main.main" { + addr = sym.Addr + break + } + } + if addr == 0 { + t.Fatal("cannot find main.main in symbols") + } + + // Open the resulting binary and examine the DWARF it contains. + // Look for the function of interest ("main.main") + // and verify that all line table entries show the same source + // file. + dw, err := f.DWARF() + if err != nil { + t.Fatalf("error parsing DWARF: %v", err) + } + rdr := dw.Reader() + ex := examiner{} + if err := ex.populate(rdr); err != nil { + t.Fatalf("error reading DWARF: %v", err) + } + + // Locate the main.main DIE + mains := ex.Named("main.main") + if len(mains) == 0 { + t.Fatalf("unable to locate DIE for main.main") + } + if len(mains) != 1 { + t.Fatalf("more than one main.main DIE") + } + maindie := mains[0] + + // Collect the start/end PC for main.main + lowpc := maindie.Val(dwarf.AttrLowpc).(uint64) + highpc := maindie.Val(dwarf.AttrHighpc).(uint64) + + // Now read the line table for the 'main' compilation unit. + mainIdx := ex.idxFromOffset(maindie.Offset) + cuentry := ex.Parent(mainIdx) + if cuentry == nil { + t.Fatalf("main.main DIE appears orphaned") + } + lnrdr, lerr := dw.LineReader(cuentry) + if lerr != nil { + t.Fatalf("error creating DWARF line reader: %v", err) + } + if lnrdr == nil { + t.Fatalf("no line table for main.main compilation unit") + } + rows := []dwarf.LineEntry{} + mainrows := 0 + var lne dwarf.LineEntry + for { + err := lnrdr.Next(&lne) + if err == io.EOF { + break + } + rows = append(rows, lne) + if err != nil { + t.Fatalf("error reading next DWARF line: %v", err) + } + if lne.Address < lowpc || lne.Address > highpc { + continue + } + if !strings.HasSuffix(lne.File.Name, "issue39757main.go") { + t.Errorf("found row with file=%s (not issue39757main.go)", lne.File.Name) + } + mainrows++ + } + f.Close() + + // Make sure we saw a few rows. + if mainrows < 3 { + t.Errorf("not enough line table rows for main.main (got %d, wanted > 3", mainrows) + for i, r := range rows { + t.Logf("row %d: A=%x F=%s L=%d\n", i, r.Address, r.File.Name, r.Line) + } + } +} diff --git a/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go b/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go new file mode 100644 index 0000000000..76e0ea1b08 --- /dev/null +++ b/src/cmd/link/internal/ld/testdata/issue39757/issue39757main.go @@ -0,0 +1,15 @@ +// 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 + +var G int + +func main() { + if G != 101 { + println("not 101") + } else { + println("well now that's interesting") + } +} -- cgit v1.2.3-54-g00ecf From 255afa2461eb45756bbc562d37f5988cc3ca29f7 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 22 Oct 2020 16:37:19 -0700 Subject: [release-branch.go1.15] cmd/compile, runtime: store pointers to go:notinheap types indirectly pointers to go:notinheap types should be treated as scalars. That means they shouldn't be stored directly in interfaces, or directly in reflect.Value.ptr. Also be sure to use uintpr to compare such pointers in reflect.DeepEqual. Fixes #42169 Change-Id: I53735f6d434e9c3108d4940bd1bae14c61ef2a74 Reviewed-on: https://go-review.googlesource.com/c/go/+/264480 Trust: Keith Randall Reviewed-by: Ian Lance Taylor (cherry picked from commit 009d71409821a6ac4f1b32aaae2c856c20a29f92) Reviewed-on: https://go-review.googlesource.com/c/go/+/265720 Run-TryBot: Keith Randall TryBot-Result: Go Bot --- src/cmd/compile/internal/gc/subr.go | 6 ++-- src/cmd/compile/internal/gc/typecheck.go | 2 +- src/reflect/deepequal.go | 12 +++++++- src/reflect/value.go | 12 +++++++- src/runtime/netpoll.go | 47 ++++++++++++++++++++++---------- test/fixedbugs/issue42076.go | 21 ++++++++++++++ 6 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 test/fixedbugs/issue42076.go diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index c82eefb63d..9118405fb1 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -1876,8 +1876,10 @@ func isdirectiface(t *types.Type) bool { } switch t.Etype { - case TPTR, - TCHAN, + case TPTR: + // Pointers to notinheap types must be stored indirectly. See issue 42076. + return !t.Elem().NotInHeap() + case TCHAN, TMAP, TFUNC, TUNSAFEPTR: diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 98b52a506a..b187612b9b 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -2502,7 +2502,7 @@ func lookdot(n *Node, t *types.Type, dostrcmp int) *types.Field { n.Left = nod(OADDR, n.Left, nil) n.Left.SetImplicit(true) n.Left = typecheck(n.Left, ctxType|ctxExpr) - } else if tt.IsPtr() && !rcvr.IsPtr() && types.Identical(tt.Elem(), rcvr) { + } else if tt.IsPtr() && (!rcvr.IsPtr() || rcvr.IsPtr() && rcvr.Elem().NotInHeap()) && types.Identical(tt.Elem(), rcvr) { n.Left = nod(ODEREF, n.Left, nil) n.Left.SetImplicit(true) n.Left = typecheck(n.Left, ctxType|ctxExpr) diff --git a/src/reflect/deepequal.go b/src/reflect/deepequal.go index 8a2bf8b09e..b99c345e7b 100644 --- a/src/reflect/deepequal.go +++ b/src/reflect/deepequal.go @@ -37,7 +37,17 @@ func deepValueEqual(v1, v2 Value, visited map[visit]bool, depth int) bool { // and it's safe and valid to get Value's internal pointer. hard := func(v1, v2 Value) bool { switch v1.Kind() { - case Map, Slice, Ptr, Interface: + case Ptr: + if v1.typ.ptrdata == 0 { + // go:notinheap pointers can't be cyclic. + // At least, all of our current uses of go:notinheap have + // that property. The runtime ones aren't cyclic (and we don't use + // DeepEqual on them anyway), and the cgo-generated ones are + // all empty structs. + return false + } + fallthrough + case Map, Slice, Interface: // Nil pointers cannot be cyclic. Avoid putting them in the visited map. return !v1.IsNil() && !v2.IsNil() } diff --git a/src/reflect/value.go b/src/reflect/value.go index c6f24a5609..1bd11c8707 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -90,6 +90,7 @@ func (f flag) ro() flag { // pointer returns the underlying pointer represented by v. // v.Kind() must be Ptr, Map, Chan, Func, or UnsafePointer +// if v.Kind() == Ptr, the base type must not be go:notinheap. func (v Value) pointer() unsafe.Pointer { if v.typ.size != ptrSize || !v.typ.pointers() { panic("can't call pointer on a non-pointer Value") @@ -1452,7 +1453,16 @@ func (v Value) Pointer() uintptr { // TODO: deprecate k := v.kind() switch k { - case Chan, Map, Ptr, UnsafePointer: + case Ptr: + if v.typ.ptrdata == 0 { + // Handle pointers to go:notinheap types directly, + // so we never materialize such pointers as an + // unsafe.Pointer. (Such pointers are always indirect.) + // See issue 42076. + return *(*uintptr)(v.ptr) + } + fallthrough + case Chan, Map, UnsafePointer: return uintptr(v.pointer()) case Func: if v.flag&flagMethod != 0 { diff --git a/src/runtime/netpoll.go b/src/runtime/netpoll.go index 34ea82a7fa..77eb3aa4c6 100644 --- a/src/runtime/netpoll.go +++ b/src/runtime/netpoll.go @@ -79,16 +79,17 @@ type pollDesc struct { lock mutex // protects the following fields fd uintptr closing bool - everr bool // marks event scanning error happened - user uint32 // user settable cookie - rseq uintptr // protects from stale read timers - rg uintptr // pdReady, pdWait, G waiting for read or nil - rt timer // read deadline timer (set if rt.f != nil) - rd int64 // read deadline - wseq uintptr // protects from stale write timers - wg uintptr // pdReady, pdWait, G waiting for write or nil - wt timer // write deadline timer - wd int64 // write deadline + everr bool // marks event scanning error happened + user uint32 // user settable cookie + rseq uintptr // protects from stale read timers + rg uintptr // pdReady, pdWait, G waiting for read or nil + rt timer // read deadline timer (set if rt.f != nil) + rd int64 // read deadline + wseq uintptr // protects from stale write timers + wg uintptr // pdReady, pdWait, G waiting for write or nil + wt timer // write deadline timer + wd int64 // write deadline + self *pollDesc // storage for indirect interface. See (*pollDesc).makeArg. } type pollCache struct { @@ -157,6 +158,7 @@ func poll_runtime_pollOpen(fd uintptr) (*pollDesc, int) { pd.wseq++ pd.wg = 0 pd.wd = 0 + pd.self = pd unlock(&pd.lock) var errno int32 @@ -271,14 +273,14 @@ func poll_runtime_pollSetDeadline(pd *pollDesc, d int64, mode int) { // Copy current seq into the timer arg. // Timer func will check the seq against current descriptor seq, // if they differ the descriptor was reused or timers were reset. - pd.rt.arg = pd + pd.rt.arg = pd.makeArg() pd.rt.seq = pd.rseq resettimer(&pd.rt, pd.rd) } } else if pd.rd != rd0 || combo != combo0 { pd.rseq++ // invalidate current timers if pd.rd > 0 { - modtimer(&pd.rt, pd.rd, 0, rtf, pd, pd.rseq) + modtimer(&pd.rt, pd.rd, 0, rtf, pd.makeArg(), pd.rseq) } else { deltimer(&pd.rt) pd.rt.f = nil @@ -287,14 +289,14 @@ func poll_runtime_pollSetDeadline(pd *pollDesc, d int64, mode int) { if pd.wt.f == nil { if pd.wd > 0 && !combo { pd.wt.f = netpollWriteDeadline - pd.wt.arg = pd + pd.wt.arg = pd.makeArg() pd.wt.seq = pd.wseq resettimer(&pd.wt, pd.wd) } } else if pd.wd != wd0 || combo != combo0 { pd.wseq++ // invalidate current timers if pd.wd > 0 && !combo { - modtimer(&pd.wt, pd.wd, 0, netpollWriteDeadline, pd, pd.wseq) + modtimer(&pd.wt, pd.wd, 0, netpollWriteDeadline, pd.makeArg(), pd.wseq) } else { deltimer(&pd.wt) pd.wt.f = nil @@ -547,3 +549,20 @@ func (c *pollCache) alloc() *pollDesc { unlock(&c.lock) return pd } + +// makeArg converts pd to an interface{}. +// makeArg does not do any allocation. Normally, such +// a conversion requires an allocation because pointers to +// go:notinheap types (which pollDesc is) must be stored +// in interfaces indirectly. See issue 42076. +func (pd *pollDesc) makeArg() (i interface{}) { + x := (*eface)(unsafe.Pointer(&i)) + x._type = pdType + x.data = unsafe.Pointer(&pd.self) + return +} + +var ( + pdEface interface{} = (*pollDesc)(nil) + pdType *_type = efaceOf(&pdEface)._type +) diff --git a/test/fixedbugs/issue42076.go b/test/fixedbugs/issue42076.go new file mode 100644 index 0000000000..3e954813c9 --- /dev/null +++ b/test/fixedbugs/issue42076.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. + +package main + +import "reflect" + +//go:notinheap +type NIH struct { +} + +var x, y NIH + +func main() { + if reflect.DeepEqual(&x, &y) != true { + panic("should report true") + } +} -- cgit v1.2.3-54-g00ecf From 068911e1dd7742939ae567d7bff30c9116c9c365 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 22 Oct 2020 13:11:16 -0700 Subject: [release-branch.go1.15] cmd/compile: fix storeType to handle pointers to go:notinheap types storeType splits compound stores up into a scalar parts and a pointer parts. The scalar part happens unconditionally, and the pointer part happens under the guard of a write barrier check. Types which are declared as pointers, but are represented as scalars because they might have "bad" values, were not handled correctly here. They ended up not getting stored in either set. Fixes #42151 Change-Id: I46f6600075c0c370e640b807066247237f93c7ac Reviewed-on: https://go-review.googlesource.com/c/go/+/264300 Trust: Keith Randall Reviewed-by: Ian Lance Taylor (cherry picked from commit 933721b8c7f981229974e2603850c2e9a7ffc5a1) Reviewed-on: https://go-review.googlesource.com/c/go/+/265719 Run-TryBot: Keith Randall TryBot-Result: Go Bot --- src/cmd/compile/internal/gc/ssa.go | 8 +++++++- test/fixedbugs/issue42032.go | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue42032.go diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 3e21450deb..7da04547c2 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -4976,7 +4976,10 @@ func (s *state) storeTypeScalars(t *types.Type, left, right *ssa.Value, skip ski case t.IsBoolean() || t.IsInteger() || t.IsFloat() || t.IsComplex(): s.store(t, left, right) case t.IsPtrShaped(): - // no scalar fields. + if t.IsPtr() && t.Elem().NotInHeap() { + s.store(t, left, right) // see issue 42032 + } + // otherwise, no scalar fields. case t.IsString(): if skip&skipLen != 0 { return @@ -5020,6 +5023,9 @@ func (s *state) storeTypeScalars(t *types.Type, left, right *ssa.Value, skip ski func (s *state) storeTypePtrs(t *types.Type, left, right *ssa.Value) { switch { case t.IsPtrShaped(): + if t.IsPtr() && t.Elem().NotInHeap() { + break // see issue 42032 + } s.store(t, left, right) case t.IsString(): ptr := s.newValue1(ssa.OpStringPtr, s.f.Config.Types.BytePtr, right) diff --git a/test/fixedbugs/issue42032.go b/test/fixedbugs/issue42032.go new file mode 100644 index 0000000000..c456b1db02 --- /dev/null +++ b/test/fixedbugs/issue42032.go @@ -0,0 +1,27 @@ +// 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 + +//go:notinheap +type NIH struct { +} + +type T struct { + x *NIH + p *int +} + +var y NIH +var z int + +func main() { + a := []T{{&y, &z}} + a = append(a, T{&y, &z}) + if a[1].x == nil { + panic("pointer not written") + } +} -- cgit v1.2.3-54-g00ecf From 8687f6d924ee3a311e08db855c6dc1024c1f9349 Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Wed, 28 Oct 2020 13:44:53 +0100 Subject: [release-branch.go1.15] cmd/go/internal/modfetch: drop gopkg.in/russross/blackfriday.v2 from TestCodeRepoVersions Follow-up for CL 265819. Given the -pre tag added recently, a new stable version is likely tagged soon. This would break TestCodeRepoVersions on the longtest builders again. Since the other test cases in codeRepoVersionsTests already provide enough coverage, drop gopkg.in/russross/blackfriday.v2 to avoid breaking TestCodeRepoVersions once the release happens. Updates #28856 Change-Id: If86a637b5e47f59faf9048fc1cbbae6e8f1dcc53 Reviewed-on: https://go-review.googlesource.com/c/go/+/265917 Trust: Tobias Klauser Run-TryBot: Tobias Klauser Reviewed-by: Bryan C. Mills Reviewed-by: Jay Conrod TryBot-Result: Go Bot (cherry picked from commit 421d4e72de802ed65cb38317660654771cfb13e9) Reviewed-on: https://go-review.googlesource.com/c/go/+/266178 Trust: Dmitri Shuralyov Trust: Jay Conrod Run-TryBot: Dmitri Shuralyov Reviewed-by: Tobias Klauser --- src/cmd/go/internal/modfetch/coderepo_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cmd/go/internal/modfetch/coderepo_test.go b/src/cmd/go/internal/modfetch/coderepo_test.go index f69c193b86..9a0cd7dba6 100644 --- a/src/cmd/go/internal/modfetch/coderepo_test.go +++ b/src/cmd/go/internal/modfetch/coderepo_test.go @@ -655,11 +655,6 @@ var codeRepoVersionsTests = []struct { path: "swtch.com/testmod", versions: []string{"v1.0.0", "v1.1.1"}, }, - { - vcs: "git", - path: "gopkg.in/russross/blackfriday.v2", - versions: []string{"v2.0.0", "v2.0.1"}, - }, { vcs: "git", path: "gopkg.in/natefinch/lumberjack.v2", -- cgit v1.2.3-54-g00ecf From 777e455106b784b49bf0fe969bcdf802a1104026 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Sat, 17 Oct 2020 15:19:53 +0000 Subject: [release-branch.go1.15] compress/flate: fix corrupted output The fastest compression mode can pick up a false match for every 2GB of input data resulting in incorrectly decompressed data. Since matches are allowed to be up to and including at maxMatchOffset we must offset the buffer by an additional element to prevent the first 4 bytes to match after an out-of-reach value after shiftOffsets has been called. We offset by `maxMatchOffset + 1` so offset 0 in the table will now fail the `if offset > maxMatchOffset` in all cases. Updates #41420. Fixes #41463. Change-Id: If1fbe01728e132b8a207e3f3f439edd832dcc710 GitHub-Last-Rev: 50fabab0da874c37543b139459a810e12e83cee2 GitHub-Pull-Request: golang/go#41477 Reviewed-on: https://go-review.googlesource.com/c/go/+/255879 Reviewed-by: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Joe Tsai Trust: Matthew Dempsky (cherry picked from commit ab541a0560408999ac65d12bec2a3057994eda38) Reviewed-on: https://go-review.googlesource.com/c/go/+/266177 Run-TryBot: Dmitri Shuralyov Trust: Dmitri Shuralyov Reviewed-by: Joe Tsai --- src/compress/flate/deflate_test.go | 57 ++++++++++++++++++++++++++++++++++++++ src/compress/flate/deflatefast.go | 11 ++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/compress/flate/deflate_test.go b/src/compress/flate/deflate_test.go index 49a0345fd1..b19cbec5a9 100644 --- a/src/compress/flate/deflate_test.go +++ b/src/compress/flate/deflate_test.go @@ -11,6 +11,7 @@ import ( "internal/testenv" "io" "io/ioutil" + "math/rand" "reflect" "runtime/debug" "sync" @@ -896,6 +897,62 @@ func TestBestSpeedMaxMatchOffset(t *testing.T) { } } +func TestBestSpeedShiftOffsets(t *testing.T) { + // Test if shiftoffsets properly preserves matches and resets out-of-range matches + // seen in https://github.com/golang/go/issues/4142 + enc := newDeflateFast() + + // testData may not generate internal matches. + testData := make([]byte, 32) + rng := rand.New(rand.NewSource(0)) + for i := range testData { + testData[i] = byte(rng.Uint32()) + } + + // Encode the testdata with clean state. + // Second part should pick up matches from the first block. + wantFirstTokens := len(enc.encode(nil, testData)) + wantSecondTokens := len(enc.encode(nil, testData)) + + if wantFirstTokens <= wantSecondTokens { + t.Fatalf("test needs matches between inputs to be generated") + } + // Forward the current indicator to before wraparound. + enc.cur = bufferReset - int32(len(testData)) + + // Part 1 before wrap, should match clean state. + got := len(enc.encode(nil, testData)) + if wantFirstTokens != got { + t.Errorf("got %d, want %d tokens", got, wantFirstTokens) + } + + // Verify we are about to wrap. + if enc.cur != bufferReset { + t.Errorf("got %d, want e.cur to be at bufferReset (%d)", enc.cur, bufferReset) + } + + // Part 2 should match clean state as well even if wrapped. + got = len(enc.encode(nil, testData)) + if wantSecondTokens != got { + t.Errorf("got %d, want %d token", got, wantSecondTokens) + } + + // Verify that we wrapped. + if enc.cur >= bufferReset { + t.Errorf("want e.cur to be < bufferReset (%d), got %d", bufferReset, enc.cur) + } + + // Forward the current buffer, leaving the matches at the bottom. + enc.cur = bufferReset + enc.shiftOffsets() + + // Ensure that no matches were picked up. + got = len(enc.encode(nil, testData)) + if wantFirstTokens != got { + t.Errorf("got %d, want %d tokens", got, wantFirstTokens) + } +} + func TestMaxStackSize(t *testing.T) { // This test must not run in parallel with other tests as debug.SetMaxStack // affects all goroutines. diff --git a/src/compress/flate/deflatefast.go b/src/compress/flate/deflatefast.go index 24f8be9d5d..6aa439f13d 100644 --- a/src/compress/flate/deflatefast.go +++ b/src/compress/flate/deflatefast.go @@ -270,6 +270,7 @@ func (e *deflateFast) matchLen(s, t int32, src []byte) int32 { func (e *deflateFast) reset() { e.prev = e.prev[:0] // Bump the offset, so all matches will fail distance check. + // Nothing should be >= e.cur in the table. e.cur += maxMatchOffset // Protect against e.cur wraparound. @@ -288,17 +289,21 @@ func (e *deflateFast) shiftOffsets() { for i := range e.table[:] { e.table[i] = tableEntry{} } - e.cur = maxMatchOffset + e.cur = maxMatchOffset + 1 return } // Shift down everything in the table that isn't already too far away. for i := range e.table[:] { - v := e.table[i].offset - e.cur + maxMatchOffset + v := e.table[i].offset - e.cur + maxMatchOffset + 1 if v < 0 { + // We want to reset e.cur to maxMatchOffset + 1, so we need to shift + // all table entries down by (e.cur - (maxMatchOffset + 1)). + // Because we ignore matches > maxMatchOffset, we can cap + // any negative offsets at 0. v = 0 } e.table[i].offset = v } - e.cur = maxMatchOffset + e.cur = maxMatchOffset + 1 } -- cgit v1.2.3-54-g00ecf From 414668cfbc41fd8cadf74e981849d1e05cc23b2e Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 22 Oct 2020 15:25:00 -0700 Subject: [release-branch.go1.15] time: support slim tzdata format Backport of part of https://golang.org/cl/261877 to support the slim tzdata format. As of tzdata 2020b, the default is to use the slim format. We need to support that format so that Go installations continue to work when tzdata is updated. Relevant part of the CL description: The reason for the failed tests was that when caching location data, the extended time format past the end of zone transitions was not considered. The respective change was introduced in (*Location).lookup by CL 215539. For #42138 Change-Id: I37f52a0917b2c6e3957e6b4612c8ef104c736e65 Reviewed-on: https://go-review.googlesource.com/c/go/+/264301 Trust: Ian Lance Taylor Reviewed-by: Tobias Klauser --- src/time/zoneinfo_read.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/time/zoneinfo_read.go b/src/time/zoneinfo_read.go index 6f789be92a..e9f1ab2ec6 100644 --- a/src/time/zoneinfo_read.go +++ b/src/time/zoneinfo_read.go @@ -325,8 +325,16 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) { l.cacheEnd = omega if i+1 < len(tx) { l.cacheEnd = tx[i+1].when + } else if l.extend != "" { + // If we're at the end of the known zone transitions, + // try the extend string. + if _, _, estart, eend, ok := tzset(l.extend, l.cacheEnd, sec); ok { + l.cacheStart = estart + l.cacheEnd = eend + } } l.cacheZone = &l.zone[tx[i].index] + break } } -- cgit v1.2.3-54-g00ecf From 5b023e693ff8058bcffe7103d7ddd120910af692 Mon Sep 17 00:00:00 2001 From: Christopher Hlubek Date: Mon, 26 Oct 2020 13:44:44 +0100 Subject: [release-branch.go1.15] time: fix LoadLocationFromTZData with slim tzdata The extend information of a time zone file with last transition < now could result in a wrong cached zone because it used the zone of the last transition. This could lead to wrong zones in systems with slim zoneinfo. For #42216 Fixes #42138 Change-Id: I7c57c35b5cfa58482ac7925b5d86618c52f5444d Reviewed-on: https://go-review.googlesource.com/c/go/+/264939 Trust: Tobias Klauser Run-TryBot: Tobias Klauser TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor (cherry picked from commit 70e022e4a83dc996ac4f108e811fbc399ad5565b) Reviewed-on: https://go-review.googlesource.com/c/go/+/266299 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Reviewed-by: Tobias Klauser --- src/time/zoneinfo_read.go | 13 +++++++++++-- src/time/zoneinfo_test.go | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/time/zoneinfo_read.go b/src/time/zoneinfo_read.go index e9f1ab2ec6..22a60f3211 100644 --- a/src/time/zoneinfo_read.go +++ b/src/time/zoneinfo_read.go @@ -323,17 +323,26 @@ func LoadLocationFromTZData(name string, data []byte) (*Location, error) { if tx[i].when <= sec && (i+1 == len(tx) || sec < tx[i+1].when) { l.cacheStart = tx[i].when l.cacheEnd = omega + zoneIdx := tx[i].index if i+1 < len(tx) { l.cacheEnd = tx[i+1].when } else if l.extend != "" { // If we're at the end of the known zone transitions, // try the extend string. - if _, _, estart, eend, ok := tzset(l.extend, l.cacheEnd, sec); ok { + if name, _, estart, eend, ok := tzset(l.extend, l.cacheEnd, sec); ok { l.cacheStart = estart l.cacheEnd = eend + // Find the zone that is returned by tzset, + // the last transition is not always the correct zone. + for i, z := range l.zone { + if z.name == name { + zoneIdx = uint8(i) + break + } + } } } - l.cacheZone = &l.zone[tx[i].index] + l.cacheZone = &l.zone[zoneIdx] break } } diff --git a/src/time/zoneinfo_test.go b/src/time/zoneinfo_test.go index 72829bc9fb..277b68f798 100644 --- a/src/time/zoneinfo_test.go +++ b/src/time/zoneinfo_test.go @@ -183,6 +183,25 @@ func TestMalformedTZData(t *testing.T) { } } +func TestLoadLocationFromTZDataSlim(t *testing.T) { + // A 2020b slim tzdata for Europe/Berlin + tzData := "TZif2\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00TZif2\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00<\x00\x00\x00\x04\x00\x00\x00\x12\xff\xff\xff\xffo\xa2a\xf8\xff\xff\xff\xff\x9b\f\x17`\xff\xff\xff\xff\x9b\xd5\xda\xf0\xff\xff\xff\xff\x9cٮ\x90\xff\xff\xff\xff\x9d\xa4\xb5\x90\xff\xff\xff\xff\x9e\xb9\x90\x90\xff\xff\xff\xff\x9f\x84\x97\x90\xff\xff\xff\xff\xc8\tq\x90\xff\xff\xff\xff\xcc\xe7K\x10\xff\xff\xff\xffͩ\x17\x90\xff\xff\xff\xff\u03a2C\x10\xff\xff\xff\xffϒ4\x10\xff\xff\xff\xffЂ%\x10\xff\xff\xff\xff\xd1r\x16\x10\xff\xff\xff\xffѶ\x96\x00\xff\xff\xff\xff\xd2X\xbe\x80\xff\xff\xff\xffҡO\x10\xff\xff\xff\xff\xd3c\x1b\x90\xff\xff\xff\xff\xd4K#\x90\xff\xff\xff\xff\xd59\xd1 \xff\xff\xff\xff\xd5g\xe7\x90\xff\xff\xff\xffըs\x00\xff\xff\xff\xff\xd6)\xb4\x10\xff\xff\xff\xff\xd7,\x1a\x10\xff\xff\xff\xff\xd8\t\x96\x10\xff\xff\xff\xff\xd9\x02\xc1\x90\xff\xff\xff\xff\xd9\xe9x\x10\x00\x00\x00\x00\x13MD\x10\x00\x00\x00\x00\x143\xfa\x90\x00\x00\x00\x00\x15#\xeb\x90\x00\x00\x00\x00\x16\x13ܐ\x00\x00\x00\x00\x17\x03͐\x00\x00\x00\x00\x17\xf3\xbe\x90\x00\x00\x00\x00\x18㯐\x00\x00\x00\x00\x19Ӡ\x90\x00\x00\x00\x00\x1aÑ\x90\x00\x00\x00\x00\x1b\xbc\xbd\x10\x00\x00\x00\x00\x1c\xac\xae\x10\x00\x00\x00\x00\x1d\x9c\x9f\x10\x00\x00\x00\x00\x1e\x8c\x90\x10\x00\x00\x00\x00\x1f|\x81\x10\x00\x00\x00\x00 lr\x10\x00\x00\x00\x00!\\c\x10\x00\x00\x00\x00\"LT\x10\x00\x00\x00\x00# Date: Thu, 29 Oct 2020 18:50:31 -0400 Subject: [release-branch.go1.15] net/http: update bundled x/net/http2 Bring in the change in CL 266158 with: go mod edit -replace=golang.org/x/net=golang.org/x/net@release-branch.go1.15-bundle GOFLAGS='-mod=mod' go generate -run=bundle std go mod edit -dropreplace=golang.org/x/net go get -d golang.org/x/net@release-branch.go1.15 go mod tidy go mod vendor Updates #39337. Fixes #42113. Change-Id: I3ebef4b90c11ad271b7a3031aafd80c423c2c241 Reviewed-on: https://go-review.googlesource.com/c/go/+/266375 Trust: Dmitri Shuralyov Run-TryBot: Dmitri Shuralyov TryBot-Result: Go Bot Reviewed-by: Emmanuel Odeke --- src/net/http/h2_bundle.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 29296991ef..b03b84d2f3 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -7168,6 +7168,7 @@ func (t *http2Transport) newClientConn(c net.Conn, singleUse bool) (*http2Client cc.inflow.add(http2transportDefaultConnFlow + http2initialWindowSize) cc.bw.Flush() if cc.werr != nil { + cc.Close() return nil, cc.werr } @@ -7533,6 +7534,15 @@ func (cc *http2ClientConn) roundTrip(req *Request) (res *Response, gotErrAfterRe bodyWriter := cc.t.getBodyWriterState(cs, body) cs.on100 = bodyWriter.on100 + defer func() { + cc.wmu.Lock() + werr := cc.werr + cc.wmu.Unlock() + if werr != nil { + cc.Close() + } + }() + cc.wmu.Lock() endStream := !hasBody && !hasTrailers werr := cc.writeHeaders(cs.ID, endStream, int(cc.maxFrameSize), hdrs) -- cgit v1.2.3-54-g00ecf From 0e953add9656c32a788e06438cd7b533e968b7f8 Mon Sep 17 00:00:00 2001 From: Alexander Rakoczy Date: Thu, 5 Nov 2020 13:12:55 -0500 Subject: [release-branch.go1.15] go1.15.4 Change-Id: Ibcd61e2c7ef7cc6f8509dadea6c3952c5dd7016e Reviewed-on: https://go-review.googlesource.com/c/go/+/267879 Run-TryBot: Alexander Rakoczy TryBot-Result: Go Bot Reviewed-by: Dmitri Shuralyov Trust: Alexander Rakoczy --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 46b20cee5e..b0378bdff8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.15.3 \ No newline at end of file +go1.15.4 \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 32159824698a82a174b60a6845e8494ae3243102 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 6 Nov 2020 09:38:38 -0800 Subject: [release-branch.go1.15-security] cmd/go, cmd/cgo: don't let bogus symbol set cgo_ldflag A hand-edited object file can have a symbol name that uses newline and other normally invalid characters. The cgo tool will generate Go files containing symbol names, unquoted. That can permit those symbol names to inject Go code into a cgo-generated file. If that Go code uses the //go:cgo_ldflag pragma, it can cause the C linker to run arbitrary code when building a package. If you build an imported package we permit arbitrary code at run time, but we don't want to permit it at package build time. This CL prevents this in two ways. In cgo, reject invalid symbols that contain non-printable or space characters, or that contain anything that looks like a Go comment. In the go tool, double check all //go:cgo_ldflag directives in generated code, to make sure they follow the existing LDFLAG restrictions. Thanks to Chris Brown and Tempus Ex for reporting this. Fixes CVE-2020-28366 Change-Id: Ia1ad8f3791ea79612690fa7d26ac451d0f6df7c1 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/895832 Reviewed-by: Than McIntosh Reviewed-by: Cherry Zhang (cherry picked from commit 6bc814dd2bbfeaafa41d314dd4cc591b575dfbf6) Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/901056 Reviewed-by: Filippo Valsorda Reviewed-by: Roland Shoemaker --- misc/cgo/errors/badsym_test.go | 216 +++++++++++++++++++++++++++++++++++++++ src/cmd/cgo/out.go | 23 +++++ src/cmd/go/internal/work/exec.go | 60 +++++++++++ 3 files changed, 299 insertions(+) create mode 100644 misc/cgo/errors/badsym_test.go diff --git a/misc/cgo/errors/badsym_test.go b/misc/cgo/errors/badsym_test.go new file mode 100644 index 0000000000..b2701bf922 --- /dev/null +++ b/misc/cgo/errors/badsym_test.go @@ -0,0 +1,216 @@ +// 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 errorstest + +import ( + "bytes" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "unicode" +) + +// A manually modified object file could pass unexpected characters +// into the files generated by cgo. + +const magicInput = "abcdefghijklmnopqrstuvwxyz0123" +const magicReplace = "\n//go:cgo_ldflag \"-badflag\"\n//" + +const cSymbol = "BadSymbol" + magicInput + "Name" +const cDefSource = "int " + cSymbol + " = 1;" +const cRefSource = "extern int " + cSymbol + "; int F() { return " + cSymbol + "; }" + +// goSource is the source code for the trivial Go file we use. +// We will replace TMPDIR with the temporary directory name. +const goSource = ` +package main + +// #cgo LDFLAGS: TMPDIR/cbad.o TMPDIR/cbad.so +// extern int F(); +import "C" + +func main() { + println(C.F()) +} +` + +func TestBadSymbol(t *testing.T) { + dir := t.TempDir() + + mkdir := func(base string) string { + ret := filepath.Join(dir, base) + if err := os.Mkdir(ret, 0755); err != nil { + t.Fatal(err) + } + return ret + } + + cdir := mkdir("c") + godir := mkdir("go") + + makeFile := func(mdir, base, source string) string { + ret := filepath.Join(mdir, base) + if err := ioutil.WriteFile(ret, []byte(source), 0644); err != nil { + t.Fatal(err) + } + return ret + } + + cDefFile := makeFile(cdir, "cdef.c", cDefSource) + cRefFile := makeFile(cdir, "cref.c", cRefSource) + + ccCmd := cCompilerCmd(t) + + cCompile := func(arg, base, src string) string { + out := filepath.Join(cdir, base) + run := append(ccCmd, arg, "-o", out, src) + output, err := exec.Command(run[0], run[1:]...).CombinedOutput() + if err != nil { + t.Log(run) + t.Logf("%s", output) + t.Fatal(err) + } + if err := os.Remove(src); err != nil { + t.Fatal(err) + } + return out + } + + // Build a shared library that defines a symbol whose name + // contains magicInput. + + cShared := cCompile("-shared", "c.so", cDefFile) + + // Build an object file that refers to the symbol whose name + // contains magicInput. + + cObj := cCompile("-c", "c.o", cRefFile) + + // Rewrite the shared library and the object file, replacing + // magicInput with magicReplace. This will have the effect of + // introducing a symbol whose name looks like a cgo command. + // The cgo tool will use that name when it generates the + // _cgo_import.go file, thus smuggling a magic //go:cgo_ldflag + // pragma into a Go file. We used to not check the pragmas in + // _cgo_import.go. + + rewrite := func(from, to string) { + obj, err := ioutil.ReadFile(from) + if err != nil { + t.Fatal(err) + } + + if bytes.Count(obj, []byte(magicInput)) == 0 { + t.Fatalf("%s: did not find magic string", from) + } + + if len(magicInput) != len(magicReplace) { + t.Fatalf("internal test error: different magic lengths: %d != %d", len(magicInput), len(magicReplace)) + } + + obj = bytes.ReplaceAll(obj, []byte(magicInput), []byte(magicReplace)) + + if err := ioutil.WriteFile(to, obj, 0644); err != nil { + t.Fatal(err) + } + } + + cBadShared := filepath.Join(godir, "cbad.so") + rewrite(cShared, cBadShared) + + cBadObj := filepath.Join(godir, "cbad.o") + rewrite(cObj, cBadObj) + + goSourceBadObject := strings.ReplaceAll(goSource, "TMPDIR", godir) + makeFile(godir, "go.go", goSourceBadObject) + + makeFile(godir, "go.mod", "module badsym") + + // Try to build our little package. + cmd := exec.Command("go", "build", "-ldflags=-v") + cmd.Dir = godir + output, err := cmd.CombinedOutput() + + // The build should fail, but we want it to fail because we + // detected the error, not because we passed a bad flag to the + // C linker. + + if err == nil { + t.Errorf("go build succeeded unexpectedly") + } + + t.Logf("%s", output) + + for _, line := range bytes.Split(output, []byte("\n")) { + if bytes.Contains(line, []byte("dynamic symbol")) && bytes.Contains(line, []byte("contains unsupported character")) { + // This is the error from cgo. + continue + } + + // We passed -ldflags=-v to see the external linker invocation, + // which should not include -badflag. + if bytes.Contains(line, []byte("-badflag")) { + t.Error("output should not mention -badflag") + } + + // Also check for compiler errors, just in case. + // GCC says "unrecognized command line option". + // clang says "unknown argument". + if bytes.Contains(line, []byte("unrecognized")) || bytes.Contains(output, []byte("unknown")) { + t.Error("problem should have been caught before invoking C linker") + } + } +} + +func cCompilerCmd(t *testing.T) []string { + cc := []string{goEnv(t, "CC")} + + out := goEnv(t, "GOGCCFLAGS") + quote := '\000' + start := 0 + lastSpace := true + backslash := false + s := string(out) + for i, c := range s { + if quote == '\000' && unicode.IsSpace(c) { + if !lastSpace { + cc = append(cc, s[start:i]) + lastSpace = true + } + } else { + if lastSpace { + start = i + lastSpace = false + } + if quote == '\000' && !backslash && (c == '"' || c == '\'') { + quote = c + backslash = false + } else if !backslash && quote == c { + quote = '\000' + } else if (quote == '\000' || quote == '"') && !backslash && c == '\\' { + backslash = true + } else { + backslash = false + } + } + } + if !lastSpace { + cc = append(cc, s[start:]) + } + return cc +} + +func goEnv(t *testing.T, key string) string { + out, err := exec.Command("go", "env", key).CombinedOutput() + if err != nil { + t.Logf("go env %s\n", key) + t.Logf("%s", out) + t.Fatal(err) + } + return strings.TrimSpace(string(out)) +} diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index cbdb4e0f5b..dcd69edb44 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -336,6 +336,8 @@ func dynimport(obj string) { if s.Version != "" { targ += "#" + s.Version } + checkImportSymName(s.Name) + checkImportSymName(targ) fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s.Name, targ, s.Library) } lib, _ := f.ImportedLibraries() @@ -351,6 +353,7 @@ func dynimport(obj string) { if len(s) > 0 && s[0] == '_' { s = s[1:] } + checkImportSymName(s) fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s, s, "") } lib, _ := f.ImportedLibraries() @@ -365,6 +368,8 @@ func dynimport(obj string) { for _, s := range sym { ss := strings.Split(s, ":") name := strings.Split(ss[0], "@")[0] + checkImportSymName(name) + checkImportSymName(ss[0]) fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", name, ss[0], strings.ToLower(ss[1])) } return @@ -382,6 +387,7 @@ func dynimport(obj string) { // Go symbols. continue } + checkImportSymName(s.Name) fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s.Name, s.Name, s.Library) } lib, err := f.ImportedLibraries() @@ -397,6 +403,23 @@ func dynimport(obj string) { fatalf("cannot parse %s as ELF, Mach-O, PE or XCOFF", obj) } +// checkImportSymName checks a symbol name we are going to emit as part +// of a //go:cgo_import_dynamic pragma. These names come from object +// files, so they may be corrupt. We are going to emit them unquoted, +// so while they don't need to be valid symbol names (and in some cases, +// involving symbol versions, they won't be) they must contain only +// graphic characters and must not contain Go comments. +func checkImportSymName(s string) { + for _, c := range s { + if !unicode.IsGraphic(c) || unicode.IsSpace(c) { + fatalf("dynamic symbol %q contains unsupported character", s) + } + } + if strings.Index(s, "//") >= 0 || strings.Index(s, "/*") >= 0 { + fatalf("dynamic symbol %q contains Go comment") + } +} + // Construct a gcc struct matching the gc argument frame. // Assumes that in gcc, char is 1 byte, short 2 bytes, int 4 bytes, long long 8 bytes. // These assumptions are checked by the gccProlog. diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 071c9d2db9..13d4c8cbb4 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -2711,6 +2711,66 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo noCompiler() } + // Double check the //go:cgo_ldflag comments in the generated files. + // The compiler only permits such comments in files whose base name + // starts with "_cgo_". Make sure that the comments in those files + // are safe. This is a backstop against people somehow smuggling + // such a comment into a file generated by cgo. + if cfg.BuildToolchainName == "gc" && !cfg.BuildN { + var flags []string + for _, f := range outGo { + if !strings.HasPrefix(filepath.Base(f), "_cgo_") { + continue + } + + src, err := ioutil.ReadFile(f) + if err != nil { + return nil, nil, err + } + + const cgoLdflag = "//go:cgo_ldflag" + idx := bytes.Index(src, []byte(cgoLdflag)) + for idx >= 0 { + // We are looking at //go:cgo_ldflag. + // Find start of line. + start := bytes.LastIndex(src[:idx], []byte("\n")) + if start == -1 { + start = 0 + } + + // Find end of line. + end := bytes.Index(src[idx:], []byte("\n")) + if end == -1 { + end = len(src) + } else { + end += idx + } + + // Check for first line comment in line. + // We don't worry about /* */ comments, + // which normally won't appear in files + // generated by cgo. + commentStart := bytes.Index(src[start:], []byte("//")) + commentStart += start + // If that line comment is //go:cgo_ldflag, + // it's a match. + if bytes.HasPrefix(src[commentStart:], []byte(cgoLdflag)) { + // Pull out the flag, and unquote it. + // This is what the compiler does. + flag := string(src[idx+len(cgoLdflag) : end]) + flag = strings.TrimSpace(flag) + flag = strings.Trim(flag, `"`) + flags = append(flags, flag) + } + src = src[end:] + idx = bytes.Index(src, []byte(cgoLdflag)) + } + } + if err := checkLinkerFlags("LDFLAGS", "go:cgo_ldflag", flags); err != nil { + return nil, nil, err + } + } + return outGo, outObj, nil } -- cgit v1.2.3-54-g00ecf From ec06b6d6be568ce1591d91a0ea4f14c190d06605 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 2 Nov 2020 21:31:06 -0800 Subject: [release-branch.go1.15-security] cmd/go: in cgoflags, permit -DX1, prohibit -Wp,-D,opt Restrict -D and -U to ASCII C identifiers, but do permit trailing digits. When using -Wp, prohibit commas in -D values. Thanks to Imre Rad (https://www.linkedin.com/in/imre-rad-2358749b) for reporting this. Fixes CVE-2020-28367 Change-Id: Ibfc4dfdd6e6c258e131448e7682610c44eee9492 Reviewed-on: https://go-review.googlesource.com/c/go/+/267277 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/899924 Reviewed-by: Filippo Valsorda --- src/cmd/go/internal/work/security.go | 8 ++++---- src/cmd/go/internal/work/security_test.go | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cmd/go/internal/work/security.go b/src/cmd/go/internal/work/security.go index 3ee68ac1b4..0d9628241f 100644 --- a/src/cmd/go/internal/work/security.go +++ b/src/cmd/go/internal/work/security.go @@ -42,8 +42,8 @@ import ( var re = lazyregexp.New var validCompilerFlags = []*lazyregexp.Regexp{ - re(`-D([A-Za-z_].*)`), - re(`-U([A-Za-z_]*)`), + re(`-D([A-Za-z_][A-Za-z0-9_]*)(=[^@\-]*)?`), + re(`-U([A-Za-z_][A-Za-z0-9_]*)`), re(`-F([^@\-].*)`), re(`-I([^@\-].*)`), re(`-O`), @@ -51,8 +51,8 @@ var validCompilerFlags = []*lazyregexp.Regexp{ re(`-W`), re(`-W([^@,]+)`), // -Wall but not -Wa,-foo. re(`-Wa,-mbig-obj`), - re(`-Wp,-D([A-Za-z_].*)`), - re(`-Wp,-U([A-Za-z_]*)`), + re(`-Wp,-D([A-Za-z_][A-Za-z0-9_]*)(=[^@,\-]*)?`), + re(`-Wp,-U([A-Za-z_][A-Za-z0-9_]*)`), re(`-ansi`), re(`-f(no-)?asynchronous-unwind-tables`), re(`-f(no-)?blocks`), diff --git a/src/cmd/go/internal/work/security_test.go b/src/cmd/go/internal/work/security_test.go index 11e74f29c6..aec9789185 100644 --- a/src/cmd/go/internal/work/security_test.go +++ b/src/cmd/go/internal/work/security_test.go @@ -13,6 +13,7 @@ var goodCompilerFlags = [][]string{ {"-DFOO"}, {"-Dfoo=bar"}, {"-Ufoo"}, + {"-Ufoo1"}, {"-F/Qt"}, {"-I/"}, {"-I/etc/passwd"}, @@ -24,6 +25,8 @@ var goodCompilerFlags = [][]string{ {"-Wall"}, {"-Wp,-Dfoo=bar"}, {"-Wp,-Ufoo"}, + {"-Wp,-Dfoo1"}, + {"-Wp,-Ufoo1"}, {"-fobjc-arc"}, {"-fno-objc-arc"}, {"-fomit-frame-pointer"}, @@ -78,6 +81,8 @@ var badCompilerFlags = [][]string{ {"-O@1"}, {"-Wa,-foo"}, {"-W@foo"}, + {"-Wp,-DX,-D@X"}, + {"-Wp,-UX,-U@X"}, {"-g@gdb"}, {"-g-gdb"}, {"-march=@dawn"}, -- cgit v1.2.3-54-g00ecf From 84150d0af193a7ccd733b3c7fa5787f43125cd2d Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Tue, 10 Nov 2020 15:54:12 -0500 Subject: [release-branch.go1.15-security] math/big: fix shift for recursive division MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous s value could cause a crash for certain inputs. Will check in tests and documentation improvements later. Thanks to the Go Ethereum team and the OSS-Fuzz project for reporting this. Thanks to Rémy Oudompheng and Robert Griesemer for their help developing and validating the fix. Fixes CVE-2020-28362 Change-Id: Ibbf455c4436bcdb07c84a34fa6551fb3422356d3 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/899974 Reviewed-by: Roland Shoemaker Reviewed-by: Filippo Valsorda (cherry picked from commit 28015462c2a83239543dc2bef651e9a5f234b633) Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/901065 --- src/math/big/nat.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/math/big/nat.go b/src/math/big/nat.go index 6a3989bf9d..8c43de69d3 100644 --- a/src/math/big/nat.go +++ b/src/math/big/nat.go @@ -928,7 +928,7 @@ func (z nat) divRecursiveStep(u, v nat, depth int, tmp *nat, temps []*nat) { // Now u < (v< Date: Thu, 12 Nov 2020 09:43:55 -0500 Subject: [release-branch.go1.15-security] go1.15.5 Change-Id: Id3b116c0f54c2131111bc8afacb8d81d06f96461 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/901407 Reviewed-by: Katie Hockman --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index b0378bdff8..d4bbebb9fc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.15.4 \ No newline at end of file +go1.15.5 \ No newline at end of file -- cgit v1.2.3-54-g00ecf