When the compiler refers to a runtime builtin, it emits an indexed
symbol reference in the object file via predetermined/preassigned ID
within the PkgIdxBuiltin pseudo-package. At link time when the loader
encounters these references, it redirects them to the corresponding
defined symbol in the runtime package. This redirection process
currently assumes that if a runtime builtin is referenced, we'll
always have a definition for it. This assumption holds in most cases,
however for the builtins "runtime.racefuncenter" and
"runtime.racefuncexit", we'll only see definitions if the runtime
package we're linking against was built with "-race".
In the bug in question, build passes "-gcflags=-race" during
compilation of the main package, but doesn't pass "-race" directly to
'go build', and as a result the final link combines a
race-instrumented main with a non-race runtime; this results in R_CALL
relocations with zero-valued target symbols, resulting in a panic
during stack checking.
This patch changes the loader's resolve method to detect situations
where we're asking for builtin "runtime.X", but the runtime package
read in doesn't contain a definition for X.
Fixes#42396.
Change-Id: Iafd38bd3b0f7f462868d120ccd4d7d1b88b27436
Reviewed-on: https://go-review.googlesource.com/c/go/+/267881
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Tools often need to associate errors not with a single position, but
with a span of source code. For example, gopls currently estimates
diagnostic spans using heuristics to expand the positions reported by
the type checker to surrounding source code. Unfortunately this is often
inaccurate.
This CL lays the groundwork to solve this within go/types by adding a
start and end position to type checker errors. This is an experimental
API, both because we are uncertain of the ideal representation for these
spans and because their initial positioning is naive. In most cases this
CL simply expands errors to the surrounding ast.Node being typechecked,
if available. This might not be the best error span to present to the
user. For these reasons the API is unexported -- gopls can read these
positions using reflection, allowing us to gain experience and improve
them during the next development cycle.
For golang/go#42290
Change-Id: I39a04d70ea2bb2134b4d4c937f32b2ddb4456430
Reviewed-on: https://go-review.googlesource.com/c/go/+/265250
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Test script expects the regexp argument for stdout, stderr, and cmp
to be the first argument after the command, but that might not be the
case if the -q or -count flags are provided. Treat the first argument
after a flag as a regexp instead.
For #39958
Change-Id: I369926109ec10cca8b2c3baca27e7a3f7baf364b
Reviewed-on: https://go-review.googlesource.com/c/go/+/267877
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
startm contains a critical section from when it takes ownership of a P
(either on function entry or call to pidleput) until it wakes the M
receiving the P. If preempted in this critical section, the owned P is
left in limbo. If preempted for a GC stop, there will be nothing to stop
the owned P and STW will wait forever.
golang.org/cl/232298 introduced the first call to startm that is not on
the system stack (via a wakep call), introducing the possibility of
preemption. Disable preemption in startm to ensure this remains
non-preemptible.
Since we're not always on the system stack anymore, we also need to be
careful in allocm.
Updates #42237
Change-Id: Icb95eef9eb262121856485316098331beea045da
Reviewed-on: https://go-review.googlesource.com/c/go/+/267257
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>
Previously, we resolved each argument to 'go get' to a package path or
module path based on what was in the build list at existing versions,
even if the argument specified a different version explicitly. That
resulted in bugs like #37438, in which we variously resolved the wrong
version or guessed the wrong argument type for what is unambiguously a
package argument at the requested version.
We were also using a two-step upgrade/downgrade algorithm, which could
not only upgrade more that is strictly necessary, but could also
unintentionally upgrade *above* the requested versions during the
downgrade step.
This change instead uses an iterative approach, with an explicit
disambiguation step for the (rare) cases where an argument could match
the same package path in multiple modules. We use a hook in the
package loader to halt package loading as soon as an incorrect version
is found — preventing over-resolving — and verify that the result
after applying downgrades successfully obtained the requested versions
of all modules.
Making 'go get' be correct and usable is especially important now that
we are defaulting to read-only mode (#40728), for which we are
recommending 'go get' more heavily.
While I'm in here refactoring, I'm also reworking the API boundary
between the modget and modload packages. Previously, the modget
package edited the build list directly, and the modload package
accepted the edited build list without validation. For lazy loading
(#36460), the modload package will need to maintain additional
metadata about the requirement graph, so it needs tighter control over
the changes to the build list.
As of this change, modget no longer invokes MVS directly, but instead
goes through the modload package. The resulting API gives clearer
reasons in case of updates, which we can use to emit more useful
errors.
Fixes#37438
Updates #36460
Updates #40728
Change-Id: I596f0020f3795870dec258147e6fc26a3292c93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/263267
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: Russ Cox <rsc@golang.org>
As far as I can tell, this bug had gone unnoticed because everything
that uses Max so far happened to only ever present the empty string as
the first argument.
For #37438
Change-Id: Ie8c42313157d2c2c17e4058c53b5bb026b95a1c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/266860
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This has no impact on the resulting build list, but provides clearer
diagnostics if reqs.Required returns an error for one of the upgraded
modules.
For #37438
Change-Id: I5cd8f72a9b7b9a0b185e1a728f46fefbd2f09b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/266897
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Also clean up the test assertions, and add a check for assertions
missing function invocations (there was one).
For #37438
Change-Id: Iafbfeae2c25217eac894181e01480b25b7cffbd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/266859
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
For #37438
Change-Id: I7df80ae0917b0b4ecad98947da39ddf8554b07c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/266717
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This allows a single QueryPattern call to resolve a path that could be
either a package or a module. It is important to be able to make a
single QueryPattern call — rather than a QueryPattern followed by a
Query for the specific module path — to provide appropriate fallback
behavior: if the proxy returns package results but does not contain a
module result, we don't want to fall back to the next proxy to look
for the (probably-nonexistent) module.
For #37438
Change-Id: I419b8bb3ab4565f443bb5cee9a8b206f453b9801
Reviewed-on: https://go-review.googlesource.com/c/go/+/266657
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
The file "cmd/internal/gc/range.go" does not exist, but should be
"cmd/compile/internal/gc/range.go".
Change-Id: I26e5560b9d0b7eea8502c6b375e45fc87aed1276
GitHub-Last-Rev: 5f19dca7e9
GitHub-Pull-Request: golang/go#42391
Reviewed-on: https://go-review.googlesource.com/c/go/+/267837
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
This reverts CL 266420.
Reason for revert: tests aren't passing on linux-{386,amd64}-longtest.
Change-Id: Icec47cded795a51ef7569dfb2d93d9211b4fb578
Reviewed-on: https://go-review.googlesource.com/c/go/+/267799
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This reverts CL 266240.
Reason for revert: tests aren't passing on windows-amd64-longtest.
Change-Id: If323c6254a42aff0418e2c0a9531f3d4c829a242
Reviewed-on: https://go-review.googlesource.com/c/go/+/267798
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
CL 242083 corrected an inaccurate error message related to the
assignability of untyped constant values. Previously the error message
was of the form "cannot convert ... to ...", which is misleading when
there is no explicit conversion in the syntax. The new error message
corrected this to "cannot use ... as ... in ...", but also appended an
inner error message that can be quite verbose. For example:
cannot use "123" (untyped string constant) as int value in assignment:
cannot convert "123" (untyped string constant) to int"
This might be more accurate, but is a regression in readability. Correct
this by only including the inner error message in the rare cases where
it is helpful: if the constant value overflows or is truncated.
For golang/go#22070
Change-Id: I8b8ee6ef713f64facc319894be09398b0b5ea500
Reviewed-on: https://go-review.googlesource.com/c/go/+/267717
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Remove all cpu features from the ARM64 struct that are not initialized
to reduce cache lines used and to avoid those features being
accidentially used without actual detection if they are present.
Add missing option to mask the CPUID feature.
Change-Id: I94bf90c0655de1af2218ac72117ac6c52adfc289
Reviewed-on: https://go-review.googlesource.com/c/go/+/267658
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Martin Möhrmann <moehrmann@google.com>
Fixes the check for RFC 5322 "obsolete time zone" to ensure
that we correctly extract the entire date from the "T" of the
implied time zone.
Obsolete Time zones come in the form:
* GMT
* PST
* MDT
etc, as per Section 4.3 of RFC 5322,
https://tools.ietf.org/html/rfc5322#section-4.3.
The prior check from CL 117596 erronenously used strings.Index
which selects the first "T", and that meant that dates containing
days "Tue" or "Thu" could not be parsed.
We also now deal with "T" in the CFWS "Comment Folding White Space".
Thus we'll now accept dates:
* Thu, 20 Nov 1997 09:55:06 MDT
* Thu, 20 Nov 1997 09:55:06 MDT (MDT)
* Fri, 21 Nov 1997 09:55:06 MDT (This comment)
* Fri, 21 Nov 1997 09:55:06 MDT (MDT
Fixes#39260
Change-Id: I6d59d99bc4f05a82582c826b5c5a080a25fd999b
Reviewed-on: https://go-review.googlesource.com/c/go/+/235200
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
The go command runs commands like git and hg to download modules.
In the past, we have had problems with security bugs in version
control systems becoming security bugs in “go get”.
The original modules draft design removed use of these commands
entirely, saying:
> We want to move away from invoking version control tools such as bzr,
> fossil, git, hg, and svn to download source code. These fragment the
> ecosystem: packages developed using Bazaar or Fossil, for example, are
> effectively unavailable to users who cannot or choose not to install
> these tools. The version control tools have also been a source of
> exciting security problems. It would be good to move them outside the
> security perimeter.
The removal of these commands was not possible in the end: being able
to fetch directly from Git repos is too important, especially for
closed source. But the security exposure has not gone away.
We remain vulnerable to problems in VCS systems, especially the less
scrutinized ones.
This change adds a GOVCS setting to let users control which version
control systems are allowed by default.
It also changes the default allowed version control systems to git and hg
for public code and any version control system for private code
(import path or module path matched by the GOPRIVATE setting).
See the changes in alldocs.go for detailed documentation.
See #41730 for proposal and discussion.
Fixes#41730.
Change-Id: I1999ddf7445b36a7572965be5897c7a1ff7f4265
Reviewed-on: https://go-review.googlesource.com/c/go/+/266420
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This patch adds a prologue_end statement to the DWARF information for
the ppc64 arch.
Prologue end is used by the Delve debugger in order to determine where
to set a breakpoint to avoid the stacksplit prologue.
Updates #36612
Change-Id: Ifb16c1476fe716a0bf493c5486d1d88ebe8d0253
GitHub-Last-Rev: 77a217206d
GitHub-Pull-Request: golang/go#42261
Reviewed-on: https://go-review.googlesource.com/c/go/+/266019
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
WalkDir is like Walk but can use ReadDir to read directories,
instead of Readdirnames + Lstat on every entry,
which is usually a significant performance improvement.
(The Lstat can still happen if the walk function calls d.Info.)
Fixes#42027.
Change-Id: Ie11024b23be2656e320d41fd81ff0d8810aa729e
Reviewed-on: https://go-review.googlesource.com/c/go/+/266240
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The wrong value for the first reg parameter was selected.
Likewise the wrong opcode was selected. This should match
rlwnm (rrr type), not rlwinm (irr type).
Similarly, fix the optab matching rules so clrlslwi does
not match reg,reg,const,reg arguments. This is not a valid
operand combination for clrlslwi.
Fixes#42368
Change-Id: I4eb16d45a760b9fd3f497ef9863f82465351d39f
Reviewed-on: https://go-review.googlesource.com/c/go/+/267421
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
The refactoring of this code while adding ReadDir stopped
pre-allocating a 100-entry slice for the results.
That seemed like a good idea in general, since many
directories have nowhere near 100 entries, but it had the
side effect of returning a nil slice for an empty directory.
Some “golden” tests that are too sensitive about nil vs not
inside Google broke because Readdirnames(-1) was now
returning nil instead of []string{} on an empty directory.
It seems likely there are other such tests in the wild, and
it doesn't seem worth breaking them.
This commit restores the non-nil-ness of the old result,
without restoring the excessive preallocation.
Fixes#42367.
Change-Id: I2be72030ac703346e859a97c2d4e456fadfce9b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/267637
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
A recent update of the Fossil SCM application changes
the line prefix when the fossil info command is used.
Instead of the revision hash starting with "uuid:", it has been
changed to "hash:".
Fossil check-in introducing this change:
https://fossil-scm.org/home/info/8ad5e4690854a81a
To support older and new versions, fossilParseStat will
now check for either version of the prefix when attempting
to find the line containing the hash of the desired revision.
Fixes#42323
Change-Id: I6eff49f9989b37b295322a8569e222a1fd02f6e3
GitHub-Last-Rev: f4e6652307
GitHub-Pull-Request: golang/go#42324
Reviewed-on: https://go-review.googlesource.com/c/go/+/267080
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
OpenBSD gained __thread support quite some time ago.
Change-Id: I7de0a5c0c4de1a7ce59e48ac939fc2daf56be8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/267318
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
The LDANDx instructions were misleading because they correspond to the
mnemonic LDCLRx as defined in the Arm Architecture Reference Manual for
Armv8. This changes the assembler to use the same mnemonic as the GNU
assembler and the manual.
The instruction has the form:
LDCLRx Rs, (Rb), Rt: *Rb -> Rt, Rs AND NOT(*Rb) -> *Rb
Change-Id: I94ae003e99e817209bba1afe960e612bf3a0b410
Reviewed-on: https://go-review.googlesource.com/c/go/+/267138
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: fannie zhang <Fannie.Zhang@arm.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: fannie zhang <Fannie.Zhang@arm.com>
Add a physPageAlignedStack boolean which if set, results in over allocation
by a physical page, the allocation being rounded to physical page alignment
and the unused memory surrounding the allocation being freed again.
OpenBSD/octeon has 16KB physical pages and requires stacks to be physical page
aligned in order for them to be remapped as MAP_STACK. This change allows Go
to work on this platform.
Based on a suggestion from mknyszek in issue #41008.
Updates #40995Fixes#41008
Change-Id: Ia5d652292b515916db473043b41f6030094461d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/266919
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Enable pie as a buildmode for linux/riscv64, along with associated tests.
Change-Id: I3fb0234d534dbeb96aa6cee6ae872304fbe02cf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/267317
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This is a partial revert of https://go-review.googlesource.com/c/go/+/248219
because we found that a non-trivial amount of code erroneously calls
ParseFloat(s, 10) or even ParseFloat(s, 0) and expects it to work --
before that change was merged, ParseFloat accepted a bitSize of
anything other than 32 or 64 to mean 64 (and ParseComplex was similar).
So revert that behavior to avoid breaking people's code, and add tests
for this.
I may add a vet check to flag ParseFloat(s, not_32_or_64) in a later
change.
See #42297 for more details.
Change-Id: I4bc0156bd74f67a39d5561b6e5fde3f2d20bd622
Reviewed-on: https://go-review.googlesource.com/c/go/+/267319
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Restrict -D and -U to ASCII C identifiers, but do permit trailing digits.
When using -Wp, prohibit commas in -D values.
Change-Id: Ibfc4dfdd6e6c258e131448e7682610c44eee9492
Reviewed-on: https://go-review.googlesource.com/c/go/+/267277
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Fixes#36641
Change-Id: I51868d83ce341d78d33b221d184c5a5110c60d14
Reviewed-on: https://go-review.googlesource.com/c/go/+/263598
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change-Id: Ia14659a9c44f9e1504eb88b5693932b9dd4bb286
Reviewed-on: https://go-review.googlesource.com/c/go/+/252939
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previous fix in issue #20929 for adding comment marker does
not check whether string field have // prefix or not.
This commit ensures string field does not contain // before adding
prefix to the line. Test also included in this commit.
Fixes#40992
Change-Id: Ibc5e8ef147eeb2ed732fb9e19815c8b21fcfb2ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/251237
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Exposes ErrProcessDone variable in windows and plan9
also returns this error code instead of
errors.New("os: process already finished")
Fixes#42311
Change-Id: Ie807b6526e7b6c27636e6bffe5ff0c904b319be4
GitHub-Last-Rev: 2153e0d702
GitHub-Pull-Request: golang/go#42313
Reviewed-on: https://go-review.googlesource.com/c/go/+/266997
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
The code I wrote in ldmacho.go in CL 266373 was plainly wrong. It
didn't carry rAdd over correctly. Fixed. Also added sign extension
(as ld64 does).
Internal linking with -race mode now works. Enable it.
Updates #38485.
Change-Id: I78aa949687bf6a0987913059059160b018c7560e
Reviewed-on: https://go-review.googlesource.com/c/go/+/267097
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
This change modifies the consistent stats implementation to keep the
per-P sequence counter on each P instead of each mcache. A valid mcache
is not available everywhere that we want to call e.g. allocSpan, as per
issue #42339. By decoupling these two, we can add a mechanism to allow
contexts without a P to update stats consistently.
In this CL, we achieve that with a mutex. In practice, it will be very
rare for an M to update these stats without a P. Furthermore, the stats
reader also only needs to hold the mutex across the update to "gen"
since once that changes, writers are free to continue updating the new
stats generation. Contention could thus only arise between writers
without a P, and as mentioned earlier, those should be rare.
A nice side-effect of this change is that the consistent stats acquire
and release API becomes simpler.
Fixes#42339.
Change-Id: Ied74ab256f69abd54b550394c8ad7c4c40a5fe34
Reviewed-on: https://go-review.googlesource.com/c/go/+/267158
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change moves the responsibility of throwing if an mcache is not
available to the caller, because the inlining cost of throw is set very
high in the compiler. Even if it was reduced down to the cost of a usual
function call, it would still be too expensive, so just move it out.
This choice also makes sense in the context of #42339 since we're going
to have to handle the case where we don't have an mcache to update stats
in a few contexts anyhow.
Also, add getMCache to the list of functions that should be inlined to
prevent future regressions.
getMCache is called on the allocation fast path and because its not
inlined actually causes a significant regression (~10%) in some
microbenchmarks.
Fixes#42305.
Change-Id: I64ac5e4f26b730bd4435ea1069a4a50f55411ced
Reviewed-on: https://go-review.googlesource.com/c/go/+/267157
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This is a roll-forward of golang.org/cl/262618, which was reverted in
golang.org/cl/267037. The only differences between this CL and the
original are the three calls to fflush from the C files in
build_overlay.txt, to guarantee that the string we're expecting is
actually written out.
This requires rewriting the paths of the files passed to the cgo tool
toolchain to use the overlaid paths instead of the disk paths of
files. Because the directories of the overlaid paths don't exist in
general, the cgo tool have been updated to run in base.Cwd instead of
the package directory.
For #39958
Change-Id: If7e5e057c62c0c22ddb724f9fe650902fc5f4832
Reviewed-on: https://go-review.googlesource.com/c/go/+/267197
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Michael Matloob <matloob@golang.org>
In Go 1.12, we changed the runtime to use MADV_FREE when available on
Linux (falling back to MADV_DONTNEED) in CL 135395 to address issue
#23687. While MADV_FREE is somewhat faster than MADV_DONTNEED, it
doesn't affect many of the statistics that MADV_DONTNEED does until
the memory is actually reclaimed under OS memory pressure. This
generally leads to poor user experience, like confusing stats in top
and other monitoring tools; and bad integration with management
systems that respond to memory usage.
We've seen numerous issues about this user experience, including
#41818, #39295, #37585, #33376, and #30904, many questions on Go
mailing lists, and requests for mechanisms to change this behavior at
run-time, such as #40870. There are also issues that may be a result
of this, but root-causing it can be difficult, such as #41444 and
#39174. And there's some evidence it may even be incompatible with
Android's process management in #37569.
This CL changes the default to prefer MADV_DONTNEED over MADV_FREE, to
favor user-friendliness and minimal surprise over performance. I think
it's become clear that Linux's implementation of MADV_FREE ultimately
doesn't meet our needs. We've also made many improvements to the
scavenger since Go 1.12. In particular, it is now far more prompt and
it is self-paced, so it will simply trickle memory back to the system
a little more slowly with this change. This can still be overridden by
setting GODEBUG=madvdontneed=0.
Fixes#42330 (meta-issue).
Fixes#41818, #39295, #37585, #33376, #30904 (many of which were
already closed as "working as intended").
Change-Id: Ib6aa7f2dc8419b32516cc5a5fc402faf576c92e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/267100
Trust: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Steps run:
$ cd $(go env GOROOT)/src
$ go get -d golang.org/x/sys
$ go mod tidy
$ go mod vendor
$ go generate syscall/... internal/syscall/...
$ cd cmd
$ go get -d golang.org/x/sys
$ go mod tidy
$ go mod vendor
$ cd ..
$ git add .
This change subsumes CL 260860.
For #36905
Change-Id: I7c677c6aa1ad61b9cbd8cf9ed208ed5a30f29c87
Reviewed-on: https://go-review.googlesource.com/c/go/+/267103
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Minor optimization. Spotted while working on that code.
Change-Id: Ia02dee10d74bce79a0bef1eaba7fac1bfc27df38
Reviewed-on: https://go-review.googlesource.com/c/go/+/266899
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: fannie zhang <Fannie.Zhang@arm.com>
Reviewed-by: David Chase <drchase@google.com>
Adjust mksyscall_windows.go to activate module mode and set
-mod=readonly, and to suppress its own deprecation warning when run
from within GOROOT/src.
We can't vendor the mkwinsyscall tool in to the std module directly,
because std-vendored dependencies (unlike the dependencies of all
other modules) turn into actual, distinct packages in 'std' when
viewed from outside the 'std' module. We don't want to introduce a
binary in the 'std' meta-pattern, but we also don't particularly want
to add more special-cases to the 'go' command right now when we have
an existing wrapper program that can do the job.
I also regenerated the affected packages to ensure that they are
consistent with the current version of mksyscall, which produced some
declaration-order changes in
internal/syscall/windows/zsyscall_windows.go.
Fixes#41916
Updates #25922
Change-Id: If6e6f8ba3dd372a7ecd6820ee6c0ca38d55f0f35
Reviewed-on: https://go-review.googlesource.com/c/go/+/261499
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
