1. 11 Jun, 2019 3 commits
  2. 10 Jun, 2019 10 commits
  3. 09 Jun, 2019 1 commit
    • Dmitri Shuralyov's avatar
      cmd/go: fix syntax mistake in a testscript file, take 2 · 323212b9
      Dmitri Shuralyov authored
      This is a followup to CL 181278 and CL 181177.
      
      According to cmd/go/testdata/script/README:
      
      	Each line is parsed into a sequence of space-separated command words,
      	with environment variable expansion and # marking an end-of-line comment.
      	Adding single quotes around text keeps spaces in that text from being treated
      	as word separators and also disables environment variable expansion.
      
      We want $HOME to be expanded, so leave it out of the single-quoted
      block of text.
      
      I tested this change on macOS, and it makes TestScript/env_write pass.
      
      Fixes #32503
      
      Change-Id: I13621aec82263e5cb6978c13a1ad71d2210a0e42
      Reviewed-on: https://go-review.googlesource.com/c/go/+/181418
      Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      323212b9
  4. 07 Jun, 2019 12 commits
  5. 06 Jun, 2019 14 commits
    • Austin Clements's avatar
      runtime/internal/atomic: remove erroneous ABI wrappers · 82521659
      Austin Clements authored
      CL 179862 introduced go:linkname directives to create ABI wrappers for
      Store and Store64 on s390x, but a concurrent change (CL 180439)
      replaced the Go definitions of these functions with assembly
      definitions. This resulted in conflicting definitions for the ABI0
      symbols, which led to a bootstrap linking failure.
      
      Fix this by removing the now-incorrect go:linkname directives for
      Store and Store64. This should fix the linux-s390x builders.
      
      Updates #31230.
      
      Change-Id: I8de8c03c23412fc217d428c0018cc56eb2f9996f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/181078Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      82521659
    • Austin Clements's avatar
      runtime/internal/atomic: export more ABI0 wrappers · 62c309c5
      Austin Clements authored
      Somehow I missed these two functions in CL 179863. This should fix the
      linux-arm builders.
      
      Updates #31230.
      
      Change-Id: I3f8bef3fac331b505a55c0850b0fbc799b7c06c5
      Reviewed-on: https://go-review.googlesource.com/c/go/+/181077
      Run-TryBot: Austin Clements <austin@google.com>
      Reviewed-by: default avatarBryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      62c309c5
    • Russ Cox's avatar
      net: remove non-cgo macOS resolver code · d32ec38f
      Russ Cox authored
      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: default avatarKeith Randall <khr@golang.org>
      d32ec38f
    • Russ Cox's avatar
      net: fix non-cgo macOS resolver code · 705830e2
      Russ Cox authored
      This code was added in April in CL 166297, for #12524.
      This CL fixes the following problems in the code:
      
       - The test for failure in the assembly stubs checked for
         64-bit -1 instead of 32-bit -1 to decide to fetch errno.
      
       - These C routines (res_init and res_search) don't set errno anyway,
         so the Go code using errno to decide success is incorrect.
         (The routines set h_errno, which is a racy global variable
         that can't safely be consulted, storing values in a different
         error space.)
      
       - The Go call passed res_search a non-NUL-terminated name.
      
       - The C res_search rejects calls asking for TypeALL as opposed to
         more specific answers like TypeA/TypeAAAA/TypeCNAME,
         breaking cgoLookupHost in all cases and cgoLookupIP
         except with IP-version-specific networks.
      
       - The DNS response packet was parsed twice, once with msg.Unpack
         (discarded), and once with the lower-level dnsmessage.Parser.
         The Parser loop was missing a call to p.SkipAllQuestions, with the
         result that no DNS response packet would ever parse successfully.
      
       - The parsing of the DNS response answers, if reached, behaved as if
         that the AResource and AAAAResource record contained textual
         IP addresses, while in fact they contain binary ones. The calls to
         parseIPv4 and parseIPv6 therefore would always returns nil,
         so that no useful result would be returned from the resolver.
      
      With these fixes, cgoLookupIP can correctly resolve google.com
      and return both the A and AAAA addresses.
      
      Even after fixing all these things, TestGoLookupIP still fails,
      because it is testing that in non-cgo builds the cgo stubs
      correctly report "I can't handle the lookup", and as written the
      code intentionally violates that expectation.
      
      This CL adds new direct tests of the pseudo-cgo routines.
      The direct IP address lookups succeed, but the CNAME query
      causes res_search to hang, and the PTR query fails unconditionally
      (a trivial C program confirms these behaviors are due to res_search itself).
      
      Traditionally, res_search is only intended for single-threaded use.
      It is unclear whether this one is safe for use from multiple goroutines.
      If you run net.test under lldb, that causes syslog messages to be
      printed to standard error suggesting double-free bugs:
      
      	2019-06-05 19:52:43.505246-0400 net.test[6256:6831076] dnssd_clientstub DNSServiceRefDeallocate called with invalid DNSServiceRef 0x5c000f0 FFFFFFFF DDDDDDDD
      	2019-06-05 19:52:43.505274-0400 net.test[6256:6831076] dnssd_clientstub DNSServiceRefDeallocate called with invalid DNSServiceRef 0x5c000f0 FFFFFFFF DDDDDDDD
      	2019-06-05 19:52:43.505303-0400 net.test[6256:6831076] dnssd_clientstub DNSServiceRefDeallocate called with invalid DNSServiceRef 0x5c000f0 FFFFFFFF DDDDDDDD
      	2019-06-05 19:52:43.505329-0400 net.test[6256:6831076] dnssd_clientstub DNSServiceRefDeallocate called with invalid DNSServiceRef 0x5c000f0 FFFFFFFF DDDDDDDD
      
      This res_search is from libsystem_info; a normal C program would
      get res_search (#defined to res_9_search) from libresolv instead.
      It is unclear what the relation between the two is.
      Issue #12524 was about supporting the /etc/resolver directory tree,
      but only libresolv contains code for that; libsystem_info does not.
      So this code probably does not enable use of /etc/resolver.
      
      In short:
      
       - Before this CL, the code clearly had never run successfully.
       - The code appears not to improve upon the usual non-cgo fallback.
       - The code carries with it no tests of improved behavior.
       - The code breaks existing tests.
       - Calling res_search does not work for PTR/CNAME queries,
         so the code breaks existing behavior, even after this CL.
       - It's unclear whether res_search is safe to call from multiple threads.
       - It's unclear whether res_search is used by any other macOS programs.
      
      Given this, it probably makes sense to delete this code rather
      than rejigger the test. This CL fixes the code first, so that there
      is a working copy to bring back later if we find out that it really
      is necessary.
      
      For #31705.
      
      Change-Id: Id2e11e8ade43098b0f90dd4d16a62ca86a7a244a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180842
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      705830e2
    • Russ Cox's avatar
      runtime: document, fix libc error checks on macOS · 7d65e3a8
      Russ Cox authored
      It matters whether we are calling a function that would
      return a 32-bit or 64-bit -1 on error. A few sites were wrong
      and this key detail was omitted from syscall/syscallX docs.
      
      Change-Id: I48a421b6cc4d2d2b5e58f790cc947e3cb2f98940
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180841
      Run-TryBot: Russ Cox <rsc@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      7d65e3a8
    • Austin Clements's avatar
      cmd/go: remove cross-package assembly reference discovery · 47df542f
      Austin Clements authored
      This removes the special case for finding assembly references to Go
      symbols in runtime and runtime/internal/atomic. These are no longer
      necessary because we've now marked all symbols in these packages that
      must be accessible from assembly in other packages.
      
      Fixes #31230.
      
      Change-Id: I70c90b70e13b922a6669f3d46c53347f98d6fc3f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/179863
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      47df542f
    • Austin Clements's avatar
      runtime: mark all Go symbols called from assembly in other packages · f5e5bc1a
      Austin Clements authored
      This marks all Go symbols called from assembly in other packages with
      "go:linkname" directives to ensure they get ABI wrappers.
      
      Now that we have this go:linkname convention, this also removes the
      abi0Syms definition in the runtime, which was used to give morestackc
      an ABI0 wrapper. Instead, we now just mark morestackc with a
      go:linkname directive.
      
      This was tested with buildall.bash in the default configuration, with
      -race, and with -gcflags=all=-d=ssa/intrinsics/off. Since I couldn't
      test cgo on non-Linux configurations, I manually grepped for runtime
      symbols in runtime/cgo.
      
      Updates #31230.
      
      Change-Id: I6c8aa56be2ca6802dfa2bf159e49c411b9071bf1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/179862
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      f5e5bc1a
    • Austin Clements's avatar
      cmd/compile: make the second argument to go:linkname optional · dde7c770
      Austin Clements authored
      The //go:linkname directive can be used to make a symbol accessible to
      another package (when it wouldn't normally be). Sometimes you want to
      do this without actually changing the symbol's object file symbol
      name; for example, in gccgo this makes unexported symbols non-static,
      and in gc this provides ABI0 wrappers for Go symbols so they can be
      called from assembly in other packages. Currently, this results in
      stutter like
      
         //go:linkname entersyscall runtime.entersyscall
      
      This CL makes the second argument to go:linkname optional for the case
      where the intent is simply to expose the symbol rather than to rename
      it in the object file.
      
      Updates #31230.
      
      Change-Id: Id06d9c4b2ec3d8e27f9b8a0d65212ab8048d734f
      Reviewed-on: https://go-review.googlesource.com/c/go/+/179861
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      dde7c770
    • Austin Clements's avatar
      cmd/compile: generate ABI wrappers for //go:linkname'd symbols · b402bd44
      Austin Clements authored
      Calling a Go symbol from assembly in another package currently results
      in a link failure because the Go symbol is defined as ABIInternal, but
      the assembly call is from ABI0. In general this is okay because you
      shouldn't do this anyway, but there are special cases where this is
      necessary, especially between the runtime and packages closely tied to
      the runtime in std.
      
      Currently, we address this for runtime symbols with a hack in cmd/go
      that knows to scan related packages when building the symabis file for
      the runtime and runtime/internal/atomic. However, in addition to being
      a messy solution in the first place, this hack causes races in cmd/go
      that are difficult to work around.
      
      We considered creating dummy references from assembly in the runtime
      to these symbols, just to make sure they get ABI0 wrappers. However,
      there are a fairly large number of these symbols on some platforms,
      and it can vary significantly depending on build flags (e.g., race
      mode), so even this solution is fairly unpalatable.
      
      This CL addresses this by providing a way to mark symbols in Go code
      that should be made available to assembly in other packages. Rather
      than introduce a new pragma, we lightly expand the meaning of
      "//go:linkname", since that pragma already generally indicates that
      you're making the symbol available in a way it wasn't before. This
      also dovetails nicely with the behavior of go:linkname in gccgo, which
      makes unexported symbols available to other packages.
      
      Follow-up CLs will make use of this and then remove the hack from
      cmd/go.
      
      Updates #31230.
      
      Change-Id: I23060c97280626581f025c5c01fb8d24bb4c5159
      Reviewed-on: https://go-review.googlesource.com/c/go/+/179860
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Chase <drchase@google.com>
      b402bd44
    • Michael Munday's avatar
      cmd/compile, runtime: make atomic loads/stores sequentially consistent on s390x · ac8dbe77
      Michael Munday authored
      The z/Architecture does not guarantee that a load following a store
      will not be reordered with that store, unless they access the same
      address. Therefore if we want to ensure the sequential consistency
      of atomic loads and stores we need to perform serialization
      operations after atomic stores.
      
      We do not need to serialize in the runtime when using StoreRel[ease]
      and LoadAcq[uire]. The z/Architecture already provides sufficient
      ordering guarantees for these operations.
      
      name              old time/op  new time/op  delta
      AtomicLoad64-16   0.51ns ± 0%  0.51ns ± 0%     ~     (all equal)
      AtomicStore64-16  0.51ns ± 0%  0.60ns ± 9%  +16.47%  (p=0.000 n=17+20)
      AtomicLoad-16     0.51ns ± 0%  0.51ns ± 0%     ~     (all equal)
      AtomicStore-16    0.51ns ± 0%  0.60ns ± 9%  +16.50%  (p=0.000 n=18+20)
      
      Fixes #32428.
      
      Change-Id: I88d19a4010c46070e4fff4b41587efe4c628d4d9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180439
      Run-TryBot: Michael Munday <mike.munday@ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      ac8dbe77
    • David Chase's avatar
      cmd/compile: correct capitalization in recordFlags parameter · 53deb812
      David Chase authored
      Tool refactoring smallStacks into smallFrames helpfully
      "corrected" the capitalization in a string, this undoes
      the help.
      
      This is necessary to ensure correct (re)building when the
      flag is used to research stack-marking GC latency bugs.
      
      Updates #27732.
      
      Change-Id: Ib7c8d4a36c9e4f9612559be68bd481f9d9cc69f1
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180958
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      53deb812
    • Meng Zhuo's avatar
      syscall: fix skip condition in skipUnprivilegedUserClone · 5ec14065
      Meng Zhuo authored
      This is a follow up CL of CL 180877:
      It will skip test create user namespaces under 3 conditions:
      
      1. sysctl file is missing
      2. file reads nothing
      3. user don't have permission to create namespaces
      
      Change-Id: I25f00a6b67213bf98d654972388637789978e1fe
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180937
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      5ec14065
    • David Chase's avatar
      cmd/compile: add -smallframes gc flag for GC latency diagnosis · 037ac2bd
      David Chase authored
      Shrinks the size of things that can be stack allocated from
      10M to 128k for declared variables and from 64k to 16k for
      implicit allocations (new(T), &T{}, etc).
      
      Usage: "go build -gcflags -smallframes hello.go"
      
      An earlier GOEXPERIMENT version of this caused only one
      problem, when a gc-should-detect-oversize-stack test no
      longer had an oversized stack to detect.  The change was
      converted to a flag to make it easier to access (for
      diagnosing "long" GC-related single-thread pauses) and to
      remove interference with the test.
      
      Includes test to verify behavior.
      
      Updates #27732.
      
      Change-Id: I1255d484331e77185e07c78389a8b594041204c2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180817
      Run-TryBot: David Chase <drchase@google.com>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      037ac2bd
    • Meng Zhuo's avatar
      syscall: skip test if unprivileged_userns_clone sysctl is missing · f31b7b9b
      Meng Zhuo authored
      The original test (CL 166460) didn't check the existence of
      /proc/sys/kernel/unprivileged_userns_clone and continue the test
      if the file doesn't exist.
      
      Fixes #32459
      
      Change-Id: Iab4938252fcaded32b61e17edf68f966c2565582
      Reviewed-on: https://go-review.googlesource.com/c/go/+/180877
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTobias Klauser <tobias.klauser@gmail.com>
      f31b7b9b