Tobias Klauser [Fri, 25 Oct 2019 18:48:07 +0000 (20:48 +0200)]
runtime: define emptyfunc as static function in assembly for freebsd/arm64
CL 198544 broke the linux/arm64 build because it declares emptyfunc for
GOARCH=arm64, but only freebsd/arm64 defines it. Make it a static
assembly function specific for freebsd/arm64 and remove the stub.
Fixes #35160
Change-Id: I5fd94249b60c6fd259c251407b6eccc8fa512934
Reviewed-on: https://go-review.googlesource.com/c/go/+/203418 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Fri, 25 Oct 2019 17:55:10 +0000 (13:55 -0400)]
os/signal: derive TestAtomicStop timeout from overall test timeout
Previously, TestAtomicStop used a hard-coded 2-second timeout.
That empirically is not long enough on certain builders. Rather than
adjusting it to a different arbitrary value, use a slice of the
overall timeout for the test binary. If everything is working, we
won't block nearly that long anyway.
runtime: ensure _Grunning Gs have a valid g.m and g.m.p
We already claim on the documentation for _Grunning that this is case,
but execute transitions to _Grunning before assigning g.m. Fix this
and make the documentation even more explicit.
runtime: make m.libcallsp check in shrinkstack panic
Currently, shrinkstack will not shrink a stack on Windows if
gp.m.libcallsp != 0. In general, we can't shrink stacks in syscalls
because the syscall may hold pointers into the stack, and in principle
this is supposed to be preventing that for libcall-based syscalls
(which are direct syscalls from the runtime). But this test is
actually broken and has been for a long time. That turns out to be
okay because it also appears it's not necessary.
This test is racy. g.m points to whatever M the G was last running on,
even if the G is in a blocked state, and that M could be doing
anything, including making libcalls. Hence, observing that libcallsp
== 0 at one moment in shrinkstack is no guarantee that it won't become
non-zero while we're shrinking the stack, and vice-versa.
It's also weird that this check is only performed on Windows, given
that we now use libcalls on macOS, Solaris, and AIX.
This check was added when stack shrinking was first implemented in CL 69580044. The history of that CL (though not the final version)
suggests this was necessary for libcalls that happened on Go user
stacks, which we never do now because of the limited stack space.
It could also be defending against user stack pointers passed to
libcall system calls from blocked Gs. But the runtime isn't allowed to
keep pointers into the user stack for blocked Gs on any OS, so it's
not clear this would be of any value.
Hence, this checks seems to be simply unnecessary.
Rather than simply remove it, this CL makes it defensive. We can't do
anything about blocked Gs, since it doesn't even make sense to look at
their M, but if a G tries to shrink its own stack while in a libcall,
that indicates a bug in the libcall code. This CL makes shrinkstack
panic in this case.
For #10958, #24543, since those are going to rearrange how we decide
that it's safe to shrink a stack.
Austin Clements [Sat, 12 Oct 2019 22:35:49 +0000 (18:35 -0400)]
cmd/internal/obj/x86: correct pcsp for ADJSP
The x86 assembler supports an "ADJSP" pseudo-op that compiles to an
ADD/SUB from SP. Unfortunately, while this seems perfect for an
instruction that would allow obj to continue to track the SP/FP delta,
obj currently doesn't do that. As a result, FP-relative references
won't work and, perhaps worse, the pcsp table will have the wrong
frame size.
We don't currently use this instruction in any assembly or generate it
in the compiler, but this is a perfect instruction for solving a
problem in #24543.
This CL makes ADJSP useful by incorporating it into the SP delta
logic.
One subtlety is that we do generate ADJSP in obj itself to open a
function's stack frame. Currently, when preprocess enters the loop to
compute the SP delta, it may or may not start at this ADJSP
instruction depending on various factors. We clean this up by instead
always starting the SP delta at 0 and always starting this loop at the
entry to the function.
Why not just recognize ADD/SUB of SP? The danger is that could change
the meaning of existing code. For example, walltime1 in
sys_linux_amd64.s saves SP, SUBs from it, and aligns it. Later, it
restores the saved copy and then does a few FP-relative references.
Currently obj doesn't know any of this is happening, but that's fine
once it gets to the FP-relative references. If we taught obj to
recognize the SUB, it would start to miscompile this code. An
alternative would be to recognize unknown instructions that write to
SP and refuse subsequent FP-relative references, but that's kind of
annoying.
This passes toolstash -cmp for std on both amd64 and 386.
Jason A. Donenfeld [Wed, 23 Oct 2019 20:27:29 +0000 (22:27 +0200)]
internal/syscall/windows/registry: allow for non-null terminated strings
According to MSDN, "If the data has the REG_SZ, REG_MULTI_SZ or
REG_EXPAND_SZ type, this size includes any terminating null character or
characters unless the data was stored without them. [...] If the data
has the REG_SZ, REG_MULTI_SZ or REG_EXPAND_SZ type, the string may not
have been stored with the proper terminating null characters. Therefore,
even if the function returns ERROR_SUCCESS, the application should
ensure that the string is properly terminated before using it;
otherwise, it may overwrite a buffer."
It's therefore dangerous to pass it off unbounded as we do, and in fact
this led to crashes on real systems.
Change-Id: I6d786211814656f036b87fd78631466634cd764a
Reviewed-on: https://go-review.googlesource.com/c/go/+/202937
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Jay Conrod [Tue, 22 Oct 2019 15:30:20 +0000 (11:30 -0400)]
cmd/go: add -modfile flag that sets go.mod file to read/write
This change adds the -modfile flag to module aware build commands and
to 'go mod' subcommands. -modfile may be set to a path to an alternate
go.mod file to be read and written. A real go.mod file must still
exist and is used to set the module root directory. However, it is not
opened.
When -modfile is set, the effective location of the go.sum file is
also changed to the -modfile with the ".mod" suffix trimmed (if
present) and ".sum" added.
Updates #34506
Change-Id: I2d1e044e18af55505a4f24bbff09b73bb9c908b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/202564
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Jay Conrod [Fri, 18 Oct 2019 21:39:31 +0000 (17:39 -0400)]
doc: add skeleton module documentation with headings
Sections will be filled in with individual CLs before Go 1.14.
NOTE: This document is currently in Markdown for ease of writing /
reviewing. Before Go 1.14, we will either ensure that x/website
can render Markdown (flavor TBD) or check in a rendered HTML file that
can be displayed directly.
Updates #33637
Change-Id: Icd43fa2bdb7d256b28a56b93214b70343f43492e
Reviewed-on: https://go-review.googlesource.com/c/go/+/202081 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Thu, 24 Oct 2019 13:17:54 +0000 (09:17 -0400)]
cmd/go: re-enable 'go list -m' with -mod=vendor for limited patterns
I had prohibited 'go list -m' with -mod=vendor because the module
graph is incomplete, but I've realized that many queries do not
actually require the full graph — and may, in fact, be driven using
modules previously reported by 'go list' for specific, vendored
packages. Queries for those modules should succeed.
Updates #33848
Change-Id: I1000b4cf586a830bb78faf620ebf62d73a3cb300
Reviewed-on: https://go-review.googlesource.com/c/go/+/203138
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Dan Scales [Mon, 24 Jun 2019 19:59:22 +0000 (12:59 -0700)]
cmd/compile, cmd/link, runtime: make defers low-cost through inline code and extra funcdata
Generate inline code at defer time to save the args of defer calls to unique
(autotmp) stack slots, and generate inline code at exit time to check which defer
calls were made and make the associated function/method/interface calls. We
remember that a particular defer statement was reached by storing in the deferBits
variable (always stored on the stack). At exit time, we check the bits of the
deferBits variable to determine which defer function calls to make (in reverse
order). These low-cost defers are only used for functions where no defers
appear in loops. In addition, we don't do these low-cost defers if there are too
many defer statements or too many exits in a function (to limit code increase).
When a function uses open-coded defers, we produce extra
FUNCDATA_OpenCodedDeferInfo information that specifies the number of defers, and
for each defer, the stack slots where the closure and associated args have been
stored. The funcdata also includes the location of the deferBits variable.
Therefore, for panics, we can use this funcdata to determine exactly which defers
are active, and call the appropriate functions/methods/closures with the correct
arguments for each active defer.
In order to unwind the stack correctly after a recover(), we need to add an extra
code segment to functions with open-coded defers that simply calls deferreturn()
and returns. This segment is not reachable by the normal function, but is returned
to by the runtime during recovery. We set the liveness information of this
deferreturn() to be the same as the liveness at the first function call during the
last defer exit code (so all return values and all stack slots needed by the defer
calls will be live).
I needed to increase the stackguard constant from 880 to 896, because of a small
amount of new code in deferreturn().
The -N flag disables open-coded defers. '-d defer' prints out the kind of defer
being used at each defer statement (heap-allocated, stack-allocated, or
open-coded).
Cost of defer statement [ go test -run NONE -bench BenchmarkDefer$ runtime ]
With normal (stack-allocated) defers only: 35.4 ns/op
With open-coded defers: 5.6 ns/op
Cost of function call alone (remove defer keyword): 4.4 ns/op
Text size increase (including funcdata) for go binary without/with open-coded defers: 0.09%
The average size increase (including funcdata) for only the functions that use
open-coded defers is 1.1%.
The cost of a panic followed by a recover got noticeably slower, since panic
processing now requires a scan of the stack for open-coded defer frames. This scan
is required, even if no frames are using open-coded defers:
Cost of panic and recover [ go test -run NONE -bench BenchmarkPanicRecover runtime ]
Without open-coded defers: 62.0 ns/op
With open-coded defers: 255 ns/op
A CGO Go-to-C-to-Go benchmark got noticeably faster because of open-coded defers:
CGO Go-to-C-to-Go benchmark [cd misc/cgo/test; go test -run NONE -bench BenchmarkCGoCallback ]
Without open-coded defers: 443 ns/op
With open-coded defers: 347 ns/op
Bryan C. Mills [Wed, 23 Oct 2019 20:10:55 +0000 (16:10 -0400)]
cmd/go/internal/list: ensure that cfg.BuildMod is initialized before reading it in 'go list -m'
The default value of cfg.BuildMod depends on the 'go' version in the
go.mod file. The go.mod file is read and parsed, and its settings are
applied, in modload.InitMod.
As it turns out, modload.Enabled does not invoke InitMod, so
cfg.BuildMod is not necessarily set even if modload.Enabled returns
true.
Updates #33848
Change-Id: I13a4dd80730528e6f1a5acc492fcfe07cb59d94e
Reviewed-on: https://go-review.googlesource.com/c/go/+/202917
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>
Robert Griesemer [Wed, 23 Oct 2019 23:44:51 +0000 (16:44 -0700)]
math/big: make Rat.Denom side-effect free
A Rat is represented via a quotient a/b where a and b are Int values.
To make it possible to use an uninitialized Rat value (with a and b
uninitialized and thus == 0), the implementation treats a 0 denominator
as 1.
Rat.Num and Rat.Denom return pointers to these values a and b. Because
b may be 0, Rat.Denom used to first initialize it to 1 and thus produce
an undesirable side-effect (by changing the Rat's denominator).
This CL changes Denom to return a new (not shared) *Int with value 1
in the rare case where the Rat was not initialized. This eliminates
the side effect and returns the correct denominator value.
While this is changing behavior of the API, the impact should now be
minor because together with (prior) CL https://golang.org/cl/202997,
which initializes Rats ASAP, Denom is unlikely used to access the
denominator of an uninitialized (and thus 0) Rat. Any operation that
will somehow set a Rat value will ensure that the denominator is not 0.
Robert Griesemer [Wed, 23 Oct 2019 21:22:32 +0000 (14:22 -0700)]
math/big: normalize unitialized denominators ASAP
A Rat is represented via a quotient a/b where a and b are Int values.
To make it possible to use an uninitialized Rat value (with a and b
uninitialized and thus == 0), the implementation treats a 0 denominator
as 1.
For each operation we check if the denominator is 0, and then treat
it as 1 (if necessary). Operations that create a new Rat result,
normalize that value such that a result denominator 1 is represened
as 0 again.
This CL changes this behavior slightly: 0 denominators are still
interpreted as 1, but whenever we (safely) can, we set an uninitialized
0 denominator to 1. This simplifies the code overall.
Cherry Zhang [Wed, 23 Oct 2019 15:42:23 +0000 (11:42 -0400)]
runtime: save/fetch g register during VDSO on ARM and ARM64
On ARM and ARM64, during a VDSO call, the g register may be
temporarily clobbered by the VDSO code. If a signal is received
during the execution of VDSO code, we may not find a valid g
reading the g register. In CL 192937, we conservatively assume
g is nil. But this approach has a problem: we cannot handle
the signal in this case. Further, if the signal is not a
profiling signal, we'll call badsignal, which calls needm, which
wants to get an extra m, but we don't have one in a non-cgo
binary, which cuases the program to hang.
This is even more of a problem with async preemption, where we
will receive more signals than before. I ran into this problem
while working on async preemption support on ARM64.
In this CL, before making a VDSO call, we save the g on the
gsignal stack. When we receive a signal, we will be running on
the gsignal stack, so we can fetch the g from there and move on.
We probably want to do the same for PPC64. Currently we rely on
that the VDSO code doesn't actually clobber the g register, but
this is not guaranteed and we don't have control with.
Dmitri Shuralyov [Wed, 23 Oct 2019 17:50:15 +0000 (13:50 -0400)]
misc: delete benchcmp forwarding script
benchcmp was moved out of misc into x/tools in CL 60100043 in 2014,
and then replaced by a forwarding script in CL 82710043.
Five years have since passed, and the forwarding script has outlived
its usefulness. It's now more confusing than helpful. Delete it.
Jason A. Donenfeld [Tue, 22 Oct 2019 20:39:30 +0000 (22:39 +0200)]
internal/syscall/windows/registry: blacklist certain registry keys in TestWalkFullRegistry
It turns out that Windows has "legitimate" keys that have bogus type
values or bogus lengths that don't correspond with their type. On up to
date Windows 10 systems, this test always fails for this reason. These
keys exist because of bugs in Microsoft's code. This commit works around
the problem by simply blacklisting known instances. It also expands the
error message a bit so that we can make adjustments should the problem
ever happen again, and reformats the messages so that it makes copy and
pasting into the blacklist easier.
Updates #35084
Change-Id: I50322828c0eb0ccecbb62d6bf4f9c726fa0b3c27
Reviewed-on: https://go-review.googlesource.com/c/go/+/202897
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Jay Conrod [Wed, 23 Oct 2019 17:47:36 +0000 (13:47 -0400)]
cmd/go: ignore '@' when cleaning local and absolute file path args
Since CL 194600, search.CleanPaths preserves characters after '@' in
each argument. This was done so that paths could be cleaned while
version queries were preserved. However, local and absolute file paths
may contain '@' characters.
With this change, '@' is treated as a normal character by
search.CleanPaths in local and absolute paths.
Fixes #35115
Change-Id: Ia7d37e0a2737442d4f1796cc2fc3a59237a8ddfe
Reviewed-on: https://go-review.googlesource.com/c/go/+/202761
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Jason A. Donenfeld [Wed, 23 Oct 2019 11:08:46 +0000 (13:08 +0200)]
syscall: reenable sysctl on iOS
This was disabled due to a report that the App Store rejects the symbol
__sysctl. However, we use the sysctl symbol, which is fine. The __sysctl
symbol is used by x/sys/unix, which needs fixing instead. So, this
commit reenables sysctl on iOS, so that things like net.InterfaceByName
can work again.
This reverts CL 193843, CL 193844, CL 193845, and CL 193846.
Fixes #35101
Updates #34133
Updates #35103
Change-Id: Ib8eb9f87b81db24965b0de29d99eb52887c7c60a
Reviewed-on: https://go-review.googlesource.com/c/go/+/202778
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: David Crawshaw <crawshaw@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Thu, 11 Apr 2019 21:20:54 +0000 (14:20 -0700)]
runtime: add race detector support for new timers
Since the new timers run on g0, which does not have a race context,
we add a race context field to the P, and use that for timer functions.
This works since all timer functions are in the standard library.
Updates #27707
Change-Id: I8a5b727b4ddc8ca6fc60eb6d6f5e9819245e395b
Reviewed-on: https://go-review.googlesource.com/c/go/+/171882
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Jason A. Donenfeld [Tue, 22 Oct 2019 20:39:30 +0000 (22:39 +0200)]
internal/syscall/windows/registry: fix strict assumptions in TestWalkFullRegistry
It turns out that Windows has "legitimate" keys that have bogus type
values or bogus lengths that don't correspond with their type. On up to
date Windows 10 systems, this test always fails for this reason.
So, this commit alters the test to simply log the discrepancy and move
on.
Fixes #35084
Change-Id: I56e12cc62aff49cfcc38ff01a19dfe53153976a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/202678
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Matthew Dempsky [Thu, 17 Oct 2019 20:24:34 +0000 (13:24 -0700)]
cmd/compile: enable -d=checkptr when -race or -msan is specified
It can still be manually disabled again using -d=checkptr=0.
It's also still disabled by default for GOOS=windows, because the
Windows standard library code has a lot of unsafe pointer conversions
that need updating.
Matthew Dempsky [Tue, 22 Oct 2019 18:59:00 +0000 (11:59 -0700)]
runtime: fix -d=checkptr failure for testing/quick
This CL extends checkptrBase to recognize pointers into the stack and
data/bss sections. I was meaning to do this eventually anyway, but
it's also an easy way to workaround #35068.
Ian Lance Taylor [Thu, 11 Apr 2019 04:54:58 +0000 (21:54 -0700)]
runtime: add new adjusttimers function
The adjusttimers function is where we check the adjustTimers field in
the P struct to see if we need to resort the heap. We walk forward in
the heap and find and resort timers that have been modified, until we
find all the timers that were modified to run earlier. Along the way
we remove deleted timers.
Updates #27707
Change-Id: I1cba7fe77b8112b7e9a9dba80b5dfb08fcc7c568
Reviewed-on: https://go-review.googlesource.com/c/go/+/171877
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Ian Lance Taylor [Thu, 11 Apr 2019 04:23:55 +0000 (21:23 -0700)]
runtime: add new dodeltimer and dodeltimer0 functions
The dodeltimer function removes a timer from a heap. The dodeltimer0
function removes the first timer from a heap; in the old timer code
this common special case was inlined in the timerproc function.
Updates #27707
Change-Id: I1b7c0af46866abb4bffa8aa4d8e7143f9ae8f402
Reviewed-on: https://go-review.googlesource.com/c/go/+/171834
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Jay Conrod [Tue, 22 Oct 2019 18:45:31 +0000 (14:45 -0400)]
cmd/go: support -modcacherw in 'go mod' subcommands
The -modcacherw flag is now registered in work.AddModCommonFlags,
which is called from work.AddBuildFlags, where it was registered
before. 'go mod' subcommands register the flag by calling
work.AddModCommonFlags directly.
Also, build commands now exit with an error if -modcacherw is set
explicitly (not in GOFLAGS) in GOPATH mode.
Updates #31481
Change-Id: I461e59a51ed31b006fff4d5c57c2a866be0bbf38
Reviewed-on: https://go-review.googlesource.com/c/go/+/202563
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
smasher164 [Tue, 22 Oct 2019 14:09:55 +0000 (10:09 -0400)]
cmd/compile: don't use FMA on plan9
CL 137156 introduces an intrinsic on AMD64 that executes vfmadd231sd
when feature detection is successful. However, because floating-point
isn't allowed in note handler, the builder disables SSE instructions,
and fails when attempting to execute this instruction. This change
disables FMA on plan9 to immediately use the software fallback.
Fixes #35063.
Change-Id: I87d8f0995bd2f15013d203e618938f5079c9eed2
Reviewed-on: https://go-review.googlesource.com/c/go/+/202617 Reviewed-by: Keith Randall <khr@golang.org>
Ian Lance Taylor [Thu, 11 Apr 2019 04:04:36 +0000 (21:04 -0700)]
runtime: add new modtimer function
This adds a new field to P, adjustTimers, that tells the P that one of
its existing timers was modified to be earlier, and that it therefore
needs to resort them.
Updates #27707
Change-Id: I4c5f5b51ed116f1d898d3f87cdddfa1b552337f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/171832
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Jean de Klerk [Wed, 1 Aug 2018 01:10:42 +0000 (18:10 -0700)]
testing: stream log output in verbose mode
Fixes #24929
Change-Id: Icc426068cd73b75b78001f55e1e5d81ccebbe854
Reviewed-on: https://go-review.googlesource.com/c/go/+/127120
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
LE Manh Cuong [Fri, 16 Aug 2019 09:05:10 +0000 (16:05 +0700)]
cmd/compile: add marker for skipping dowidth when tracing typecheck
The root cause of #33658 is that fmt.Printf does have side effects when
printing Type.
typefmt for TINTER will call Type.Fields to get all embedded fields and
methods. The thing is that type.Fields itself will call dowidth, which will
expand the embedded interface, make it non-embedded anymore.
To fix it, we add a marker while we are tracing, so dowidth can know and
return immediately without doing anything.
Fixes #33658
Change-Id: Id4b70ff68a3b802675deae96793fdb8f7ef1a4a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/190537
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Tue, 22 Oct 2019 00:45:59 +0000 (17:45 -0700)]
reflect: fix unsafe conversions reported by -d=checkptr
The code for generating gcdata was (technically) unsafe. It was also
rather repetitive. This CL refactors it a bit and abstracts use of
gcdata into a helper gcSlice method.
Updates #34972.
Change-Id: Ie86d7822eafe263f1d3d150eedf0ec66be1ec85d
Reviewed-on: https://go-review.googlesource.com/c/go/+/202582
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Matthew Dempsky [Tue, 22 Oct 2019 00:06:02 +0000 (17:06 -0700)]
sync/atomic: suppress checkptr errors for hammerStoreLoadPointer
This test could be updated to use unsafe.Pointer arithmetic properly
(e.g., see discussion at #34972), but it doesn't seem worthwhile. The
test is just checking that LoadPointer and StorePointer are atomic.
Updates #34972.
Change-Id: I85a8d610c1766cd63136cae686aa8a240a362a18
Reviewed-on: https://go-review.googlesource.com/c/go/+/202597
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills [Tue, 22 Oct 2019 16:44:44 +0000 (12:44 -0400)]
net: convert TestTCPServer to use subtests
My fix in CL 202618 inadvertently violated an invariant in the inner
loop of TestTCPServer (namely, that len(trchs) == i). That causes a
panic when one or more of the channels is omitted due to a flake.
Instead of trying to fix up the test, let's just factor out a subtest
and skip the whole thing if the transceiver's Dial flakes out.
Robert Griesemer [Mon, 21 Oct 2019 21:25:45 +0000 (14:25 -0700)]
go/types: don't update package-external types when checking validity
The recently added type-validity check uses a new field of Named
types for marking (to detect cycles). That field was modified even
if the type was not part of the current package or belonged to the
Universe scope (error type). This led to race conditions if the
package's type was imported by multiple, concurrently type-checked
packages.
A test would be nice but it's a bit cumbersome to set one up.
Verified manually that package-external types are left alone.
Fixes #35049.
Change-Id: I51686bef47fcca48b99b91ecb1b2e9d58e135ea6
Reviewed-on: https://go-review.googlesource.com/c/go/+/202483 Reviewed-by: Bryan C. Mills <bcmills@google.com>
David du Colombier [Tue, 22 Oct 2019 11:07:15 +0000 (13:07 +0200)]
cmd/go: fix TestScript/list_ambiguous_path on Plan 9
CL 198459 added TestScript/list_ambiguous_path. This
test is failing on Plan 9, because the expected error
doesn't match the error message returned on Plan 9.
This change fixes the test by matching the correct
error message on Plan 9.
Fixes #35072.
Change-Id: If8cdb641e0e9544ae4ac24f8d0c54859a3b23a69
Reviewed-on: https://go-review.googlesource.com/c/go/+/202447
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Mon, 21 Oct 2019 16:55:23 +0000 (09:55 -0700)]
runtime: correctly negate errno value for *BSD ARM
Fixes #35037
Change-Id: I0b9bcd001556cd409994d83dabcdd6e32b001d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/202441
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Jason A. Donenfeld [Mon, 21 Oct 2019 14:12:22 +0000 (16:12 +0200)]
syscall: respect permission bits on file opening on Windows
On Windows, os.Chmod and syscall.Chmod toggle the FILE_ATTRIBUTES_
READONLY flag depending on the permission bits. That's a bit odd but I
guess some compromises were made at some point and this is what was
chosen to map to a Unix concept that Windows doesn't really have in the
same way. That's fine. However, the logic used in Chmod was forgotten
from os.Open and syscall.Open, which then manifested itself in various
places, most recently, go modules' read-only behavior.
This makes syscall.Open consistent with syscall.Chmod and adds a test
for the permission _behavior_ using ioutil. By testing the behavior
instead of explicitly testing for the attribute bits we care about, we
make sure this doesn't regress in unforeseen ways in the future, as well
as ensuring the test works on platforms other than Windows.
In the process, we fix some tests that never worked and relied on broken
behavior, as well as tests that were disabled on Windows due to the
broken behavior and had TODO notes.
Fixes #35033
Change-Id: I6f7cf54517cbe5f6b1678d1c24f2ab337edcc7f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/202439
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Tue, 22 Oct 2019 07:38:08 +0000 (00:38 -0700)]
runtime: force testing calls of netpoll to run on system stack
Fixes #35053
Change-Id: I31853d434610880044c169e0c1e9732f97ff1bdb
Reviewed-on: https://go-review.googlesource.com/c/go/+/202444
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David du Colombier <0intro@gmail.com>
Cuong Manh Le [Sat, 19 Oct 2019 08:18:34 +0000 (15:18 +0700)]
runtime: fix unsafe.Pointer alignment on Linux
Caught by go test -a -short -gcflags=all=-d=checkptr runtime
TestMincoreErrorSign intentionally uses uintptr(1) to get -EINVAL,
but it violates unsafe pointer rules 2. So use another misaligned
pointer add(new(int32), 1), but do not violate unsafe pointer rules.
TestEpollctlErrorSign passes an unsafe.Pointer of &struct{}{} to
Epollctl, which is then casted to epollevent, causes mis-alignment.
Fixing it by exporting epollevent on runtime_test package, so it can be
passed to Epollctl.
Updates #34972
Change-Id: I78ebfbeaf706fd1d372272af0bbc4e2cabca4631
Reviewed-on: https://go-review.googlesource.com/c/go/+/202157
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ian Lance Taylor [Thu, 11 Apr 2019 00:38:26 +0000 (17:38 -0700)]
runtime, syscall, time: prepare for adding timers to P's
Add new fields to runtime.timer, and adjust the various timer
functions in preparation for adding timers to P's. This continues to
use the old timer code.
Updates #6239
Updates #27707
Change-Id: I9adb3814f657e083ec5e22736c4b5b52b77b6a3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/171829
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Matthew Dempsky [Thu, 17 Oct 2019 21:29:16 +0000 (14:29 -0700)]
cmd/compile: recognize (*[Big]T)(ptr)[:n:m] pattern for -d=checkptr
A common idiom for turning an unsafe.Pointer into a slice is to write:
s := (*[Big]T)(ptr)[:n:m]
This technically violates Go's unsafe pointer rules (rule #1 says T2
can't be bigger than T1), but it's fairly common and not too difficult
to recognize, so might as well allow it for now so we can make
progress on #34972.
Dmitri Shuralyov [Mon, 21 Oct 2019 19:30:13 +0000 (15:30 -0400)]
net/http: remove parseURL variable
The parseURL variable was introduced in CL 49930 in order to work
around the fact that the name "url" was shadowed by a parameter of
exported functions, and couldn't be renamed without sacrificing
documentation readability. Documentation readability takes higher
priority than internal implementation details.
Back then, I considered renaming the net/url import but saw that it
would be too disruptive of a change to the large net/http package.
Now I see a better way: it's possible to import net/url both as url
and as urlpkg (the package is still imported just once, but it becomes
available via two names). This way we eliminate the need for wasting
(a little) memory on the parseURL variable, improve code readability
slightly, and delete some lines of code and comments.
Dmitri Shuralyov [Mon, 21 Oct 2019 19:04:54 +0000 (15:04 -0400)]
database/sql: remove forced log import from test
This var _ = log.Printf line was added 8 years ago, in CL 4973055,
which created the database/sql package and its tests. There was no
goimports back then, so this was likely added to make it easier to
use log package during development of tests.
It's no longer needed, so remove it. It can always be conveniently
re-added via goimports whenever needed.
Bryan C. Mills [Mon, 21 Oct 2019 18:12:26 +0000 (14:12 -0400)]
crypto/tls: retry net.Dial flakes on Dragonfly
localPipe currently flakes in various crypto/tls tests. Since that
function doesn't seem to flake anywhere else, I suspect a kernel bug.
To make the test less flaky, retry the Dial if we suspect that it is
affected. (Worst case, we delay the test by a few seconds before
erroring out as usual.)
smasher164 [Mon, 15 Oct 2018 07:14:57 +0000 (03:14 -0400)]
cmd/compile: add fma intrinsic for arm
This change introduces an arm intrinsic that generates the FMULAD
instruction for the fused-multiply-add operation on systems that
support it. System support is detected via cpu.ARM.HasVFPv4. A rewrite
rule translates the generic intrinsic to FMULAD.
Updates #25819.
Change-Id: I8459e5dd1cdbdca35f88a78dbeb7d387f1e20efa
Reviewed-on: https://go-review.googlesource.com/c/go/+/142117
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Ian Lance Taylor [Thu, 11 Apr 2019 00:23:05 +0000 (17:23 -0700)]
runtime, syscall, time: add and use resettimer
As a small step toward speeding up timers, restrict modification
of the timer.when field to the timer code itself. Other code that
wants to change the when field of an existing timer must now call
resettimer rather than changing the when field and calling addtimer.
The new resettimer function also works for a new timer.
This is just a refactoring in preparation for later code.
Updates #27707
Change-Id: Iccd5dcad415ffbeac4c2a3cf015e91f82692acf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/171825
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>