Before this CL we accepted timeouts in TestUDPZeroBytePayload to avoid
flakiness and because, according to CL 9194, the test didn't work on
some platforms. On Windows, before CL 132781, the read would always
timeout, and so since the test accepted timeouts it would pass
incorrectly. CL 132781 fixed Windows, and changed the test to not
accept timeouts in the ReadFrom case.
However, the timeout was short, and so on a loaded system the Read
might timeout not due to an error in the code, but just because the
read was not delivered. So ignoring timeouts made the test flaky, as
reported in issue #29225.
This CL tries to get to a better state by increasing the timeout to a
large value and not permitting timeouts at all. If there are systems
where the test fails, we will need to explicitly skip the test on
those systems.
Fixes#29225
Change-Id: I26863369898a69cac866b34fcb5b6ffbffab31f6
Reviewed-on: https://go-review.googlesource.com/c/154759
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If TMP environment variable is set to Z:\, TempDir returns Z:.
But Z: refers to current directory on Z:, while Z:\ refers to root
directory on Z:. Adjust TempDir to return Z:\.
Fixes#29291
Change-Id: If04d0c7977a8ac2d9d558307502e81beb68776ef
Reviewed-on: https://go-review.googlesource.com/c/154384
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When a locked M has its G exit without calling UnlockOSThread, then
lockedExt on it was getting cleared. Unfortunately, this meant that
during P handoff, if a new M was started, it might get forked (on
most OSs besides Windows) from the locked M, which could have kernel
state attached to it.
To solve this, just don't clear lockedExt. At the point where the
locked M has its G exit, it will also exit in accordance with the
LockOSThread API. So, we can safely assume that it's lockedExt state
will no longer be used. For the case of the main thread where it just
gets wedged instead of exiting, it's probably better for it to keep
the locked marker since it more accurately represents its state.
Fixed#28979.
Change-Id: I7d3d71dd65bcb873e9758086d2cbcb9a06429b0f
Reviewed-on: https://go-review.googlesource.com/c/153078
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
This change disables the test TestArenaCollision on Darwin in race mode
to deal with the fact that Darwin 10.10 must use MAP_FIXED in race mode
to ensure we retain our heap in a particular portion of the address
space which the race detector needs. The test specifically checks to
make sure a manually mapped region's space isn't re-used, which is
definitely possible with MAP_FIXED because it replaces whatever mapping
already exists at a given address.
This change then also makes it so that MAP_FIXED is only used in race
mode and on Darwin, not all BSDs, because using MAP_FIXED breaks this
test for FreeBSD in addition to Darwin.
Updates #26475.
Fixes#29340.
Change-Id: I1c59349408ccd7eeb30c4bf2593f48316b23ab2f
Reviewed-on: https://go-review.googlesource.com/c/155097
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts change https://golang.org/cl/154758.
Restore the previous implementations of nanotime and time.now, which
are sufficiently high resolution and more efficient than
QueryPerformanceCounter. The intent of the change was to improve
resolution of tracing timestamps, but the change was overly broad
as it was only necessary to fix cputicks(). cputicks() is fixed in
a subsequent change.
Updates #26148
Change-Id: Ib9883d02fe1af2cc4940e866d8f6dc7622d47781
Reviewed-on: https://go-review.googlesource.com/c/154761
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix two typos and don't indent the go vet example.
Change-Id: Iccec56ca5decfbae45547a00115500ed13b703e1
Reviewed-on: https://go-review.googlesource.com/c/154721
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This was accidentally broken by CL 127755.
Fixes#29333
Change-Id: I5e92048c64a55c1699d6c38eb4dbbd51c817b820
Reviewed-on: https://go-review.googlesource.com/c/155037
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
startpanic_m could be called correctly in a context where there's a
valid G, a valid M, but no P, for example in a signal handler which
panics. Currently, startpanic_m has write barriers enabled because
write barriers are permitted if a G's M is dying. However, all the
current write barrier implementations assume the current G has a P.
Therefore, in this change we disable write barriers in startpanic_m,
remove the only pointer write which clears g.writebuf, and fix up gwrite
to ignore the writebuf if the current G's M is dying, rather than
relying on it being nil in the dying case.
Fixes#26575.
Change-Id: I9b29e6b9edf00d8e99ffc71770c287142ebae086
Reviewed-on: https://go-review.googlesource.com/c/154837
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The previous implementation of nanotime and time.now used a time source
that was updated on the system clock tick, which has a maximum
resolution of about 1ms. On 386 and amd64, this time source maps to
the system performance counter, so has much higher resolution.
On ARM, use QueryPerformanceCounter() to get a high resolution timestamp.
Updates #26148
Change-Id: I1abc99baf927a95b472ac05020a7788626c71d08
Reviewed-on: https://go-review.googlesource.com/c/154758
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Permission bits are most commonly viewed in string form (rwx-- etc) or
in octal form (0755), but the latter is relatively rare in Go.
Demonstrate how to print a FileMode in readable octal format.
Change-Id: I41feb801bcecb5077d4eabafdea27c149fc179a1
Reviewed-on: https://go-review.googlesource.com/c/154423
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change makes it so that reserving more of the address space for the
heap calls mmap with MAP_FIXED in race mode. Race mode requires certain
guarantees on where the heap is located in the address space, and on
Darwin 10.10 it appears that the kernel may end up ignoring the hint
quite often (#26475). Using MAP_FIXED is relatively OK in race mode
because nothing else should be mapped in the memory region provided by
the initial hints.
Fixes#26475.
Change-Id: Id7ac1534ee74f6de491bc04441f27dbda09f0285
Reviewed-on: https://go-review.googlesource.com/c/153897
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When using //line directives and unformatted code it is possible for
positions to repeat. Increment the final column position to avoid that.
Fixes#27350
Change-Id: I2faccc31360075e9814d4a024b0f98b117f8ce97
Reviewed-on: https://go-review.googlesource.com/c/153061
Run-TryBot: Rob Pike <r@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This ensures that "go test cmd/cover" tests the current cover program,
not the installed cover program.
Change-Id: I58e718ded7eb1cd8da448d0194262209bb025b20
Reviewed-on: https://go-review.googlesource.com/c/153058
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 138675 added the race detector support on Linux/ARM64, but it
didn't enable the race detector tests in cmd/dist (therefore in
all.bash). Enable them.
Updates #28848
Change-Id: I4306dad2fb4167021d568436076b9f535d7f6e07
Reviewed-on: https://go-review.googlesource.com/c/149967
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also update a Go 1 compatibility promise link to canonical URL.
Updates #27592
Updates #28264
Change-Id: I5994a0a63e0870c1795c65016590dfad829d26a7
Reviewed-on: https://go-review.googlesource.com/c/154618
Reviewed-by: Richard Musiol <neelance@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit fixes backtrace if a crash or an exit signal is received
during a C syscall on aix/ppc64.
This is similar to Solaris, Darwin or Windows implementation.
Change-Id: I6040c0b1577a9f5b298f58bd4ee6556258a135ef
Reviewed-on: https://go-review.googlesource.com/c/154718
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, we flush the write barrier buffer on every write barrier
once throwOnGCWork is set, but not during the mark completion
algorithm itself. As seen in recent failures like
https://build.golang.org/log/317369853b803b4ee762b27653f367e1aa445ac1
by the time we actually catch a late gcWork put, the write barrier
buffer is full-size again.
As a result, we're probably not catching the actual problematic write
barrier, which is probably somewhere in the buffer.
Fix this by using the gcWork pause generation to also keep the write
barrier buffer small between the mark completion flushes it and when
mark completion is done.
For #27993.
Change-Id: I77618169441d42a7d562fb2a998cfaa89891edb2
Reviewed-on: https://go-review.googlesource.com/c/154638
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Add support for cgo on openbsd/arm.The gcc shipped with base OpenBSD armv7
is old/inadequate, so use clang by default.
Change-Id: I945a26d369378952d357727718e69249411e1127
Reviewed-on: https://go-review.googlesource.com/c/154381
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The formatting routines for types use a depth limit as primitive
mechanism to detect cycles. For now, increase the limit from 100
to 250 and file #29312 so we don't drop this on the floor.
Also, adjust some fatal error messages elsewhere to use
better formatting.
Fixes#29264.
Updates #29312.
Change-Id: Idd529f6682d478e0dcd2d469cb802192190602f6
Reviewed-on: https://go-review.googlesource.com/c/154583
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change modifies the behavior of span allocations to no longer
prefer the free treap over the scavenged treap.
While there is an additional cost to allocating out of the scavenged
treap, the current behavior of preferring the unscavenged spans can
lead to unbounded growth of a program's virtual memory footprint.
In small programs (low # of Ps, low resident set size, low allocation
rate) this behavior isn't really apparent and is difficult to
reproduce.
However, in relatively large, long-running programs we see this
unbounded growth in free spans, and an unbounded amount of heap
growths.
It still remains unclear how this policy change actually ends up
increasing the number of heap growths over time, but switching the
policy back to best-fit does indeed solve the problem.
Change-Id: Ibb88d24f9ef6766baaa7f12b411974cc03341e7b
Reviewed-on: https://go-review.googlesource.com/c/148979
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This change adds the treapIter type which provides an iterator
abstraction for walking over an mTreap. In particular, the mTreap type
now has iter() and rev() for iterating both forwards (smallest to
largest) and backwards (largest to smallest). It also has an erase()
method for erasing elements at the iterator's current position.
For #28479.
While the expectation is that this change will slow down Go programs,
the impact on Go1 and Garbage is negligible.
Go1: https://perf.golang.org/search?q=upload:20181214.6
Garbage: https://perf.golang.org/search?q=upload:20181214.11
Change-Id: I60dbebbbe73cbbe7b78d45d2093cec12cc0bc649
Reviewed-on: https://go-review.googlesource.com/c/151537
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
This commit move data addresses to 0x200000000 for XCOFF executables.
.data and .bss must always be position-independent on AIX. This
modification allows to detect more easily if they aren't, as segfault
will be triggered.
Change-Id: Ied7a5b72b9f4ff9f870a1626cf07c48110635e62
Reviewed-on: https://go-review.googlesource.com/c/151040
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When a println arg contains a call to an inlineable function
that itself contains a switch, that switch statement will be
walked twice, once by the walkexprlist formerly in the
OPRINT/OPRINTN case, then by walkexprlistcheap in walkprint.
Remove the first walkexprlist, it is not necessary.
walkexprlist =
s[i] = walkexpr(s[i], init)
walkexprlistcheap = {
s[i] = cheapexpr(n, init)
s[i] = walkexpr(s[i], init)
}
Seems like this might be possible in other places, i.e.,
calls to inlineable switch-containing functions.
See also #25776.
Fixes#29220.
Change-Id: I3781e86aad6688711597b8bee9bc7ebd3af93601
Reviewed-on: https://go-review.googlesource.com/c/154497
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
sysUsed on Windows cares about the result from the VirtualAlloc syscall
returning exactly the address that was passed to it. However,
VirtualAlloc aligns the address its given to the kernel's allocation
granularity, so the returned address may not be the same.
Note that this wasn't an issue in the past because we only sysUsed
regions owned by spans, and spans are always a multiple of 8K, which
is a multiple of the allocation granularity on most Windows machines.
Change-Id: I3f5ccd63c6bbbd8b7995945ecedee17573b31667
Reviewed-on: https://go-review.googlesource.com/c/153677
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
This captures the stack trace where mark completion observed that each
P had no work, and then dumps this if that P later discovers more
work. Hopefully this will help bound where the work was created.
For #27993.
Change-Id: I4f29202880d22c433482dc1463fb50ab693b6de6
Reviewed-on: https://go-review.googlesource.com/c/154599
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Many of the crashes observed in #27993 involve committing the new
_defer object at the end of newdefer. It would be helpful to know if
the _defer was just allocated or was retrieved from the defer pool. In
order to indicate this in the traceback, this CL duplicates the tail
of newdefer so that the PC/line number will tell us whether d is new
or not.
For #27993.
Change-Id: Icd3e23dbcf00461877bb082b6f18df701149a607
Reviewed-on: https://go-review.googlesource.com/c/154598
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently we only know the slot address and the value being written in
the throwOnGCWork crash tracebacks, and we have to infer the old value
from what's dumped by gcWork.checkPut. Sometimes these old values
don't make sense, like when we see a write of a nil pointer to a
freshly-allocated object, yet we observe marking a value (where did
that pointer come from?).
This CL adds the old value of the slot and the first two pointers in
the buffer to the traceback.
For #27993.
Change-Id: Ib70eead1afb9c06e8099e520172c3a2acaa45f80
Reviewed-on: https://go-review.googlesource.com/c/154597
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This commit updates the new symbol addressing made for aix/ppc64 according
to feedbacks given in CL 151039.
Change-Id: Ic4eb9943dc520d65f7d084adf8fa9a2530f4d3f9
Reviewed-on: https://go-review.googlesource.com/c/151302
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A prior optimization (https://golang.org/cl/106175) removed the
generation of unnecessary method expression wrappers, but also
eliminated the generation of the wrapper for error.Error which
was still required.
Special-case error type in the optimization.
Fixes#29304.
Change-Id: I54c8afc88a2c6d1906afa2d09c68a0a3f3e2f1e3
Reviewed-on: https://go-review.googlesource.com/c/154578
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently we reset the write barrier buffer before processing the
pointers in it. As a result, if there were any write barriers in the
code that processes the buffer, it would corrupt the write barrier
buffer and cause us to mark objects without later scanning them.
As far as I can tell, this shouldn't be happening, but rather than
relying on hope (and incomplete static analysis), this CL changes
wbBufFlush1 to poison the write barrier buffer while processing it,
and only reset it once it's done.
Updates #27993. (Unlike many of the other changes for this issue,
there's no need to roll back this CL. It's a good change in its own
right.)
Change-Id: I6d2d9f1b69b89438438b9ee624f3fff9f009e29d
Reviewed-on: https://go-review.googlesource.com/c/154537
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
It appears calling GetFileInformationByHandleEx with
FILE_ATTRIBUTE_TAG_INFO fails on FAT file system. FAT does not
support symlinks, so assume there are no symlnks when
GetFileInformationByHandleEx returns ERROR_INVALID_PARAMETER.
Fixes#29214
Change-Id: If2d9f3288bd99637681ab5fd4e4581c77b578a69
Reviewed-on: https://go-review.googlesource.com/c/154377
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The OpenBSD armv7 port has working VFPv3 these days - re-enable the VFP
detection code so that GOARM=7 is used by default on openbsd/arm.
Change-Id: I0271d81c048d2d55becd2803c19e5f1542076357
Reviewed-on: https://go-review.googlesource.com/c/154378
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This fixes a few different issues that led to hangs and general
flakiness in the TestDebugCall* tests.
1. This fixes missing wake-ups in two error paths of the SIGTRAP
signal handler. If the goroutine was in an unknown state, or if
there was an unknown debug call status, we currently don't wake the
injection coordinator. These are terminal states, so this resulted
in a hang.
2. This adds a retry if the target goroutine is in a transient state
that prevents us from injecting a call. The most common failure
mode here is that the target goroutine is in _Grunnable, but this
was previously masked because it deadlocked the test.
3. Related to 2, this switches the "ready" signal from the target
goroutine from a blocking channel send to a non-blocking channel
send. This makes it much less likely that we'll catch this
goroutine while it's in the runtime performing that send.
4. This increases GOMAXPROCS from 2 to 8 during these tests. With the
current setting of 2, we can have at most the non-preemptible
goroutine we're injecting a call in to and the goroutine that's
trying to make it exit. If anything else comes along, it can
deadlock. One particular case I observed was in TestDebugCallGC,
where runtime.GC() returns before the forEachP that prepares
sweeping on all goroutines has finished. When this happens, the
forEachP blocks on the non-preemptible loop, which means we now
have at least three goroutines that need to run.
Fixes#25519.
Updates #29124.
Change-Id: I7bc41dc0b865b7d0bb379cb654f9a1218bc37428
Reviewed-on: https://go-review.googlesource.com/c/154112
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
