From 0b12684a4a79b93444a3517e4a53e543ce736388 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 2 Dec 2022 14:52:34 -0500 Subject: [release-branch.go1.18] cmd/go: skip TestScript/mod_replace_gopkgin (Until it can be made hermetic.) The gopkg.in service has had a lot of flakiness lately. Go users in general are isolated from that flakiness by the Go module mirror (proxy.golang.org), but this test intentionally bypasses the module mirror because the mirror itself uses cmd/go to download the module. In the long term, we can redirect the gopkg.in URL to the local (in-process) vcweb server added for #27494. In the meantime, let's skip the test to reduce the impact of upstream outages. Fixes #57057. Updates #54503. Change-Id: Icf3de7ca416db548e53864a71776fe22b444fcea Reviewed-on: https://go-review.googlesource.com/c/go/+/454503 Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Heschi Kreinick (cherry picked from commit c5f5cb659adda026d01b7fa9bd39b2ad3b58c5bf) Reviewed-on: https://go-review.googlesource.com/c/go/+/454840 --- src/cmd/go/testdata/script/mod_replace_gopkgin.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cmd/go/testdata/script/mod_replace_gopkgin.txt b/src/cmd/go/testdata/script/mod_replace_gopkgin.txt index df752d9716..8cb034c36c 100644 --- a/src/cmd/go/testdata/script/mod_replace_gopkgin.txt +++ b/src/cmd/go/testdata/script/mod_replace_gopkgin.txt @@ -4,6 +4,9 @@ # even if there is an explicit go.mod file containing the # gopkg.in path. +skip 'skipping test that depends on an unreliable third-party server; see https://go.dev/issue/54503' + # TODO(#54043): Make this test hermetic and re-enable it. + [short] skip [!net] skip [!exec:git] skip -- cgit v1.2.1 From 11f8a85e7eec74fa149b0fa90971d78240486ec3 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 30 Nov 2022 13:57:33 -0500 Subject: [release-branch.go1.18] net: reenable SRV tests with _ldap._tcp.google.com TestLookupDotsWithRemoteSource and TestLookupGoogleSRV were disabled because they look up the no-longer-present SRV record for _xmpp-server._tcp.google.com. Change the tests to look for _ldap._tcp.google.com and reenable them. For #56708. Fixes #56711. Change-Id: I26475fa3ff6fc008048a4e5f24f0e96ee12f655c Reviewed-on: https://go-review.googlesource.com/c/go/+/453861 Run-TryBot: Damien Neil Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot (cherry picked from commit cb42bbddeb80ff08599079ee6b94088e5be88e5b) Reviewed-on: https://go-review.googlesource.com/c/go/+/454295 Auto-Submit: Damien Neil Reviewed-by: Jenny Rakoczy --- src/net/lookup_test.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index 655543c7d2..32e7f8ef99 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -49,21 +49,21 @@ var lookupGoogleSRVTests = []struct { cname, target string }{ { - "xmpp-server", "tcp", "google.com", + "ldap", "tcp", "google.com", "google.com.", "google.com.", }, { - "xmpp-server", "tcp", "google.com.", + "ldap", "tcp", "google.com.", "google.com.", "google.com.", }, // non-standard back door { - "", "", "_xmpp-server._tcp.google.com", + "", "", "_ldap._tcp.google.com", "google.com.", "google.com.", }, { - "", "", "_xmpp-server._tcp.google.com.", + "", "", "_ldap._tcp.google.com.", "google.com.", "google.com.", }, } @@ -71,10 +71,6 @@ var lookupGoogleSRVTests = []struct { var backoffDuration = [...]time.Duration{time.Second, 5 * time.Second, 30 * time.Second} func TestLookupGoogleSRV(t *testing.T) { - // TODO(mknyszek): Figure out next steps for this test. This is just - // a quick fix. - t.Skip("fails consistently due to an upstream DNS change; see #56707.") - t.Parallel() mustHaveExternalNetwork(t) @@ -635,10 +631,6 @@ func TestLookupDotsWithLocalSource(t *testing.T) { } func TestLookupDotsWithRemoteSource(t *testing.T) { - // TODO(mknyszek): Figure out next steps for this test. This is just - // a quick fix. - t.Skip("fails consistently due to an upstream DNS change; see #56707.") - if runtime.GOOS == "darwin" || runtime.GOOS == "ios" { testenv.SkipFlaky(t, 27992) } @@ -709,16 +701,16 @@ func testDots(t *testing.T, mode string) { } } - cname, srvs, err := LookupSRV("xmpp-server", "tcp", "google.com") + cname, srvs, err := LookupSRV("ldap", "tcp", "google.com") if err != nil { - t.Errorf("LookupSRV(xmpp-server, tcp, google.com): %v (mode=%v)", err, mode) + t.Errorf("LookupSRV(ldap, tcp, google.com): %v (mode=%v)", err, mode) } else { if !hasSuffixFold(cname, ".google.com.") { - t.Errorf("LookupSRV(xmpp-server, tcp, google.com) returned cname=%v, want name ending in .google.com. with trailing dot (mode=%v)", cname, mode) + t.Errorf("LookupSRV(ldap, tcp, google.com) returned cname=%v, want name ending in .google.com. with trailing dot (mode=%v)", cname, mode) } for _, srv := range srvs { if !hasSuffixFold(srv.Target, ".google.com.") { - t.Errorf("LookupSRV(xmpp-server, tcp, google.com) returned addrs=%v, want names ending in .google.com. with trailing dot (mode=%v)", srvString(srvs), mode) + t.Errorf("LookupSRV(ldap, tcp, google.com) returned addrs=%v, want names ending in .google.com. with trailing dot (mode=%v)", srvString(srvs), mode) break } } -- cgit v1.2.1 From 2b989668973ae2b00bdcb30cf2ed141da9d22655 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 2 Dec 2022 14:11:00 -0500 Subject: [release-branch.go1.18] cmd/go: remove TestScript/version_buildvcs_git_gpg This was a regression test added for a 'git' command line used for build stamping. Unfortunately, 'gpg' has proved to be extremely fragile: * In recent versions, it appears to always require 'gpg-agent' to be installed for anything involving secret keys, but for some reason is not normally marked as requiring gpg-agent in Debian's package manager. * It tries to create a Unix domain socket in a subdirectory of $TMPDIR without checking the path length, which fails when $TMPDIR is too long to fit in the 'sun_path' field of a sockaddr_un struct (which typically tops out somewhere between 92 and 108 bytes). We could theoretically address those by artificially reducing the script's TMPDIR length and checking for gpg-agent in addition to gpg, but arguably those should both be fixed upstream instead. On balance, the incremental value that this test provides does not seem worth the complexity of dealing with such a fragile third-party tool. Updates #50675. Updates #48802. Updates #57034. Fixes #57054. Change-Id: Ia3288c2f84f8db86ddfa139b4d1c0112d67079ef Reviewed-on: https://go-review.googlesource.com/c/go/+/454502 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Cherry Mui (cherry picked from commit 45f5ef4ed7a774b6911650319a265e17ee9e6e0e) Reviewed-on: https://go-review.googlesource.com/c/go/+/454956 --- .../testdata/script/version_buildvcs_git_gpg.txt | 105 --------------------- 1 file changed, 105 deletions(-) delete mode 100644 src/cmd/go/testdata/script/version_buildvcs_git_gpg.txt diff --git a/src/cmd/go/testdata/script/version_buildvcs_git_gpg.txt b/src/cmd/go/testdata/script/version_buildvcs_git_gpg.txt deleted file mode 100644 index dcf97d7c44..0000000000 --- a/src/cmd/go/testdata/script/version_buildvcs_git_gpg.txt +++ /dev/null @@ -1,105 +0,0 @@ -# This test checks that VCS information is stamped into Go binaries even when -# the current commit is signed and the use has configured git to display commit -# signatures. - -[!exec:git] skip -[!exec:gpg] skip -[short] skip -env GOBIN=$GOPATH/bin -env GNUPGHOME=$WORK/.gpupg -mkdir $GNUPGHOME -chmod 0700 $GNUPGHOME - -# Create GPG key -exec gpg --batch --passphrase '' --quick-generate-key gopher@golang.org -exec gpg --list-secret-keys --with-colons gopher@golang.org -cp stdout keyinfo.txt -go run extract_key_id.go keyinfo.txt -cp stdout keyid.txt - -# Initialize repo -cd repo/ -exec git init -exec git config user.email gopher@golang.org -exec git config user.name 'J.R. Gopher' -exec git config --add log.showSignature true -go run ../configure_signing_key.go ../keyid.txt - -# Create signed commit -cd a -exec git add -A -exec git commit -m 'initial commit' --gpg-sign -exec git log - -# Verify commit signature does not interfere with versioning -go install -go version -m $GOBIN/a -stdout '^\tbuild\tvcs\.revision=' -stdout '^\tbuild\tvcs\.time=' -stdout '^\tbuild\tvcs\.modified=false$' - --- repo/README -- -Far out in the uncharted backwaters of the unfashionable end of the western -spiral arm of the Galaxy lies a small, unregarded yellow sun. --- repo/a/go.mod -- -module example.com/a - -go 1.18 --- repo/a/a.go -- -package main - -func main() {} - --- extract_key_id.go -- -package main - -import "fmt" -import "io/ioutil" -import "os" -import "strings" - -func main() { - err := run(os.Args[1]) - if err != nil { - panic(err) - } -} - -func run(keyInfoFilePath string) error { - contents, err := ioutil.ReadFile(keyInfoFilePath) - if err != nil { - return err - } - lines := strings.Split(string(contents), "\n") - for _, line := range lines { - fields := strings.Split(line, ":") - if fields[0] == "sec" { - fmt.Print(fields[4]) - return nil - } - } - return fmt.Errorf("key ID not found in: %s", keyInfoFilePath) -} - --- configure_signing_key.go -- -package main - -import "io/ioutil" -import "os" -import "os/exec" - -func main() { - err := run(os.Args[1]) - if err != nil { - panic(err) - } -} - -func run(keyIdFilePath string) error { - keyId, err := ioutil.ReadFile(keyIdFilePath) - if err != nil { - return err - } - gitCmd := exec.Command("git", "config", "user.signingKey", string(keyId)) - return gitCmd.Run() -} -- cgit v1.2.1 From 63dd776220bb3a443e6b5c0766a389ec33dc4b69 Mon Sep 17 00:00:00 2001 From: "Paul E. Murphy" Date: Wed, 16 Nov 2022 14:53:39 -0600 Subject: [release-branch.go1.18] cmd/link/internal/ppc64: fix trampoline reuse distance calculation If a compatible trampoline has been inserted by a previously laid function in the same section, and is known to be sufficiently close, it can be reused. When testing if the trampoline can be reused, the addend of the direct call should be ignored. It is already encoded in the trampoline. If the addend is non-zero, and the target sufficiently far away, and just beyond direct call reach, this may cause the trampoline to be incorrectly reused. This was observed on go1.17.13 and openshift-installer commit f3c53b382 building in release mode with the following error: github.com/aliyun/alibaba-cloud-sdk-go/services/cms.(*Client).DescribeMonitoringAgentAccessKeyWithChan.func1: direct call too far: runtime.duffzero+1f0-tramp0-1 -2000078 Fixes #56833 Change-Id: I54af957302506d4e3cd5d3121542c83fe980e912 Reviewed-on: https://go-review.googlesource.com/c/go/+/451415 Reviewed-by: Cherry Mui Run-TryBot: Paul Murphy TryBot-Result: Gopher Robot Reviewed-by: Lynn Boger Reviewed-by: Than McIntosh Reviewed-on: https://go-review.googlesource.com/c/go/+/451916 Reviewed-by: Joedian Reid --- .../go/testdata/script/trampoline_reuse_test.txt | 100 +++++++++++++++++++++ src/cmd/link/internal/ppc64/asm.go | 5 +- 2 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 src/cmd/go/testdata/script/trampoline_reuse_test.txt diff --git a/src/cmd/go/testdata/script/trampoline_reuse_test.txt b/src/cmd/go/testdata/script/trampoline_reuse_test.txt new file mode 100644 index 0000000000..bca897c16d --- /dev/null +++ b/src/cmd/go/testdata/script/trampoline_reuse_test.txt @@ -0,0 +1,100 @@ +# Verify PPC64 does not reuse a trampoline which is too far away. +# This tests an edge case where the direct call relocation addend should +# be ignored when computing the distance from the direct call to the +# already placed trampoline +[short] skip +[!ppc64] [!ppc64le] skip +[aix] skip + +# Note, this program does not run. Presumably, 'DWORD $0' is simpler to +# assembly 2^26 or so times. +# +# We build something which should be laid out as such: +# +# bar.Bar +# main.Func1 +# bar.Bar+400-tramp0 +# main.BigAsm +# main.Func2 +# bar.Bar+400-tramp1 +# +# bar.Bar needs to be placed far enough away to generate relocations +# from main package calls. and main.Func1 and main.Func2 are placed +# a bit more than the direct call limit apart, but not more than 0x400 +# bytes beyond it (to verify the reloc calc). + +go build + +-- go.mod -- + +module foo + +go 1.19 + +-- main.go -- + +package main + +import "foo/bar" + +func Func1() + +func main() { + Func1() + bar.Bar2() +} + +-- foo.s -- + +TEXT main·Func1(SB),0,$0-0 + CALL bar·Bar+0x400(SB) + CALL main·BigAsm(SB) +// A trampoline will be placed here to bar.Bar + +// This creates a gap sufficiently large to prevent trampoline reuse +#define NOP64 DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; +#define NOP256 NOP64 NOP64 NOP64 NOP64 +#define NOP2S10 NOP256 NOP256 NOP256 NOP256 +#define NOP2S12 NOP2S10 NOP2S10 NOP2S10 NOP2S10 +#define NOP2S14 NOP2S12 NOP2S12 NOP2S12 NOP2S12 +#define NOP2S16 NOP2S14 NOP2S14 NOP2S14 NOP2S14 +#define NOP2S18 NOP2S16 NOP2S16 NOP2S16 NOP2S16 +#define NOP2S20 NOP2S18 NOP2S18 NOP2S18 NOP2S18 +#define NOP2S22 NOP2S20 NOP2S20 NOP2S20 NOP2S20 +#define NOP2S24 NOP2S22 NOP2S22 NOP2S22 NOP2S22 +#define BIGNOP NOP2S24 NOP2S24 +TEXT main·BigAsm(SB),0,$0-0 + // Fill to the direct call limit so Func2 must generate a new trampoline. + // As the implicit trampoline above is just barely unreachable. + BIGNOP + MOVD $main·Func2(SB), R3 + +TEXT main·Func2(SB),0,$0-0 + CALL bar·Bar+0x400(SB) +// Another trampoline should be placed here. + +-- bar/bar.s -- + +#define NOP64 DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; DWORD $0; +#define NOP256 NOP64 NOP64 NOP64 NOP64 +#define NOP2S10 NOP256 NOP256 NOP256 NOP256 +#define NOP2S12 NOP2S10 NOP2S10 NOP2S10 NOP2S10 +#define NOP2S14 NOP2S12 NOP2S12 NOP2S12 NOP2S12 +#define NOP2S16 NOP2S14 NOP2S14 NOP2S14 NOP2S14 +#define NOP2S18 NOP2S16 NOP2S16 NOP2S16 NOP2S16 +#define NOP2S20 NOP2S18 NOP2S18 NOP2S18 NOP2S18 +#define NOP2S22 NOP2S20 NOP2S20 NOP2S20 NOP2S20 +#define NOP2S24 NOP2S22 NOP2S22 NOP2S22 NOP2S22 +#define BIGNOP NOP2S24 NOP2S24 NOP2S10 +// A very big not very interesting function. +TEXT bar·Bar(SB),0,$0-0 + BIGNOP + +-- bar/bar.go -- + +package bar + +func Bar() + +func Bar2() { +} diff --git a/src/cmd/link/internal/ppc64/asm.go b/src/cmd/link/internal/ppc64/asm.go index 73c2718a33..879adaa965 100644 --- a/src/cmd/link/internal/ppc64/asm.go +++ b/src/cmd/link/internal/ppc64/asm.go @@ -809,8 +809,9 @@ func trampoline(ctxt *ld.Link, ldr *loader.Loader, ri int, rs, s loader.Sym) { if ldr.SymValue(tramp) == 0 { break } - - t = ldr.SymValue(tramp) + r.Add() - (ldr.SymValue(s) + int64(r.Off())) + // Note, the trampoline is always called directly. The addend of the original relocation is accounted for in the + // trampoline itself. + t = ldr.SymValue(tramp) - (ldr.SymValue(s) + int64(r.Off())) // With internal linking, the trampoline can be used if it is not too far. // With external linking, the trampoline must be in this section for it to be reused. -- cgit v1.2.1 From f92317e69bc0d3a19f472d9cfb3d2c6e5c8463e7 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 9 Dec 2022 11:22:32 -0800 Subject: [release-branch.go1.18] os: skip size test in TestLstat if the file is a symlink Tested by temporarily changing sysdir to use a directory where the expected files were all symlinks. We should consider using a different approach that doesn't rely on sysdir, but for now do a minimal fix. For #57210 Fixes #57213 Change-Id: Ifb1becef03e014ceb48290ce13527b3e103c0e07 Reviewed-on: https://go-review.googlesource.com/c/go/+/456557 Reviewed-by: Ian Lance Taylor Reviewed-by: Bryan Mills Run-TryBot: Ian Lance Taylor Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gopher Robot (cherry picked from commit 9b8750f53ed89fb326e4d811524e647683136bac) Reviewed-on: https://go-review.googlesource.com/c/go/+/456561 Reviewed-by: Austin Clements --- src/os/os_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/os/os_test.go b/src/os/os_test.go index 4124be13cc..3721e15027 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -252,9 +252,11 @@ func TestLstat(t *testing.T) { if !equal(sfname, dir.Name()) { t.Error("name should be ", sfname, "; is", dir.Name()) } - filesize := size(path, t) - if dir.Size() != filesize { - t.Error("size should be", filesize, "; is", dir.Size()) + if dir.Mode()&ModeSymlink == 0 { + filesize := size(path, t) + if dir.Size() != filesize { + t.Error("size should be", filesize, "; is", dir.Size()) + } } } -- cgit v1.2.1 From d17cf5654105ff4f9490870586175990de3334a7 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 14 Dec 2022 10:14:18 -0800 Subject: [release-branch.go1.18] all: upgrade golang.org/x/net to v0.0.0-20221214163811-6143a133e5c9 Update x/net to include the fix for #53960. For #53960 For #56323 Change-Id: I825212ecdf7bf2f52c2fda1faf1739b593063653 Reviewed-on: https://go-review.googlesource.com/c/go/+/457596 TryBot-Result: Gopher Robot Reviewed-by: Than McIntosh Run-TryBot: Damien Neil --- src/go.mod | 2 +- src/go.sum | 4 ++-- src/net/http/h2_bundle.go | 7 +------ src/vendor/modules.txt | 2 +- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/go.mod b/src/go.mod index 4613bb6825..e11c6d7df8 100644 --- a/src/go.mod +++ b/src/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 - golang.org/x/net v0.0.0-20220907013725-0a43f88f7ef0 + golang.org/x/net v0.0.0-20221214163811-6143a133e5c9 ) require ( diff --git a/src/go.sum b/src/go.sum index 07cb8d82d5..d2bfed64dc 100644 --- a/src/go.sum +++ b/src/go.sum @@ -1,7 +1,7 @@ golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 h1:0es+/5331RGQPcXlMfP+WrnIIS6dNnNRe0WB02W0F4M= golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/net v0.0.0-20220907013725-0a43f88f7ef0 h1:XXaSUSplyi6wsRNJGB7vUBvDjbxc8UPYBsf9ukBQ3KA= -golang.org/x/net v0.0.0-20220907013725-0a43f88f7ef0/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20221214163811-6143a133e5c9 h1:gcbGP3ZkgsHGpX/48qvg7Q/YmTtzZRWc/zpvN8XGSBg= +golang.org/x/net v0.0.0-20221214163811-6143a133e5c9/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5 h1:y/woIyUBFbpQGKS0u1aHF/40WUDnek3fPOyD08H5Vng= golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.3.8-0.20211105212822-18b340fc7af2 h1:GLw7MR8AfAG2GmGcmVgObFOHXYypgGjnGno25RDwn3Y= diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 381f91c8ad..3a21ecac3d 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -5760,12 +5760,6 @@ func (sc *http2serverConn) newWriterAndRequest(st *http2stream, f *http2MetaHead return nil, nil, sc.countError("bad_path_method", http2streamError(f.StreamID, http2ErrCodeProtocol)) } - bodyOpen := !f.StreamEnded() - if rp.method == "HEAD" && bodyOpen { - // HEAD requests can't have bodies - return nil, nil, sc.countError("head_body", http2streamError(f.StreamID, http2ErrCodeProtocol)) - } - rp.header = make(Header) for _, hf := range f.RegularFields() { rp.header.Add(sc.canonicalHeader(hf.Name), hf.Value) @@ -5778,6 +5772,7 @@ func (sc *http2serverConn) newWriterAndRequest(st *http2stream, f *http2MetaHead if err != nil { return nil, nil, err } + bodyOpen := !f.StreamEnded() if bodyOpen { if vv, ok := rp.header["Content-Length"]; ok { if cl, err := strconv.ParseUint(vv[0], 10, 63); err == nil { diff --git a/src/vendor/modules.txt b/src/vendor/modules.txt index 109a99e44a..157c1fe602 100644 --- a/src/vendor/modules.txt +++ b/src/vendor/modules.txt @@ -9,7 +9,7 @@ golang.org/x/crypto/curve25519/internal/field golang.org/x/crypto/hkdf golang.org/x/crypto/internal/poly1305 golang.org/x/crypto/internal/subtle -# golang.org/x/net v0.0.0-20220907013725-0a43f88f7ef0 +# golang.org/x/net v0.0.0-20221214163811-6143a133e5c9 ## explicit; go 1.17 golang.org/x/net/dns/dnsmessage golang.org/x/net/http/httpguts -- cgit v1.2.1 From 337138c10c7aff6b4bdcb5472128a3ba11d57620 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 9 Dec 2022 10:55:28 -0800 Subject: [release-branch.go1.18] cmd/compile: fix conditional select rule ARM64 maintains booleans in the low byte of registers. Upper parts of that register are junk. This rule is using all 32 bits of a boolean-containing register, which is wrong. Change the rule to only look at the low bit. Fixes #57211 Change-Id: Ibbef86b2be859df3d06d993db00e1231c481c428 Reviewed-on: https://go-review.googlesource.com/c/go/+/456556 Auto-Submit: Keith Randall TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Reviewed-by: Keith Randall Run-TryBot: Keith Randall Reviewed-on: https://go-review.googlesource.com/c/go/+/456558 Reviewed-by: Than McIntosh --- src/cmd/compile/internal/ssa/gen/ARM64.rules | 4 +-- src/cmd/compile/internal/ssa/rewriteARM64.go | 6 ++--- test/fixedbugs/issue57184.go | 40 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 test/fixedbugs/issue57184.go diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules index 09e70ad13b..ad99960078 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64.rules +++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules @@ -316,9 +316,9 @@ (FCMPD x (FMOVDconst [0])) => (FCMPD0 x) (FCMPD (FMOVDconst [0]) x) => (InvertFlags (FCMPD0 x)) -// CSEL needs a flag-generating argument. Synthesize a CMPW if necessary. +// CSEL needs a flag-generating argument. Synthesize a TSTW if necessary. (CondSelect x y boolval) && flagArg(boolval) != nil => (CSEL [boolval.Op] x y flagArg(boolval)) -(CondSelect x y boolval) && flagArg(boolval) == nil => (CSEL [OpARM64NotEqual] x y (CMPWconst [0] boolval)) +(CondSelect x y boolval) && flagArg(boolval) == nil => (CSEL [OpARM64NotEqual] x y (TSTWconst [1] boolval)) (OffPtr [off] ptr:(SP)) && is32Bit(off) => (MOVDaddr [int32(off)] ptr) (OffPtr [off] ptr) => (ADDconst [off] ptr) diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go index 1ee25c2eee..ad1052f88d 100644 --- a/src/cmd/compile/internal/ssa/rewriteARM64.go +++ b/src/cmd/compile/internal/ssa/rewriteARM64.go @@ -23409,7 +23409,7 @@ func rewriteValueARM64_OpCondSelect(v *Value) bool { } // match: (CondSelect x y boolval) // cond: flagArg(boolval) == nil - // result: (CSEL [OpARM64NotEqual] x y (CMPWconst [0] boolval)) + // result: (CSEL [OpARM64NotEqual] x y (TSTWconst [1] boolval)) for { x := v_0 y := v_1 @@ -23419,8 +23419,8 @@ func rewriteValueARM64_OpCondSelect(v *Value) bool { } v.reset(OpARM64CSEL) v.AuxInt = opToAuxInt(OpARM64NotEqual) - v0 := b.NewValue0(v.Pos, OpARM64CMPWconst, types.TypeFlags) - v0.AuxInt = int32ToAuxInt(0) + v0 := b.NewValue0(v.Pos, OpARM64TSTWconst, types.TypeFlags) + v0.AuxInt = int32ToAuxInt(1) v0.AddArg(boolval) v.AddArg3(x, y, v0) return true diff --git a/test/fixedbugs/issue57184.go b/test/fixedbugs/issue57184.go new file mode 100644 index 0000000000..1384b50be8 --- /dev/null +++ b/test/fixedbugs/issue57184.go @@ -0,0 +1,40 @@ +// run + +// Copyright 2022 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 ( + "log" + "reflect" + "sort" +) + +func main() { + const length = 257 + x := make([]int64, length) + for i := 0; i < length; i++ { + x[i] = int64(i) * 27644437 % int64(length) + } + + isLessStatic := func(i, j int) bool { + return x[i] < x[j] + } + + isLessReflect := reflect.MakeFunc(reflect.TypeOf(isLessStatic), func(args []reflect.Value) []reflect.Value { + i := args[0].Int() + j := args[1].Int() + b := x[i] < x[j] + return []reflect.Value{reflect.ValueOf(b)} + }).Interface().(func(i, j int) bool) + + sort.SliceStable(x, isLessReflect) + + for i := 0; i < length-1; i++ { + if x[i] >= x[i+1] { + log.Fatalf("not sorted! (length=%v, idx=%v)\n%v\n", length, i, x) + } + } +} -- cgit v1.2.1 From 6aa1e6d52d6e2791f356ccf2b1177510ba4dade1 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 9 Dec 2022 19:49:53 -0800 Subject: [release-branch.go1.18] cmd/compile: fix conditional move rule on PPC64 Similar to CL 456556 but for ppc64 instead of arm64. Change docs about how booleans are stored in registers for ppc64. We now don't promise to keep the upper bits zeroed; they might be junk. To test, I changed the boolean generation instructions (MOVBZload* and ISEL* with boolean type) to OR in 0x100 to the result. all.bash still passed, so I think nothing else is depending on the upper bits of booleans. Update #57211 Change-Id: Ie66f8934a0dafa34d0a8c2a37324868d959a852c Reviewed-on: https://go-review.googlesource.com/c/go/+/456437 Run-TryBot: Keith Randall Reviewed-by: Cherry Mui Reviewed-by: Keith Randall TryBot-Result: Gopher Robot Reviewed-by: KAMPANAT THUMWONG (KONG PC) <1992kongpc.kth@gmail.com> Run-TryBot: Archana Ravindar Reviewed-on: https://go-review.googlesource.com/c/go/+/456735 Reviewed-by: Than McIntosh --- src/cmd/compile/internal/ssa/gen/PPC64.rules | 4 ++-- src/cmd/compile/internal/ssa/gen/PPC64Ops.go | 4 ++-- src/cmd/compile/internal/ssa/rewritePPC64.go | 25 ++++++++++++++++--------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/PPC64.rules b/src/cmd/compile/internal/ssa/gen/PPC64.rules index a90a3d0937..f83ed78bab 100644 --- a/src/cmd/compile/internal/ssa/gen/PPC64.rules +++ b/src/cmd/compile/internal/ssa/gen/PPC64.rules @@ -562,9 +562,9 @@ ((EQ|NE|LT|LE|GT|GE) (CMPconst [0] z:(XOR x y)) yes no) && z.Uses == 1 => ((EQ|NE|LT|LE|GT|GE) (XORCC x y) yes no) // Only lower after bool is lowered. It should always lower. This helps ensure the folding below happens reliably. -(CondSelect x y bool) && flagArg(bool) == nil => (ISEL [6] x y (CMPWconst [0] bool)) +(CondSelect x y bool) && flagArg(bool) == nil => (ISEL [6] x y (Select1 (ANDCCconst [1] bool))) // Fold any CR -> GPR -> CR transfers when applying the above rule. -(ISEL [6] x y (CMPWconst [0] (ISELB [c] one cmp))) => (ISEL [c] x y cmp) +(ISEL [6] x y (Select1 (ANDCCconst [1] (ISELB [c] one cmp)))) => (ISEL [c] x y cmp) // Lowering loads (Load ptr mem) && (is64BitInt(t) || isPtr(t)) => (MOVDload ptr mem) diff --git a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go index 59d8af1a9d..d18cbcc787 100644 --- a/src/cmd/compile/internal/ssa/gen/PPC64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/PPC64Ops.go @@ -11,8 +11,8 @@ import "strings" // Notes: // - Less-than-64-bit integer types live in the low portion of registers. -// For now, the upper portion is junk; sign/zero-extension might be optimized in the future, but not yet. -// - Boolean types are zero or 1; stored in a byte, but loaded with AMOVBZ so the upper bytes of a register are zero. +// The upper portion is junk. +// - Boolean types are zero or 1; stored in a byte, with upper bytes of the register containing junk. // - *const instructions may use a constant larger than the instruction can encode. // In this case the assembler expands to multiple instructions and uses tmp // register (R31). diff --git a/src/cmd/compile/internal/ssa/rewritePPC64.go b/src/cmd/compile/internal/ssa/rewritePPC64.go index 5a28b9d4f7..c7bcc248fc 100644 --- a/src/cmd/compile/internal/ssa/rewritePPC64.go +++ b/src/cmd/compile/internal/ssa/rewritePPC64.go @@ -1167,9 +1167,10 @@ func rewriteValuePPC64_OpCondSelect(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] b := v.Block + typ := &b.Func.Config.Types // match: (CondSelect x y bool) // cond: flagArg(bool) == nil - // result: (ISEL [6] x y (CMPWconst [0] bool)) + // result: (ISEL [6] x y (Select1 (ANDCCconst [1] bool))) for { x := v_0 y := v_1 @@ -1179,9 +1180,11 @@ func rewriteValuePPC64_OpCondSelect(v *Value) bool { } v.reset(OpPPC64ISEL) v.AuxInt = int32ToAuxInt(6) - v0 := b.NewValue0(v.Pos, OpPPC64CMPWconst, types.TypeFlags) - v0.AuxInt = int32ToAuxInt(0) - v0.AddArg(bool) + v0 := b.NewValue0(v.Pos, OpSelect1, types.TypeFlags) + v1 := b.NewValue0(v.Pos, OpPPC64ANDCCconst, types.NewTuple(typ.Int, types.TypeFlags)) + v1.AuxInt = int64ToAuxInt(1) + v1.AddArg(bool) + v0.AddArg(v1) v.AddArg3(x, y, v0) return true } @@ -5895,7 +5898,7 @@ func rewriteValuePPC64_OpPPC64ISEL(v *Value) bool { v.AddArg(y) return true } - // match: (ISEL [6] x y (CMPWconst [0] (ISELB [c] one cmp))) + // match: (ISEL [6] x y (Select1 (ANDCCconst [1] (ISELB [c] one cmp)))) // result: (ISEL [c] x y cmp) for { if auxIntToInt32(v.AuxInt) != 6 { @@ -5903,15 +5906,19 @@ func rewriteValuePPC64_OpPPC64ISEL(v *Value) bool { } x := v_0 y := v_1 - if v_2.Op != OpPPC64CMPWconst || auxIntToInt32(v_2.AuxInt) != 0 { + if v_2.Op != OpSelect1 { break } v_2_0 := v_2.Args[0] - if v_2_0.Op != OpPPC64ISELB { + if v_2_0.Op != OpPPC64ANDCCconst || auxIntToInt64(v_2_0.AuxInt) != 1 { + break + } + v_2_0_0 := v_2_0.Args[0] + if v_2_0_0.Op != OpPPC64ISELB { break } - c := auxIntToInt32(v_2_0.AuxInt) - cmp := v_2_0.Args[1] + c := auxIntToInt32(v_2_0_0.AuxInt) + cmp := v_2_0_0.Args[1] v.reset(OpPPC64ISEL) v.AuxInt = int32ToAuxInt(c) v.AddArg3(x, y, cmp) -- cgit v1.2.1 From e6adccb3c56385f8d840165008f2606098e13e56 Mon Sep 17 00:00:00 2001 From: Alex Brachet Date: Sat, 3 Dec 2022 18:52:26 +0000 Subject: [release-branch.go1.18] cmd/cgo: allow DW_TAG_variable's with no name https://reviews.llvm.org/D123534 is emitting DW_TAG_variable's that don't have a DW_AT_name. This is allowed in the DWARF standard. It is adding DIE's for string literals for better symbolization on buffer overlows etc on these strings. They no associated name because they are not user provided variables. Fixes #57044 Updates #53000 Change-Id: I2cf063160508687067c7672cef0517bccd707d7b Reviewed-on: https://go-review.googlesource.com/c/go/+/406816 TryBot-Result: Gopher Robot Run-TryBot: Ian Lance Taylor Auto-Submit: Ian Lance Taylor Reviewed-by: Ian Lance Taylor (cherry picked from commit e66f895667cd51d0d28c42d369a803c12db8bb35) Change-Id: I2cf063160508687067c7672cef0517bccd707d7b GitHub-Last-Rev: 32208e4fa20272b80cfffd95a4701e1473f47ca9 GitHub-Pull-Request: golang/go#57067 Reviewed-on: https://go-review.googlesource.com/c/go/+/455055 Reviewed-by: Ian Lance Taylor Run-TryBot: David Chase TryBot-Result: Gopher Robot --- src/cmd/cgo/gcc.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index dc5639812a..79d27c3e37 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -577,8 +577,23 @@ func (p *Package) loadDWARF(f *File, conv *typeConv, names []*Name) { switch e.Tag { case dwarf.TagVariable: name, _ := e.Val(dwarf.AttrName).(string) + // As of https://reviews.llvm.org/D123534, clang + // now emits DW_TAG_variable DIEs that have + // no name (so as to be able to describe the + // type and source locations of constant strings + // like the second arg in the call below: + // + // myfunction(42, "foo") + // + // If a var has no name we won't see attempts to + // refer to it via "C.", so skip these vars + // + // See issue 53000 for more context. + if name == "" { + break + } typOff, _ := e.Val(dwarf.AttrType).(dwarf.Offset) - if name == "" || typOff == 0 { + if typOff == 0 { if e.Val(dwarf.AttrSpecification) != nil { // Since we are reading all the DWARF, // assume we will see the variable elsewhere. -- cgit v1.2.1 From 856ec21021a78faa48dfa7688840d8013279a947 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Dec 2022 13:54:33 -0800 Subject: [release-branch.go1.18] syscall, internal/poll: fall back to accept on linux-arm Our minimum Linux version is 2.6.32, and the accept4 system call was introduced in 2.6.28, so we use accept4 everywhere. Unfortunately, it turns out that the accept4 system call was only added to linux-arm in 2.6.36, so for linux-arm only we need to try the accept4 system call and then fall back to accept if it doesn't work. The code we use on linux-arm is the code we used in Go 1.17. On non-arm platforms we continue using the simpler code introduced in Go 1.18. Adding accept4 to the ARM Linux kernel was: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=21d93e2e29722d7832f61cc56d73fb953ee6578e For #57333 Fixes #57338 Change-Id: I6680cb54dd4d3514a6887dda8906e6708c64459d Reviewed-on: https://go-review.googlesource.com/c/go/+/457997 Reviewed-by: Tobias Klauser Run-TryBot: Ian Lance Taylor Reviewed-by: Than McIntosh Reviewed-by: Ian Lance Taylor Reviewed-by: Brad Fitzpatrick TryBot-Result: Gopher Robot Reviewed-by: David Chase Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor --- src/internal/poll/sock_cloexec.go | 2 +- src/internal/poll/sock_cloexec_accept.go | 51 ++++++++++++++++++++++++++++++++ src/syscall/syscall_linux.go | 15 ---------- src/syscall/syscall_linux_accept.go | 32 ++++++++++++++++++++ src/syscall/syscall_linux_accept4.go | 25 ++++++++++++++++ 5 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 src/internal/poll/sock_cloexec_accept.go create mode 100644 src/syscall/syscall_linux_accept.go create mode 100644 src/syscall/syscall_linux_accept4.go diff --git a/src/internal/poll/sock_cloexec.go b/src/internal/poll/sock_cloexec.go index d849fda0b0..4c70abd9b1 100644 --- a/src/internal/poll/sock_cloexec.go +++ b/src/internal/poll/sock_cloexec.go @@ -5,7 +5,7 @@ // This file implements accept for platforms that provide a fast path for // setting SetNonblock and CloseOnExec. -//go:build dragonfly || freebsd || illumos || linux || netbsd || openbsd +//go:build dragonfly || freebsd || illumos || (linux && !arm) || netbsd || openbsd package poll diff --git a/src/internal/poll/sock_cloexec_accept.go b/src/internal/poll/sock_cloexec_accept.go new file mode 100644 index 0000000000..4b86de59e0 --- /dev/null +++ b/src/internal/poll/sock_cloexec_accept.go @@ -0,0 +1,51 @@ +// 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. + +// This file implements accept for platforms that provide a fast path for +// setting SetNonblock and CloseOnExec, but don't necessarily have accept4. +// This is the code we used for accept in Go 1.17 and earlier. +// On Linux the accept4 system call was introduced in 2.6.28 kernel, +// and our minimum requirement is 2.6.32, so we simplified the function. +// Unfortunately, on ARM accept4 wasn't added until 2.6.36, so for ARM +// only we continue using the older code. + +//go:build linux && arm + +package poll + +import "syscall" + +// Wrapper around the accept system call that marks the returned file +// descriptor as nonblocking and close-on-exec. +func accept(s int) (int, syscall.Sockaddr, string, error) { + ns, sa, err := Accept4Func(s, syscall.SOCK_NONBLOCK|syscall.SOCK_CLOEXEC) + switch err { + case nil: + return ns, sa, "", nil + default: // errors other than the ones listed + return -1, sa, "accept4", err + case syscall.ENOSYS: // syscall missing + case syscall.EINVAL: // some Linux use this instead of ENOSYS + case syscall.EACCES: // some Linux use this instead of ENOSYS + case syscall.EFAULT: // some Linux use this instead of ENOSYS + } + + // See ../syscall/exec_unix.go for description of ForkLock. + // It is probably okay to hold the lock across syscall.Accept + // because we have put fd.sysfd into non-blocking mode. + // However, a call to the File method will put it back into + // blocking mode. We can't take that risk, so no use of ForkLock here. + ns, sa, err = AcceptFunc(s) + if err == nil { + syscall.CloseOnExec(ns) + } + if err != nil { + return -1, nil, "accept", err + } + if err = syscall.SetNonblock(ns, true); err != nil { + CloseFunc(ns) + return -1, nil, "setnonblock", err + } + return ns, sa, "", nil +} diff --git a/src/syscall/syscall_linux.go b/src/syscall/syscall_linux.go index f32020ca6c..f694d8ca35 100644 --- a/src/syscall/syscall_linux.go +++ b/src/syscall/syscall_linux.go @@ -546,21 +546,6 @@ func anyToSockaddr(rsa *RawSockaddrAny) (Sockaddr, error) { return nil, EAFNOSUPPORT } -func Accept(fd int) (nfd int, sa Sockaddr, err error) { - var rsa RawSockaddrAny - var len _Socklen = SizeofSockaddrAny - nfd, err = accept4(fd, &rsa, &len, 0) - if err != nil { - return - } - sa, err = anyToSockaddr(&rsa) - if err != nil { - Close(nfd) - nfd = 0 - } - return -} - func Accept4(fd int, flags int) (nfd int, sa Sockaddr, err error) { var rsa RawSockaddrAny var len _Socklen = SizeofSockaddrAny diff --git a/src/syscall/syscall_linux_accept.go b/src/syscall/syscall_linux_accept.go new file mode 100644 index 0000000000..2ba60f0636 --- /dev/null +++ b/src/syscall/syscall_linux_accept.go @@ -0,0 +1,32 @@ +// Copyright 2009 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. + +// We require Linux kernel version 2.6.32. The accept4 system call was +// added in version 2.6.28, so in general we can use accept4. +// Unfortunately, for ARM only, accept4 was added in version 2.6.36. +// Handle that case here, by using a copy of the Accept function that +// we used in Go 1.17. + +//go:build linux && arm + +package syscall + +func Accept(fd int) (nfd int, sa Sockaddr, err error) { + var rsa RawSockaddrAny + var len _Socklen = SizeofSockaddrAny + // Try accept4 first for Android and newer kernels. + nfd, err = accept4(fd, &rsa, &len, 0) + if err == ENOSYS { + nfd, err = accept(fd, &rsa, &len) + } + if err != nil { + return + } + sa, err = anyToSockaddr(&rsa) + if err != nil { + Close(nfd) + nfd = 0 + } + return +} diff --git a/src/syscall/syscall_linux_accept4.go b/src/syscall/syscall_linux_accept4.go new file mode 100644 index 0000000000..74898672c0 --- /dev/null +++ b/src/syscall/syscall_linux_accept4.go @@ -0,0 +1,25 @@ +// Copyright 2009 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. + +// This file provides the Accept function used on all systems +// other than arm. See syscall_linux_accept.go for why. + +//go:build linux && !arm + +package syscall + +func Accept(fd int) (nfd int, sa Sockaddr, err error) { + var rsa RawSockaddrAny + var len _Socklen = SizeofSockaddrAny + nfd, err = accept4(fd, &rsa, &len, 0) + if err != nil { + return + } + sa, err = anyToSockaddr(&rsa) + if err != nil { + Close(nfd) + nfd = 0 + } + return +} -- cgit v1.2.1 From 07b6ffb79c906a73573784e9fc71f98f1143d8fb Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 22 Dec 2022 09:44:56 -0500 Subject: [release-branch.go1.18] runtime: call __fork instead of fork on darwin Issues #33565 and #56784 were caused by hangs in the child process after fork, while it ran atfork handlers that ran into slow paths that didn't work in the child. CL 451735 worked around those two issues by calling a couple functions at startup to try to warm up those child paths. That mostly worked, but it broke programs using cgo with certain macOS frameworks (#57263). CL 459175 reverted CL 451735. This CL introduces a different fix: bypass the atfork child handlers entirely. For a general fork call where the child and parent are both meant to keep executing the original program, atfork handlers can be necessary to fix any state that would otherwise be tied to the parent process. But Go only uses fork as preparation for exec, and it takes care to limit what it attempts to do in the child between the fork and exec. In particular it doesn't use any of the things that the macOS atfork handlers are trying to fix up (malloc, xpc, others). So we can use the low-level fork system call (__fork) instead of the atfork-wrapped one. The full list of functions that can be called in a child after fork in exec_libc2.go is: - ptrace - setsid - setpgid - getpid - ioctl - chroot - setgroups - setgid - setuid - chdir - dup2 - fcntl - close - execve - write - exit I disassembled all of these while attached to a hung exec.test binary and confirmed that nearly all of them are making direct kernel calls, not using anything that the atfork handler needs to fix up. The exceptions are ioctl, fcntl, and exit. The ioctl and fcntl implementations do some extra work around the kernel call but don't call any other functions, so they should still be OK. (If not, we could use __ioctl and __fcntl instead, but without a good reason, we should keep using the standard entry points.) The exit implementation calls atexit handlers. That is almost certainly inappropriate in a failed fork child, so this CL changes that call to __exit on darwin. To avoid making unnecessary changes at this point in the release cycle, this CL leaves OpenBSD calling plain exit, even though that is probably a bug in the OpenBSD port (filed #57446). Fixes #33565. Fixes #56784. Fixes #57263. Fixes #56836. Change-Id: I26812c26a72bdd7fcf72ec41899ba11cf6b9c4ab Reviewed-on: https://go-review.googlesource.com/c/go/+/459176 Reviewed-by: David Chase Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot Run-TryBot: Russ Cox Reviewed-on: https://go-review.googlesource.com/c/go/+/459179 --- src/syscall/exec_libc2.go | 4 ++-- src/syscall/syscall_darwin.go | 31 ++++++++++++++++++++++++++++++- src/syscall/syscall_openbsd_libc.go | 5 +++++ src/syscall/zsyscall_darwin_amd64.go | 22 ++++++++++++++++++---- src/syscall/zsyscall_darwin_amd64.s | 6 ++++-- src/syscall/zsyscall_darwin_arm64.go | 22 ++++++++++++++++++---- src/syscall/zsyscall_darwin_arm64.s | 6 ++++-- 7 files changed, 81 insertions(+), 15 deletions(-) diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go index b05f053bbf..bd2ce6820d 100644 --- a/src/syscall/exec_libc2.go +++ b/src/syscall/exec_libc2.go @@ -76,7 +76,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr // About to call fork. // No more allocation or calls of non-assembly functions. runtime_BeforeFork() - r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) + r1, _, err1 = rawSyscall(forkTrampoline, 0, 0, 0) if err1 != 0 { runtime_AfterFork() return 0, err1 @@ -260,6 +260,6 @@ childerror: // send error code on pipe rawSyscall(abi.FuncPCABI0(libc_write_trampoline), uintptr(pipe), uintptr(unsafe.Pointer(&err1)), unsafe.Sizeof(err1)) for { - rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0) + rawSyscall(exitTrampoline, 253, 0, 0) } } diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go index 902d6e77e1..346e6862cd 100644 --- a/src/syscall/syscall_darwin.go +++ b/src/syscall/syscall_darwin.go @@ -17,6 +17,34 @@ import ( "unsafe" ) +// These are called from exec_libc2.go in the child of fork. +// The names differ between macOS and OpenBSD, so we need +// to declare the specific ones used here to keep the exec_libc2.go +// code portable. +// +// We use __fork and __exit, not fork and exit, to avoid the libc atfork +// and atexit handlers. The atfork handlers have caused fork child +// hangs in the past (see #33565, #56784). The atexit handlers have +// not, but the non-libc ports all invoke the system call, so doing +// the same here makes sense. In general we wouldn't expect +// atexit handlers to work terribly well in a fork child anyway. +// (Also, perhaps the atfork handlers clear the atexit handlers, +// in which case we definitely need to avoid calling the libc exit +// if we bypass the libc fork.) +// +// Other calls that are made in the child after the fork are +// ptrace, setsid, setpgid, getpid, ioctl, chroot, setgroups, +// setgid, setuid, chdir, dup2, fcntl, close, execve, and write. +// Those are all simple kernel wrappers that should be safe +// to be called directly. The fcntl and ioctl functions do run +// some code around the kernel call, but they don't call any +// other functions, so for now we keep using them instead of +// calling the lower-level __fcntl and __ioctl functions. +var ( + exitTrampoline = abi.FuncPCABI0(libc___exit_trampoline) + forkTrampoline = abi.FuncPCABI0(libc___fork_trampoline) +) + type SockaddrDatalink struct { Len uint8 Family uint8 @@ -202,11 +230,12 @@ func Kill(pid int, signum Signal) (err error) { return kill(pid, int(signum), 1) //sys writev(fd int, iovecs []Iovec) (cnt uintptr, err error) //sys mmap(addr uintptr, length uintptr, prot int, flag int, fd int, pos int64) (ret uintptr, err error) //sys munmap(addr uintptr, length uintptr) (err error) -//sysnb fork() (pid int, err error) +//sysnb __fork() (pid int, err error) //sysnb ioctl(fd int, req int, arg int) (err error) //sysnb ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = SYS_ioctl //sysnb execve(path *byte, argv **byte, envp **byte) (err error) //sysnb exit(res int) (err error) +//sysnb __exit(res int) (err error) //sys sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) //sys fcntlPtr(fd int, cmd int, arg unsafe.Pointer) (val int, err error) = SYS_fcntl //sys unlinkat(fd int, path string, flags int) (err error) diff --git a/src/syscall/syscall_openbsd_libc.go b/src/syscall/syscall_openbsd_libc.go index 15b68fd0fc..ff9fa00259 100644 --- a/src/syscall/syscall_openbsd_libc.go +++ b/src/syscall/syscall_openbsd_libc.go @@ -10,6 +10,11 @@ import ( "internal/abi" ) +var ( + exitTrampoline = abi.FuncPCABI0(libc_exit_trampoline) + forkTrampoline = abi.FuncPCABI0(libc_fork_trampoline) +) + func init() { execveOpenBSD = execve } diff --git a/src/syscall/zsyscall_darwin_amd64.go b/src/syscall/zsyscall_darwin_amd64.go index 0ccdaf2d0e..2ad03801b2 100644 --- a/src/syscall/zsyscall_darwin_amd64.go +++ b/src/syscall/zsyscall_darwin_amd64.go @@ -1714,8 +1714,8 @@ func libc_munmap_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func fork() (pid int, err error) { - r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) +func __fork() (pid int, err error) { + r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0) pid = int(r0) if e1 != 0 { err = errnoErr(e1) @@ -1723,9 +1723,9 @@ func fork() (pid int, err error) { return } -func libc_fork_trampoline() +func libc___fork_trampoline() -//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib" // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT @@ -1781,6 +1781,20 @@ func libc_exit_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT +func __exit(res int) (err error) { + _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0) + if e1 != 0 { + err = errnoErr(e1) + } + return +} + +func libc___exit_trampoline() + +//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib" + +// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT + func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) { var _p0 unsafe.Pointer if len(mib) > 0 { diff --git a/src/syscall/zsyscall_darwin_amd64.s b/src/syscall/zsyscall_darwin_amd64.s index 563083d441..1815703803 100644 --- a/src/syscall/zsyscall_darwin_amd64.s +++ b/src/syscall/zsyscall_darwin_amd64.s @@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_mmap(SB) TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_munmap(SB) -TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0 - JMP libc_fork(SB) +TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0 + JMP libc___fork(SB) TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_ioctl(SB) TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0 JMP libc_execve(SB) TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0 JMP libc_exit(SB) +TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0 + JMP libc___exit(SB) TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_sysctl(SB) TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0 diff --git a/src/syscall/zsyscall_darwin_arm64.go b/src/syscall/zsyscall_darwin_arm64.go index 09bf34bb3c..559bb49c4b 100644 --- a/src/syscall/zsyscall_darwin_arm64.go +++ b/src/syscall/zsyscall_darwin_arm64.go @@ -1714,8 +1714,8 @@ func libc_munmap_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func fork() (pid int, err error) { - r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) +func __fork() (pid int, err error) { + r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0) pid = int(r0) if e1 != 0 { err = errnoErr(e1) @@ -1723,9 +1723,9 @@ func fork() (pid int, err error) { return } -func libc_fork_trampoline() +func libc___fork_trampoline() -//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib" // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT @@ -1781,6 +1781,20 @@ func libc_exit_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT +func __exit(res int) (err error) { + _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0) + if e1 != 0 { + err = errnoErr(e1) + } + return +} + +func libc___exit_trampoline() + +//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib" + +// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT + func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) { var _p0 unsafe.Pointer if len(mib) > 0 { diff --git a/src/syscall/zsyscall_darwin_arm64.s b/src/syscall/zsyscall_darwin_arm64.s index 0567a42fa3..1666cf985c 100644 --- a/src/syscall/zsyscall_darwin_arm64.s +++ b/src/syscall/zsyscall_darwin_arm64.s @@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_mmap(SB) TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_munmap(SB) -TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0 - JMP libc_fork(SB) +TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0 + JMP libc___fork(SB) TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_ioctl(SB) TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0 JMP libc_execve(SB) TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0 JMP libc_exit(SB) +TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0 + JMP libc___exit(SB) TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_sysctl(SB) TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0 -- cgit v1.2.1 From eeaf508d13b463c0bd2224275e239c01e06f4ade Mon Sep 17 00:00:00 2001 From: David Chase Date: Mon, 19 Dec 2022 11:59:13 -0500 Subject: [release-branch.go1.18] cmd/compile: sign-extend the 2nd argument of the LoweredAtomicCas32 on mips64x,riscv64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function LoweredAtomicCas32 is implemented using the LL-SC instruction pair on loong64, mips64x, riscv64. However,the LL instruction on loong64, mips64x, riscv64 is sign-extended, so it is necessary to sign-extend the 2nd parameter "old" of the LoweredAtomicCas32, so that the instruction BNE after LL can get the desired result. The function prototype of LoweredAtomicCas32 in golang: func Cas32(ptr *uint32, old, new uint32) bool When using an intrinsify implementation: case 1: (*ptr) <= 0x80000000 && old < 0x80000000 E.g: (*ptr) = 0x7FFFFFFF, old = Rarg1= 0x7FFFFFFF After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0x7FFFFFFF Rtmp ! = Rarg1(old) is false, the result we expect case 2: (*ptr) >= 0x80000000 && old >= 0x80000000 E.g: (*ptr) = 0x80000000, old = Rarg1= 0x80000000 After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0xFFFFFFFF_80000000 Rtmp ! = Rarg1(old) is true, which we do not expect When using an non-intrinsify implementation: Because Rarg1 is loaded from the stack using sign-extended instructions ld.w, the situation described in Case 2 above does not occur Benchmarks on linux/loong64: name old time/op new time/op delta Cas 50.0ns ± 0% 50.1ns ± 0% ~ (p=1.000 n=1+1) Cas64 50.0ns ± 0% 50.1ns ± 0% ~ (p=1.000 n=1+1) Cas-4 56.0ns ± 0% 56.0ns ± 0% ~ (p=1.000 n=1+1) Cas64-4 56.0ns ± 0% 56.0ns ± 0% ~ (p=1.000 n=1+1) Benchmarks on Loongson 3A4000 (GOARCH=mips64le, 1.8GHz) name old time/op new time/op delta Cas 70.4ns ± 0% 70.3ns ± 0% ~ (p=1.000 n=1+1) Cas64 70.7ns ± 0% 70.6ns ± 0% ~ (p=1.000 n=1+1) Cas-4 81.1ns ± 0% 80.8ns ± 0% ~ (p=1.000 n=1+1) Cas64-4 80.9ns ± 0% 80.9ns ± 0% ~ (p=1.000 n=1+1) Fixes #57344 Change-Id: I190a7fc648023b15fa392f7fdda5ac18c1561bac Reviewed-on: https://go-review.googlesource.com/c/go/+/457135 Run-TryBot: Than McIntosh Reviewed-by: Cherry Mui Reviewed-by: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Wayne Zuo Reviewed-by: Than McIntosh Reviewed-by: David Chase Reviewed-on: https://go-review.googlesource.com/c/go/+/458357 Run-TryBot: David Chase Auto-Submit: Dmitri Shuralyov --- src/cmd/compile/internal/ssa/gen/MIPS64.rules | 3 ++- src/cmd/compile/internal/ssa/gen/RISCV64.rules | 2 +- src/cmd/compile/internal/ssa/rewriteMIPS64.go | 24 +++++++++++++++++++-- src/cmd/compile/internal/ssa/rewriteRISCV64.go | 24 +++++++++++++++++++-- src/runtime/internal/atomic/atomic_test.go | 30 ++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/internal/ssa/gen/MIPS64.rules b/src/cmd/compile/internal/ssa/gen/MIPS64.rules index 292ff2fc79..0d6d30fa4c 100644 --- a/src/cmd/compile/internal/ssa/gen/MIPS64.rules +++ b/src/cmd/compile/internal/ssa/gen/MIPS64.rules @@ -392,7 +392,8 @@ (AtomicAdd(32|64) ...) => (LoweredAtomicAdd(32|64) ...) -(AtomicCompareAndSwap(32|64) ...) => (LoweredAtomicCas(32|64) ...) +(AtomicCompareAndSwap32 ptr old new mem) => (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem) +(AtomicCompareAndSwap64 ...) => (LoweredAtomicCas64 ...) // checks (NilCheck ...) => (LoweredNilCheck ...) diff --git a/src/cmd/compile/internal/ssa/gen/RISCV64.rules b/src/cmd/compile/internal/ssa/gen/RISCV64.rules index 7aea622c5e..acef3df389 100644 --- a/src/cmd/compile/internal/ssa/gen/RISCV64.rules +++ b/src/cmd/compile/internal/ssa/gen/RISCV64.rules @@ -568,7 +568,7 @@ (AtomicAnd32 ...) => (LoweredAtomicAnd32 ...) -(AtomicCompareAndSwap32 ...) => (LoweredAtomicCas32 ...) +(AtomicCompareAndSwap32 ptr old new mem) => (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem) (AtomicCompareAndSwap64 ...) => (LoweredAtomicCas64 ...) (AtomicExchange32 ...) => (LoweredAtomicExchange32 ...) diff --git a/src/cmd/compile/internal/ssa/rewriteMIPS64.go b/src/cmd/compile/internal/ssa/rewriteMIPS64.go index 1fbd556b5c..6a0fd3ad6e 100644 --- a/src/cmd/compile/internal/ssa/rewriteMIPS64.go +++ b/src/cmd/compile/internal/ssa/rewriteMIPS64.go @@ -52,8 +52,7 @@ func rewriteValueMIPS64(v *Value) bool { v.Op = OpMIPS64LoweredAtomicAdd64 return true case OpAtomicCompareAndSwap32: - v.Op = OpMIPS64LoweredAtomicCas32 - return true + return rewriteValueMIPS64_OpAtomicCompareAndSwap32(v) case OpAtomicCompareAndSwap64: v.Op = OpMIPS64LoweredAtomicCas64 return true @@ -697,6 +696,27 @@ func rewriteValueMIPS64_OpAddr(v *Value) bool { return true } } +func rewriteValueMIPS64_OpAtomicCompareAndSwap32(v *Value) bool { + v_3 := v.Args[3] + v_2 := v.Args[2] + v_1 := v.Args[1] + v_0 := v.Args[0] + b := v.Block + typ := &b.Func.Config.Types + // match: (AtomicCompareAndSwap32 ptr old new mem) + // result: (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem) + for { + ptr := v_0 + old := v_1 + new := v_2 + mem := v_3 + v.reset(OpMIPS64LoweredAtomicCas32) + v0 := b.NewValue0(v.Pos, OpSignExt32to64, typ.Int64) + v0.AddArg(old) + v.AddArg4(ptr, v0, new, mem) + return true + } +} func rewriteValueMIPS64_OpAvg64u(v *Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] diff --git a/src/cmd/compile/internal/ssa/rewriteRISCV64.go b/src/cmd/compile/internal/ssa/rewriteRISCV64.go index 6828d97ff8..b277979061 100644 --- a/src/cmd/compile/internal/ssa/rewriteRISCV64.go +++ b/src/cmd/compile/internal/ssa/rewriteRISCV64.go @@ -61,8 +61,7 @@ func rewriteValueRISCV64(v *Value) bool { case OpAtomicAnd8: return rewriteValueRISCV64_OpAtomicAnd8(v) case OpAtomicCompareAndSwap32: - v.Op = OpRISCV64LoweredAtomicCas32 - return true + return rewriteValueRISCV64_OpAtomicCompareAndSwap32(v) case OpAtomicCompareAndSwap64: v.Op = OpRISCV64LoweredAtomicCas64 return true @@ -765,6 +764,27 @@ func rewriteValueRISCV64_OpAtomicAnd8(v *Value) bool { return true } } +func rewriteValueRISCV64_OpAtomicCompareAndSwap32(v *Value) bool { + v_3 := v.Args[3] + v_2 := v.Args[2] + v_1 := v.Args[1] + v_0 := v.Args[0] + b := v.Block + typ := &b.Func.Config.Types + // match: (AtomicCompareAndSwap32 ptr old new mem) + // result: (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem) + for { + ptr := v_0 + old := v_1 + new := v_2 + mem := v_3 + v.reset(OpRISCV64LoweredAtomicCas32) + v0 := b.NewValue0(v.Pos, OpSignExt32to64, typ.Int64) + v0.AddArg(old) + v.AddArg4(ptr, v0, new, mem) + return true + } +} func rewriteValueRISCV64_OpAtomicOr8(v *Value) bool { v_2 := v.Args[2] v_1 := v.Args[1] diff --git a/src/runtime/internal/atomic/atomic_test.go b/src/runtime/internal/atomic/atomic_test.go index 2ae60b8507..2427bfd211 100644 --- a/src/runtime/internal/atomic/atomic_test.go +++ b/src/runtime/internal/atomic/atomic_test.go @@ -345,6 +345,36 @@ func TestBitwiseContended(t *testing.T) { } } +func TestCasRel(t *testing.T) { + const _magic = 0x5a5aa5a5 + var x struct { + before uint32 + i uint32 + after uint32 + o uint32 + n uint32 + } + + x.before = _magic + x.after = _magic + for j := 0; j < 32; j += 1 { + x.i = (1 << j) + 0 + x.o = (1 << j) + 0 + x.n = (1 << j) + 1 + if !atomic.CasRel(&x.i, x.o, x.n) { + t.Fatalf("should have swapped %#x %#x", x.o, x.n) + } + + if x.i != x.n { + t.Fatalf("wrong x.i after swap: x.i=%#x x.n=%#x", x.i, x.n) + } + + if x.before != _magic || x.after != _magic { + t.Fatalf("wrong magic: %#x _ %#x != %#x _ %#x", x.before, x.after, _magic, _magic) + } + } +} + func TestStorepNoWB(t *testing.T) { var p [2]*int for i := range p { -- cgit v1.2.1 From 476384ec7b6e4770ab25a754f656da9d8115cb66 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 21 Nov 2022 16:47:39 -0800 Subject: [release-branch.go1.18] crypto/x509: return typed verification errors on macOS On macOS return the error code from SecTrustEvaluateWithError, and use it to create typed errors that can be returned from Verify. Updates #56891 Fixes #57426 Change-Id: Ib597ce202abb60702f730e75da583894422e4c14 Reviewed-on: https://go-review.googlesource.com/c/go/+/452620 Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov (cherry picked from commit c9a10d48a8f0e8479f5b9d98c5bd81b64a90d23d) Reviewed-on: https://go-review.googlesource.com/c/go/+/460896 Reviewed-by: Carlos Amedee Auto-Submit: Heschi Kreinick Reviewed-by: Heschi Kreinick Run-TryBot: Filippo Valsorda --- src/crypto/x509/internal/macos/corefoundation.go | 7 +++++++ src/crypto/x509/internal/macos/corefoundation.s | 2 ++ src/crypto/x509/internal/macos/security.go | 19 ++++++++++++++----- src/crypto/x509/root_darwin.go | 14 ++++++++++++-- src/crypto/x509/root_darwin_test.go | 10 +++++----- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/crypto/x509/internal/macos/corefoundation.go b/src/crypto/x509/internal/macos/corefoundation.go index 75c212910b..2b20b52dc6 100644 --- a/src/crypto/x509/internal/macos/corefoundation.go +++ b/src/crypto/x509/internal/macos/corefoundation.go @@ -184,6 +184,13 @@ func CFErrorCopyDescription(errRef CFRef) CFRef { } func x509_CFErrorCopyDescription_trampoline() +//go:cgo_import_dynamic x509_CFErrorGetCode CFErrorGetCode "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" + +func CFErrorGetCode(errRef CFRef) int { + return int(syscall(abi.FuncPCABI0(x509_CFErrorGetCode_trampoline), uintptr(errRef), 0, 0, 0, 0, 0)) +} +func x509_CFErrorGetCode_trampoline() + //go:cgo_import_dynamic x509_CFStringCreateExternalRepresentation CFStringCreateExternalRepresentation "/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation" func CFStringCreateExternalRepresentation(strRef CFRef) CFRef { diff --git a/src/crypto/x509/internal/macos/corefoundation.s b/src/crypto/x509/internal/macos/corefoundation.s index d69f72f795..49cd084467 100644 --- a/src/crypto/x509/internal/macos/corefoundation.s +++ b/src/crypto/x509/internal/macos/corefoundation.s @@ -37,5 +37,7 @@ TEXT ·x509_CFDataCreate_trampoline(SB),NOSPLIT,$0-0 JMP x509_CFDataCreate(SB) TEXT ·x509_CFErrorCopyDescription_trampoline(SB),NOSPLIT,$0-0 JMP x509_CFErrorCopyDescription(SB) +TEXT ·x509_CFErrorGetCode_trampoline(SB),NOSPLIT,$0-0 + JMP x509_CFErrorGetCode(SB) TEXT ·x509_CFStringCreateExternalRepresentation_trampoline(SB),NOSPLIT,$0-0 JMP x509_CFStringCreateExternalRepresentation(SB) diff --git a/src/crypto/x509/internal/macos/security.go b/src/crypto/x509/internal/macos/security.go index ef64bda49f..0b7958eaa2 100644 --- a/src/crypto/x509/internal/macos/security.go +++ b/src/crypto/x509/internal/macos/security.go @@ -8,7 +8,6 @@ package macOS import ( "errors" - "fmt" "internal/abi" "strconv" "unsafe" @@ -51,6 +50,15 @@ const ( SecTrustSettingsDomainSystem ) +const ( + // various macOS error codes that can be returned from + // SecTrustEvaluateWithError that we can map to Go cert + // verification error types. + ErrSecCertificateExpired = -67818 + ErrSecHostNameMismatch = -67602 + ErrSecNotTrusted = -67843 +) + type OSStatus struct { call string status int32 @@ -190,17 +198,18 @@ func x509_SecTrustGetResult_trampoline() //go:cgo_import_dynamic x509_SecTrustEvaluateWithError SecTrustEvaluateWithError "/System/Library/Frameworks/Security.framework/Versions/A/Security" -func SecTrustEvaluateWithError(trustObj CFRef) error { +func SecTrustEvaluateWithError(trustObj CFRef) (int, error) { var errRef CFRef ret := syscall(abi.FuncPCABI0(x509_SecTrustEvaluateWithError_trampoline), uintptr(trustObj), uintptr(unsafe.Pointer(&errRef)), 0, 0, 0, 0) if int32(ret) != 1 { errStr := CFErrorCopyDescription(errRef) - err := fmt.Errorf("x509: %s", CFStringToString(errStr)) + err := errors.New(CFStringToString(errStr)) + errCode := CFErrorGetCode(errRef) CFRelease(errRef) CFRelease(errStr) - return err + return errCode, err } - return nil + return 0, nil } func x509_SecTrustEvaluateWithError_trampoline() diff --git a/src/crypto/x509/root_darwin.go b/src/crypto/x509/root_darwin.go index ad365f577e..c35885ace8 100644 --- a/src/crypto/x509/root_darwin.go +++ b/src/crypto/x509/root_darwin.go @@ -7,6 +7,7 @@ package x509 import ( macOS "crypto/x509/internal/macos" "errors" + "fmt" ) func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate, err error) { @@ -54,8 +55,17 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate // always enforce its SCT requirements, and there are still _some_ people // using TLS or OCSP for that. - if err := macOS.SecTrustEvaluateWithError(trustObj); err != nil { - return nil, err + if ret, err := macOS.SecTrustEvaluateWithError(trustObj); err != nil { + switch ret { + case macOS.ErrSecCertificateExpired: + return nil, CertificateInvalidError{c, Expired, err.Error()} + case macOS.ErrSecHostNameMismatch: + return nil, HostnameError{c, opts.DNSName} + case macOS.ErrSecNotTrusted: + return nil, UnknownAuthorityError{Cert: c} + default: + return nil, fmt.Errorf("x509: %s", err) + } } chain := [][]*Certificate{{}} diff --git a/src/crypto/x509/root_darwin_test.go b/src/crypto/x509/root_darwin_test.go index 90a464f624..299cecf556 100644 --- a/src/crypto/x509/root_darwin_test.go +++ b/src/crypto/x509/root_darwin_test.go @@ -42,23 +42,23 @@ func TestPlatformVerifier(t *testing.T) { { name: "expired leaf", host: "expired.badssl.com", - expectedErr: "x509: “*.badssl.com” certificate is expired", + expectedErr: "x509: certificate has expired or is not yet valid: “*.badssl.com” certificate is expired", }, { name: "wrong host for leaf", host: "wrong.host.badssl.com", verifyName: "wrong.host.badssl.com", - expectedErr: "x509: “*.badssl.com” certificate name does not match input", + expectedErr: "x509: certificate is valid for *.badssl.com, badssl.com, not wrong.host.badssl.com", }, { name: "self-signed leaf", host: "self-signed.badssl.com", - expectedErr: "x509: “*.badssl.com” certificate is not trusted", + expectedErr: "x509: certificate signed by unknown authority", }, { name: "untrusted root", host: "untrusted-root.badssl.com", - expectedErr: "x509: “BadSSL Untrusted Root Certificate Authority” certificate is not trusted", + expectedErr: "x509: certificate signed by unknown authority", }, { name: "revoked leaf", @@ -74,7 +74,7 @@ func TestPlatformVerifier(t *testing.T) { name: "expired leaf (custom time)", host: "google.com", verifyTime: time.Time{}.Add(time.Hour), - expectedErr: "x509: “*.google.com” certificate is expired", + expectedErr: "x509: certificate has expired or is not yet valid: “*.google.com” certificate is expired", }, { name: "valid chain (custom time)", -- cgit v1.2.1 From 87105e5b2e3462cb5d62d9a8e08b9e45f65ed671 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 8 Jan 2023 17:33:12 -0500 Subject: [release-branch.go1.18] runtime: revert "call __fork instead of fork on darwin" A recent comment on #57263 reports an unexplained crash in a cgo program that is fixed by reverting the __fork fix. We don't have any viable fix for the os/exec bug at this point, so give up on a fix for the January point releases. This reverts CL 459179 (commit 07b6ffb79c90). Fixes #57689. Change-Id: I3b81de6bded399f47862325129e86a65c83d8e3b Reviewed-on: https://go-review.googlesource.com/c/go/+/461116 Reviewed-by: Ian Lance Taylor Run-TryBot: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui --- src/syscall/exec_libc2.go | 4 ++-- src/syscall/syscall_darwin.go | 31 +------------------------------ src/syscall/syscall_openbsd_libc.go | 5 ----- src/syscall/zsyscall_darwin_amd64.go | 22 ++++------------------ src/syscall/zsyscall_darwin_amd64.s | 6 ++---- src/syscall/zsyscall_darwin_arm64.go | 22 ++++------------------ src/syscall/zsyscall_darwin_arm64.s | 6 ++---- 7 files changed, 15 insertions(+), 81 deletions(-) diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go index bd2ce6820d..b05f053bbf 100644 --- a/src/syscall/exec_libc2.go +++ b/src/syscall/exec_libc2.go @@ -76,7 +76,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr // About to call fork. // No more allocation or calls of non-assembly functions. runtime_BeforeFork() - r1, _, err1 = rawSyscall(forkTrampoline, 0, 0, 0) + r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) if err1 != 0 { runtime_AfterFork() return 0, err1 @@ -260,6 +260,6 @@ childerror: // send error code on pipe rawSyscall(abi.FuncPCABI0(libc_write_trampoline), uintptr(pipe), uintptr(unsafe.Pointer(&err1)), unsafe.Sizeof(err1)) for { - rawSyscall(exitTrampoline, 253, 0, 0) + rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0) } } diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go index 346e6862cd..902d6e77e1 100644 --- a/src/syscall/syscall_darwin.go +++ b/src/syscall/syscall_darwin.go @@ -17,34 +17,6 @@ import ( "unsafe" ) -// These are called from exec_libc2.go in the child of fork. -// The names differ between macOS and OpenBSD, so we need -// to declare the specific ones used here to keep the exec_libc2.go -// code portable. -// -// We use __fork and __exit, not fork and exit, to avoid the libc atfork -// and atexit handlers. The atfork handlers have caused fork child -// hangs in the past (see #33565, #56784). The atexit handlers have -// not, but the non-libc ports all invoke the system call, so doing -// the same here makes sense. In general we wouldn't expect -// atexit handlers to work terribly well in a fork child anyway. -// (Also, perhaps the atfork handlers clear the atexit handlers, -// in which case we definitely need to avoid calling the libc exit -// if we bypass the libc fork.) -// -// Other calls that are made in the child after the fork are -// ptrace, setsid, setpgid, getpid, ioctl, chroot, setgroups, -// setgid, setuid, chdir, dup2, fcntl, close, execve, and write. -// Those are all simple kernel wrappers that should be safe -// to be called directly. The fcntl and ioctl functions do run -// some code around the kernel call, but they don't call any -// other functions, so for now we keep using them instead of -// calling the lower-level __fcntl and __ioctl functions. -var ( - exitTrampoline = abi.FuncPCABI0(libc___exit_trampoline) - forkTrampoline = abi.FuncPCABI0(libc___fork_trampoline) -) - type SockaddrDatalink struct { Len uint8 Family uint8 @@ -230,12 +202,11 @@ func Kill(pid int, signum Signal) (err error) { return kill(pid, int(signum), 1) //sys writev(fd int, iovecs []Iovec) (cnt uintptr, err error) //sys mmap(addr uintptr, length uintptr, prot int, flag int, fd int, pos int64) (ret uintptr, err error) //sys munmap(addr uintptr, length uintptr) (err error) -//sysnb __fork() (pid int, err error) +//sysnb fork() (pid int, err error) //sysnb ioctl(fd int, req int, arg int) (err error) //sysnb ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = SYS_ioctl //sysnb execve(path *byte, argv **byte, envp **byte) (err error) //sysnb exit(res int) (err error) -//sysnb __exit(res int) (err error) //sys sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) //sys fcntlPtr(fd int, cmd int, arg unsafe.Pointer) (val int, err error) = SYS_fcntl //sys unlinkat(fd int, path string, flags int) (err error) diff --git a/src/syscall/syscall_openbsd_libc.go b/src/syscall/syscall_openbsd_libc.go index ff9fa00259..15b68fd0fc 100644 --- a/src/syscall/syscall_openbsd_libc.go +++ b/src/syscall/syscall_openbsd_libc.go @@ -10,11 +10,6 @@ import ( "internal/abi" ) -var ( - exitTrampoline = abi.FuncPCABI0(libc_exit_trampoline) - forkTrampoline = abi.FuncPCABI0(libc_fork_trampoline) -) - func init() { execveOpenBSD = execve } diff --git a/src/syscall/zsyscall_darwin_amd64.go b/src/syscall/zsyscall_darwin_amd64.go index 2ad03801b2..0ccdaf2d0e 100644 --- a/src/syscall/zsyscall_darwin_amd64.go +++ b/src/syscall/zsyscall_darwin_amd64.go @@ -1714,8 +1714,8 @@ func libc_munmap_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func __fork() (pid int, err error) { - r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0) +func fork() (pid int, err error) { + r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) pid = int(r0) if e1 != 0 { err = errnoErr(e1) @@ -1723,9 +1723,9 @@ func __fork() (pid int, err error) { return } -func libc___fork_trampoline() +func libc_fork_trampoline() -//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib" // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT @@ -1781,20 +1781,6 @@ func libc_exit_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func __exit(res int) (err error) { - _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0) - if e1 != 0 { - err = errnoErr(e1) - } - return -} - -func libc___exit_trampoline() - -//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib" - -// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT - func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) { var _p0 unsafe.Pointer if len(mib) > 0 { diff --git a/src/syscall/zsyscall_darwin_amd64.s b/src/syscall/zsyscall_darwin_amd64.s index 1815703803..563083d441 100644 --- a/src/syscall/zsyscall_darwin_amd64.s +++ b/src/syscall/zsyscall_darwin_amd64.s @@ -219,16 +219,14 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_mmap(SB) TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_munmap(SB) -TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0 - JMP libc___fork(SB) +TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0 + JMP libc_fork(SB) TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_ioctl(SB) TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0 JMP libc_execve(SB) TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0 JMP libc_exit(SB) -TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0 - JMP libc___exit(SB) TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_sysctl(SB) TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0 diff --git a/src/syscall/zsyscall_darwin_arm64.go b/src/syscall/zsyscall_darwin_arm64.go index 559bb49c4b..09bf34bb3c 100644 --- a/src/syscall/zsyscall_darwin_arm64.go +++ b/src/syscall/zsyscall_darwin_arm64.go @@ -1714,8 +1714,8 @@ func libc_munmap_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func __fork() (pid int, err error) { - r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0) +func fork() (pid int, err error) { + r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) pid = int(r0) if e1 != 0 { err = errnoErr(e1) @@ -1723,9 +1723,9 @@ func __fork() (pid int, err error) { return } -func libc___fork_trampoline() +func libc_fork_trampoline() -//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib" +//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib" // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT @@ -1781,20 +1781,6 @@ func libc_exit_trampoline() // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT -func __exit(res int) (err error) { - _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0) - if e1 != 0 { - err = errnoErr(e1) - } - return -} - -func libc___exit_trampoline() - -//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib" - -// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT - func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) { var _p0 unsafe.Pointer if len(mib) > 0 { diff --git a/src/syscall/zsyscall_darwin_arm64.s b/src/syscall/zsyscall_darwin_arm64.s index 1666cf985c..0567a42fa3 100644 --- a/src/syscall/zsyscall_darwin_arm64.s +++ b/src/syscall/zsyscall_darwin_arm64.s @@ -219,16 +219,14 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_mmap(SB) TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0 JMP libc_munmap(SB) -TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0 - JMP libc___fork(SB) +TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0 + JMP libc_fork(SB) TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_ioctl(SB) TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0 JMP libc_execve(SB) TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0 JMP libc_exit(SB) -TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0 - JMP libc___exit(SB) TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0 JMP libc_sysctl(SB) TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0 -- cgit v1.2.1 From aa0c01fb35bc7f95d46c77020bc56a58eaa80edd Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 17 Nov 2022 15:29:27 -0500 Subject: [release-branch.go1.18] misc/cgo/testcshared: handle unsuffixed dlltool path Adapt the testcshared tests to handle the case where the path output by invoking gcc -print-prog-name=dlltool is a path lacking the final ".exe" suffix (this seems to be what clang is doing); tack it on before using if this is the case. Updates #57704. Fixes #57705. Change-Id: I04fb7b9fc90677880b1ced4a4ad2a8867a3f5f86 Reviewed-on: https://go-review.googlesource.com/c/go/+/451816 TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Run-TryBot: Than McIntosh Reviewed-by: Cherry Mui (cherry picked from commit 771a98d6b19c9ca4bbd3fbeba03d7d512f77c166) Reviewed-on: https://go-review.googlesource.com/c/go/+/461176 Reviewed-by: Heschi Kreinick Reviewed-by: Carlos Amedee --- misc/cgo/testcshared/cshared_test.go | 60 +++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go index c9e9e5fe63..1cd8c2ab4a 100644 --- a/misc/cgo/testcshared/cshared_test.go +++ b/misc/cgo/testcshared/cshared_test.go @@ -317,30 +317,46 @@ func createHeaders() error { if err != nil { return fmt.Errorf("unable to find dlltool path: %v\n%s\n", err, out) } - args := []string{strings.TrimSpace(string(out)), "-D", args[6], "-l", libgoname, "-d", "libgo.def"} - - // This is an unfortunate workaround for https://github.com/mstorsjo/llvm-mingw/issues/205 in which - // we basically reimplement the contents of the dlltool.sh wrapper: https://git.io/JZFlU - dlltoolContents, err := os.ReadFile(args[0]) - if err != nil { - return fmt.Errorf("unable to read dlltool: %v\n", err) + dlltoolpath := strings.TrimSpace(string(out)) + if filepath.Ext(dlltoolpath) == "" { + // Some compilers report slash-separated paths without extensions + // instead of ordinary Windows paths. + // Try to find the canonical name for the path. + if lp, err := exec.LookPath(dlltoolpath); err == nil { + dlltoolpath = lp + } } - if bytes.HasPrefix(dlltoolContents, []byte("#!/bin/sh")) && bytes.Contains(dlltoolContents, []byte("llvm-dlltool")) { - base, name := filepath.Split(args[0]) - args[0] = filepath.Join(base, "llvm-dlltool") - var machine string - switch prefix, _, _ := strings.Cut(name, "-"); prefix { - case "i686": - machine = "i386" - case "x86_64": - machine = "i386:x86-64" - case "armv7": - machine = "arm" - case "aarch64": - machine = "arm64" + + args := []string{dlltoolpath, "-D", args[6], "-l", libgoname, "-d", "libgo.def"} + + if filepath.Ext(dlltoolpath) == "" { + // This is an unfortunate workaround for + // https://github.com/mstorsjo/llvm-mingw/issues/205 in which + // we basically reimplement the contents of the dlltool.sh + // wrapper: https://git.io/JZFlU. + // TODO(thanm): remove this workaround once we can upgrade + // the compilers on the windows-arm64 builder. + dlltoolContents, err := os.ReadFile(args[0]) + if err != nil { + return fmt.Errorf("unable to read dlltool: %v\n", err) } - if len(machine) > 0 { - args = append(args, "-m", machine) + if bytes.HasPrefix(dlltoolContents, []byte("#!/bin/sh")) && bytes.Contains(dlltoolContents, []byte("llvm-dlltool")) { + base, name := filepath.Split(args[0]) + args[0] = filepath.Join(base, "llvm-dlltool") + var machine string + switch prefix, _, _ := strings.Cut(name, "-"); prefix { + case "i686": + machine = "i386" + case "x86_64": + machine = "i386:x86-64" + case "armv7": + machine = "arm" + case "aarch64": + machine = "arm64" + } + if len(machine) > 0 { + args = append(args, "-m", machine) + } } } -- cgit v1.2.1 From 581603cb7d02019bbf4ff508014038f3120a3dcb Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Tue, 10 Jan 2023 17:57:35 +0000 Subject: [release-branch.go1.18] go1.18.10 Change-Id: I8cc4645d07defe595f3bf00eedb989a5edc5b3b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/461357 TryBot-Result: Gopher Robot Reviewed-by: Carlos Amedee Run-TryBot: Gopher Robot Reviewed-by: Heschi Kreinick Auto-Submit: Gopher Robot --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index a7cd7b8f21..a246dd8151 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.18.9 \ No newline at end of file +go1.18.10 \ No newline at end of file -- cgit v1.2.1