Skip to content

G
go
Switch branch/tag
  1. 07 Jun, 2019 11 commits
  3. 06 Jun, 2019 18 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
    • Russ Cox's avatar
      runtime: fix non-tab indentation in lookup_darwin_*.s · 064ce85c
      Russ Cox authored 
      Change-Id: Ie00494f098bd2bce9bfd1b18dbf9543cf46faad6
Reviewed-on: https://go-review.googlesource.com/c/go/+/180840
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      064ce85c
    • Russ Cox's avatar
      runtime: fix scattered non-tab indentation in assembly · 26a5f6a3
      Russ Cox authored 
      Change-Id: I6940a4c747f2da871263afa6a4e3386395d5cf54
Reviewed-on: https://go-review.googlesource.com/c/go/+/180839
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      26a5f6a3
    • Russ Cox's avatar
      net: fix conf.teardown call in TestGoLookupIPOrderFallbackToFile · 4ed2e193
      Russ Cox authored 
      If the test fails, conf.teardown wouldn't be.
It doesn't look like it matters much, but clean up anyway.

Change-Id: I45c18095abfd49422975d061be20cbd971a98f8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/180780
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      4ed2e193
    • Russ Cox's avatar
      runtime: use default system stack size, not 64 kB, on non-cgo macOS · c00ff65d
      Russ Cox authored 
      At least one libc call we make
(res_search, which calls _mdns_query and then mdns_item_call)
pushes a 64 kB stack frame onto the stack.
Then it faults on the guard page.

Use the default system stack size, under the assumption
that the C code being called is compatible with that stack size.

For #31705.

Change-Id: I1b0bfc2e54043c49f0709255988ef920ce30ee82
Reviewed-on: https://go-review.googlesource.com/c/go/+/180779
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      c00ff65d
  5. 05 Jun, 2019 10 commits
  7. 04 Jun, 2019 1 commit
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7