]> Cypherpunks repositories - gostls13.git/commitdiff
net: remove non-cgo macOS resolver code
authorRuss Cox <rsc@golang.org>
Wed, 5 Jun 2019 23:59:58 +0000 (19:59 -0400)
committerRuss Cox <rsc@golang.org>
Thu, 6 Jun 2019 19:56:56 +0000 (19:56 +0000)
The built-in Go resolver works significantly better.
In particular, the use of res_search does not support
CNAME or PTR queries and may not even be thread-safe.

This CL is essentially a revert of CL 166297 plus fixes,
including CL 180842.

See CL 180842 for additional notes about problems
with this approach.

Fixes #31705.

Change-Id: I0a30a0de2fbd04f6c461520fd34378c84aadf66c
Reviewed-on: https://go-review.googlesource.com/c/go/+/180843
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
src/net/cgo_darwin_stub.go [deleted file]
src/net/cgo_darwin_stub_test.go [deleted file]
src/net/cgo_stub.go
src/net/conf.go
src/runtime/lookup_darwin.go [deleted file]
src/runtime/lookup_darwin_386.s [deleted file]
src/runtime/lookup_darwin_amd64.s [deleted file]
src/runtime/lookup_darwin_arm.s [deleted file]
src/runtime/lookup_darwin_arm64.s [deleted file]

diff --git a/src/net/cgo_darwin_stub.go b/src/net/cgo_darwin_stub.go
deleted file mode 100644 (file)
index fc50809..0000000
+++ /dev/null
@@ -1,242 +0,0 @@
-// Copyright 2019 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 is intended to be used in non-cgo builds of darwin binaries,
-// in particular when cross-compiling a darwin binary from a non-darwin machine.
-// All OS calls on darwin have to be done via C libraries, and this code makes such
-// calls with the runtime's help. (It is a C call but does not require the cgo tool to
-// be compiled, and as such it is possible to build even when cross-compiling.)
-//
-// The specific C library calls are to res_init and res_search from /usr/lib/system/libsystem_info.dylib.
-// Unfortunately, an ordinary C program calling these names would actually end up with
-// res_9_init and res_9_search from /usr/lib/libresolv.dylib, not libsystem_info.
-// It may well be that the libsystem_info routines are completely unused on macOS systems
-// except for this code. At the least, they have the following problems:
-//
-//     - TypeALL requests do not work, so if we want both IPv4 and IPv6 addresses,
-//       we have to do two requests, one for TypeA and one for TypeAAAA.
-//     - TypeCNAME requests hang indefinitely.
-//     - TypePTR requests fail unconditionally.
-//     - Detailed error information is stored in the global h_errno value,
-//       which cannot be accessed safely (it is not per-thread like errno).
-//     - The routines may not be safe to call from multiple threads.
-//       If you run net.test under lldb, that emits syslog prints to stderr
-//       that suggest double-free problems. (If not running under lldb,
-//       it is unclear where the syslog prints go, if anywhere.)
-//
-// This code is marked for deletion. If it is to be revived, it should be changed to use
-// res_9_init and res_9_search from libresolv and special care should be paid to
-// error detail and thread safety.
-
-// +build !netgo,!cgo
-// +build darwin
-
-package net
-
-import (
-       "context"
-       "errors"
-       "sync"
-
-       "golang.org/x/net/dns/dnsmessage"
-)
-
-type addrinfoErrno int
-
-func (eai addrinfoErrno) Error() string   { return "<nil>" }
-func (eai addrinfoErrno) Temporary() bool { return false }
-func (eai addrinfoErrno) Timeout() bool   { return false }
-
-func cgoLookupHost(ctx context.Context, name string) (addrs []string, err error, completed bool) {
-       // The 4-suffix indicates IPv4, TypeA lookups.
-       // The 6-suffix indicates IPv6, TypeAAAA lookups.
-       // If resSearch is updated to call the libresolv res_9_search (see comment at top of file),
-       // it may be possible to make one call for TypeALL
-       // and get both address kinds out.
-       r4, err4 := resSearch(ctx, name, int32(dnsmessage.TypeA), int32(dnsmessage.ClassINET))
-       if err4 == nil {
-               addrs, err4 = appendHostsFromResources(addrs, r4)
-       }
-       r6, err6 := resSearch(ctx, name, int32(dnsmessage.TypeAAAA), int32(dnsmessage.ClassINET))
-       if err6 == nil {
-               addrs, err6 = appendHostsFromResources(addrs, r6)
-       }
-       if err4 != nil && err6 != nil {
-               return nil, err4, false
-       }
-       return addrs, nil, true
-}
-
-func cgoLookupPort(ctx context.Context, network, service string) (port int, err error, completed bool) {
-       port, err = goLookupPort(network, service) // we can just use netgo lookup
-       return port, err, err == nil
-}
-
-func cgoLookupIP(ctx context.Context, network, name string) (addrs []IPAddr, err error, completed bool) {
-       // The 4-suffix indicates IPv4, TypeA lookups.
-       // The 6-suffix indicates IPv6, TypeAAAA lookups.
-       // If resSearch is updated to call the libresolv res_9_search (see comment at top of file),
-       // it may be possible to make one call for TypeALL (when vers != '6' and vers != '4')
-       // and get both address kinds out.
-       var r4, r6 []dnsmessage.Resource
-       var err4, err6 error
-       vers := ipVersion(network)
-       if vers != '6' {
-               r4, err4 = resSearch(ctx, name, int32(dnsmessage.TypeA), int32(dnsmessage.ClassINET))
-               if err4 == nil {
-                       addrs, err4 = appendIPsFromResources(addrs, r4)
-               }
-       }
-       if vers != '4' {
-               r6, err6 = resSearch(ctx, name, int32(dnsmessage.TypeAAAA), int32(dnsmessage.ClassINET))
-               if err6 == nil {
-                       addrs, err6 = appendIPsFromResources(addrs, r6)
-               }
-       }
-       if err4 != nil && err6 != nil {
-               return nil, err4, false
-       }
-
-       return addrs, nil, true
-}
-
-func cgoLookupCNAME(ctx context.Context, name string) (cname string, err error, completed bool) {
-       resources, err := resSearch(ctx, name, int32(dnsmessage.TypeCNAME), int32(dnsmessage.ClassINET))
-       if err != nil {
-               return
-       }
-       cname, err = parseCNAMEFromResources(resources)
-       if err != nil {
-               return "", err, false
-       }
-       return cname, nil, true
-}
-
-func cgoLookupPTR(ctx context.Context, addr string) (ptrs []string, err error, completed bool) {
-       resources, err := resSearch(ctx, addr, int32(dnsmessage.TypePTR), int32(dnsmessage.ClassINET))
-       if err != nil {
-               return
-       }
-       ptrs, err = parsePTRsFromResources(resources)
-       if err != nil {
-               return
-       }
-       return ptrs, nil, true
-}
-
-var (
-       resInitOnce   sync.Once
-       resInitResult int32
-)
-
-// resSearch will make a call to the 'res_search' routine in libSystem
-// and parse the output as a slice of resource resources which can then be parsed
-func resSearch(ctx context.Context, hostname string, rtype, class int32) ([]dnsmessage.Resource, error) {
-       // We have to use res_init and res_search, but these do not set errno on failure.
-       // (They set h_errno, which is a global int shared by all threads and therefore
-       // racy to use.)
-       // https://opensource.apple.com/source/Libinfo/Libinfo-517.200.9/dns.subproj/res_query.c.auto.html
-       resInitOnce.Do(func() {
-               resInitResult = res_init()
-       })
-       if resInitResult < 0 {
-               return nil, errors.New("res_init failure")
-       }
-
-       // res_search does not set errno.
-       // It returns the size of the DNS response packet.
-       // But if the DNS response packet contains failure-like response codes,
-       // res_search returns -1 even though it has copied the packet into buf,
-       // giving us no way to find out how big the packet is.
-       // For now, we are willing to take res_search's word that there's nothing
-       // useful in the response, even though there *is* a response.
-       name := make([]byte, len(hostname)+1) // +1 for NUL at end for C
-       copy(name, hostname)
-       var buf [1024]byte
-       size, _ := res_search(&name[0], class, rtype, &buf[0], int32(len(buf)))
-       if size <= 0 {
-               return nil, errors.New("res_search failure")
-       }
-
-       var p dnsmessage.Parser
-       if _, err := p.Start(buf[:size]); err != nil {
-               return nil, err
-       }
-       p.SkipAllQuestions()
-       resources, err := p.AllAnswers()
-       if err != nil {
-               return nil, err
-       }
-       return resources, nil
-}
-
-func copyBytes(x []byte) []byte {
-       y := make([]byte, len(x))
-       copy(y, x)
-       return y
-}
-
-func appendHostsFromResources(answers []string, resources []dnsmessage.Resource) ([]string, error) {
-       for i := range resources {
-               switch resources[i].Header.Type {
-               case dnsmessage.TypeA:
-                       b := resources[i].Body.(*dnsmessage.AResource)
-                       answers = append(answers, IP(b.A[:]).String())
-               case dnsmessage.TypeAAAA:
-                       b := resources[i].Body.(*dnsmessage.AAAAResource)
-                       answers = append(answers, IP(b.AAAA[:]).String())
-               default:
-                       return nil, errors.New("could not parse an A or AAAA response from message buffer")
-               }
-       }
-       return answers, nil
-}
-
-func appendIPsFromResources(answers []IPAddr, resources []dnsmessage.Resource) ([]IPAddr, error) {
-       for i := range resources {
-               switch resources[i].Header.Type {
-               case dnsmessage.TypeA:
-                       b := resources[i].Body.(*dnsmessage.AResource)
-                       answers = append(answers, IPAddr{IP: IP(copyBytes(b.A[:]))})
-               case dnsmessage.TypeAAAA:
-                       b := resources[i].Body.(*dnsmessage.AAAAResource)
-                       answers = append(answers, IPAddr{IP: IP(copyBytes(b.AAAA[:]))})
-               default:
-                       return nil, errors.New("could not parse an A or AAAA response from message buffer")
-               }
-       }
-       return answers, nil
-}
-
-func parseCNAMEFromResources(resources []dnsmessage.Resource) (string, error) {
-       if len(resources) == 0 {
-               return "", errors.New("no CNAME record received")
-       }
-       c, ok := resources[0].Body.(*dnsmessage.CNAMEResource)
-       if !ok {
-               return "", errors.New("could not parse CNAME record")
-       }
-       return c.CNAME.String(), nil
-}
-
-func parsePTRsFromResources(resources []dnsmessage.Resource) ([]string, error) {
-       var answers []string
-       for i := range resources {
-               switch resources[i].Header.Type {
-               case dnsmessage.TypePTR:
-                       p := resources[0].Body.(*dnsmessage.PTRResource)
-                       answers = append(answers, p.PTR.String())
-               default:
-                       return nil, errors.New("could not parse a PTR response from message buffer")
-
-               }
-       }
-       return answers, nil
-}
-
-// res_init and res_search are defined in runtime/lookup_darwin.go
-
-func res_init() int32
-
-func res_search(dname *byte, class int32, rtype int32, answer *byte, anslen int32) (int32, int32)
diff --git a/src/net/cgo_darwin_stub_test.go b/src/net/cgo_darwin_stub_test.go
deleted file mode 100644 (file)
index f694e2a..0000000
+++ /dev/null
@@ -1,80 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build !netgo,!cgo
-// +build darwin
-
-package net
-
-import (
-       "context"
-       "strings"
-       "testing"
-)
-
-func TestPseudoCgoLookupHost(t *testing.T) {
-       addrs, err, ok := cgoLookupHost(context.Background(), "google.com")
-       t.Logf("cgoLookupHost google.com: %v %v %v", addrs, err, ok)
-       if !ok {
-               t.Fatal("cgoLookupHost ok=false")
-       }
-       if err != nil {
-               t.Fatalf("cgoLookupHost: %v", err)
-       }
-       // cgoLookupHost need not return IPv4 before IPv6 in general,
-       // but for the current implementation it does.
-       // If that changes, this test will need updating.
-       if len(addrs) < 1 || strings.Count(addrs[0], ".") != 3 || !strings.Contains(addrs[len(addrs)-1], "::") {
-               t.Fatalf("cgoLookupHost google.com = %v, want IPv4 and IPv6", addrs)
-       }
-}
-
-func TestPseudoCgoLookupIP(t *testing.T) {
-       ips, err, ok := cgoLookupIP(context.Background(), "ip", "google.com")
-       t.Logf("cgoLookupIP google.com: %v %v %v", ips, err, ok)
-       if !ok {
-               t.Fatal("cgoLookupIP ok=false")
-       }
-       if err != nil {
-               t.Fatalf("cgoLookupIP: %v", err)
-       }
-       // cgoLookupIP need not return IPv4 before IPv6 in general,
-       // but for the current implementation it does.
-       // If that changes, this test will need updating.
-       if len(ips) < 1 || len(ips[0].IP) != 4 || len(ips[len(ips)-1].IP) != 16 {
-               t.Fatalf("cgoLookupIP google.com = %v, want IPv4 and IPv6", ips)
-       }
-}
-
-func TestPseudoCgoLookupCNAME(t *testing.T) {
-       t.Skip("res_search on macOS hangs in TypeCNAME queries (even in plain C programs)")
-
-       cname, err, ok := cgoLookupCNAME(context.Background(), "redirect.swtch.com")
-       t.Logf("cgoLookupCNAME redirect.swtch.com: %v %v %v", cname, err, ok)
-       if !ok {
-               t.Fatal("cgoLookupCNAME ok=false")
-       }
-       if err != nil {
-               t.Fatalf("cgoLookupCNAME: %v", err)
-       }
-       if !strings.HasSuffix(cname, ".com") {
-               t.Fatalf("cgoLookupCNAME redirect.swtch.com = %v, want *.com", cname)
-       }
-}
-
-func TestPseudoCgoLookupPTR(t *testing.T) {
-       t.Skip("res_search on macOS does not support TypePTR")
-
-       ptrs, err, ok := cgoLookupPTR(context.Background(), "8.8.8.8")
-       t.Logf("cgoLookupPTR 8.8.8.8: %v %v %v", ptrs, err, ok)
-       if !ok {
-               t.Fatal("cgoLookupPTR ok=false")
-       }
-       if err != nil {
-               t.Fatalf("cgoLookupPTR: %v", err)
-       }
-       if len(ptrs) < 1 || ptrs[0] != "google-public-dns-a.google.com" {
-               t.Fatalf("cgoLookupPTR = %v, want google-public-dns-a.google.com", ptrs)
-       }
-}
index aa15ab4dc1043d4a48fbd3eba84f594924ceb4f1..041f8af12966a4363c3c4822061f7b956e478b40 100644 (file)
@@ -3,7 +3,6 @@
 // license that can be found in the LICENSE file.
 
 // +build !cgo netgo
-// +build !darwin
 
 package net
 
index 1c88f096bad33185a154689481b056c3358d0035..971b1a399a1bb842c89deb90c13932b8be790caa 100644 (file)
@@ -70,11 +70,6 @@ func initConfVal() {
        // their own DNS requests. So always use cgo instead, which
        // avoids that.
        if runtime.GOOS == "darwin" {
-               // Normally we force netGo to be true if building without cgo enabled.
-               // On Darwin, we can call libc even if cgo is not enabled, so only set netGo to true
-               // if explicitly requested.
-               confVal.netGo = dnsMode == "go"
-
                confVal.forceCgoLookupHost = true
                return
        }
diff --git a/src/runtime/lookup_darwin.go b/src/runtime/lookup_darwin.go
deleted file mode 100644 (file)
index c39b937..0000000
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2019 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package runtime
-
-import (
-       "unsafe"
-)
-
-//go:linkname res_init net.res_init
-//go:nosplit
-//go:cgo_unsafe_args
-func res_init() int32 {
-       return libcCall(unsafe.Pointer(funcPC(res_init_trampoline)), nil)
-}
-func res_init_trampoline()
-
-//go:linkname res_search net.res_search
-//go:nosplit
-//go:cgo_unsafe_args
-func res_search(dname *byte, class int32, rtype int32, answer *byte, anslen int32) (int32, int32) {
-       args := struct {
-               dname                   *byte
-               class, rtype            int32
-               answer                  *byte
-               anslen, retSize, retErr int32
-       }{dname, class, rtype, answer, anslen, 0, 0}
-       libcCall(unsafe.Pointer(funcPC(res_search_trampoline)), unsafe.Pointer(&args))
-       return args.retSize, args.retErr
-}
-func res_search_trampoline()
-
-//go:cgo_import_dynamic libc_res_search res_search "/usr/lib/libSystem.B.dylib"
-//go:cgo_import_dynamic libc_res_init res_init "/usr/lib/libSystem.B.dylib"
diff --git a/src/runtime/lookup_darwin_386.s b/src/runtime/lookup_darwin_386.s
deleted file mode 100644 (file)
index e185532..0000000
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright 2019 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.
-
-#include "go_asm.h"
-#include "go_tls.h"
-#include "textflag.h"
-
-TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0
-       PUSHL   BP
-       MOVL    SP, BP
-       SUBL    $8, SP
-       CALL    libc_res_init(SB)
-       CMPL    AX, $-1
-       JNE ok
-       CALL    libc_error(SB)
-ok:
-       MOVL    BP, SP
-       POPL    BP
-       RET
-
-TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0
-       PUSHL   BP
-       MOVL    SP, BP
-       SUBL    $24, SP
-       MOVL    32(SP), CX
-       MOVL    16(CX), AX      // arg 5 anslen
-       MOVL    AX, 16(SP)
-       MOVL    12(CX), AX      // arg 4 answer
-       MOVL    AX, 12(SP)
-       MOVL    8(CX), AX       // arg 3 type
-       MOVL    AX, 8(SP)
-       MOVL    4(CX), AX       // arg 2 class
-       MOVL    AX, 4(SP)
-       MOVL    0(CX), AX       // arg 1 name
-       MOVL    AX, 0(SP)
-       CALL    libc_res_search(SB)
-       XORL    DX, DX
-       CMPL    AX, $-1
-       JNE ok
-       CALL    libc_error(SB)
-       MOVL    (AX), DX
-       XORL    AX, AX
-ok:
-       MOVL    32(SP), CX
-       MOVL    AX, 20(CX)
-       MOVL    DX, 24(CX)
-       MOVL    BP, SP
-       POPL    BP
-       RET
diff --git a/src/runtime/lookup_darwin_amd64.s b/src/runtime/lookup_darwin_amd64.s
deleted file mode 100644 (file)
index 587e436..0000000
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2019 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.
-
-#include "go_asm.h"
-#include "go_tls.h"
-#include "textflag.h"
-
-TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0
-       PUSHQ   BP
-       MOVQ    SP, BP
-       CALL    libc_res_init(SB)
-       CMPL    AX, $-1
-       JNE ok
-       CALL    libc_error(SB)
-ok:
-       POPQ    BP
-       RET
-
-TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0
-       PUSHQ   BP
-       MOVQ    SP, BP
-       MOVQ    DI, BX  // move DI into BX to preserve struct addr
-       MOVL    24(BX), R8      // arg 5 anslen
-       MOVQ    16(BX), CX      // arg 4 answer
-       MOVL    12(BX), DX      // arg 3 type
-       MOVL    8(BX), SI       // arg 2 class
-       MOVQ    0(BX), DI       // arg 1 name
-       CALL    libc_res_search(SB)
-       XORL    DX, DX
-       CMPL    AX, $-1
-       JNE ok
-       CALL    libc_error(SB)
-       MOVLQSX (AX), DX        // move return from libc_error into DX
-       XORL    AX, AX  // size on error is 0
-ok:
-       MOVL    AX, 28(BX) // size
-       MOVL    DX, 32(BX) // error code
-       POPQ    BP
-       RET
diff --git a/src/runtime/lookup_darwin_arm.s b/src/runtime/lookup_darwin_arm.s
deleted file mode 100644 (file)
index c74419f..0000000
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright 2015 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// System calls and other sys.stuff for ARM64, Darwin
-// System calls are implemented in libSystem, this file contains
-// trampolines that convert from Go to C calling convention.
-
-#include "go_asm.h"
-#include "go_tls.h"
-#include "textflag.h"
-
-// On darwin/arm, the runtime always uses runtime/cgo
-// for resolution. This will just exit with a nominal
-// exit code.
-
-TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0
-       MOVW    $90, R0
-       BL      libc_exit(SB)
-       RET
-
-TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0
-       MOVW    $91, R0
-       BL      libc_exit(SB)
-       RET
diff --git a/src/runtime/lookup_darwin_arm64.s b/src/runtime/lookup_darwin_arm64.s
deleted file mode 100644 (file)
index e13662b..0000000
+++ /dev/null
@@ -1,21 +0,0 @@
-// Copyright 2019 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.
-
-#include "go_asm.h"
-#include "go_tls.h"
-#include "textflag.h"
-
-// On darwin/arm, the runtime always uses runtime/cgo
-// for resolution. This will just exit with a nominal
-// exit code.
-
-TEXT runtime·res_search_trampoline(SB),NOSPLIT,$0
-       MOVW    $90, R0
-       BL      libc_exit(SB)
-       RET
-
-TEXT runtime·res_init_trampoline(SB),NOSPLIT,$0
-       MOVW    $91, R0
-       BL      libc_exit(SB)
-       RET