We still don't understand what's causing there to be remaining GC work
when we enter mark termination, but in order to move forward on this
issue, this CL implements a work-around for the problem.
If debugCachedWork is false, this CL does a second check for remaining
GC work as soon as it stops the world for mark termination. If it
finds any work, it starts the world again and re-enters concurrent
mark. This will increase STW time by a small amount proportional to
GOMAXPROCS, but fixes a serious correctness issue.
This works-around #27993.
Change-Id: Ia23b85dd6c792ee8d623428bd1a3115631e387b8
Reviewed-on: https://go-review.googlesource.com/c/156140
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Rick Hudson <rlh@golang.org>
This test case failed on the longtest builder. It relied on
runtime/internal/atomic not being compiled with the -l flag in the
cache. The test case now creates its own GOCACHE, similar to
build_cache_compile and a few others.
Also, mention the correct issue the test case verifies.
Fixes#29395
Change-Id: Id50e9dfc50db03fb11582d3dd6b69c3e1ed750eb
Reviewed-on: https://go-review.googlesource.com/c/156237
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
As a followon to CL 152537, modify the panic-printing traceback
to also handle mid-stack inlining correctly.
Also declare -fm functions (aka method functions) as wrappers, so that
they get elided during traceback. This fixes part 2 of #26839.
Fixes#28640Fixes#24488
Update #26839
Change-Id: I1c535a9b87a9a1ea699621be1e6526877b696c21
Reviewed-on: https://go-review.googlesource.com/c/153477
Reviewed-by: David Chase <drchase@google.com>
Use the length of the bitmap to decide how much to pass to the
write barrier, not the total length of the arguments.
The test needs enough arguments so that two distinct bitmaps
get interpreted as a single longer bitmap.
Update #29362
Change-Id: I78f3f7f9ec89c2ad4678f0c52d3d3def9cac8e72
Reviewed-on: https://go-review.googlesource.com/c/156123
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
After CL 31455, "go fun(n)" may put "n" to write barrier buffer
when there are no pointers in fun's arguments.
Fixes#29362
Change-Id: Icfa42b8759ce8ad9267dcb3859c626feb6fda381
Reviewed-on: https://go-review.googlesource.com/c/155779
Reviewed-by: Keith Randall <khr@golang.org>
In the twoNonZero function in hash_test, the buffer is sliced as [:] three times. This change deletes them.
Change-Id: I0701d0c810b4f3e267f80133a0dcdb4ed81fe356
Reviewed-on: https://go-review.googlesource.com/c/156138
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes bug introduced by https://golang.org/cl/129059 where
gcflags='all=...' and ldflags='all=...' would not be applied to some
packages built by 'go test'.
LoadImport used to set gcflags/ldflags for the Package objects it
created, in https://golang.org/cl/129059 this code was factored out to
setToolFlags. The codepath of `go build` was updated to call
setToolFlags appropriatley, but the codepath of `go test -c` wasn't,
resulting in gcflags/ldflags being applied inconsistently when building
tests.
This commit changes TestPackagesFor to call setToolFlags on the package
objects it creates.
Fixes#27681
Change-Id: Idcbec0c989ee96ec066207184611f08818873e8d
Reviewed-on: https://go-review.googlesource.com/c/136275
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
CL 152537 introduced a new use for ginsnop, the arch-dependent
routine that generates nops. The previous s390x nop clobbered flags.
It turns out the previous uses of this nop did not require flags
to be preserved, but the new use does.
Use a real nop: the 4-byte preferred nop.
Fixes#29453
Change-Id: I95310dfdd831932e26f5d5b6608324687f4c3162
Reviewed-on: https://go-review.googlesource.com/c/155926
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Recent changes to compiler backtraces perturbed the line
number assignment, some better, some worse, probably net
worse. For purposes of passing the long tests, update the
reference files (delve's file was also stale).
TODO: Figure out a less delicate way to locate statement
boundaries for 1.13.
Fixes#29511.
Change-Id: If0e488341d848ba6012045b126c86b1250408d65
Reviewed-on: https://go-review.googlesource.com/c/156021
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently it's possible for the runtime to deadlock if checkPut is
called in a non-preemptible context. In this case, checkPut may spin,
so it won't leave the non-preemptible context, but the thread running
gcMarkDone needs to preempt all of the goroutines before it can
release the checkPut spin loops.
Fix this by returning from checkPut if it's called under any of the
conditions that would prevent gcMarkDone from preempting it. In this
case, it leaves a note behind that this happened; if the runtime does
later detect left-over work it can at least indicate that it was
unable to catch it in the act.
For #27993.
Updates #29385 (may fix it).
Change-Id: Ic71c10701229febb4ddf8c104fb10e06d84b122e
Reviewed-on: https://go-review.googlesource.com/c/156017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
In the case of x+d >= w, where d and w are constants, we are
deriving x is within the bound of min=w-d and max=maxInt-d. When
there is an overflow (min >= max), we know only one of x >= min
or x <= max is true, and we derive this by excluding the other.
When excluding x >= min, we did not consider the equal case, so
we could incorrectly derive x <= max when x == min.
Fixes#29502.
Change-Id: Ia9f7d814264b1a3ddf78f52e2ce23377450e6e8a
Reviewed-on: https://go-review.googlesource.com/c/156019
Reviewed-by: David Chase <drchase@google.com>
methodbyname was used for sorting in bexport.go, until
https://golang.org/cl/139338 removed the code that
invoked sorting function.
R=1.13
Change-Id: I13e313fb60111a142ed3883d81916af254445fdc
Reviewed-on: https://go-review.googlesource.com/c/155959
Run-TryBot: Iskander Sharipov <quasilyte@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A notetsleepg may get stuck if its timeout callback gets invoked
exactly on its deadline due to low precision of nanotime. This change
fixes the comparison so it also resolves the note if the timestamps are
equal.
Updates #28975
Change-Id: I045d2f48b7f41cea0caec19b56876e9de01dcd6c
Reviewed-on: https://go-review.googlesource.com/c/153558
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
whereas this is a longstanding tradition
and insofaras it is worth continuing such traditions
and notwithstanding an attempt at future-proofing
thetruthofthematter is that I have been waiting for years to send this change
so despiteallobjections I have updated the copyright year.
Change-Id: I55961b15a7eda35d84fdd9250afdbe19f0bf8412
Reviewed-on: https://go-review.googlesource.com/c/155928
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
CL 138595 introduced the new names when the hardcoded stat8 definitions was replaced
with a cgo generated one.
Fixes#29393
Updates #22448
Change-Id: I6309958306329ff301c17344b2e0ead0cc874224
Reviewed-on: https://go-review.googlesource.com/c/155958
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Use EnumTimeFormatsEx() to test panics across callback boundaries
instead of EnumWindows(). EnumWindows() is incompatible with Go's panic
unwinding mechanism. See the associated issue for more information.
Updates #26148
Change-Id: If1dd70885d9c418b980b6827942cb1fd16c73803
Reviewed-on: https://go-review.googlesource.com/c/155923
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
CL 155917 added a -race test that shouldn't be run when cgo is not
enabled. Enforce this in the test file, with a buildflag.
Fixes the nocgo builder.
Change-Id: I9fe0d8f21da4d6e2de3f8fe9395e1fa7e9664b02
Reviewed-on: https://go-review.googlesource.com/c/155957
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reorg map flags a bit so we don't need any extra space for the extra flag.
Fixes#23734
Change-Id: I436812156240ae90de53d0943fe1aabf3ea37417
Reviewed-on: https://go-review.googlesource.com/c/155918
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We can't remove race instrumentation unless there are no calls,
not just no static calls. Closure and interface calls also count.
The problem in issue 29329 is that there was a racefuncenter, an
InterCall, and a racefuncexit. The racefuncenter was removed, then
the InterCall was rewritten to a StaticCall. That prevented the
racefuncexit from being removed. That caused an imbalance in
racefuncenter/racefuncexit calls, which made the race detector barf.
Bug introduced at CL 121235
Fixes#29329
Change-Id: I2c94ac6cf918dd910b74b2a0de5dc2480d236f16
Reviewed-on: https://go-review.googlesource.com/c/155917
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Work involved in getting a stack trace is divided between
runtime.Callers and runtime.CallersFrames.
Before this CL, runtime.Callers returns a pc per runtime frame.
runtime.CallersFrames is responsible for expanding a runtime frame
into potentially multiple user frames.
After this CL, runtime.Callers returns a pc per user frame.
runtime.CallersFrames just maps those to user frame info.
Entries in the result of runtime.Callers are now pcs
of the calls (or of the inline marks), not of the instruction
just after the call.
Fixes#29007Fixes#28640
Update #26320
Change-Id: I1c9567596ff73dc73271311005097a9188c3406f
Reviewed-on: https://go-review.googlesource.com/c/152537
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Use SyscallConn to avoid calling the Fd method in sendFile on Unix
systems, since Fd has the side effect of putting the descriptor into
blocking mode.
Fixes#28330
Change-Id: If093417a225fe44092bd2c0dbbc3937422e98c0b
Reviewed-on: https://go-review.googlesource.com/c/155137
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(SGTconst [c] (SRLconst _ [d])) && 0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c) -> (MOVWconst [1])
This rule is problematic. 1<<(32-uint32(d)) <= int32(c) meant to
say that it is true if c is greater than the largest possible
value of the right shift. But when d==1, 1<<(32-1) is negative
and results in the wrong comparison.
Rewrite the rules in a more direct way.
Fixes#29402.
Change-Id: I5940fc9538d9bc3a4bcae8aa34672867540dc60e
Reviewed-on: https://go-review.googlesource.com/c/155798
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fix comment as w&1 is the parity of 'x', not of 'n'.
Change-Id: Ia0e448f7e5896412ff9b164459ce15561ab624cc
GitHub-Last-Rev: 54ba08ab10
GitHub-Pull-Request: golang/go#29419
Reviewed-on: https://go-review.googlesource.com/c/155743
Reviewed-by: Robert Griesemer <gri@golang.org>
Last of the Macos libSystem changes, hopefully.
Fixes#17490
Change-Id: I88b303bafd92494cc4ddde712213d2ef976ce4e2
Reviewed-on: https://go-review.googlesource.com/c/155737
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updates #20969
Change-Id: Ibcf0bf932d5b1de67c22c63dd8514ed7a5d198fb
Reviewed-on: https://go-review.googlesource.com/c/155538
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Rather than return os.ErrNotExist for /path/to/existing_file/,
walkSymLinks now returns syscall.ENOTDIR.
This is consistent with behavior of os.Lstat.
Fixes#29372
Change-Id: Id5c471d901db04b2f35d60f60a81b2a0be93cae9
Reviewed-on: https://go-review.googlesource.com/c/155597
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
UnsafePointer is a valid type kind to call IsNil on.
Fixes#29381
Change-Id: Iaf65d582c67f4be52cd1885badf40f174920500b
Reviewed-on: https://go-review.googlesource.com/c/155797
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On Go 1.11.x, if one ran 'go build' on a main package within a module,
while a needed vcs program like git was missing, a confusing error would
show up:
build testmod: cannot find module for path rsc.io/quote
The error should instead point at the source of the problem, which is
the missing vcs program. Thankfully, Go 1.12 doesn't have this bug, even
though it doesn't seem like the bug was fixed directly and
intentionally.
To ensure that this particular edge case isn't broken again, add a
regression test. Piggyback on mod_vcs_missing, since it already requires
a missing vcs program and network access.
I double-checked that Go 1.11 fails this test via /usr/bin/go, which is
1.11.3 on my system:
$ PATH=~/tip/bin go test -v -run Script/mod_vcs_missing
[...]
> exec /usr/bin/go build
[stderr]
build m: cannot find module for path launchpad.net/gocheck
Fixes#28948.
Change-Id: Iff1bcf77d9f7c11d15935cb87d6f58d7981d33d2
Reviewed-on: https://go-review.googlesource.com/c/155537
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This appears to have been an oversight and/or
left over from development.
Setting the genfile means that extra sanity
checks are executed when regenerating SSA files.
They already pass.
Change-Id: Icc01ecf85020d3d51355e8bccfbc521b52371747
Reviewed-on: https://go-review.googlesource.com/c/154459
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>