Damien Neil [Thu, 30 May 2019 16:46:56 +0000 (09:46 -0700)]
testing: provide additional information when test funcs panic
Flush the output log up to the root when a test panics. Prior to
this change, only the current test's output log was flushed to its
parent, resulting in no output when a subtest panics.
For the following test function:
func Test(t *testing.T) {
for i, test := range []int{1, 0, 2} {
t.Run(fmt.Sprintf("%v/%v", i, test), func(t *testing.T) {
_ = 1 / test
})
}
}
Output before this change:
panic: runtime error: integer divide by zero [recovered]
panic: runtime error: integer divide by zero
(stack trace follows)
Output after this change:
--- FAIL: Test (0.00s)
--- FAIL: Test/1/0 (0.00s)
panic: runtime error: integer divide by zero [recovered]
(stack trace follows)
Fixes #32121
Change-Id: Ifee07ccc005f0493a902190a8be734943123b6b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/179599
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sam Whited [Tue, 21 Aug 2018 22:11:30 +0000 (17:11 -0500)]
encoding/xml: fix token decoder on early EOF
The documentation for TokenReader suggests that implementations of the
interface may return a token and io.EOF together, indicating that it is
the last token in the stream. This is similar to io.Reader. However, if
you wrap such a TokenReader in a Decoder it complained about the EOF.
A test was added to ensure this behavior on Decoder's.
Clément Chigot [Tue, 29 Oct 2019 14:39:42 +0000 (15:39 +0100)]
runtime: fix nbpipe_test for AIX
Fcntl can't be called using syscall.Syscall as it doesn't work on AIX.
Moreover, fcntl isn't exported by syscall package.
However, it can be accessed by exporting it from runtime package
using export_aix_test.go.
Change-Id: Ib6af66d9d7eacb9ca0525ebc4cd4c92951735f1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/204059
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Clément Chigot [Wed, 30 Oct 2019 12:56:12 +0000 (13:56 +0100)]
runtime: fix netpollBreak for AIX
Change-Id: I2629711ce02d935130fb2aab29f9028b62ba9fe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/204318
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Lorenz Bauer [Wed, 30 Oct 2019 11:30:57 +0000 (11:30 +0000)]
syscall: treat ENFILE as a temporary error
ENFILE is returned from accept when the whole system has run out of
file descriptors. Mark the error as temporary, so that accept loops
continue working.
Fixes #35131
Updates #1891
Change-Id: Idf44c084731898ff4c720d06c250d3b8a42de312
Reviewed-on: https://go-review.googlesource.com/c/go/+/203117
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Elias Naur [Wed, 16 Oct 2019 08:20:12 +0000 (10:20 +0200)]
cmd/link/internal/ld: remove flags incompatible with -fembed-bitcode
The flags -headerpad, -Wl,-no_pie and -pagezero_size are incompatible with
the -fembed-bitcode flag used by `gomobile build`. Than McIntosh
suggested we might not need the offending flags; this change removes
the flags on darwin/arm64 and -headerpad, -pagezero_size on darwin/arm.
The -Wl,-no_pie flag is left for darwin/arm because linking fails
without it:
ld: warning: PIE disabled. Absolute addressing (perhaps -mdynamic-no-pic) not allowed in code signed PIE, but used in _runtime.rodata from /var/folders/qq/qxn86k813bn9fjxydm095rxw0000gp/T/workdir-host-darwin-amd64-zenly-ios/tmp/go-link-225285265/go.o. To fix this warning, don't compile with -mdynamic-no-pic or link with -Wl,-no_pie
Cholerae Hu [Tue, 29 Oct 2019 05:50:31 +0000 (13:50 +0800)]
cmd/compile: resolve TODO of Mpflt.SetString
Number literal strings returned by the lexer (internal/syntax package) and other
arguments to SetString never contain leading whitespace. There's no need (anymore)
to trim the argument.
Change-Id: Ib060d109f46f79a364a5c8aa33c4f625fe849264
Reviewed-on: https://go-review.googlesource.com/c/go/+/203997 Reviewed-by: Robert Griesemer <gri@golang.org>
Cherry Zhang [Tue, 29 Oct 2019 21:07:21 +0000 (17:07 -0400)]
runtime: clear m.gsignal when the M exits
On some platforms (currently ARM and ARM64), when calling into
VDSO we store the G to the gsignal stack, if there is one, so if
we receive a signal during VDSO we can find the G.
When an M exits, it frees the gsignal stack. But m.gsignal.stack
still points to that stack. When we call nanotime on this M, we
will write to the already freed gsignal stack, which is bad.
Prevent this by unlinking the freed stack from the M.
Should fix #35235.
Change-Id: I338b1fc8ec62aae036f38afaca3484687e11a40d
Reviewed-on: https://go-review.googlesource.com/c/go/+/204158
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Tue, 29 Oct 2019 20:14:58 +0000 (13:14 -0700)]
cmd/compile: fix typing of IData opcodes
The rules for extracting the interface data word don't leave
the result typed correctly. If I do i.([1]*int)[0], the result
should have type *int, not [1]*int. Using (IData x) for the result
keeps the typing of the original top-level Value.
I don't think this would ever cause a real codegen bug, bug fixing it
at least makes the typing shown in ssa.html more consistent.
Change-Id: I239d821c394e58347639387981b0510d13b2f7b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/204042
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jay Conrod [Wed, 16 Oct 2019 19:26:15 +0000 (15:26 -0400)]
cmd/go: delete internal packages moved to x/mod
This change deletes several internal packages, replaces imports to
them with the equivalent golang.org/x/mod packages, updates x/mod, and
re-runs 'go mod vendor'.
Dan Scales [Mon, 28 Oct 2019 22:55:36 +0000 (15:55 -0700)]
cmd/compile: handle some missing cases of non-SSAable values for args of open-coded defers
In my experimentation, I had found that most non-SSAable expressions were
converted to autotmp variables during AST evaluation. However, this was not true
generally, as witnessed by issue #35213, which has a non-SSAable field reference
of a struct that is not converted to an autotmp. So, I fixed openDeferSave() to
handle non-SSAable nodes more generally, and make sure that these non-SSAable
expressions are not evaluated more than once (which could incorrectly repeat side
effects).
Fixes #35213
Change-Id: I8043d5576b455e94163599e930ca0275e550d594
Reviewed-on: https://go-review.googlesource.com/c/go/+/203888
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Jay Conrod [Tue, 29 Oct 2019 19:25:22 +0000 (15:25 -0400)]
cmd/go/internal/modfile: don't use cmd/internal/diff
This is a partial revert of CL 203218.
cmd/go/internal/modfile is about to be deleted and replaced with
golang.org/x/mod/modfile in CL 202698. cmd/internal/diff is not
visible from golang.org/x/mod/modfile, and it doesn't make sense to
extract it into a new package there.
Updates #31761
Change-Id: I3bbbc4cae81120020e1092c1138524729530b415
Reviewed-on: https://go-review.googlesource.com/c/go/+/204103
Run-TryBot: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Bryan C. Mills [Mon, 28 Oct 2019 15:37:24 +0000 (11:37 -0400)]
go/build: use the main module's root when locating module sources
Previously, we were using srcDir, which would apply the wrong module
dependencies (including the wrong 'replace' and 'exclude' directives)
when locating an import path within a module.
Fixes #34860
Change-Id: Ie59dcc2075a7b51ba40f7cd2f62dae27bf58c9b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/203820 Reviewed-by: Jay Conrod <jayconrod@google.com>
Clément Chigot [Tue, 29 Oct 2019 14:35:42 +0000 (15:35 +0100)]
runtime: initialize netpoll in TestNetpollBreak
Netpoll must be always be initialized when TestNetpollBreak is launched.
However, when it is run in standalone, it won't be the case, so it must
be forced.
Updates: #27707
Change-Id: I28147f3834f3d6aca982c6a298feadc09b55f66e
Reviewed-on: https://go-review.googlesource.com/c/go/+/204058
Run-TryBot: Clément Chigot <clement.chigot%atos.net@gtempaccount.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Than McIntosh [Thu, 10 Oct 2019 16:11:06 +0000 (12:11 -0400)]
cmd/compile: fix spurious R_TLE_LE reloc on android/386
When compiling for GOARCH=386 GOOS=android, the compiler was attaching
R_TLS_LE relocations inappropriately -- as of Go 1.13 the TLS access
recipe for Android refers to a runtime symbol and no longer needs this
type of relocation (which was causing a crash when the linker tried to
process it).
Updates #29674.
Fixes #34788.
Change-Id: Ida01875011b524586597b1f7e273aa14e11815d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/200337
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Elias Naur <mail@eliasnaur.com> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Austin Clements [Tue, 29 Oct 2019 02:14:04 +0000 (22:14 -0400)]
runtime: unblock SIGUSR1 for TestPreemptM
TestPreemptM tests signal delivery using SIGUSR1, but (for unknown
reasons) SIGUSR1 is blocked by default on android/arm and
android/arm64, causing the test to fail.
This fixes the test by ensuring that SIGUSR1 is unblocked for this
test.
Lynn Boger [Mon, 28 Oct 2019 18:40:27 +0000 (14:40 -0400)]
crypto/elliptic: clean up ppc64le implementation slightly
As suggested by comments from the review of CL 168478, this adds
Go code to do reverse bytes and removes the asm code, as well
as making a few cosmetic changes.
Alex Brainman [Sun, 27 Oct 2019 06:13:45 +0000 (17:13 +1100)]
internal/syscall/windows/registry: make '-gcflags=all=-d=checkptr' flag work
Mostly replaced [:x] slice operation with [:x:x].
According to @mdempsky, compiler specially recognizes when you combine
a pointer conversion with a full slice operation in a single expression
and makes an exception.
Updates golang/go#34972
Change-Id: I07d9de3b31da254d55f50d14c18155f8fc8f3ece
Reviewed-on: https://go-review.googlesource.com/c/go/+/203442 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cuong Manh Le [Mon, 21 Oct 2019 16:25:32 +0000 (23:25 +0700)]
cmd/compile: hard fail if n.Opt() is not nil in walkCheckPtrArithmetic
n.Opt() is used in walkCheckPtrArithmetic to prevent infinite loops. The
fact that it's used today because n.Opt() is not used for OCONVNOP
during walk.go. If that changes, then it's not safe to repalce it
anymore. So doing hard fail if that case happens, the author of new
changes will be noticed and must change the usage of n.Opt() inside
walkCheckPtrArithmetic, too.
Change-Id: Ic7094baa1759c647fc10e82457c19026099a0d47
Reviewed-on: https://go-review.googlesource.com/c/go/+/202497
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Cherry Zhang [Sun, 27 Oct 2019 02:48:15 +0000 (22:48 -0400)]
cmd/internal/obj/mips: fix encoding of FCR registers
The asm encoder generally assumes that the lowest 5 bits of the
REG_XX constants match the machine instruction encoding, i.e.
the lowest 5 bits is the register number. This was not true for
FCR registers and M registers. Make it so.
MOV Rx, FCRy was encoded as two machine instructions. The first
is unnecessary. Remove.
Mikhail Fesenko [Mon, 28 Oct 2019 21:51:00 +0000 (21:51 +0000)]
cmd/fix, cmd/go, cmd/gofmt: refactor common code into new internal diff package
Change-Id: Idac8473d1752059bf2f617fd7a781000ee2c3af4
GitHub-Last-Rev: 02a3aa1a3241d3ed4422518f1c954cd54bbe858e
GitHub-Pull-Request: golang/go#35141
Reviewed-on: https://go-review.googlesource.com/c/go/+/203218
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Shenghou Ma [Fri, 25 Oct 2019 01:29:30 +0000 (21:29 -0400)]
cmd/compile/internal/gc: reword "declared and not used" error message
"declared and not used" is technically correct, but might confuse
the user. Switching "and" to "but" will hopefully create the
contrast for the users: they did one thing (declaration), but
not the other --- actually using the variable.
This new message is still not ideal (specifically, declared is not
entirely precise here), but at least it matches the other parsers
and is one step in the right direction.
Change-Id: I725c7c663535f9ab9725c4b0bf35b4fa74b0eb20
Reviewed-on: https://go-review.googlesource.com/c/go/+/203282
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Agniva De Sarker [Wed, 13 Feb 2019 03:19:52 +0000 (08:49 +0530)]
go/ast: fix SortImports to handle block comments (take 2)
This is a 2nd attempt at fixing CL 162337 which had an off-by-one error.
We were unconditionally getting the position of the start of the next line
from the last import without checking whether it is the end of the file or not.
Fix the code to check for that and move the testcase added in CL 190523
to the end of the file for it to trigger the issue properly.
Fixes #18929
Change-Id: I59e77256e256570b160fea6a17bce9ef49e810df
Reviewed-on: https://go-review.googlesource.com/c/go/+/190480
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
John Papandriopoulos [Sun, 29 Sep 2019 23:59:56 +0000 (16:59 -0700)]
cmd/link: pass-through undefined call targets in external link mode
Allows Go asm calls referencing a function in a .syso file to be
passed through to the external linker, that would have otherwise
raised a "relocation target X not defined" error in cmd/link.
Fixes #33139
Change-Id: I2a8eb6063ebcd05fac96f141acf7652cf9189766
Reviewed-on: https://go-review.googlesource.com/c/go/+/198798
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Constantin Konstantinidis [Sat, 26 Oct 2019 18:01:47 +0000 (20:01 +0200)]
os: remove read-only directories in RemoveAll on Windows
Remove skipping of TestRemoveUnreadableDir on Windows.
Fixes #26295
Change-Id: I364a3caa55406c855ece807759f6298f7e4ddf1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/203599
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Jay Conrod [Wed, 23 Oct 2019 21:14:09 +0000 (17:14 -0400)]
cmd/dist: support GOROOT vendoring
In the second step of make.bash, cmd/dist builds cmd/go by invoking
the compiler, linker, and other tools directly on transitive
dependencies of cmd/go. Essentially, cmd/dist acts as a minimal
version of 'go install' when building go_toolchain.
Until now, cmd/go has had no transitive dependencies in vendor
directories. This changes in CL 202698, where several packages are
deleted and equivalent versions in golang.org/x/mod are used
instead. So this CL adds support to cmd/dist for vendor directories.
Updates #31761
Change-Id: Iab4cdc7e505069a8df296287d16fbaa871944955
Reviewed-on: https://go-review.googlesource.com/c/go/+/203537
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Joshua M. Clulow [Mon, 28 Oct 2019 16:19:48 +0000 (09:19 -0700)]
runtime: make NumCPU respect zone CPU cap on illumos
On illumos systems, check for the "zone.cpu-cap" resource control when
determining how many usable CPUs are available. If the resource control
is not set, or we are unable to read it, ignore the failure and return
the value we used to return; i.e., the CPU count from
sysconf(_SC_NPROCESSORS_ONLN).
Cherry Zhang [Fri, 25 Oct 2019 20:43:08 +0000 (16:43 -0400)]
cmd/compile: delete ZeroAuto
ZeroAuto was used with the ambiguously live logic. The
ambiguously live logic is removed as we switched to stack
objects. It is now never called. Remove.
Lynn Boger [Mon, 28 Oct 2019 13:29:40 +0000 (09:29 -0400)]
runtime: fix textOff for multiple text sections
If a compilation has multiple text sections, code in
textOff must compare the offset argument against the range
for each text section to determine which one it is in.
The comparison looks like this:
if uintptr(off) >= sectaddr && uintptr(off) <= sectaddr+sectlen
If the off value being compared is equal to sectaddr+sectlen then it
is not within the range of the text section but after it. The
comparison should be just '<'.
Lynn Boger [Wed, 20 Mar 2019 13:30:27 +0000 (09:30 -0400)]
crypto/elliptic: add asm implementation for p256 on ppc64le
This adds an asm implementation of the p256 functions used
in crypto/elliptic, utilizing VMX, VSX to improve performance.
On a power9 the improvement is:
This implemenation is based on the s390x implementation, using
comparable instructions for most with some minor changes where the
instructions are not quite the same.
Some changes were also needed since s390x is big endian and ppc64le
is little endian.
Phil Pearl [Sun, 27 Oct 2019 16:05:54 +0000 (16:05 +0000)]
encoding/json: remove allocation when using a Marshaler with value receiver
If we marshal a non-pointer struct field whose type implements Marshaler with
a non-pointer receiver, then we avoid an allocation if we take the address of
the field before casting it to an interface.
name old time/op new time/op delta
EncodeMarshaler-8 104ns ± 1% 92ns ± 2% -11.72% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
EncodeMarshaler-8 36.0B ± 0% 4.0B ± 0% -88.89% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
EncodeMarshaler-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
Test coverage already looks good enough for this change. TestRefValMarshal
already covers all possible combinations of value & pointer receivers on
value and pointer struct fields.
Change-Id: I6fc7f72396396d98f9a90c3c86e813690f41c099
Reviewed-on: https://go-review.googlesource.com/c/go/+/203608 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Phil Pearl [Sun, 13 Oct 2019 12:01:58 +0000 (13:01 +0100)]
encoding/json: improve performance of Compact
This change improves performance of Compact by using a sync.Pool to allow re-use
of a scanner. This also has the side-effect of removing an allocation for each
field that implements Marshaler when marshalling JSON.
name old time/op new time/op delta
EncodeMarshaler-8 118ns ± 2% 104ns ± 1% -12.21% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
EncodeMarshaler-8 100B ± 0% 36B ± 0% -64.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
EncodeMarshaler-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=8+8)
Change-Id: Ic70c61a0a6354823da5220f5aad04b94c054f233
Reviewed-on: https://go-review.googlesource.com/c/go/+/200864 Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Jason A. Donenfeld [Sat, 26 Oct 2019 21:05:22 +0000 (23:05 +0200)]
internal/syscall/windows/registry: remove TestWalkFullRegistry due to false assumptions
This test's existence was predicated upon assumptions about the full
range of known data types and known data into those types. However,
we've learned from Microsoft that there are several undocumented secret
registry types that are in use by various parts of Windows, and we've
learned from inspection that many Microsoft uses of registry types don't
strictly adhere to the recommended value size. It's therefore foolhardy
to make any assumptions about what goes in and out of the registry, and
so this test, as well as its "blacklist", are meaningless.
Fixes #35084
Change-Id: I6c3fe5fb0e740e88858321b3b042c0ff1a23284e
Reviewed-on: https://go-review.googlesource.com/c/go/+/203604
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Giovanni Bajo [Fri, 27 Sep 2019 22:05:54 +0000 (00:05 +0200)]
cmd/compile: in poset, implement path collapsing
Sometimes, poset needs to collapse a path making all nodes in
the path aliases. For instance, we know that A<=N1<=B and we
learn that B<=A, we can deduce A==N1==B, and thus we can
collapse all paths from A to B into a single aliased node.
Currently, this is a TODO. This CL implements the path-collapsing
primitive by doing a DFS walk to build a bitset of all nodes
across all paths, and then calling the new aliasnodes that allow
to mark multiple nodes as aliases of a single master node.
This helps only 4 times in std+cmd, but it will be fundamental
when we will rely on poset to calculate numerical limits, to
calculate the correct values.
This also fixes #35157, a bug uncovered by a previous CL in this
serie. A testcase will be added soon.
Change-Id: I5fc54259711769d7bd7c2d166a5abc1cddc26350
Reviewed-on: https://go-review.googlesource.com/c/go/+/200861
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Giovanni Bajo [Fri, 27 Sep 2019 21:39:42 +0000 (23:39 +0200)]
cmd/compile: in poset, allow multiple aliases in a single pass
Change aliasnode into aliasnodes, to allow for recording
multiple aliases in a single pass. The nodes being aliased
are passed as bitset for performance reason (O(1) lookups).
It does look worse in the existing case of SetEqual where
we now need to allocate a bitset just for a single node,
but the new API will allow to fully implement a path-collapsing
primitive in next CL.
No functional changes, passes toolstash -cmp.
Change-Id: I06259610e8ef478106b36852464ed2caacd29ab5
Reviewed-on: https://go-review.googlesource.com/c/go/+/200860 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Giovanni Bajo [Fri, 27 Sep 2019 22:08:45 +0000 (00:08 +0200)]
cmd/compile: in poset, refactor aliasnode
In preparation for allowing to make multiple nodes as aliases
in a single pass, refactor aliasnode splitting out the case
in which one of the nodes is not in the post into a new
funciton (aliasnewnode).
No functional changes, passes toolstash -cmp
Change-Id: I19ca6ef8426f8aec9f2622b6151c5c617dbb25b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/200859 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Ben Shi [Tue, 25 Jun 2019 10:38:21 +0000 (10:38 +0000)]
runtime: save/restore callee-save registers in arm's sigtramp
ARM's R4-R8 & R10-R11 are callee-save registers, and R9
may be callee-save or not. This CL saves them at the beginning
of sigtramp and restores them in the end.
Austin Clements [Mon, 14 Oct 2019 21:05:56 +0000 (17:05 -0400)]
runtime: M-targeted signals for BSDs
For these, we split up the existing runtime.raise assembly
implementation into its constituent "get thread ID" and "signal
thread" parts. This lets us implement signalM and reimplement raise in
pure Go. (NetBSD conveniently already had lwp_self.)
We also change minit to store the procid directly, rather than
depending on newosproc to do so. This is because newosproc isn't
called for the bootstrap M, but we need a procid for every M. This is
also simpler overall.
Cuong Manh Le [Wed, 23 Oct 2019 07:31:29 +0000 (14:31 +0700)]
runtime: mark findObject nosplit
findObject takes the pointer argument as uintptr. If the pointer is to
the local stack and calling findObject happens to require the stack to
be reallocated, then spanOf is called for the old pointer.
runtime: only shrink stacks at synchronous safe points
We're about to introduce asynchronous safe points, where we won't have
precise pointer maps for all stack frames. That's okay for scanning
the stack (conservatively), but not for shrinking the stack.
Hence, this CL prepares for this by only shrinking the stack as part
of the stack scan if the goroutine is stopped at a synchronous safe
point. Otherwise, it queues up the stack shrink for the next
synchronous safe point.
We already have one condition under which we can't shrink the stack
for very similar reasons: syscalls. Currently, we just give up on
shrinking the stack if it's in a syscall. But with this mechanism, we
defer that stack shrink until the next synchronous safe point.
runtime: make copystack/sudog synchronization more explicit
When we copy a stack of a goroutine blocked in a channel operation, we
have to be very careful because other goroutines may be writing to
that goroutine's stack. To handle this, stack copying acquires the
locks for the channels a goroutine is waiting on.
One complication is that stack growth may happen while a goroutine
holds these locks, in which case stack copying must *not* acquire
these locks because that would self-deadlock.
Currently, stack growth never acquires these locks because stack
growth only happens when a goroutine is running, which means it's
either not blocking on a channel or it's holding the channel locks
already. Stack shrinking always acquires these locks because shrinking
happens asynchronously, so the goroutine is never running, so there
are either no locks or they've been released by the goroutine.
However, we're about to change when stack shrinking can happen, which
is going to break the current rules. Rather than find a new way to
derive whether to acquire these locks or not, this CL simply adds a
flag to the g struct that indicates that stack copying should acquire
channel locks. This flag is set while the goroutine is blocked on a
channel op.
Currently, gcscanvalid is used to resolve a race between attempts to
scan a stack. Now that there's a clear owner of the stack scan
operation, there's no longer any danger of racing or attempting to
scan a stack more than once, so this CL eliminates gcscanvalid.
I double-checked my reasoning by first adding a throw if gcscanvalid
was set in scanstack and verifying that all.bash still passed.
Currently, the process of suspending a goroutine is tied to stack
scanning. In preparation for non-cooperative preemption, this CL
abstracts this into general purpose suspendG/resumeG functions.
suspendG and resumeG closely follow the existing scang and restartg
functions with one exception: the addition of a _Gpreempted status.
Currently, preemption tasks (stack scanning) are carried out by the
target goroutine if it's in _Grunning. In this new approach, the task
is always carried out by the goroutine that called suspendG. Thus, we
need a reliable way to drive the target goroutine out of _Grunning
until the requesting goroutine is ready to resume it. The new
_Gpreempted state provides the handshake: when a runnable goroutine
responds to a preemption request, it now parks itself and enters
_Gpreempted. The requesting goroutine races to put it in _Gwaiting,
which gives it ownership, but also the responsibility to start it
again.
This CL adds several TODOs about improving the synchronization on the
G status. The existing code already has these problems; we're just
taking note of them.
The next CL will remove the now-dead scang and preemptscan.
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>