The gold linker is used by default in the Android NDK, except on
arm64:
https://github.com/android-ndk/ndk/issues/148
The Go linker already forces the use of the gold linker on arm and
arm64 (CL 22141) for other reasons. However, the test.bash script in
testcshared doesn't, resulting in linker errors on android/arm64:
warning: liblog.so, needed by ./libgo.so, not found (try using -rpath or
-rpath-link)
Add -fuse-ld=gold when running testcshared on Android. Fixes the
android/arm64 builder.
Change-Id: I35ca96f01f136bae72bec56d71b7ca3f344df1ed
Reviewed-on: https://go-review.googlesource.com/38832
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
After https://golang.org/cl/31490 we break false
output dependency for CVTS.. in compiler generated code.
I've looked through asm code, which uses CVTS..
and added XOR to the only case where it affected performance.
Log-6 21.6ns ± 0% 19.9ns ± 0% -7.87% (p=0.000 n=10+10)
Change-Id: I25d9b405e3041a3839b40f9f9a52e708034bb347
Reviewed-on: https://go-review.googlesource.com/38771
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
There is always 128 bytes available below the stackguard. Allow functions
with medium-sized stack frames to use this, potentially allowing them to
avoid growing the stack.
This change makes all architectures use the same calculation as x86.
Change-Id: I2afb1a7c686ae5a933e50903b31ea4106e4cd0a0
Reviewed-on: https://go-review.googlesource.com/38734
Reviewed-by: Cherry Zhang <cherryyz@google.com>
By overwhelming popular demand, exclude vendored packages from ... matches,
by making ... never match the "vendor" element above a vendored package.
go help packages now reads:
An import path is a pattern if it includes one or more "..." wildcards,
each of which can match any string, including the empty string and
strings containing slashes. Such a pattern expands to all package
directories found in the GOPATH trees with names matching the
patterns.
To make common patterns more convenient, there are two special cases.
First, /... at the end of the pattern can match an empty string,
so that net/... matches both net and packages in its subdirectories, like net/http.
Second, any slash-separted pattern element containing a wildcard never
participates in a match of the "vendor" element in the path of a vendored
package, so that ./... does not match packages in subdirectories of
./vendor or ./mycode/vendor, but ./vendor/... and ./mycode/vendor/... do.
Note, however, that a directory named vendor that itself contains code
is not a vendored package: cmd/vendor would be a command named vendor,
and the pattern cmd/... matches it.
Fixes#19090.
Change-Id: I985bf9571100da316c19fbfd19bb1e534a3c9e5f
Reviewed-on: https://go-review.googlesource.com/38745
Reviewed-by: Alan Donovan <adonovan@google.com>
This reverts commit 041ecb697f.
Reason for revert: Not working on S390x and some 386 archs.
I have a guess why the S390x is failing. No clue on the 386 yet.
Revert until I can figure it out.
Change-Id: I64f1ce78fa6d1037ebe7ee2a8a8107cb4c1db70c
Reviewed-on: https://go-review.googlesource.com/38790
Reviewed-by: Keith Randall <khr@golang.org>
rsc.io/toolstash is gone; use rsc.io/pprof_mac_fix.
This fixes a bug in the test. It turns out the code being tested here
is also broken, so the test still doesn't pass after this CL (filed #19769).
Change-Id: Ieb725c321d7fab600708e133ae28f531e55521ad
Reviewed-on: https://go-review.googlesource.com/38743
Reviewed-by: Alan Donovan <adonovan@google.com>
The uintptr-typed Data field in reflect.SliceHeader and
reflect.StringHeader needs special treatment because it is
really a pointer. Add the special treatment in walk for
bug #19168 to escape analysis.
Includes extra debugging that was helpful.
Fixes#19743.
Change-Id: I6dab5002f0d436c3b2a7cdc0156e4fc48a43d6fe
Reviewed-on: https://go-review.googlesource.com/38738
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, an inlined call to wg.Done() in package main would have the
following incorrect symbol name:
main.(*sync.WaitGroup).Done
This change modifies methodname to return the correct symbol name:
sync.(*WaitGroup).Done
This fix was suggested by @mdempsky.
Fixes#19467.
Change-Id: I0117838679ac5353789299c618ff8c326712d94d
Reviewed-on: https://go-review.googlesource.com/37866
Reviewed-by: Austin Clements <austin@google.com>
The `skip` argument passed to runtime.Caller and runtime.Callers should
be interpreted as the number of logical calls to skip (rather than the
number of physical stack frames to skip). This changes runtime.Callers
to skip inlined calls in addition to physical stack frames.
The result value of runtime.Callers is a slice of program counters
([]uintptr) representing physical stack frames. If the `skip` parameter
to runtime.Callers skips part-way into a physical frame, there is no
convenient way to encode that in the resulting slice. To avoid changing
the API in an incompatible way, our solution is to store the number of
skipped logical calls of the first frame in the _second_ uintptr
returned by runtime.Callers. Since this number is a small integer, we
encode it as a valid PC value into a small symbol called:
runtime.skipPleaseUseCallersFrames
For example, if f() calls g(), g() calls `runtime.Callers(2, pcs)`, and
g() is inlined into f, then the frame for f will be partially skipped,
resulting in the following slice:
pcs = []uintptr{pc_in_f, runtime.skipPleaseUseCallersFrames+1, ...}
We store the skip PC in pcs[1] instead of pcs[0] so that `pcs[i:]` will
truncate the captured stack trace rather than grow it for all i.
Updates #19348.
Change-Id: I1c56f89ac48c29e6f52a5d085567c6d77d499cf1
Reviewed-on: https://go-review.googlesource.com/37854
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Previously, we could not run tests with -l=4 on NaCl since the buildrun
action is not supported on NaCl. This lets us run tests with build flags
on NaCl.
Change-Id: I103370c7b823b4ff46f47df97e802da0dc2bc7c3
Reviewed-on: https://go-review.googlesource.com/38170
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The aLongTimeAgo time value in net and net/http is used to cancel
in-flight read and writes. It was set to time.Unix(233431200, 0)
which seemed like far enough in the past.
But Raspberry Pis, lacking a real time clock, had to spoil the fun and
boot in 1970 at the Unix epoch time, breaking assumptions in net and
net/http.
So change aLongTimeAgo to time.Unix(1, 0), which seems like the
earliest safe value. I don't trust subsecond values on all operating
systems, and I don't trust the Unix zero time. The Raspberry Pis do
advance their clock at least. And the reported problem was that Hijack
on a ResponseWriter hung forever, waiting for the connection read
operation to finish. So now, even if kernel + userspace boots in under
a second (unlikely), the Hijack will just have to wait for up to a
second.
Fixes#19747
Change-Id: Id59430de2e7b5b5117d4903a788863e9d344e53a
Reviewed-on: https://go-review.googlesource.com/38785
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
We have lots of rewrite rules that vary only in the fact that
we have 2 versions for the 2 different orderings of various
commuting ops. For example:
(ADDL x (MOVLconst [c])) -> (ADDLconst [c] x)
(ADDL (MOVLconst [c]) x) -> (ADDLconst [c] x)
It can get unwieldly quickly, especially when there is more than
one commuting op in a rule.
Our existing "fix" for this problem is to have rules that
canonicalize the operations first. For example:
(Eq64 x (Const64 <t> [c])) && x.Op != OpConst64 -> (Eq64 (Const64 <t> [c]) x)
Subsequent rules can then assume if there is a constant arg to Eq64,
it will be the first one. This fix kinda works, but it is fragile and
only works when we remember to include the required extra rules.
The fundamental problem is that the rule matcher doesn't
know anything about commuting ops. This CL fixes that fact.
We already have information about which ops commute. (The register
allocator takes advantage of commutivity.) The rule generator now
automatically generates multiple rules for a single source rule when
there are commutative ops in the rule. We can now drop all of our
almost-duplicate source-level rules and the canonicalization rules.
I have some CLs in progress that will be a lot less verbose when
the rule generator handles commutivity for me.
I had to reorganize the load-combining rules a bit. The 8-way OR rules
generated 128 different reorderings, which was causing the generator
to put too much code in the rewrite*.go files (the big ones were going
from 25K lines to 132K lines). Instead I reorganized the rules to
combine pairs of loads at a time. The generated rule files are now
actually a bit (5%) smaller.
[Note to reviewers: check these carefully. Most of the other rule
changes are trivial.]
Make.bash times are ~unchanged.
Compiler benchmarks are not observably different. Probably because
we don't spend much compiler time in rule matching anyway.
I've also done a pass over all of our ops adding commutative markings
for ops which hadn't had them previously.
Fixes#18292
Change-Id: I999b1307272e91965b66754576019dedcbe7527a
Reviewed-on: https://go-review.googlesource.com/38666
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
reflect.callReflect heap-allocates a stack frame and then constructs
pointers to the arguments and result areas of that frame. However, if
there are no results, the results pointer will point past the end of
the frame allocation. If there are also no arguments, the arguments
pointer will also point past the end of the frame allocation. If the
GC observes either these pointers, it may panic.
Fix this by not constructing these pointers if these areas of the
frame are empty.
This adds a test of calling no-argument/no-result methods via reflect,
since nothing in std did this before. However, it's quite difficult to
demonstrate the actual failure because it depends on both exact
allocation patterns and on GC scanning the goroutine's stack while
inside one of the typedmemmovepartial calls.
I also audited other uses of typedmemmovepartial and
memclrNoHeapPointers in reflect, since these are the most susceptible
to this. These appear to be the only two cases that can construct
out-of-bounds arguments to these functions.
Fixes#19724.
Change-Id: I4b83c596b5625dc4ad0567b1e281bad4faef972b
Reviewed-on: https://go-review.googlesource.com/38736
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
"go env" prints Go environment information as a shell script format by
default but it's difficult for some tools (e.g. editor packages) to
interpret it.
The -json flag prints the environment in JSON format which
can be easily interpreted by a lot of tools.
$ go env -json
{
"CC": "gcc",
"CGO_CFLAGS": "-g -O2",
"CGO_CPPFLAGS": "",
"CGO_CXXFLAGS": "-g -O2",
"CGO_ENABLED": "1",
"CGO_FFLAGS": "-g -O2",
"CGO_LDFLAGS": "-g -O2",
"CXX": "g++",
"GCCGO": "gccgo",
"GOARCH": "amd64",
"GOBIN": "/home/haya14busa/go/bin",
"GOEXE": "",
"GOGCCFLAGS": "-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build498013955=/tmp/go-build -gno-record-gcc-switches",
"GOHOSTARCH": "amd64",
"GOHOSTOS": "linux",
"GOOS": "linux",
"GOPATH": "/home/haya14busa",
"GORACE": "",
"GOROOT": "/home/haya14busa/src/go.googlesource.com/go",
"GOTOOLDIR": "/home/haya14busa/src/go.googlesource.com/go/pkg/tool/linux_amd64",
"PKG_CONFIG": "pkg-config"
}
Also, it supports arguments with -json flag.
$ go env -json GOROOT GOPATH GOBIN
{
"GOBIN": "/home/haya14busa/go/bin",
"GOPATH": "/home/haya14busa",
"GOROOT": "/home/haya14busa/src/go.googlesource.com/go"
}
Fixes#12567
Change-Id: I75db3780f14a8ab8c7fa58cc3c9cc488ef7b66a1
Reviewed-on: https://go-review.googlesource.com/38757
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Gccgo crashed compiling a function that returned multiple zero-sized values.
Change-Id: I499112cc310e4a4f649962f4d2bc9fee95dee1b6
Reviewed-on: https://go-review.googlesource.com/38772
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was cherry-picked to 1.8 as CL 38587, but on master issue was fixed
by CL 37661. Add still relevant part (test) and close issue, since test passes.
Fixes#19201
Change-Id: I6415792e2c465dc6d9bd6583ba1e54b107bcf5cc
Reviewed-on: https://go-review.googlesource.com/37376
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
It is currently possible in the compiler to create a struct type,
calculate the widths of types that depend on it,
and then alter the struct type.
transformclosure has local protection against this.
Protect against it at a deeper level.
This is preparation to call dowidth automatically,
rather than explicitly.
This is a re-roll of CL 38469.
Change-Id: Ic5b4baa250618504611fc57cbf51ab01d1eddf80
Reviewed-on: https://go-review.googlesource.com/38534
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Prior to this CL, Type.Width != 0 was the mark
of a Type whose Width had been calculated.
As a result, dowidth always recalculated
the width of struct{}.
This, combined with the prohibition on calculating
the width of a FuncArgsStruct and the use of
struct{} as a function argument,
meant that there were circumstances in which
it was forbidden to call dowidth on a type.
This inhibits refactoring to call dowidth automatically,
rather than explicitly.
Instead add a helper method, Type.WidthCalculated,
and implement as Type.Align > 0.
Type.Width is not a good candidate for tracking
whether the width has been calculated;
0 is a value type width, and Width is subject to
too much magic value game-playing.
For good measure, add a test for #11354.
Change-Id: Ie9a9fb5d924e7a2010c1904ae5e38ed4a38eaeb2
Reviewed-on: https://go-review.googlesource.com/38468
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It is explicitly assigned in each of the
assemblers as needed.
I plan to remove Cursym entirely eventually,
but this is a helpful intermediate step.
Passes toolstash-check -all.
Updates #15756
Change-Id: Id7ddefae2def439af44d03053886ca8cc935731f
Reviewed-on: https://go-review.googlesource.com/38727
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It is insufficiently canonical;
see the discussion at issue 19719.
Fixes#19719
Change-Id: I0559ff3b1b39d7bc4b446d104f36fdf8ce3da50e
Reviewed-on: https://go-review.googlesource.com/38722
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Remove the global obj.Link.Curp.
In asmz.go, replace the only use by passing it as an argument.
In asm0.go and asm9.go, it was written but never read.
In asm5.go and asm7.go, thread it through as an argument.
Passes toolstash-check -all.
Updates #15756
Change-Id: I1a0faa89e768820f35d73a8b37ec8088d78d15f7
Reviewed-on: https://go-review.googlesource.com/38715
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fold the printing of the offending instruction
into the neighboring Diag call, if it is not
already present.
Change-Id: I310f1479e16a4d2a24ff3c2f7e2c60e5e2015c1b
Reviewed-on: https://go-review.googlesource.com/38714
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The result from CFBundleCopyResourceURL is owned by the caller. This
CL adds the necessary CFRelease to release it after use.
Fixes#19722
Change-Id: I7afe22ef241d21922a7f5cef6498017e6269a5c3
Reviewed-on: https://go-review.googlesource.com/38639
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
If the user created an httptest.Server directly without using a
constructor it won't have the new unexported 'client' field. So don't
assume it's non-nil.
Fixes#19729
Change-Id: Ie92e5da66cf4e7fb8d95f3ad0f4e3987d3ae8b77
Reviewed-on: https://go-review.googlesource.com/38710
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Kevin Burke <kev@inburke.com>
We reused the old C stack check mechanism for the implementation of
//go:systemstack, so when we execute a //go:systemstack function on a
user stack, the system fails by calling morestackc. However,
morestackc's message still talks about "executing C code".
Fix morestackc's message to reflect its modern usage.
Change-Id: I7e70e7980eab761c0520f675d3ce89486496030f
Reviewed-on: https://go-review.googlesource.com/38572
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On Android, the thread local offset is found by looping through memory
starting at the TLS base address. The search is limited to
PTHREAD_KEYS_MAX, but issue 19472 made it clear that in some cases, the
slot is located further from the TLS base.
The limit is merely a sanity check in case our assumptions about the
thread-local storage layout are wrong, so this CL raises it to 384, which
is enough for the test case in issue 19472.
Fixes#19472
Change-Id: I89d1db3e9739d3a7fff5548ae487a7483c0a278a
Reviewed-on: https://go-review.googlesource.com/38636
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These fields are used to encode a single instruction.
Add them to AsmBuf, which is also per-instruction,
and which is not global.
Updates #15756
Change-Id: I0e5ea22ffa641b07291e27de6e2ff23b6dc534bd
Reviewed-on: https://go-review.googlesource.com/38668
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Thread it through as an argument instead of using a global.
Passes toolstash-check -all.
Updates #15756
Change-Id: Ia8c6ce09b43dbb2e6c7d889ded8dbaeb5366048d
Reviewed-on: https://go-review.googlesource.com/38667
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Empirically, p == ctxt.Curp here.
A scan of (the thousands of lines of) asm6.go
shows no clear opportunity for them to diverge.
Passes toolstash-check -all.
Updates #15756
Change-Id: I9f5ee9585a850fbe24be3b851d8fdc2c966c65ce
Reviewed-on: https://go-review.googlesource.com/38665
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
