If an exported function has named return variables, then show the names
as comments in the return struct we create in the header file.
Example here:
https://groups.google.com/forum/#!topic/golang-nuts/r393ne4zIfY
Change-Id: I21fb4ca2673f6977bec35ccab0cef7d42b311f96
Reviewed-on: https://go-review.googlesource.com/13061
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously t.zero always pointed to runtime.zerovalue. Change the hashmap code
to always return a runtime pointer directly, and change that pointer to point
to a larger buffer if one is needed.
(It might be better to only copy from the pointer returned by the mapaccess
functions when the value type is small enough and have the compiler insert
explicit zeroing for larger value types, but I tried and failed to do this).
This removes all uses of the zero field of the type data; the field itself can
be removed in a separate change.
Fixes#11491
Change-Id: I5b81752ff4067d74a5a281c41e88f151bae0171e
Reviewed-on: https://go-review.googlesource.com/13784
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was always a bit confusing, but it also fixes a problem: runtime.firstmoduledata
was always overridden in the linker to be a local symbol but cmd/internal/obj had
already rewritten code accessing it to access it via the GOT. This works on amd64, but
causes link failures on other platforms (e.g. arm64).
Change-Id: I9b8153af74b4d0f092211d63a000d15818f39773
Reviewed-on: https://go-review.googlesource.com/13786
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add blocks to remove critical edges, even when it looks like
there's no phi that requires it. Regalloc still likes to have
critical-edge-free graphs for other reasons.
Change-Id: I69f8eaecbc5d79ab9f2a257c2e289d60b18e43c8
Reviewed-on: https://go-review.googlesource.com/13933
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The issue 12226 has been caused by the allocation of the same register
for the equality check of two byte values. The code in cgen.go freed the
register for the second operand before the allocation of the register
for the first operand.
Fixes#12226
Change-Id: Ie4dc33a488bd48a17f8ae9b497fd63c1ae390555
Reviewed-on: https://go-review.googlesource.com/13771
Reviewed-by: Russ Cox <rsc@golang.org>
Add a new function and generic operation to handle
bounds checking for slices. Unlike the index
bounds checking the index can be equal to the upper
bound.
Do gc-friendly slicing that generates proper code for
0-length result slices.
This is a takeover of Alexandru's original change,
(https://go-review.googlesource.com/#/c/12764/)
submittable now that the decompose phase is in.
Change-Id: I17d164cf42ed7839f84ca949c6ad3289269c9160
Reviewed-on: https://go-review.googlesource.com/13903
Reviewed-by: David Chase <drchase@google.com>
Basic ops, no particular optimization in the pattern
matching yet (e.g. x!=x for Nan detection, x cmp constant,
etc.)
Change-Id: I0043564081d6dc0eede876c4a9eb3c33cbd1521c
Reviewed-on: https://go-review.googlesource.com/13704
Reviewed-by: Keith Randall <khr@golang.org>
A comparison of the form l == r where l is an interface and r is
concrete performs a type assertion on l to convert it to r's type.
However, the compiler fails to zero the temporary where the result of
the type assertion is written, so if the type is a pointer type and a
stack scan occurs while in the type assertion, it may see an invalid
pointer on the stack.
Fix this by zeroing the temporary. This is equivalent to the fix for
type switches from c4092ac.
Fixes#12253.
Change-Id: Iaf205d456b856c056b317b4e888ce892f0c555b9
Reviewed-on: https://go-review.googlesource.com/13872
Reviewed-by: Russ Cox <rsc@golang.org>
MOVXload and MOVXstore opcodes have both an auxint offset
and an aux offset (a symbol name, like a local or arg or global).
Make sure we keep those values during rewrites.
Change-Id: Ic9fd61bf295b5d1457784c281079a4fb38f7ad3b
Reviewed-on: https://go-review.googlesource.com/13849
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
nmspinning has a value range of [0, 2^31-1]. Update the comment to
indicate this and fix the comparison so it's not always false.
Fixes#11280
Change-Id: Iedaf0654dcba5e2c800645f26b26a1a781ea1991
Reviewed-on: https://go-review.googlesource.com/13877
Reviewed-by: Minux Ma <minux@golang.org>
gobuf.g is a guintptr, so without hex(), it will be printed as
a decimal, which is not very helpful and inconsistent with how
other pointers are printed.
Change-Id: I7c0432e9709e90a5c3b3e22ce799551a6242d017
Reviewed-on: https://go-review.googlesource.com/13879
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Simplify slice/map literal expressions.
Caught with gofmt -d -s, fixed with gofmt -w -s
Reformatted some expressions to improve readability.
Change-Id: Iaf123e6bd49162ec45c59297ad3b002ca59443bc
Reviewed-on: https://go-review.googlesource.com/13850
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While we're at it, also fix a typo.
Change-Id: Id436f33cffa5683e2a8450cce5b545960cf2877e
Reviewed-on: https://go-review.googlesource.com/13878
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is not a functional change. nr is a uint64 and can never be less
than zero, remove the no-op comparison.
Fixes#11279
Change-Id: Iebb36cc8fe97428b503e65d01b5e67d2b2bc7369
Reviewed-on: https://go-review.googlesource.com/13876
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The nohup command doesn't work in tmux on darwin.
Fixes#5135.
Change-Id: I1c21073d8bd54b49dd6b0bad86ef088d6d8e7a5f
Reviewed-on: https://go-review.googlesource.com/13883
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL takes a simple approach to spilling and loading flags.
We never spill. When a load is needed, we recalculate,
loading the arguments as needed.
This is simple and architecture-independent.
It is not very efficient, but as of this CL,
there are fewer than 200 flag spills during make.bash.
This was tested by manually reverting CLs 13813 and 13843,
causing SETcc, MOV, and LEA instructions to clobber flags,
which dramatically increases the number of flags spills.
With that done, all stdlib tests that used to pass
still pass.
For future reference, here are some other, more efficient
amd64-only schemes that we could adapt in the future if needed.
(1) Spill exactly the flags needed.
For example, if we know that the flags will be needed
by a SETcc or Jcc op later, we could use SETcc to
extract just the relevant flag. When needed,
we could use TESTB and change the op to JNE/SETNE.
(Alternatively, we could leave the op unaltered
and prepare an appropriate CMPB instruction
to produce the desired flag.)
However, this requires separate handling for every
instruction that uses the flags register,
including (say) SBBQcarrymask.
We could enable this on an ad hoc basis for common cases
and fall back to recalculation for other cases.
(2) Spill all flags with PUSHF and POPF
This modifies SP, which the runtime won't like.
It also requires coordination with stackalloc to
make sure that we have a stack slot ready for use.
(3) Spill almost all flags with LAHF, SETO, and SAHF
See http://blog.freearrow.com/archives/396
for details. This would handle all the flags we currently
use. However, LAHF and SAHF are not universally available
and it requires arranging for AX to be free.
Change-Id: Ie36600fd8e807ef2bee83e2e2ae3685112a7f276
Reviewed-on: https://go-review.googlesource.com/13844
Reviewed-by: Keith Randall <khr@golang.org>
This further reduces the number of flags spills
during make.bash by about 50%.
Note that GetG is implemented by one or two MOVs,
which is why it does not clobber flags.
Change-Id: I6fede8c027b7dc340e00d1e15df1b87bf2b2d9ec
Reviewed-on: https://go-review.googlesource.com/13843
Reviewed-by: Keith Randall <khr@golang.org>
The URL is shown on go docs and is an eye-sore.
For go1.6.
Change-Id: I8b8ea3751200d06ed36acfe22f47ebb38107f8db
Reviewed-on: https://go-review.googlesource.com/13282
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In short, %c should just give you the next rune, period.
Apparently this is the design. I use the term loosely.
Fixes#12275
Change-Id: I6f30bed442c0e88eac2244d465c7d151b29cf393
Reviewed-on: https://go-review.googlesource.com/13821
Reviewed-by: Andrew Gerrand <adg@golang.org>
All of Go passes. No changes for the text repo.
Fixes#10153
Change-Id: I313369bf471c8974390a6d42075e5c54f6a81750
Reviewed-on: https://go-review.googlesource.com/13667
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It is already validated by isDoaminName.
Change-Id: I7a955b632a5143e16b012641cf12bad452900753
Reviewed-on: https://go-review.googlesource.com/13789
Reviewed-by: Andrew Gerrand <adg@golang.org>
Mark these as unimplemented so we don't generate bad code.
Change-Id: I101190c40a753faaa82193ac37e2978b20a96e4e
Reviewed-on: https://go-review.googlesource.com/13748
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Add benchmarks for for sparsely escaped and densely escaped strings.
Then speed up the sparse unescaping part heavily by using IndexByte and
copy to skip the parts containing no escaping very fast.
Unescaping densely escaped strings slower because of
the new function call overhead. But sparsely encoded strings are seen
more often in the utf8 enabled web.
We win part of the speed back by looking up entityName differently.
benchmark old ns/op new ns/op delta
BenchmarkEscape 31680 31396 -0.90%
BenchmarkEscapeNone 6507 6872 +5.61%
BenchmarkUnescape 36481 48298 +32.39%
BenchmarkUnescapeNone 332 325 -2.11%
BenchmarkUnescapeSparse 8836 3221 -63.55%
BenchmarkUnescapeDense 30639 32224 +5.17%
Change-Id: If606cb01897a40eefe35ba98f2ff23bb25251606
Reviewed-on: https://go-review.googlesource.com/10172
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current implementations of MarshalJSON and MarshalText use
time.Format which returns a string (converted from a byte slice),
only to convert it back to a byte slice.
Avoid the conversion (and thus an allocation) by directly appending
the formatted time to a preallocated byte slice, using the new
AppendFormat function, introduced in golang.org/cl/1760.
This reduces the allocations done in Marshal[Text|JSON] by 50%.
benchmark old ns/op new ns/op delta
BenchmarkMarshalJSON 626 507 -19.01%
BenchmarkMarshalText 598 511 -14.55%
benchmark old allocs new allocs delta
BenchmarkMarshalJSON 2 1 -50.00%
BenchmarkMarshalText 2 1 -50.00%
benchmark old bytes new bytes delta
BenchmarkMarshalJSON 96 48 -50.00%
BenchmarkMarshalText 96 48 -50.00%
Fixes#11025
Change-Id: I468f78d075a6ecc1cdc839df7fb407fbc6ff2e70
Reviewed-on: https://go-review.googlesource.com/10555
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently an expression like
var v = 0 >> 1000
is rejected by gc with a "stupid shift" error, while gotype
compiles it successfully.
As suggested by gri on the issue tracker, allow an rsh right
operand to be any valid uint value.
Fixes#11328
Change-Id: I6ccb3b7f842338d91fd26ae37dd4fa279d7fc440
Reviewed-on: https://go-review.googlesource.com/13777
Reviewed-by: Robert Griesemer <gri@golang.org>
This updates the big package used by the compiler to match the
public big package which contains some updates and bug fixes.
Obtained by running vendor.bash in the internal/big directory.
No manual changes.
Change-Id: I299aecc6599d4a745a721ce48def32449640dbb2
Reviewed-on: https://go-review.googlesource.com/13815
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When there is no browser available on the system,
we should print the URL instead of failing.
Change-Id: I4a2b099e17609394273eff150062c285d76bbac1
Reviewed-on: https://go-review.googlesource.com/13774
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
This CL makes function printing and HTML generation
accurate after regalloc.
Prior to this CL, text and HTML function outputs
showed live values and blocks as dead.
Change-Id: I70669cd8641af841447fc5d2ecbd754b281356f0
Reviewed-on: https://go-review.googlesource.com/13812
Reviewed-by: Keith Randall <khr@golang.org>
Even though the umul/uquo functions expect two valid, finite big.Floats
arguments, SetString was calling them with possibly Inf values, which
resulted in bogus return values.
Replace umul and udiv calls with Mul and Quo calls to fix this. Also,
fix two wrong tests.
See relevant issue on issue tracker for a detailed explanation.
Fixes#11341
Change-Id: Ie35222763a57a2d712a5f5f7baec75cab8189a53
Reviewed-on: https://go-review.googlesource.com/13778
Reviewed-by: Robert Griesemer <gri@golang.org>
This reduces the number of flags spilled during
make.bash by > 90%.
I am working (slowly) on the rest.
Change-Id: I3c08ae228c33e2f726f615962996f0350c8d592b
Reviewed-on: https://go-review.googlesource.com/13813
Reviewed-by: David Chase <drchase@google.com>
This is not a functional change.
Also:
- minor cleanups, better comments
- uniform spelling of noun "zeros" (per OED)
Fixes#11277.
Change-Id: I1726f358ce15907bd2410f646b02cf8b11b919cd
Reviewed-on: https://go-review.googlesource.com/11267
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
I cannot find where it's being used.
This addresses a duplicate symbol issue encountered in golang/go#9327.
Change-Id: I8efda45a006ad3e19423748210c78bd5831215e0
Reviewed-on: https://go-review.googlesource.com/13615
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 9184 changed the runtime and syscall packages to link Solaris binaries
directly instead of using dlopen/dlsym but did not remove the unused (and
now broken) references to dlopen, dlclose, and dlsym.
Fixes#11923
Change-Id: I36345ce5e7b371bd601b7d48af000f4ccacd62c0
Reviewed-on: https://go-review.googlesource.com/13410
Reviewed-by: Aram Hăvărneanu <aram@mgk.ro>
Could go in 1.5, although not critical.
See also #12107
Change-Id: I7f1608b58581d21df4db58f0db654fef79e33a90
Reviewed-on: https://go-review.googlesource.com/13481
Reviewed-by: Dave Cheney <dave@cheney.net>
Even Linux systems may not have _SC_GETPW_R_SIZE_MAX if using a
different libc than glibc (e.g. musl). Instead of having special-cases
for the BSDs, handle -1 correctly by always using a default buffer size.
Fixes#11319.
Change-Id: I8b1b260eb9830e6dbe7667f3f33d115ae4de4ce8
Reviewed-on: https://go-review.googlesource.com/13772
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Decompose breaks compound objects up into pieces that can be
operated on by the target architecture. The decompose pass only
does phi ops, the rest is done by the rewrite rules in generic.rules.
Compound objects include strings,slices,interfaces,structs,arrays.
Arrays aren't decomposed because of indexing (we could support
constant indexes, but dynamic indexes can't be handled using SSA).
Structs will come in a subsequent CL.
TODO: after this pass we have lost the association between, e.g.,
a string's pointer and its size. It would be nice if we could keep
that information around for debugging info somehow.
Change-Id: I6379ab962a7beef62297d0f68c421f22aa0a0901
Reviewed-on: https://go-review.googlesource.com/13683
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Implement index check panics (and slice check panics, for when
we need those).
Clean up nil check. Now that the new regalloc is in we can use
the register we just tested as the address 0 destination.
Remove jumps after panic calls, they are unreachable.
Change-Id: Ifee6e510cdea49cc7c7056887e4f06c67488d491
Reviewed-on: https://go-review.googlesource.com/13687
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
for i, v := range a {
}
Walk converts this to a regular for loop, like this:
for i := 0, p := &a[0]; i < len(a); i++, p++ {
v := *p
}
Unfortunately, &a[0] fails its bounds check when a is
the empty slice (or string). The old compiler gets around this
by marking &a[0] as Bounded, meaning "don't emit bounds checks
for this index op". This change makes SSA honor that same mark.
The SSA compiler hasn't implemented bounds check panics yet,
so the failed bounds check just causes the current routine
to return immediately.
Fixes bytes package tests.
Change-Id: Ibe838853ef4046c92f76adbded8cca3b1e449e0b
Reviewed-on: https://go-review.googlesource.com/13685
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is especially important for LookupAddr, which used to be pure Go
(lightweight, one goroutine per call) and without this CL is now
unconditionally cgo (heavy, one thread per call).
Fixes#12190.
Change-Id: I43436a942bc1838b024225893e156f280a1e80cf
Reviewed-on: https://go-review.googlesource.com/13698
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Go 1.4 and before have always returned DNS names with a trailing dot
for reverse lookups, as they do for basically all other routines returning
DNS names. Go 1.4 and before always implemented LookupAddr using
pure Go (not C library calls).
Go 1.5 added the ability to make a C library call to implement LookupAddr.
Unfortunately the C library call returns a DNS name without a trailing dot
(an unrooted name), meaning that if turn off cgo during make.bash then
you still get the rooted name but with cgo on you get an unrooted name.
The unrooted name is inconsistent with the pure Go implementation
and with all previous Go releases, so change it to a rooted name.
Fixes#12189.
Change-Id: I3d6b72277c121fe085ea6af30e5fe8019fc490ad
Reviewed-on: https://go-review.googlesource.com/13697
Reviewed-by: Rob Pike <r@golang.org>
Adds support for high multiply which is used by the frontend when
rewriting const division. The frontend currently only does this for 8,
16, and 32 bit integer arithmetic.
Change-Id: I9b6c6018f3be827a50ee6c185454ebc79b3094c8
Reviewed-on: https://go-review.googlesource.com/13696
Reviewed-by: Keith Randall <khr@golang.org>
Store ops now need their size in the auxint field. I missed this one.
Change-Id: I050fd6b5b00579883731702c426edafa3a5f7561
Reviewed-on: https://go-review.googlesource.com/13682
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This was missed when we did the rename months ago
because cmd/vet did not live in the main tree.
Now vet's asmdecl checks will apply to ppc64 assembly too.
Change-Id: I687cba89fef702f29dd118de76a7ca1041c414f6
Reviewed-on: https://go-review.googlesource.com/13677
Reviewed-by: Andrew Gerrand <adg@golang.org>
Tested by hand.
Only lines of code changing are protected by Fieldtrack_enabled > 0,
which is never true in standard Go distributions.
Fixes#12171.
Change-Id: I963b9997dac10829db8ad4bfc97a7d6bf14b55c6
Reviewed-on: https://go-review.googlesource.com/13676
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The text segment starts farther into the binary when using
external linking on the mac. Test and fix.
Fixes#12173.
Change-Id: I1f0c81814bf70cd9decfceac3022784f4608eeef
Reviewed-on: https://go-review.googlesource.com/13672
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Found in a Google program running under the race detector.
No test, but verified that this fixes the race with go run -race of:
package main
import (
"crypto/tls"
"fmt"
"net"
"net/http"
"net/http/httptest"
)
func main() {
for {
ts := httptest.NewTLSServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {}))
conf := &tls.Config{} // non-nil
a, b := net.Pipe()
go func() {
sconn := tls.Server(a, conf)
sconn.Handshake()
}()
tr := &http.Transport{
TLSClientConfig: conf,
}
req, _ := http.NewRequest("GET", ts.URL, nil)
_, err := tr.RoundTrip(req)
println(fmt.Sprint(err))
a.Close()
b.Close()
ts.Close()
}
}
Also modified cmd/vet to report the copy-of-mutex bug statically
in CL 13646, and fixed two other instances in the code found by vet.
But vet could not have told us about cloneTLSConfig vs cloneTLSClientConfig.
Confirmed that original report is also fixed by this.
Fixes#12099.
Change-Id: Iba0171549e01852a5ec3438c25a1951c98524dec
Reviewed-on: https://go-review.googlesource.com/13453
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Implement a global (whole function) register allocator.
This replaces the local (per basic block) register allocator.
Clobbering of registers by instructions is handled properly.
A separate change will add the correct clobbers to all the instructions.
Change-Id: I38ce4dc7dccb8303c1c0e0295fe70247b0a3f2ea
Reviewed-on: https://go-review.googlesource.com/13622
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Todd Neal <todd@tneal.org>
This was missing from CL 13472
due to a badly synced client.
Change-Id: If59fc669125dd1caa335dacfbf0f8dbd7b074312
Reviewed-on: https://go-review.googlesource.com/13639
Reviewed-by: Keith Randall <khr@golang.org>
Added F32 and F64 load, store, and addition.
Added F32 and F64 multiply.
Added F32 and F64 subtraction and division.
Added X15 to "clobber" for FP sub/div
Added FP constants
Added separate FP test in gc/testdata
Change-Id: Ifa60dbad948a40011b478d9605862c4b0cc9134c
Reviewed-on: https://go-review.googlesource.com/13612
Reviewed-by: Keith Randall <khr@golang.org>
Correct 'an' to 'on' in the comment above the Stable() function.
Change-Id: I714e38b2d3a79dfd539d5368967d1c6b519cb948
Reviewed-on: https://go-review.googlesource.com/13662
Reviewed-by: Rob Pike <r@golang.org>
Fixesgolang/go#12133
CL 13630 fixed the use of a stale reg[] array in the various arch
backends which was causing the check in clearfat to pass
unconditionally on arm64.
With this check fixed, arm64 now considers REGRT1 to always be in use
as it is part of the reserved register set, see arm64/gsubr.go.
However, ppc64 does not consider REGRT1 and REGRT2 to be part of its
reserved set, so its identical clearfat check passes.
This CL removes the Reginuse check inside clearfat as REGRT1 is
guarenteed always be free on arm64.
Change-Id: I4719150d3c3378fae155b863c474529df18d4c17
Reviewed-on: https://go-review.googlesource.com/13650
Reviewed-by: Russ Cox <rsc@golang.org>
Using the type of the store argument is not safe, it may change
during rewriting, giving us the wrong store width.
(Store ptr (Trunc32to16 val) mem)
This should be a 2-byte store. But we have the rule:
(Trunc32to16 x) -> x
So if the Trunc rewrite happens before the Store -> MOVW rewrite,
then the Store thinks that the value it is storing is 4 bytes
in size and uses a MOVL. Bad things ensue.
Fix this by encoding the store width explicitly in the auxint field.
In general, we can't rely on the type of arguments, as they may
change during rewrites. The type of the op itself (as used by
the Load rules) is still ok to use.
Change-Id: I9e2359e4f657bb0ea0e40038969628bf0f84e584
Reviewed-on: https://go-review.googlesource.com/13636
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Use runtime.GOROOT instead of os.Getenv("GOROOT") to reference
trace-viewer html file. GOROOT env var is not necessary set,
runtime.GOROOT has a default value for such case.
Change-Id: I906a720f6822915bd9575756e6cbf6d622857c2b
Reviewed-on: https://go-review.googlesource.com/13593
Reviewed-by: Russ Cox <rsc@golang.org>
The reg[] array in .../gc is where truth lies. The copy in .../ARCH
is incorrect as it is mostly not updated to reflect regalloc decisions.
This bug was introduced in the rewrite
https://go-review.googlesource.com/#/c/7853/. The new reg[] array was
introduced in .../gc but not all of the uses were removed in the
.../ARCH directories.
Fixes#12133
Change-Id: I6364fc403cdab92d802d17f2913ba1607734037c
Reviewed-on: https://go-review.googlesource.com/13630
Reviewed-by: Russ Cox <rsc@golang.org>
Mul8 is lowered to MULW, but the rules for constant
folding do not handle the fact that the operands
are int8.
Change-Id: I2c336686d86249393a8079a471c6ff74e6228f3d
Reviewed-on: https://go-review.googlesource.com/13642
Reviewed-by: Keith Randall <khr@golang.org>
Disable CX as output for shift operations.
Change-Id: I85e6b22d09009b38847082dc375b6108c2dee80a
Reviewed-on: https://go-review.googlesource.com/13370
Reviewed-by: Keith Randall <khr@golang.org>
This is an initial implementation.
There are many rough edges and TODOs,
which will hopefully be polished out
with use.
Fixes#12071.
Change-Id: I1d6fd5a343063b5200623bceef2c2cfcc885794e
Reviewed-on: https://go-review.googlesource.com/13472
Reviewed-by: Keith Randall <khr@golang.org>
This omission was causing the new regalloc to fail.
Change-Id: If7ba7be38a436dbd0dd443828ddd7ebf6e35be0e
Reviewed-on: https://go-review.googlesource.com/13632
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Generating logging code every time causes large
diffs for small changes.
Since the intent is to use this for debugging only,
generate logging code only when requested.
Committed generated code will be logging free.
Change-Id: I9ef9e29c88b76c2557bad4c6b424b9db1255ec8b
Reviewed-on: https://go-review.googlesource.com/13623
Reviewed-by: Keith Randall <khr@golang.org>
Fix one test that build a violating CFG.
Change-Id: Ie0296ced602984d914a70461c76559c507ce2510
Reviewed-on: https://go-review.googlesource.com/13621
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This makes it easier to investigate and
understand rewrite behavior.
Change-Id: I790e8964922caf98362ce8a6d6972f52d83eefa8
Reviewed-on: https://go-review.googlesource.com/13588
Reviewed-by: Keith Randall <khr@golang.org>
This claims to be autogenerated from go tool dist,
but I don't see where.
In any case, the update is trivial.
Change-Id: I58daaba755f3d34a0396005046b89411a02ada7e
Reviewed-on: https://go-review.googlesource.com/13584
Reviewed-by: Keith Randall <khr@golang.org>
We need to move the memory variable update back to before endBlock
so that all successors use the right memory value.
See https://go-review.googlesource.com/13560
Change-Id: Id72e5526c56e5e070b933d3b28dc503a5a2978dc
Reviewed-on: https://go-review.googlesource.com/13586
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Update #12108
If DUFFZERO is used within a tail call method it will overwrite the
link register.
Change-Id: I6abd2fde0f0ad909ccd55eb119b992673a74f0e2
Reviewed-on: https://go-review.googlesource.com/13570
Reviewed-by: Russ Cox <rsc@golang.org>
Fix the test broken with dee1f2 by implementing Elem()
Change-Id: I7a4a487885267c24fdc52d79fb7d450231328812
Reviewed-on: https://go-review.googlesource.com/13551
Reviewed-by: Keith Randall <khr@golang.org>
Consider OpZero to be a store so it can be eliminated by dse.
Change-Id: Idebb6a190657b76966f0c5b20f2ec9f52fe47499
Reviewed-on: https://go-review.googlesource.com/13447
Reviewed-by: Keith Randall <khr@golang.org>
Introduce pseudo-ops PanicMem and LoweredPanicMem.
PanicMem could be rewritten directly into MOVL
during lowering, but then we couldn't log nil checks.
With this change, runnable nil check tests pass:
GOSSAPKG=main go run run.go -- nil*.go
Compiler output nil check tests fail:
GOSSAPKG=p go run run.go -- nil*.go
This is due to several factors:
* SSA has improved elimination of unnecessary nil checks.
* SSA is missing elimination of implicit nil checks.
* SSA is missing extra logging about why nil checks were removed.
I'm not sure how best to resolve these failures,
particularly in a world in which the two backends
will live side by side for some time.
For now, punt on the problem.
Change-Id: Ib2ca6824551671f92e0e1800b036f5ca0905e2a3
Reviewed-on: https://go-review.googlesource.com/13474
Reviewed-by: Keith Randall <khr@golang.org>
We were not recording function calls as
changing the state of memory.
As a result, the scheduler was not aware that
storing values to the stack in order to make a
function call must happen *after* retrieving
results from the stack from a just-completed
function call.
This fixes the container/ring tests.
This was my first experience debugging an issue
using the HTML output. I'm feeling quite
pleased with it.
Change-Id: I9e8276846be9fd7a60422911b11816c5175e3d0a
Reviewed-on: https://go-review.googlesource.com/13560
Reviewed-by: Keith Randall <khr@golang.org>
p.ImportPath is the directory-derived path (like cmd/go).
p.Name is the actual package name.
Fixes#12089.
Change-Id: Ief76d42a85f811b0dfe2218affb48551527a7d44
Reviewed-on: https://go-review.googlesource.com/13530
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Hardcoded the limit on constants only allowed.
Change-Id: Idb9b07b4871db7a752a79e492671e9b41207b956
Reviewed-on: https://go-review.googlesource.com/13257
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Fixes#12086
Belongs in 1.5
There remains a question of why the Statenames table's elements
are not printed. What purpose does that serve?
Change-Id: I83fd57b81d5e5065c3397a66ed457fc0d1c041bd
Reviewed-on: https://go-review.googlesource.com/13462
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Move the known-non-nil scan outside the work loop to resolve an issue
with values that were declared outside the block being operated on.
Also consider phis whose arguments are all non-nil, as non-nil.
Change-Id: I4d5b840042de9eb181f2cb918f36913fb5d517a2
Reviewed-on: https://go-review.googlesource.com/13441
Reviewed-by: Keith Randall <khr@golang.org>
Use a version of Floyd's cycle finding algorithm,
but advance by 1 and 1/2 steps per cycle rather
than by 1 and 2. It is simpler and should be cheaper
in the normal, acyclic case.
This should fix the 386 and arm builds,
which are currently hung.
Change-Id: If8bd443011b28a5ecb004a549239991d3dfc862b
Reviewed-on: https://go-review.googlesource.com/13473
Reviewed-by: Keith Randall <khr@golang.org>
We must make sure that all loads that use a store are scheduled
before the next store. Add additional dependency edges to the
value graph to enforce this constraint.
Change-Id: Iab83644f68bc4c30637085b82ca7467b9d5513a5
Reviewed-on: https://go-review.googlesource.com/13470
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
When scanning a hex byte at EOF, the code was ungetting the eof,
which backed up the input and caused double-scanning of a byte.
Delete the call to UnreadRune.
This line appeared in 1.5 for some reason; it was not in 1.4 and
should be removed again for 1.5
Fixes#12090.
Change-Id: Iad1ce8e7db8ec26615c5271310f4b0228cca7d78
Reviewed-on: https://go-review.googlesource.com/13461
Reviewed-by: Andrew Gerrand <adg@golang.org>
Changes the torture test in #12068 from failing about 1/10 times
to not failing in almost 2,000 runs.
This was only happening in -race mode because functions are
bigger in -race mode, so a few of the helpers for heapBitsBulkBarrier
were not being inlined, and they were not marked nosplit,
so (only in -race mode) the write barrier was being preempted by GC,
causing missed pointer updates.
Filed issue #12069 for diagnosis of any other similar errors.
Fixes#12068.
Change-Id: Ic174d9b050ba278b18b08ab0d85a73c33bd5b175
Reviewed-on: https://go-review.googlesource.com/13364
Reviewed-by: Austin Clements <austin@google.com>
Also, crash early on non-Linux SMP ARM systems when GOARM < 7;
without the proper synchronization, SMP cannot work.
Linux is okay because we call kernel-provided routines for
synchronization and barriers, and the kernel takes care of
providing the right routines for the current system.
On non-Linux systems we are left to fend for ourselves.
It is possible to use different synchronization on GOARM=6,
but it's too late to do that in the Go 1.5 cycle.
We don't believe there are any non-Linux SMP GOARM=6 systems anyway.
Fixes#12067.
Change-Id: I771a556e47893ed540ec2cd33d23c06720157ea3
Reviewed-on: https://go-review.googlesource.com/13363
Reviewed-by: Austin Clements <austin@google.com>
Don't nilcheck values that were constructed as a result of OpAddr or
OpAddPtr.
Change-Id: I38053e905d1b76a2a64e77f84e444d38a5217108
Reviewed-on: https://go-review.googlesource.com/13256
Reviewed-by: David Chase <drchase@google.com>
Rather than require an explicit Copy on the RHS of rewrite rules,
use rulegen magic to add it.
The advantages to handling this in rulegen are:
* simpler rules
* harder to accidentally miss a Copy
Change-Id: I46853bade83bdf517eee9495bf5a553175277b53
Reviewed-on: https://go-review.googlesource.com/13242
Reviewed-by: Keith Randall <khr@golang.org>
Currently, runtime.Goexit() calls goexit()—the goroutine exit stub—to
terminate the goroutine. This *mostly* works, but can cause a
"leftover stack barriers" panic if the following happens:
1. Goroutine A has a reasonably large stack.
2. The garbage collector scan phase runs and installs stack barriers
in A's stack. The top-most stack barrier happens to fall at address X.
3. Goroutine A unwinds the stack far enough to be a candidate for
stack shrinking, but not past X.
4. Goroutine A calls runtime.Goexit(), which calls goexit(), which
calls goexit1().
5. The garbage collector enters mark termination.
6. Goroutine A is preempted right at the prologue of goexit1() and
performs a stack shrink, which calls gentraceback.
gentraceback stops as soon as it sees goexit on the stack, which is
only two frames up at this point, even though there may really be many
frames above it. More to the point, the stack barrier at X is above
the goexit frame, so gentraceback never sees that stack barrier. At
the end of gentraceback, it checks that it saw all of the stack
barriers and panics because it didn't see the one at X.
The fix is simple: call goexit1, which actually implements the process
of exiting a goroutine, rather than goexit, the exit stub.
To make sure this doesn't happen again in the future, we also add an
argument to the stub prototype of goexit so you really, really have to
want to call it in order to call it. We were able to reliably
reproduce the above sequence with a fair amount of awful code inserted
at the right places in the runtime, but chose to change the goexit
prototype to ensure this wouldn't happen again rather than pollute the
runtime with ugly testing code.
Change-Id: Ifb6fb53087e09a252baddadc36eebf954468f2a8
Reviewed-on: https://go-review.googlesource.com/13323
Reviewed-by: Russ Cox <rsc@golang.org>
This makes TestTraceStressStartStop much less flaky.
Running under stress, it changes the failure rate from
above 1/100 to under 1/50000. That very unlikely
failure happens when an unexpected GoSysExit is
written. Not sure how that happens yet, but it is much
less important.
Fixes#11953.
Change-Id: I034671936334b4f3ab733614ef239aa121d20247
Reviewed-on: https://go-review.googlesource.com/13321
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
The lowering rules were missing the non-64 bit case.
SBBLcarrymask can be folded to a int32 integer whose
type has a smaller bit size. Without the new AND rules
the following would be generated:
v19 = MOVLconst <uint8> [-1] : SI
v20 = ANDB <uint8> v18 v19 : DI
which is obviously a NOP.
Fixes#12022
Change-Id: I5f4209f78edc0f118e5b9b2908739f09cefebca4
Reviewed-on: https://go-review.googlesource.com/13301
Reviewed-by: Keith Randall <khr@golang.org>
Make sure all referenced Blocks and Values are really there.
Fix deadcode to generate SSA graphs that pass this new test.
Change-Id: Ib002ce20e33490eb8c919bd189d209f769d61517
Reviewed-on: https://go-review.googlesource.com/13147
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
No functional changes.
The intent is just to make this
easier to read and maintain.
Change-Id: Iec207546482cd62bcb22eaae8efe5be6c4f15378
Reviewed-on: https://go-review.googlesource.com/13284
Reviewed-by: Keith Randall <khr@golang.org>
regalloc expects to find all OpSP and OpSB values
in the entry block.
There is no value to moving them; don't.
Change-Id: I775198f03ce7420348721ffc5e7d2bab065465b1
Reviewed-on: https://go-review.googlesource.com/13266
Reviewed-by: Keith Randall <khr@golang.org>
Given (say)
b1: <- b2 b3
v1 = Phi <t> v2 v3
b2:
v2 = ...
b3:
...
tighten will move v2 to b1, since it is only used in b1.
This is wrong; v2 needs to be evaluated before entering b1.
Fix it.
Change-Id: I2cc3b30e3ffd221cf594e36cec534dfd9cf3c6a7
Reviewed-on: https://go-review.googlesource.com/13264
Reviewed-by: Keith Randall <khr@golang.org>
The old code was only allowing the chars we choose not to escape.
We sometimes prefer to escape chars that do not strictly need it.
Allowing those to be used in RawPath lets people override that
preference, which is in fact the whole point of RawPath (new in Go 1.5).
While we are here, also allow [ ] in RawPath.
This is not strictly spec-compliant, but it is what modern browers
do and what at least some people expect, and the [ ] do not cause
any ambiguity (the usual reason they would be escaped, as they are
part of the RFC gen-delims class).
The argument for allowing them now instead of waiting until Go 1.6
is that this way RawPath has one fixed meaning at the time it is
introduced, that we should not need to change or expand.
Fixes#5684.
Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c
Reviewed-on: https://go-review.googlesource.com/13258
Reviewed-by: Andrew Gerrand <adg@golang.org>
The code in question was added as part of allowing zone identifiers
in IPv6 literals like http://[ipv6%zone]:port/foo, in golang.org/cl/2431.
The old condition makes no sense. It refers to §3.2.1, which is the wrong section
of the RFC, it excludes all the sub-delims, which §3.2.2 (the right section)
makes clear are valid, and it allows ':', which is not actually valid,
without an explanation as to why (because we keep :port in the Host field
of the URL struct).
The new condition allows all the sub-delims, as specified in RFC 3986,
plus the additional characters [ ] : seen in IP address literals and :port suffixes,
which we also keep in the Host field.
This allows mysql://a,b,c/path to continue to parse, as it did in Go 1.4 and earlier.
This CL does not break any existing tests, suggesting the over-conservative
behavior was not intended and perhaps not realized.
It is especially important not to over-escape the host field, because
Go does not unescape the host field during parsing: it rejects any
host field containing % characters.
Fixes#12036.
Change-Id: Iccbe4985957b3dc58b6dfb5dcb5b63a51a6feefb
Reviewed-on: https://go-review.googlesource.com/13254
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.
The CL during Go 1.5 that broke this syntax was
trying to fix#11208 and was probably too aggressive.
I added a test case for #11208 to make sure that stays
fixed.
Relaxing the check did not re-break #11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.
Fixes#12023.
Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Strengthening VerifyHostname exposed the fact that for resumed
connections, ConnectionState().VerifiedChains was not being saved
and restored during the ClientSessionCache operations.
Do that.
This change just saves the verified chains in the client's session
cache. It does not re-verify the certificates when resuming a
connection.
There are arguments both ways about this: we want fast, light-weight
resumption connections (thus suggesting that we shouldn't verify) but
it could also be a little surprising that, if the verification config
is changed, that would be ignored if the same session cache is used.
On the server side we do re-verify client-auth certificates, but the
situation is a little different there. The client session cache is an
object in memory that's reset each time the process restarts. But the
server's session cache is a conceptual object, held by the clients, so
can persist across server restarts. Thus the chance of a change in
verification config being surprisingly ignored is much higher in the
server case.
Fixes#12024.
Change-Id: I3081029623322ce3d9f4f3819659fdd9a381db16
Reviewed-on: https://go-review.googlesource.com/13164
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Prior to this change, broken trailers would be handled by body.Read, and
an error would be returned to its caller (likely a Handler), but that
error would go completely unnoticed by the rest of the server flow
allowing a broken connection to be reused. This is a possible request
smuggling vector.
Fixes#12027.
Change-Id: I077eb0b8dff35c5d5534ee5f6386127c9954bd58
Reviewed-on: https://go-review.googlesource.com/13148
Reviewed-by: Russ Cox <rsc@golang.org>
This change alters the certificate used in many tests so that it's no
longer self-signed. This allows some tests to exercise the standard
certificate verification paths in the future.
Change-Id: I9c3fcd6847eed8269ff3b86d9b6966406bf0642d
Reviewed-on: https://go-review.googlesource.com/13244
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
88e945f introduced a non-speculative double check of the heap trigger
before actually starting a concurrent GC. This was necessary to fix a
race for heap-triggered GC, but broke sysmon-triggered periodic GC,
since the heap check will of course fail for periodically triggered
GC.
Fix this by telling startGC whether or not this GC was triggered by
heap size or a timer and only doing the heap size double check for GCs
triggered by heap size.
Fixes#12026.
Change-Id: I7c3f6ec364545c36d619f2b4b3bf3b758e3bcbd6
Reviewed-on: https://go-review.googlesource.com/13168
Reviewed-by: Russ Cox <rsc@golang.org>
Now that it works we need to turn it back on.
Fixes#10119.
Change-Id: I9c62d3026f7bb62c49a601ad73f33bf655372915
Reviewed-on: https://go-review.googlesource.com/13162
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is just far too slow.
I have a CL for Go 1.6 that makes many of these into internal tests.
That will improve the coverage.
It does not matter much, because basically none of the go command
tests are architecture dependent, so the other builders will catch
any problems.
Fixes freebsd-arm builder.
Change-Id: I8b2f6ac2cc1e7657019f7731c6662dc43e20bfb5
Reviewed-on: https://go-review.googlesource.com/13166
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This works after golang.org/cl/13120 is running on the
coordinator (maybe it already is).
Change-Id: I4053d8e2f32fafd47b927203a6f66d5858e23376
Reviewed-on: https://go-review.googlesource.com/13165
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is what is causing freebsd/arm to crash mysteriously when using cgo.
The bug was introduced in golang.org/cl/4030, which moved this code out
of rt0_go and into its own function. The ARM ABI says that calls must
be made with the stack pointer at an 8-byte boundary, but only FreeBSD
seems to crash when this is violated.
Fixes#10119.
Change-Id: Ibdbe76b2c7b80943ab66b8abbb38b47acb70b1e5
Reviewed-on: https://go-review.googlesource.com/13161
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Implement ITAB, selecting the itable field of an interface.
Soften the lowering check to allow lowerings that leave
generic but dead ops behind. (The ITAB lowering does this.)
Change-Id: Icc84961dd4060d143602f001311aa1d8be0d7fc0
Reviewed-on: https://go-review.googlesource.com/13144
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
I find myself always adding this in temporarily.
Make it permanent.
Change-Id: I1646b3930a07d0ea01840736ccd449b7fd24f06e
Reviewed-on: https://go-review.googlesource.com/13141
Reviewed-by: Keith Randall <khr@golang.org>
Failure to treat control ops as live can lead
to them being eliminated when they live in
other blocks.
Change-Id: I604a1977a3d3884b1f4516bea4e15885ce38272d
Reviewed-on: https://go-review.googlesource.com/13138
Reviewed-by: Keith Randall <khr@golang.org>
They were being omitted after scheduling.
Change-Id: Ia20e2dcb61fde9ec854918b958c3897bafd282a6
Reviewed-on: https://go-review.googlesource.com/13140
Reviewed-by: Keith Randall <khr@golang.org>
Don't put them in the control value's block.
That may be many blocks up the dominator tree.
Change-Id: Iab3ea36a890ffe0e355dadec7aeb676901c4f070
Reviewed-on: https://go-review.googlesource.com/13134
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This is helpful when debugging generated code.
Change-Id: I268efa3593a03bb2c4e9f07d9034c004cd40df41
Reviewed-on: https://go-review.googlesource.com/13099
Reviewed-by: Keith Randall <khr@golang.org>
When commit 510fd13 enabled assists during the scan phase, it failed
to also update the code in the GC controller that computed the assist
CPU utilization and adjusted the trigger based on it. Fix that code so
it uses the start of the scan phase as the wall-clock time when
assists were enabled rather than the start of the mark phase.
Change-Id: I05013734b4448c3e2c730dc7b0b5ee28c86ed8cf
Reviewed-on: https://go-review.googlesource.com/13048
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
At the start of a GC cycle, the garbage collector computes the assist
ratio based on the total scannable heap size. This was intended to be
conservative; after all, this assumes the entire heap may be reachable
and hence needs to be scanned. But it only assumes that the *current*
entire heap may be reachable. It fails to account for heap allocated
during the GC cycle. If the trigger ratio is very low (near zero), and
most of the heap is reachable when GC starts (which is likely if the
trigger ratio is near zero), then it's possible for the mutator to
create new, reachable heap fast enough that the assists won't keep up
based on the assist ratio computed at the beginning of the cycle. As a
result, the heap can grow beyond the heap goal (by hundreds of megs in
stress tests like in issue #11911).
We already have some vestigial logic for dealing with situations like
this; it just doesn't run often enough. Currently, every 10 ms during
the GC cycle, the GC revises the assist ratio. This was put in before
we switched to a conservative assist ratio (when we really were using
estimates of scannable heap), and it turns out to be exactly what we
need now. However, every 10 ms is far too infrequent for a rapidly
allocating mutator.
This commit reuses this logic, but replaces the 10 ms timer with
revising the assist ratio every time the heap is locked, which
coincides precisely with when the statistics used to compute the
assist ratio are updated.
Fixes#11911.
Change-Id: I377b231ab064946228378fa10422a46d1b50f4c5
Reviewed-on: https://go-review.googlesource.com/13047
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
This was useful in debugging the mutator assist behavior for #11911,
and it fits with the other gcpacertrace output.
Change-Id: I1e25590bb4098223a160de796578bd11086309c7
Reviewed-on: https://go-review.googlesource.com/13046
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Proportional concurrent sweep is currently based on a ratio of spans
to be swept per bytes of object allocation. However, proportional
sweeping is performed during span allocation, not object allocation,
in order to minimize contention and overhead. Since objects are
allocated from spans after those spans are allocated, the system tends
to operate in debt, which means when the next GC cycle starts, there
is often sweep debt remaining, so GC has to finish the sweep, which
delays the start of the cycle and delays enabling mutator assists.
For example, it's quite likely that many Ps will simultaneously refill
their span caches immediately after a GC cycle (because GC flushes the
span caches), but at this point, there has been very little object
allocation since the end of GC, so very little sweeping is done. The
Ps then allocate objects from these cached spans, which drives up the
bytes of object allocation, but since these allocations are coming
from cached spans, nothing considers whether more sweeping has to
happen. If the sweep ratio is high enough (which can happen if the
next GC trigger is very close to the retained heap size), this can
easily represent a sweep debt of thousands of pages.
Fix this by making proportional sweep proportional to the number of
bytes of spans allocated, rather than the number of bytes of objects
allocated. Prior to allocating a span, both the small object path and
the large object path ensure credit for allocating that span, so the
system operates in the black, rather than in the red.
Combined with the previous commit, this should eliminate all sweeping
from GC start up. On the stress test in issue #11911, this reduces the
time spent sweeping during GC (and delaying start up) by several
orders of magnitude:
mean 99%ile max
pre fix 1 ms 11 ms 144 ms
post fix 270 ns 735 ns 916 ns
Updates #11911.
Change-Id: I89223712883954c9d6ec2a7a51ecb97172097df3
Reviewed-on: https://go-review.googlesource.com/13044
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently it's possible for the next_gc heap size trigger computed for
the next GC cycle to be less than the current allocated heap size.
This means the next cycle will start immediately, which means there's
no time to perform the concurrent sweep between GC cycles. This places
responsibility for finishing the sweep on GC itself, which delays GC
start-up and hence delays mutator assist.
Fix this by ensuring that next_gc is always at least a little higher
than the allocated heap size, so we won't trigger the next cycle
instantly.
Updates #11911.
Change-Id: I74f0b887bf187518d5fedffc7989817cbcf30592
Reviewed-on: https://go-review.googlesource.com/13043
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Currently there are two sensitive periods during which a mutator can
allocate past the heap goal but mutator assists can't be enabled: 1)
at the beginning of GC between when the heap first passes the heap
trigger and sweep termination and 2) at the end of GC between mark
termination and when the background GC goroutine parks. During these
periods there's no back-pressure or safety net, so a rapidly
allocating mutator can allocate past the heap goal. This is
exacerbated if there are many goroutines because the GC coordinator is
scheduled as any other goroutine, so if it gets preempted during one
of these periods, it may stay preempted for a long period (10s or 100s
of milliseconds).
Normally the mutator does scan work to create back-pressure against
allocation, but there is no scan work during these periods. Hence, as
a fall back, if a mutator would assist but can't yet, simply yield the
CPU. This delays the mutator somewhat, but more importantly gives more
CPU time to the GC coordinator for it to complete the transition.
This is obviously a workaround. Issue #11970 suggests a far better but
far more invasive way to fix this.
Updates #11911. (This very nearly fixes the issue, but about once
every 15 minutes I get a GC cycle where the assists are enabled but
don't do enough work.)
Change-Id: I9768b79e3778abd3e06d306596c3bd77f65bf3f1
Reviewed-on: https://go-review.googlesource.com/13026
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently allocation checks the GC trigger speculatively during
allocation and then triggers the GC without rechecking. As a result,
it's possible for G 1 and G 2 to detect the trigger simultaneously,
both enter startGC, G 1 actually starts GC while G 2 gets preempted
until after the whole GC cycle, then G 2 immediately starts another GC
cycle even though the heap is now well under the trigger.
Fix this by re-checking the GC trigger non-speculatively just before
actually kicking off a new GC cycle.
This contributes to #11911 because when this happens, we definitely
don't finish the background sweep before starting the next GC cycle,
which can significantly delay the start of concurrent scan.
Change-Id: I560ab79ba5684ba435084410a9765d28f5745976
Reviewed-on: https://go-review.googlesource.com/13025
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Do not assume that if stat shows multiple links that we should mark the
file as a hardlink in the tar format. If the hardlink link was not
referenced, this caused a link to "/". On an overlay file system, all
files have multiple links.
The caller must keep the inode references and set TypeLink, Size = 0,
and LinkName themselves.
Change-Id: I873b8a235bc8f8fbb271db74ee54232da36ca013
Reviewed-on: https://go-review.googlesource.com/13045
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The buildmode docs mention exported functions, but don't say anything
about how to export them. Mention the cgo tool to make this somewhat
clearer.
Fixes#11955.
Change-Id: Ie5420445daa87f5aceec6ad743465d5d32d0a786
Reviewed-on: https://go-review.googlesource.com/13080
Reviewed-by: Russ Cox <rsc@golang.org>
Rewrite ^{n}x to be ^{n % 2}x. This will eventually resolve a fuzz
issue that breaks v1.5.
Updates #11352
Change-Id: I1b3f93872d06222f9ff5f6fd5580178ebaf4c003
Reviewed-on: https://go-review.googlesource.com/13110
Reviewed-by: Keith Randall <khr@golang.org>
For niceness, when go/exact was moved from x/tools, it
was renamed go/constant.
For simplicity, when go/types was moved from x/tools, its
imports of (now) go/constant were done with a rename:
import exact "go/constant"
This kept the code just as it was before and avoided the issue
of what to call the internal constant called, um, constant.
But not all was hidden, as the text of some fields of structs and
the like leaked the old name, so things like "exact.Value" appeared
in type definitions and function signatures in the documentation.
This is unacceptable.
Fix the documentation issue by fixing the code. Rename the constant
constant constant_, and remove the renaming import.
This should go into 1.5. It's mostly a mechanical change, is
internal to the package, and fixes the documentation. It contains
no semantic changes except to fix a benchmark that was broken
in the original transition.
Fixes#11949.
Change-Id: Ieb94b6558535b504180b1378f19e8f5a96f92d3c
Reviewed-on: https://go-review.googlesource.com/13051
Reviewed-by: Russ Cox <rsc@golang.org>
The DFS scheduler doesn't do the right thing. If a Value x is used by
more than one other Value, then x is put into the DFS queue when
its first user (call it y) is visited. It is not removed and reinserted
when the second user of x (call it z) is visited, so the dependency
between x and z is not respected. There is no easy way to fix this with
the DFS queue because we'd have to rip values out of the middle of the
DFS queue.
The new scheduler works from the end of the block backwards, scheduling
instructions which have had all of their uses already scheduled.
A simple priority scheme breaks ties between multiple instructions that
are ready to schedule simultaneously.
Keep track of whether we've scheduled or not, and make print() use
the scheduled order if we have.
Fix some shift tests that this change tickles. Add unsigned right shift tests.
Change-Id: I44164c10bb92ae8ab8f76d7a5180cbafab826ea1
Reviewed-on: https://go-review.googlesource.com/13069
Reviewed-by: Todd Neal <todd@tneal.org>
Modify tests to use a known value instead of comparing the backends
directly.
Change-Id: I32e804e12515885bd94c4f83644cbca03b018fea
Reviewed-on: https://go-review.googlesource.com/13042
Reviewed-by: Keith Randall <khr@golang.org>
This was confusing when I was trying to fix go build -o.
Perhaps due to that fix, this can now be simplified from
three functions to one.
Change-Id: I878a6d243b14132a631e7c62a3bb6d101bc243ea
Reviewed-on: https://go-review.googlesource.com/13027
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Quoting the new docs:
«
If the arguments to build are a list of .go files, build treats
them as a list of source files specifying a single package.
When compiling a single main package, build writes
the resulting executable to an output file named after
the first source file ('go build ed.go rx.go' writes 'ed' or 'ed.exe')
or the source code directory ('go build unix/sam' writes 'sam' or 'sam.exe').
The '.exe' suffix is added when writing a Windows executable.
When compiling multiple packages or a single non-main package,
build compiles the packages but discards the resulting object,
serving only as a check that the packages can be built.
The -o flag, only allowed when compiling a single package,
forces build to write the resulting executable or object
to the named output file, instead of the default behavior described
in the last two paragraphs.
»
There is a change in behavior here, namely that 'go build -o x.a x.go'
where x.go is not a command (not package main) did not write any
output files (back to at least Go 1.2) but now writes x.a.
This seems more reasonable than trying to explain that -o is
sometimes silently ignored.
Otherwise the behavior is unchanged.
The lines being deleted in goFilesPackage look like they are
setting up 'go build x.o' to write 'x.a', but they were overridden
by the p.target = "" in runBuild. Again back to at least Go 1.2,
'go build x.go' for a non-main package has never produced
output. It seems better to keep it that way than to change it,
both for historical consistency and for consistency with
'go build strings' and 'go build std'.
All of this behavior is now tested.
Fixes#10865.
Change-Id: Iccdf21f366fbc8b5ae600a1e50dfe7fc3bff8b1c
Reviewed-on: https://go-review.googlesource.com/13024
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
It was failing with multiple goroutines a few out of every thousand
runs (with errRequestCanceled) because it was using the same
*http.Request for all 5 RoundTrips, but the RoundTrips' goroutines
(notably the readLoop method) were all still running, sharing that
same pointer. Because the response has no body (which is what
TestZeroLengthPostAndResponse tests), the readLoop was marking the
connection as reusable early (before the caller read until the body's
EOF), but the Transport code was clearing the Request's cancelation
func *AFTER* the caller had already received it from RoundTrip. This
let the test continue looping and do the next request with the same
pointer, fetch a connection, and then between getConn and roundTrip
have an invariant violated: the Request's cancelation func was nil,
tripping this check:
if !pc.t.replaceReqCanceler(req.Request, pc.cancelRequest) {
pc.t.putIdleConn(pc)
return nil, errRequestCanceled
}
The solution is to clear the request cancelation func in the readLoop
goroutine in the no-body case before it's returned to the caller.
This now passes reliably:
$ go test -race -run=TestZeroLengthPostAndResponse -count=3000
I think we've only seen this recently because we now randomize scheduling
of goroutines in race mode (https://golang.org/cl/11795). This race
has existed for a long time but the window was hard to hit.
Change-Id: Idb91c582919f85aef5b9e5ef23706f1ba9126e9a
Reviewed-on: https://go-review.googlesource.com/13070
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Introduced in https://go-review.googlesource.com/12865 (git rev c2db5f4c).
This fix doesn't add any new lock acquistions: it just moves the
existing one taken by the unreadDataSize method and moves it out
wider.
It became flaky at rev c2db5f4c, but now reliably passes again:
$ go test -v -race -run=TestTransportAndServerSharedBodyRace -count=100 net/http
Fixes#11985
Change-Id: I6956d62839fd7c37e2f7441b1d425793f4a0db30
Reviewed-on: https://go-review.googlesource.com/12909
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
HTTP servers attempt to entirely consume a request body before sending a
response. However, when doing so, it previously would ignore any errors
encountered.
Unfortunately, the errors triggered at this stage are indicative of at
least a couple problems: read timeouts and chunked encoding errors.
This means properly crafted and/or timed requests could lead to a
"smuggled" request.
The fix is to inspect the errors created by the response body Reader,
and treat anything other than io.EOF or ErrBodyReadAfterClose as
fatal to the connection.
Fixes#11930
Change-Id: I0bf18006d7d8f6537529823fc450f2e2bdb7c18e
Reviewed-on: https://go-review.googlesource.com/12865
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes the receiver name consistent with the rest of the methods on
type Server.
Change-Id: Ic2a007d3b5eb50bd87030e15405e9856109cf590
Reviewed-on: https://go-review.googlesource.com/13035
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
The test uses external linking mode, which is probably not available
if cgo does not work.
Fixes#11969.
Change-Id: Id1c2828cd2540391e16b422bf51674ba6ff084b0
Reviewed-on: https://go-review.googlesource.com/13005
Reviewed-by: Russ Cox <rsc@golang.org>
The percolation of errors upward in the load process could
drop errors, meaning that a build tree could, depending on the
processing order, import the same directory as both "p/vendor/x"
and as "x". That's not supposed to be allowed. But then, worse,
the build would generate two jobs for building that directory,
which would use the same work space and overwrite each other's files,
leading to very strange failures.
Two fixes:
1. Fix the propagation of errors upward (prefer errors over success).
2. Check explicitly for duplicated packages before starting a build.
New test for #1.
Since #2 can't happen, tested #2 by hand after reverting fix for #1.
Fixes#11913.
Change-Id: I6d2fc65f93b8fb5f3b263ace8d5f68d803a2ae5c
Reviewed-on: https://go-review.googlesource.com/13022
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On most systems, a pointer is the worst case alignment, so adding
a pointer field at the end of a struct guarantees there will be no
padding added after that field (to satisfy overall struct alignment
due to some more-aligned field also present).
In the runtime, the map implementation needs a quick way to
get to the overflow pointer, which is last in the bucket struct,
so it uses size - sizeof(pointer) as the offset.
NaCl/amd64p32 is the exception, as always.
The worst case alignment is 64 bits but pointers are 32 bits.
There's a long history that is not worth going into, but when
we moved the overflow pointer to the end of the struct,
we didn't get the padding computation right.
The compiler computed the regular struct size and then
on amd64p32 added another 32-bit field.
And the runtime assumed it could step back two 32-bit fields
(one 64-bit register size) to get to the overflow pointer.
But in fact if the struct needed 64-bit alignment, the computation
of the regular struct size would have added a 32-bit pad already,
and then the code unconditionally added a second 32-bit pad.
This placed the overflow pointer three words from the end, not two.
The last two were padding, and since the runtime was consistent
about using the second-to-last word as the overflow pointer,
no harm done in the sense of overwriting useful memory.
But writing the overflow pointer to a non-pointer word of memory
means that the GC can't see the overflow blocks, so it will
collect them prematurely. Then bad things happen.
Correct all this in a few steps:
1. Add an explicit check at the end of the bucket layout in the
compiler that the overflow field is last in the struct, never
followed by padding.
2. When padding is needed on nacl (not always, just when needed),
insert it before the overflow pointer, to preserve the "last in the struct"
property.
3. Let the compiler have the final word on the width of the struct,
by inserting an explicit padding field instead of overwriting the
results of the width computation it does.
4. For the same reason (tell the truth to the compiler), set the type
of the overflow field when we're trying to pretend its not a pointer
(in this case the runtime maintains a list of the overflow blocks
elsewhere).
5. Make the runtime use "last in the struct" as its location algorithm.
This fixes TestTraceStress on nacl/amd64p32.
The 'bad map state' and 'invalid free list' failures no longer occur.
Fixes#11838.
Change-Id: If918887f8f252d988db0a35159944d2b36512f92
Reviewed-on: https://go-review.googlesource.com/12971
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This fixes the crypto/subtle tests.
Change-Id: Ie6e721eec3481f67f13de1bfbd7988e227793148
Reviewed-on: https://go-review.googlesource.com/13000
Reviewed-by: Keith Randall <khr@golang.org>
Fixes some minor issues regarding quoted-string when parsing
the local-part.
Those strings should return an error:
- quoted-string without any content: `""@test.com`
- quoted-string containing tab: "\"\t\"@test.com"
Fixes#11293
Change-Id: Ied93eb6831915c9b1f8e727cea14168af21f8d3b
Reviewed-on: https://go-review.googlesource.com/12905
Reviewed-by: Russ Cox <rsc@golang.org>
The code already fixed large non-stack offsets
but explicitly excluded stack references.
Perhaps you could get away with that before,
but current versions of nacl reject such stack
references. Rewrite them the same as the others.
For #11956 but probably not the last problem.
Change-Id: I0db4e3a1ed4f88ccddf0d30228982960091d9fb7
Reviewed-on: https://go-review.googlesource.com/13010
Reviewed-by: Dave Cheney <dave@cheney.net>
Dangling pointer error. Unlikely to trigger in practice, but still.
Found by running GODEBUG=efence=1 GOGC=1 trace.test.
Change-Id: Ice474dedcf62dd33ab77526287a023ba3b166db9
Reviewed-on: https://go-review.googlesource.com/12991
Reviewed-by: Austin Clements <austin@google.com>
In https://golang.org/cl/12080 we forbade installing cross-compiled
binaries into a subdirectory of $GOBIN, in order to fix
https://golang.org/issue/9769. However, that fix was too aggressive,
in that it also forbade installing into a subdirectory of $GOPATH/bin.
This patch permits installing cross-compiled binaries into a
subdirectory $GOPATH/bin while continuing to forbid installing into a
subdirectory of $GOBIN.
Fixes#11778.
Change-Id: Ibc9919554e8c275beff54ec8bf919cfaa03b11ba
Reviewed-on: https://go-review.googlesource.com/12938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The only types that remain in the ssa package
are special compiler-only types.
Change-Id: If957abf128ec0778910d67666c297f97f183b7ee
Reviewed-on: https://go-review.googlesource.com/12933
Reviewed-by: Keith Randall <khr@golang.org>
This should fix the solaris/amd64 builder.
Change-Id: Idd6460cc9e842f7b874c9757379986aa723c974c
Reviewed-on: https://go-review.googlesource.com/12922
Reviewed-by: Austin Clements <austin@google.com>
Fixes#11918
Replace calls to lchown(2) with fchownat(2) for linux/arm64 as the former is not suppored.
This change has also landed on the x/sys repo as CL 12837.
Change-Id: I58d4b144e051e36dd650ec9b7f3a02610ea943e5
Reviewed-on: https://go-review.googlesource.com/12833
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Russ Cox <rsc@golang.org>
From compiling go there were 260 functions where XOR was needed.
Much of the required changes for implementing XOR were already
done in 12813.
Change-Id: I5a68aa028f5ed597bc1d62cedbef3620753dfe82
Reviewed-on: https://go-review.googlesource.com/12901
Reviewed-by: Keith Randall <khr@golang.org>
The existing backend simply elides OCONVNOP.
There's no reason for us to do any differently.
Rather than insert ConvNops and then rewrite them
away, stop creating them in the first place.
Change-Id: I4bcbe2229fcebd189ae18df24f2c612feb6e215e
Reviewed-on: https://go-review.googlesource.com/12810
Reviewed-by: Keith Randall <khr@golang.org>
This only triggers on ARMv7+.
If there are important SMP ARMv6 machines we can reconsider.
Makes TestLFStress tests pass and sync/atomic tests not time out
on Apple iPad Mini 3.
Fixes#7977.
Fixes#10189.
Change-Id: Ie424dea3765176a377d39746be9aa8265d11bec4
Reviewed-on: https://go-review.googlesource.com/12950
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Was not allocating space for the frame above sigpanic,
nor was it pushing the LR into the right place.
Because traceback past sigpanic only needs the
LR for faulting leaves, this was not noticed too much.
But it did break the sync/atomic nil deref tests.
Change-Id: Icba53fffa193423aab744c37f21ee893ce2ee3ac
Reviewed-on: https://go-review.googlesource.com/12926
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Convert shift ops to also encode the size of the shift amount.
Change signed right shift from using CMOV to using bit twiddles.
It is a little bit better (5 instructions instead of 4, but fewer
bytes and slightly faster code). It's also a bit faster than
the 4-instruction branch version, even with a very predictable
branch. As tested on my machine, YMMV.
Implement OCOM while we are here.
Change-Id: I8ca12dd62fae5d626dc0e6da5d4bbd34fd9640d2
Reviewed-on: https://go-review.googlesource.com/12867
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
ODOTTYPE should be treated a whole lot like ODOT,
but it was missing completely from the switch in
escwalk and thus escape status did not propagate
to fields.
Since interfaces are required to trigger this bug,
the test was added to escape_iface.go.
Fixes#11931.
Change-Id: Id0383981cc4b1a160f6ad447192a112eed084538
Reviewed-on: https://go-review.googlesource.com/12921
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
There is absolutely no information about how this was failing.
If we reenable the test then at least we can get a build log from
darwin/arm.
There are not even freebsd/arm or netbsd/arm builders,
so not too worried about those. (That is another problem.)
Change-Id: I0e739a4dd2897adbe110aa400d720d8fa02ae65f
Reviewed-on: https://go-review.googlesource.com/12920
Reviewed-by: Russ Cox <rsc@golang.org>
Instead of pushing the denominator argument on the stack,
the denominator is now passed in m.
This fixes a variety of bugs related to trying to take stack traces
backwards from the middle of the software div/mod routines.
Some of those bugs have been kludged around in the past,
but others have not. Instead of trying to patch up after breaking
the stack, this CL stops breaking the stack.
This is an update of https://golang.org/cl/19810043,
which was rolled back in https://golang.org/cl/20350043.
The problem in the original CL was that there were divisions
at bad times, when m was not available. These were divisions
by constant denominators, either in C code or in assembly.
The Go compiler knows how to generate division by multiplication
for constant denominators, but the C compiler did not.
There is no longer any C code, so that's taken care of.
There was one problematic DIV in runtime.usleep (assembly)
but https://golang.org/cl/12898 took care of that one.
So now this approach is safe.
Reject DIV/MOD in NOSPLIT functions to keep them from
coming back.
Fixes#6681.
Fixes#6699.
Fixes#10486.
Change-Id: I09a13c76ad08ba75b3bd5d46a3eb78e66a84ab38
Reviewed-on: https://go-review.googlesource.com/12899
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In order to fix issue #9401 the compiler was changed to add a padding
byte to any non-empty Go struct that ends in a zero-sized field. That
causes the Go version of such a C struct to have a different size than
the C struct, which can considerable confusion. Change cgo so that it
discards any such zero-sized fields, so that the Go and C structs are
the same size.
This is a change from previous releases, in that it used to be
possible to refer to a zero-sized trailing field (by taking its
address), and with this change it no longer is. That is unfortunate,
but something has to change. It seems better to visibly break
programs that do this rather than to silently break programs that rely
on the struct sizes being the same.
Update #9401.
Fixes#11925.
Change-Id: I3fba3f02f11265b3c41d68616f79dedb05b81225
Reviewed-on: https://go-review.googlesource.com/12864
Reviewed-by: Russ Cox <rsc@golang.org>
We want to adjust the DIV calling convention to use m,
and usleep can be called without an m, so switch to a
multiplication by the reciprocal (and test).
Step toward a fix for #6699 and #10486.
Change-Id: Iccf76a18432d835e48ec64a2fa34a0e4d6d4b955
Reviewed-on: https://go-review.googlesource.com/12898
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If a function is large enough to need to flush the constant pool
mid-function, the line number assignment code was forcing the
line numbers not just for the constant pool but for all the instructions
that follow it. This made the line number information completely
wrong for all but the beginning of large functions on arm.
Same problem in code copied into arm64.
This broke runtime/trace's TestTraceSymbolize.
Fixes arm build.
Change-Id: I84d9fb2c798c4085f69b68dc766ab4800c7a6ca4
Reviewed-on: https://go-review.googlesource.com/12894
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This allows running a cross-compile like
GOOS=darwin GOARCH=arm go build std
to check that everything builds.
Otherwise there is a redefinition error because both
root_nocgo_darwin.go and root_darwin_armx.go
supply initSystemRoots.
Change-Id: Ic95976b2b698d28c629bfc93d8dac0048b023578
Reviewed-on: https://go-review.googlesource.com/12897
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The test expects the dial to take 1.0 seconds
on Windows and allows it to go to 1.095 seconds.
That's far too optimistic.
Recent failures are reporting roughly 1.2 seconds.
Let it have 1.5.
Change-Id: Id69811ccb65bf4b4c159301a2b4767deb6ee8d28
Reviewed-on: https://go-review.googlesource.com/12895
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Urge users of math/rand to consider using crypto/rand when doing
security-sensitive work.
Related to issue #11871. While we haven't reached consensus on how
to make the package inherently safer, everyone agrees that the docs
for math/rand can be improved.
Change-Id: I576a312e51b2a3445691da6b277c7b4717173197
Reviewed-on: https://go-review.googlesource.com/12900
Reviewed-by: Rob Pike <r@golang.org>
For the android/arm builder.
Change-Id: Iad4881689223cd6479870da9541524a8cc458cce
Reviewed-on: https://go-review.googlesource.com/12859
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Fixes arm64 builder crash.
The bug is possible on all architectures; you just have to get lucky
and hit a preemption or a stack growth on entry to assertE2I2.
The test stacks the deck.
Change-Id: I8419da909b06249b1ad15830cbb64e386b6aa5f6
Reviewed-on: https://go-review.googlesource.com/12890
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It says to disable until #7564 is fixed. It was fixed in April 2014.
Change-Id: I9bebfe96802bafdd2d1a0a47591df346d91b000c
Reviewed-on: https://go-review.googlesource.com/12858
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also make invalidptr control the recently added GC pointer check,
as documented.
Change-Id: Iccfdf49480219d12be8b33b8f03d8312d8ceabed
Reviewed-on: https://go-review.googlesource.com/12857
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The skips added in CL 12579, based on incorrect time stamps,
should be sufficient to identify and exclude all the time-related
flakiness on these systems.
If there is other flakiness, we want to find out.
For #10512.
Change-Id: I5b588ac1585b2e9d1d18143520d2d51686b563e3
Reviewed-on: https://go-review.googlesource.com/12746
Reviewed-by: Austin Clements <austin@google.com>
Nearly all the flaky failures we've seen in trace tests have been
due to the use of time stamps to determine relative event ordering.
This is tricky for many reasons, including:
- different cores might not have exactly synchronized clocks
- VMs are worse than real hardware
- non-x86 chips have different timer resolution than x86 chips
- on fast systems two events can end up with the same time stamp
Stop trying to make time reliable. It's clearly not going to be for Go 1.5.
Instead, record an explicit event sequence number for ordering.
Using our own counter solves all of the above problems.
The trace still contains time stamps, of course. The sequence number
is just used for ordering.
Should alleviate #10554 somewhat. Then tickDiv can be chosen to
be a useful time unit instead of having to be exact for ordering.
Separating ordering and time stamps lets the trace parser diagnose
systems where the time stamp order and actual order do not match
for one reason or another. This CL adds that check to the end of
trace.Parse, after all other sequence order-based checking.
If that error is found, we skip the test instead of failing it.
Putting the check in trace.Parse means that cmd/trace will pick
up the same check, refusing to display a trace where the time stamps
do not match actual ordering.
Using net/http's BenchmarkClientServerParallel4 on various CPU counts,
not tracing vs tracing:
name old time/op new time/op delta
ClientServerParallel4 50.4µs ± 4% 80.2µs ± 4% +59.06% (p=0.000 n=10+10)
ClientServerParallel4-2 33.1µs ± 7% 57.8µs ± 5% +74.53% (p=0.000 n=10+10)
ClientServerParallel4-4 18.5µs ± 4% 32.6µs ± 3% +75.77% (p=0.000 n=10+10)
ClientServerParallel4-6 12.9µs ± 5% 24.4µs ± 2% +89.33% (p=0.000 n=10+10)
ClientServerParallel4-8 11.4µs ± 6% 21.0µs ± 3% +83.40% (p=0.000 n=10+10)
ClientServerParallel4-12 14.4µs ± 4% 23.8µs ± 4% +65.67% (p=0.000 n=10+10)
Fixes#10512.
Change-Id: I173eecf8191e86feefd728a5aad25bf1bc094b12
Reviewed-on: https://go-review.googlesource.com/12579
Reviewed-by: Austin Clements <austin@google.com>
Otherwise the GC may see uninitialized memory there,
which might be old pointers that are retained, or it might
trigger the invalid pointer check.
Fixes#11907.
Change-Id: I67e306384a68468eef45da1a8eb5c9df216a77c0
Reviewed-on: https://go-review.googlesource.com/12852
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Lots and lots of ops!
Also XOR for good measure.
Add a pass to the compiler generator to check that all of the
architecture-specific opcodes are handled by genValue. We will
catch any missing ones if we come across them during compilation,
but probably better to catch them statically.
Change-Id: Ic4adfbec55c8257f88117bc732fa664486262868
Reviewed-on: https://go-review.googlesource.com/12813
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The last time we tried this, linux/arm64 broke.
The series of CLs leading to this one fixes that problem.
Let's try again.
Fixes#9880.
Change-Id: I67bc1d959175ec972d4dcbe4aa6f153790f74251
Reviewed-on: https://go-review.googlesource.com/12849
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The layout code has to date insisted on stack frames that are 16-aligned
including the saved LR, and it ensured this by growing the frame itself.
This breaks code that refers to values near the top of the frame by positive
offset from SP, and in general it's too magical: if you see TEXT xxx, $N,
you expect that the frame size is actually N, not sometimes N and sometimes N+8.
This led to a serious bug in the compiler where ambiguously live values
were not being zeroed correctly, which in turn triggered an assertion
in the GC about finding only valid pointers. The compiler has been
fixed to always emit aligned frames, and the hand-written assembly
has also been fixed.
Now that everything is aligned, make unaligned an error instead of
something to "fix" silently.
For #9880.
Change-Id: I05f01a9df174d64b37fa19b36a6b6c5f18d5ba2d
Reviewed-on: https://go-review.googlesource.com/12848
Reviewed-by: Austin Clements <austin@google.com>
The nosplit stack overflow checks were confused about morestack.
The comment about not having correct SP information at the call
to morestack was true, but that was a real bug, not something to
work around. I fixed that problem in CL 12144. With that fixed,
no need to special-case morestack in the way done here.
This cleanup and simplification of the code was the first step
to fixing a bug that happened when I started working on the
arm64 frame size adjustments, but the cleanup was sufficient
to make the bug go away.
For #9880.
Change-Id: I16b69a5c16b6b8cb4090295d3029c42d606e3b9b
Reviewed-on: https://go-review.googlesource.com/12846
Reviewed-by: Austin Clements <austin@google.com>
arm64 requires either no stack frame or a frame with a size that is 8 mod 16
(adding the saved LR will make it 16-aligned).
The cmd/internal/obj/arm64 has been silently aligning frames, but it led to
a terrible bug when the compiler and obj disagreed on the frame size,
and it's just generally confusing, so we're going to make misaligned frames
an error instead of something that is silently changed.
This CL prepares by updating assembly files.
Note that the changes in this CL are already being done silently by
cmd/internal/obj/arm64, so there is no semantic effect here,
just a clarity effect.
For #9880.
Change-Id: Ibd6928dc5fdcd896c2bacd0291bf26b364591e28
Reviewed-on: https://go-review.googlesource.com/12845
Reviewed-by: Austin Clements <austin@google.com>
If the compiler doesn't do it, cmd/internal/obj/arm64 will,
and that will break the zeroing of ambiguously live values
done in zerorange, which in turn produces uninitialized
pointer cells that the GC trips over.
For #9880.
Change-Id: Ice97c30bc8b36d06b7b88d778d87fab8e1827fdc
Reviewed-on: https://go-review.googlesource.com/12847
Reviewed-by: Austin Clements <austin@google.com>
From compiling go there were 761 functions where OR was needed.
Change-Id: Ied8bf59cec50a3175273387bc7416bd042def6d8
Reviewed-on: https://go-review.googlesource.com/12766
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This adds a GCCPUFraction field to MemStats that reports the
cumulative fraction of the program's execution time spent in the
garbage collector. This is equivalent to the utilization percent shown
in the gctrace output and makes this available programmatically.
This does make one small effect on the gctrace output: we now report
the duration of mark termination up to just before the final
start-the-world, rather than up to just after. However, unlike
stop-the-world, I don't believe there's any way that start-the-world
can block, so it should take negligible time.
While there are many statistics one might want to expose via MemStats,
this is one of the few that will undoubtedly remain meaningful
regardless of future changes to the memory system.
The diff for this change is larger than the actual change. Mostly it
lifts the code for computing the GC CPU utilization out of the
debug.gctrace path.
Updates #10323.
Change-Id: I0f7dc3fdcafe95e8d1233ceb79de606b48acd989
Reviewed-on: https://go-review.googlesource.com/12844
Reviewed-by: Russ Cox <rsc@golang.org>
Currently we only capture GC phase transition times if
debug.gctrace>0, but we're about to compute GC CPU utilization
regardless of whether debug.gctrace is set, so we need these
regardless of debug.gctrace.
Change-Id: If3acf16505a43d416e9a99753206f03287180660
Reviewed-on: https://go-review.googlesource.com/12843
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The following sequence of events can lead to the runtime attempting an
out-of-bounds access on a stack barrier slice:
1. A SIGPROF comes in on a thread while the G on that thread is in
_Gsyscall. The sigprof handler calls gentraceback, which saves a
local copy of the G's stkbar slice. Currently the G has no stack
barriers, so this slice is empty.
2. On another thread, the GC concurrently scans the stack of the
goroutine being profiled (it considers it stopped because it's in
_Gsyscall) and installs stack barriers.
3. Back on the sigprof thread, gentraceback comes across a stack
barrier in the stack and attempts to look it up in its (zero
length) copy of G's old stkbar slice, which causes an out-of-bounds
access.
This commit fixes this by adding a simple cas spin to synchronize the
SIGPROF handler with stack barrier insertion.
In general I would prefer that this synchronization be done through
the G status, since that's how stack scans are otherwise synchronized,
but adding a new lock is a much smaller change and G statuses are full
of subtlety.
Fixes#11863.
Change-Id: Ie89614a6238bb9c6a5b1190499b0b48ec759eaf7
Reviewed-on: https://go-review.googlesource.com/12748
Reviewed-by: Russ Cox <rsc@golang.org>
The scheduler, work buffer's dispose, and write barriers
can conspire to hide the a pointer from the GC's concurent
mark phase. If this pointer is the only path to a large
amount of marking the STW mark termination phase may take
a lot of time.
Consider the following:
1) dispose places a work buffer on the partial queue
2) the GC is busy so it does not immediately remove and
process the work buffer
3) the scheduler runs a mutator whose write barrier dequeues the
work buffer from the partial queue so the GC won't see it
This repeats until the GC reaches the mark termination
phase where the GC finally discovers the pointer along
with a lot of work to do.
This CL fixes the problem by having the mutator
dispose of the buffer to the full queue instead of
the partial queue. Since the write buffer never asks for full
buffers the conspiracy described above is not possible.
Updates #11694.
Change-Id: I2ce832f9657a7570f800e8ce4459cd9e304ef43b
Reviewed-on: https://go-review.googlesource.com/12840
Reviewed-by: Austin Clements <austin@google.com>
These are the old assemblers written in C, and now they are
not needed.
Fixes#10510.
Change-Id: Id9337ffc8eccfd93c84b2e23f427fb1a576b543d
Reviewed-on: https://go-review.googlesource.com/12784
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
At this stage, dist is only building go_bootstrap as cmd/compile and
the rest of the Go toolchain has already been built.
Change-Id: I6f99fa00ff1d3585e215f4ce84d49344c4fcb8a5
Reviewed-on: https://go-review.googlesource.com/12779
Reviewed-by: Russ Cox <rsc@golang.org>
With this, all non-float, non-complex
binary ops found in the standard library
are implemented.
Change-Id: I6087f115229888c0dce10ab35db3fd36a0e0a8b1
Reviewed-on: https://go-review.googlesource.com/12799
Reviewed-by: Keith Randall <khr@golang.org>
