Other test flags passed to the test binary, such as -run or -count, are
equally pointless when -c or -i are used, since the test binary is never
run. However, custom flags in that scenario are far more likely to be
due to human error, such as:
# note the "ldflags" typo, which silently did nothing
go test -c -lflags=-w
Instead, make this scenario error. It seems unlikely that anyone is
using -c along with intended custom-defined test flags, and if they are,
removing those extra flags that do nothing is probably a good idea
anyway.
We don't add this restriction for the flags defined in 'go help
testflag', since they are far less likely to be typos or unintended
mistakes. Another reason not to do that change is that other commands
similarly silently ignore no-op flags, such as:
# -d disables the build, so -ldflags is never used
go get -d -ldflags=-w
Fixes#39484.
Change-Id: I6ba2f6866562fe8f8fceaf4cd862d874bf5cd978
Reviewed-on: https://go-review.googlesource.com/c/go/+/237697
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If a signal is sent to t.signal before the panic is triggered,
a panicking test may end up with "warning: no tests to run" because
the tRunner that invokes the test in t.Run calls runtime.Goexit on
panic, which causes the panicking test not be recorded in runTests.
Send the signal if and only if there is no panic.
Fixes#41479
Change-Id: I812f1303bfe02c443a1902732e68d21620d6672e
Reviewed-on: https://go-review.googlesource.com/c/go/+/256098
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Bryan C. Mills <bcmills@google.com>
Now reports an error if cyclic maps and slices are to be encoded
instead of an infinite recursion. This case wasn't handled in CL 187920.
Fixes#40745.
Change-Id: Ia34b014ecbb71fd2663bb065ba5355a307dbcc15
GitHub-Last-Rev: 6f874944f4
GitHub-Pull-Request: golang/go#40756
Reviewed-on: https://go-review.googlesource.com/c/go/+/248358
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Allow ';' as a valid character for json field keys and struct tags.
Fixes#39189
Change-Id: I4b602a1b0674ff028db07623682f0d1e8e9fd6c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/234818
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
CL 250357 exported net.ErrClosed to allow more reliable detection
of closed network connection errors. Use that error in crypto/tls
as well.
The error message is changed from "tls: use of closed connection"
to "use of closed network connection", so the code that detected such
errors by looking for that text in the error message will need to be
updated to use errors.Is(err, net.ErrClosed) instead.
Fixes#41066
Change-Id: Ic05c0ed6a4f57af2a0302d53b00851a59200be2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/256897
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Toolstash-check successful for remaining rules using GOARM value.
Change-Id: I254f80d17839ef4957c1b7afbdb4db363a3b9367
Reviewed-on: https://go-review.googlesource.com/c/go/+/240997
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
The IndexString implementation in the bytealg package requires that
the string passed into it be in the range '2 <= len(s) <= MaxLen'
where MaxLen may be any value (including 0).
CL 156998 added calls to bytealg.IndexString where MaxLen was not
first checked. This led to an illegal instruction on s390x with
the vector facility disabled.
This CL guards the calls to bytealg.IndexString with a MaxLen check.
If the check fails then the code now falls back to the pre CL 156998
implementation (a loop over the runes in the string).
Since the MaxLen check is now in place the generic implementation is
no longer called so I have returned it to its original unimplemented
state.
In future we may want to drop MaxLen to prevent this kind of
confusion.
Fixes#41552.
Change-Id: Ibeb3f08720444a05c08d719ed97f6cef2423bbe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/256717
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>
add type casting to int32: L148-L156, L774-L778
Toolstash-check successful
Change-Id: Ib6544c1d7853c2811def5b18786e1fc5c18086ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/256097
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Introduce GOOS=ios for iOS systems. GOOS=ios matches "darwin"
build tag, like GOOS=android matches "linux" and GOOS=illumos
matches "solaris". Only ios/arm64 is supported (ios/amd64 is
not).
GOOS=ios and GOOS=darwin remain essentially the same at this
point. They will diverge at later time, to differentiate macOS
and iOS.
Uses of GOOS=="darwin" are changed to (GOOS=="darwin" || GOOS=="ios"),
except if it clearly means macOS (e.g. GOOS=="darwin" && GOARCH=="amd64"),
it remains GOOS=="darwin".
Updates #38485.
Change-Id: I4faacdc1008f42434599efb3c3ad90763a83b67c
Reviewed-on: https://go-review.googlesource.com/c/go/+/254740
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The instructions have already been updated in greater
detail in "Step 2: Configure git authentication", but
the overview needs updated to reflect the new workflow.
Change-Id: I6f411a3dc500a9058036a4a828403c0153e4220a
Reviewed-on: https://go-review.googlesource.com/c/go/+/256857
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
This does not work yet for SSA-able aggregates.
Change-Id: Ib16b9c6158b25bb957145c5f934040b2bab9babd
Reviewed-on: https://go-review.googlesource.com/c/go/+/245132
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
"mul by constant" until "div by constant"
L547-L609
Change-Id: I19ebb5694e383878f505d34df2591a51fe38431a
Reviewed-on: https://go-review.googlesource.com/c/go/+/254662
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
The sk_max_ack_backlog was increased from uint16 to uint32 in kernel
version 4.1 and above, so adopt that change to maxListenerBacklog.
See becb74f0acFixes#41470
Change-Id: I63a142eb28f3ac3acaca57f0903c085c6cb15a6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/255898
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
End of "constant folding in *shift ops" until EOF
(L1070-)
Toolstash-check is successful.
Change-Id: I55846a459aca5238f831750f04132e13a0baeed7
Reviewed-on: https://go-review.googlesource.com/c/go/+/234198
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Go Bot <gobot@golang.org>
From "absorb InvertFlags" until "constant folding in *shift ops"
L666-L1011
Toolstash-check is successful.
Change-Id: Ieed7d4643dc3dc2b3649477e87aebd22c81d1322
Reviewed-on: https://go-review.googlesource.com/c/go/+/234197
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Go Bot <gobot@golang.org>
Toolstash-check successful from L270 until L543.
Change-Id: Ic39ab86c80f970bfb21e318284f6bb3e8a994220
Reviewed-on: https://go-review.googlesource.com/c/go/+/233439
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Go Bot <gobot@golang.org>
Toolstash-check successful from L0 until L268
Change-Id: Ifc55ea1e4177c21107c521fc72da2da7b507b8ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/232811
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
QueryPackage was a wrapper around QueryPattern with extra validation,
called only once from within the same package. Most of that validation
was already performed much earlier, in (*loader).Load. Inline the
remaining validation and remove the needless indirection.
For #36460
Change-Id: I108a01d416197db8f886889554e07b29f0c37f3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/256057
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Direct syscalls are no longer supported on darwin, instead wrap the
existing func ioctl for tests.
Change-Id: Ie2c5b6e5f54e992f4d6b21513ca8f89fcf28ef10
Reviewed-on: https://go-review.googlesource.com/c/go/+/256219
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For #41315
Change-Id: I3989bcb051ae57dd2d8f89759d241d4cdce49659
Reviewed-on: https://go-review.googlesource.com/c/go/+/255969
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This flag, like the -e flag to 'go list', instructs the command to
make a best effort to continue in spite of errors for specific packages.
Fixes#26603
Change-Id: I5ee2f50c71870ae8ef3f9b3e5b045474adcca525
Reviewed-on: https://go-review.googlesource.com/c/go/+/255960
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This change replaces ImportPaths, ImportPathsQuiet, LoadALL, and
LoadVendor with a single LoadPackages function, with a LoadOpts struct
that more clearly documents the variations in behavior.
It also eliminates the cmd/go/internal/load.ImportPaths function,
which was undocumented and had only one call site (within its own
package).
The modload.LoadTests global variable is subsumed by a field in the
new LoadOpts struct, and is no longer needed for callers that invoke
LoadPackages directly. It has been (temporarily) replaced with a
similar global variable, load.ModResolveTests, which can itself be
converted to an explicit, local argument.
For #37438
For #36460
Updates #40775Fixes#26977
Change-Id: I4fb6086c01b04de829d98875db19cf0118d40f8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/255938
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Functions that require holding sched.lock now have an assertion.
A few places with missing locks have been fixed in this CL:
Additionally, locking is added around the call to procresize in
schedinit. This doesn't technically need a lock since the program is
still starting (thus no concurrency) when this is called, but lock held
checking doesn't know that.
Updates #40677
Change-Id: I198d3cbaa727f7088e4d55ba8fa989cf1ee8f9cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/250261
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When lock ranking is enabled, we can now assert that lock preconditions
are met by checking that the caller holds required locks on function
entry.
This change adds the infrastructure to add assertions. Actual assertions
will be added for various locks in subsequent changes.
Some functions are protected by locks that are not directly accessible
in the function. In that case, we can use assertRankHeld to check that
any lock with the rank is held. This is less precise, but it avoids
requiring passing the lock into the functions.
Updates #40677
Change-Id: I843c6874867f975e90a063f087b6e2ffc147877b
Reviewed-on: https://go-review.googlesource.com/c/go/+/245484
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Avoid leaking fd in case fd.Stat() fails in the fall back implementation
of Getwd.
Change-Id: I8656d42e8dbc8893b7159873f173d6bf0d4febe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/256221
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
While at it, make "typekind" awares of "types.Ideal*" types.
Passes toolstash-check.
Change-Id: I092fa8c57ab6b8d9ba0f25d8e1ea44fba48675e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/256438
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
The Go 1.16 development cycle has started. This is the time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
Those versions have already gone through code review, and now they
will undergo additional testing during the development period.
If there are new issues in these dependencies discovered, we have
development period to deal with that. We will do this update once
more at the end of the development cycle, by the code freeze, and
so doing it now will make that update smaller and safer.
Overall, this change will help us build confidence that the
Go 1.16 release and its selected dependencies will be robust.
Also increment the Go language version to 1.16 in standard library
go.mod files.
This change was created with a program from CL 256357 patch set 1
(which updates golang.org/x modules only) and the bundle tool at
CL 255053 patch set 1:
$ updatestd -goroot=$HOME/gotip -branch=master
> go version
go version devel +eda1d40544 Mon Sep 21 16:50:07 2020 +0000 darwin/amd64
> go env GOROOT
/Users/dmitshur/gotip
> go version -m /Users/dmitshur/go/bin/bundle
/Users/dmitshur/go/bin/bundle: go1.15.2
path golang.org/x/tools/cmd/bundle
mod golang.org/x/tools (devel) # CL 255053 PS 1
dep golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
dep golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
updating module cmd in /Users/dmitshur/gotip/src/cmd
skipping github.com/chzyer/logex (out of scope, it's not a golang.org/x dependency)
skipping github.com/chzyer/readline (out of scope, it's not a golang.org/x dependency)
skipping github.com/chzyer/test (out of scope, it's not a golang.org/x dependency)
skipping github.com/google/pprof (out of scope, it's not a golang.org/x dependency)
skipping github.com/ianlancetaylor/demangle (out of scope, it's not a golang.org/x dependency)
skipping github.com/yuin/goldmark (out of scope, it's not a golang.org/x dependency)
skipping rsc.io/pdf (out of scope, it's not a golang.org/x dependency)
> go mod edit -go=1.16
> go get -d golang.org/x/arch@b19915210f009e139b20abfd6a6052c7acc1f445 golang.org/x/crypto@5c72a883971a4325f8c62bf07b6d38c20ea47a6a golang.org/x/mod@ce943fd02449f621243c9ea6e64098e84752b92b golang.org/x/net@62affa334b73ec65ed44a326519ac12c421905e3 golang.org/x/sync@6e8e738ad208923de99951fe0b48239bfd864f28 golang.org/x/sys@af09f7315aff1cbc48fb21d21aa55d67b4f914c5 golang.org/x/text@a8b4671254579a87fadf9f7fa577dc7368e9d009 golang.org/x/tools@d647fc2532668b2b75a92f468487b8085e6ed58b golang.org/x/xerrors@5ec99f83aff198f5fbd629d6c8d8eb38a04218ca
go: golang.org/x/sys af09f7315aff1cbc48fb21d21aa55d67b4f914c5 => v0.0.0-20200918174421-af09f7315aff
go: golang.org/x/text a8b4671254579a87fadf9f7fa577dc7368e9d009 => v0.3.4-0.20200826142016-a8b467125457
go: golang.org/x/tools d647fc2532 => v0.0.0-20200918232735-d647fc253266
go: golang.org/x/net 62affa334b73ec65ed44a326519ac12c421905e3 => v0.0.0-20200904194848-62affa334b73
go: golang.org/x/crypto 5c72a883971a4325f8c62bf07b6d38c20ea47a6a => v0.0.0-20200820211705-5c72a883971a
go: golang.org/x/arch b19915210f009e139b20abfd6a6052c7acc1f445 => v0.0.0-20200826200359-b19915210f00
go: golang.org/x/xerrors 5ec99f83aff198f5fbd629d6c8d8eb38a04218ca => v0.0.0-20200804184101-5ec99f83aff1
> go mod tidy
> go mod vendor
updating module std in /Users/dmitshur/gotip/src
> go mod edit -go=1.16
> go get -d golang.org/x/crypto@5c72a883971a4325f8c62bf07b6d38c20ea47a6a golang.org/x/net@62affa334b73ec65ed44a326519ac12c421905e3 golang.org/x/sys@af09f7315aff1cbc48fb21d21aa55d67b4f914c5 golang.org/x/text@a8b4671254579a87fadf9f7fa577dc7368e9d009 golang.org/x/tools@d647fc2532668b2b75a92f468487b8085e6ed58b
go: golang.org/x/crypto 5c72a883971a4325f8c62bf07b6d38c20ea47a6a => v0.0.0-20200820211705-5c72a883971a
go: golang.org/x/text a8b4671254579a87fadf9f7fa577dc7368e9d009 => v0.3.4-0.20200826142016-a8b467125457
go: golang.org/x/sys af09f7315aff1cbc48fb21d21aa55d67b4f914c5 => v0.0.0-20200918174421-af09f7315aff
go: golang.org/x/tools d647fc2532 => v0.0.0-20200918232735-d647fc253266
> go mod tidy
> go mod vendor
updating bundles in /Users/dmitshur/gotip/src
> go generate -run=bundle std cmd
Other non-golang.org/x module dependencies (pprof and demangle)
still need to be updated in a future CL.
For #36905.
Change-Id: I83a350bf8714ebc249284c0d69abe4941700565e
Reviewed-on: https://go-review.googlesource.com/c/go/+/255860
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Adds deprecation warning for -insecure flag on go get in both modules
and GOPATH mode.
Updates #37519.
Change-Id: Ie2efeeb4a91e6dda92955295969e9715314ae50e
GitHub-Last-Rev: a9ebe21fe0
GitHub-Pull-Request: golang/go#41497
Reviewed-on: https://go-review.googlesource.com/c/go/+/255882
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
For a common pattern of iterating only over top-level compilation units (CU)
Reader.SkipChildren has decode and meterialize all CU subentries just
to skip them, because DW_TAG_compile_unit does not have DW_AT_sibling.
However, CUs have total size encoded before the unit and we already parse them
and know all unit sizes.
Optimize Reader.SkipChildren to use that size when skipping CUs children.
This speeds up iteration over a 1.3GB object file from 7.5s to 0.73s.
Change-Id: I2a8f00955159b4bd13571409f4817805f934cb69
Reviewed-on: https://go-review.googlesource.com/c/go/+/256217
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Than McIntosh <thanm@google.com>
Currently, if make.bash is run with a GOOS or GOARCH that the
bootstrap toolchain doesn't support, it will print an ominous but
harmless warning like:
2020/09/21 09:05:27 unsupported GOARCH arm64
This comes from the invocation of "go version" to get the exact
bootstrap toolchain version.
Since the GOOS and GOARCH don't matter for this purpose, this CL
simply clears them on the invocation of the bootstrap toolchain's "go
version".
Fixes#41525.
Change-Id: I17d44eaafed9999b9fa7dcb9fb100b5fd5e554d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/256297
Trust: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
unlockf is called after the G is put into _Gwaiting, meaning another G
may have readied this one before unlockf is called.
This is implied by the current doc, but add additional notes to call out
this behavior, as it can be quite surprising.
Updates #40641
Change-Id: I60b1ccc6a4dd9ced8ad2aa1f729cb2e973100b59
Reviewed-on: https://go-review.googlesource.com/c/go/+/256058
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently activeStackChans is set before a goroutine blocks on a channel
operation in an unlockf passed to gopark. The trouble is that the
unlockf is called *after* the G's status is changed, and the G's status
is what is used by a concurrent mark worker (calling suspendG) to
determine that a G has successfully been suspended. In this window
between the status change and unlockf, the mark worker could try to
shrink the G's stack, and in particular observe that activeStackChans is
false. This observation will cause the mark worker to *not* synchronize
with concurrent channel operations when it should, and so updating
pointers in the sudog for the blocked goroutine (which may point to the
goroutine's stack) races with channel operations which may also
manipulate the pointer (read it, dereference it, update it, etc.).
Fix the problem by adding a new atomically-updated flag to the g struct
called parkingOnChan, which is non-zero in the race window above. Then,
in isShrinkStackSafe, check if parkingOnChan is zero. The race is
resolved like so:
* Blocking G sets parkingOnChan, then changes status in gopark.
* Mark worker successfully suspends blocking G.
* If the mark worker observes parkingOnChan is non-zero when checking
isShrinkStackSafe, then it's not safe to shrink (we're in the race
window).
* If the mark worker observes parkingOnChan as zero, then because
the mark worker observed the G status change, it can be sure that
gopark's unlockf completed, and gp.activeStackChans will be correct.
The risk of this change is low, since although it reduces the number of
places that stack shrinking is allowed, the window here is incredibly
small. Essentially, every place that it might crash now is replaced with
no shrink.
This change adds a test, but the race window is so small that it's hard
to trigger without a well-placed sleep in park_m. Also, this change
fixes stackGrowRecursive in proc_test.go to actually allocate a 128-byte
stack frame. It turns out the compiler was destructuring the "pad" field
and only allocating one uint64 on the stack.
Fixes#40641.
Change-Id: I7dfbe7d460f6972b8956116b137bc13bc24464e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/247050
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
In the current implementation, we can observe crashes after calling
debug.SetMaxStack and allocating a stack larger than 4GB since
stackalloc works with 32-bit sizes. To avoid this, we define an upper
limit as the largest feasible point we can grow a stack to and provide a
better error message when we get a stack overflow.
Fixes#41228
Change-Id: I55fb0a824f47ed9fb1fcc2445a4dfd57da9ef8d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/255997
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
Document the way EncodeRune currently handles runes which are
out of range. Also add an example showing that behaviour.
Change-Id: I0f8e7645ae053474ec319085a2bb6d7f73bc137c
Reviewed-on: https://go-review.googlesource.com/c/go/+/255998
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Trust: Giovanni Bajo <rasky@develer.com>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Go Bot <gobot@golang.org>
