Skip to content

G
go
Switch branch/tag
  1. 12 Jun, 2019 2 commits
  3. 11 Jun, 2019 6 commits
  5. 10 Jun, 2019 10 commits
  7. 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
  9. 07 Jun, 2019 12 commits
  11. 06 Jun, 2019 9 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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7