Joe Tsai [Mon, 18 Apr 2022 17:05:24 +0000 (10:05 -0700)]
reflect: derive correct Value method in panic messages
methodName was brittle in that it assumed exactly where
in the call stack the exported Value method is.
This broke since recent inlining optimizations changed
exactly which frame the exported method was located.
Instead, iterate through a sufficient number of stack entries
and dynamically determined the exported Value method name.
This is more maintainable, but slightly slower.
The slowdown is acceptable since panics are not the common case.
Rhys Hiltner [Tue, 3 May 2022 20:55:40 +0000 (13:55 -0700)]
runtime: re-add import in trace.go
CL 400795, which uses the runtime/internal/atomic package in trace.go,
raced against CL 397014 removing that import. Re-add the import.
Change-Id: If847ec23f9a0fdff91dab07e93d9fb1b2efed85b
Reviewed-on: https://go-review.googlesource.com/c/go/+/403845
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv> Reviewed-by: Ian Lance Taylor <iant@google.com>
Paul E. Murphy [Tue, 3 May 2022 13:33:42 +0000 (08:33 -0500)]
cmd/internal/notsha256: revert PPC64 removal, and fix PPC64 asm
This reverts commit a41e37f56a4fc2523ac88a76bf54ba3e45dcf533, and
updates the ASM usage to be go1.8 compliant. go 1.18 added support
for using VR's in place of VSR arguments.
Rhys Hiltner [Fri, 1 Apr 2022 19:56:49 +0000 (12:56 -0700)]
runtime: split mprof locks
The profiles for memory allocations, sync.Mutex contention, and general
blocking store their data in a shared hash table. The bookkeeping work
at the end of a garbage collection cycle involves maintenance on each
memory allocation record. Previously, a single lock guarded access to
the hash table and the contents of all records. When a program has
allocated memory at a large number of unique call stacks, the
maintenance following every garbage collection can hold that lock for
several milliseconds. That can prevent progress on all other goroutines
by delaying acquirep's call to mcache.prepareForSweep, which needs the
lock in mProf_Free to report when a profiled allocation is no longer in
use. With no user goroutines making progress, it is in effect a
multi-millisecond GC-related stop-the-world pause.
Split the lock so the call to mProf_Flush no longer delays each P's call
to mProf_Free: mProf_Free uses a lock on the memory records' N+1 cycle,
and mProf_Flush uses locks on the memory records' accumulator and their
N cycle. mProf_Malloc also no longer competes with mProf_Flush, as it
uses a lock on the memory records' N+2 cycle. The profiles for
sync.Mutex contention and general blocking now share a separate lock,
and another lock guards insertions to the shared hash table (uncommon in
the steady-state). Consumers of each type of profile take the matching
accumulator lock, so will observe consistent count and magnitude values
for each record.
For #45894
Change-Id: I615ff80618d10e71025423daa64b0b7f9dc57daa
Reviewed-on: https://go-review.googlesource.com/c/go/+/399956 Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Rhys Hiltner <rhys@justin.tv> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Rhys Hiltner [Mon, 18 Apr 2022 19:32:37 +0000 (12:32 -0700)]
runtime: add CPU samples to execution trace
When the CPU profiler and execution tracer are both active, report the
CPU profile samples in the execution trace data stream.
Include only samples that arrive on the threads known to the runtime,
but include them even when running g0 (such as near the scheduler) or if
there's no P (such as near syscalls).
Render them in "go tool trace" as instantaneous events.
For #16895
Change-Id: I0aa501a7b450c971e510961c0290838729033f7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/400795 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Rhys Hiltner [Fri, 18 Feb 2022 18:56:16 +0000 (10:56 -0800)]
runtime: decrease STW pause for goroutine profile
The goroutine profile needs to stop the world to get a consistent
snapshot of all goroutines in the app. Leaving the world stopped while
iterating over allgs leads to a pause proportional to the number of
goroutines in the app (or its high-water mark).
Instead, do only a fixed amount of bookkeeping while the world is
stopped. Install a barrier so the scheduler confirms that a goroutine
appears in the profile, with its stack recorded exactly as it was during
the stop-the-world pause, before it allows that goroutine to execute.
Iterate over allgs while the app resumes normal operations, adding each
to the profile unless they've been scheduled in the meantime (and so
have profiled themselves). Stop the world a second time to remove the
barrier and do a fixed amount of cleanup work.
This increases both the fixed overhead and per-goroutine CPU-time cost
of GoroutineProfile. It also increases the wall-clock latency of the
call to GoroutineProfile, since the scheduler may interrupt it to
execute other goroutines.
Rhys Hiltner [Mon, 14 Feb 2022 20:16:22 +0000 (12:16 -0800)]
runtime/pprof: add race annotations for goroutine profiles
The race annotations for goroutine label maps covered the special type
of read necessary to create CPU profiles. Extend that to include
goroutine profiles. Annotate the copy involved in creating new
goroutines.
Michael Anthony Knyszek [Tue, 3 May 2022 19:28:25 +0000 (19:28 +0000)]
runtime: store consistent total allocation stats as uint64
Currently the consistent total allocation stats are managed as uintptrs,
which means they can easily overflow on 32-bit systems. Fix this by
storing these stats as uint64s. This will cause some minor performance
degradation on 32-bit systems, but there really isn't a way around this,
and it affects the correctness of the metrics we export.
Fixes #52680.
Change-Id: I7e6ca44047d46b4bd91c6f87c2d29f730e0d6191
Reviewed-on: https://go-review.googlesource.com/c/go/+/403758
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Austin Clements <austin@google.com>
Ian Lance Taylor [Fri, 22 Apr 2022 22:38:00 +0000 (15:38 -0700)]
vendor, cmd/vendor: update to current x/sys repo
Ran, in src and src/cmd:
go get -u golang.org/x/sys
go mod vendor
go mod tidy
This brings in loong64 support.
Change-Id: Ide30bd7bd073f473be9d8329e6a4f1d2c903d9a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/401855 Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
teivah [Tue, 3 May 2022 09:54:48 +0000 (09:54 +0000)]
encoding/base32: decoder output depends on chunking of underlying reader
After an analysis, I figured that a way to do it could be to check, after
the call to readEncodedData whether the decoder already saw the end or not.
Fixes #38657
Change-Id: I06fd718ea4ee6ded2cb26c2866b28581ad86e271
GitHub-Last-Rev: d0b7bb38e4301a2ae9b8e588944488dbd88b39c4
GitHub-Pull-Request: golang/go#52631
Reviewed-on: https://go-review.googlesource.com/c/go/+/403315
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Tobias Klauser [Tue, 3 May 2022 09:47:44 +0000 (11:47 +0200)]
net: remove fallback path in sysSocket
Support for operating system versions requiring this fallback path was
dropped from recent Go versions. The minimum Linux kernel version is
2.6.32 as of Go 1.18. FreeBSD 10 is no longer supported as of Go 1.13.
Change-Id: I7e74768146dd43a36d0d26fcb08eed9ace82189f
Reviewed-on: https://go-review.googlesource.com/c/go/+/403634
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Kale Blankenship [Tue, 3 May 2022 16:32:19 +0000 (09:32 -0700)]
archive/zip: remove unused File.descErr field
Found via staticcheck. Unused as of CL 357489.
Change-Id: I3aa409994ba4388912ac7e7809168529a5b6e31c
Reviewed-on: https://go-review.googlesource.com/c/go/+/403814
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Kale B <kale@lemnisys.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
Change-Id: I898733dff529a40eeec9f9db2a0a59a6757c3827
Reviewed-on: https://go-review.googlesource.com/c/go/+/402515
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Eli Bendersky <eliben@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Michael Anthony Knyszek [Sun, 10 Apr 2022 20:34:17 +0000 (20:34 +0000)]
runtime: redesign scavenging algorithm
Currently the runtime's scavenging algorithm involves running from the
top of the heap address space to the bottom (or as far as it gets) once
per GC cycle. Once it treads some ground, it doesn't tread it again
until the next GC cycle.
This works just fine for the background scavenger, for heap-growth
scavenging, and for debug.FreeOSMemory. However, it breaks down in the
face of a memory limit for small heaps in the tens of MiB. Basically,
because the scavenger never retreads old ground, it's completely
oblivious to new memory it could scavenge, and that it really *should*
in the face of a memory limit.
Also, every time some thread goes to scavenge in the runtime, it
reserves what could be a considerable amount of address space, hiding it
from other scavengers.
This change modifies and simplifies the implementation overall. It's
less code with complexities that are much better encapsulated. The
current implementation iterates optimistically over the address space
looking for memory to scavenge, keeping track of what it last saw. The
new implementation does the same, but instead of directly iterating over
pages, it iterates over chunks. It maintains an index of chunks (as a
bitmap over the address space) that indicate which chunks may contain
scavenge work. The page allocator populates this index, while scavengers
consume it and iterate over it optimistically.
This has a two key benefits:
1. Scavenging is much simpler: find a candidate chunk, and check it,
essentially just using the scavengeOne fast path. There's no need for
the complexity of iterating beyond one chunk, because the index is
lock-free and already maintains that information.
2. If pages are freed to the page allocator (always guaranteed to be
unscavenged), the page allocator immediately notifies all scavengers
of the new source of work, avoiding the hiding issues of the old
implementation.
One downside of the new implementation, however, is that it's
potentially more expensive to find pages to scavenge. In the past, if
a single page would become free high up in the address space, the
runtime's scavengers would ignore it. Now that scavengers won't, one or
more scavengers may need to iterate potentially across the whole heap to
find the next source of work. For the background scavenger, this just
means a potentially less reactive scavenger -- overall it should still
use the same amount of CPU. It means worse overheads for memory limit
scavenging, but that's not exactly something with a baseline yet.
In practice, this shouldn't be too bad, hopefully since the chunk index
is extremely compact. For a 48-bit address space, the index is only 8
MiB in size at worst, but even just one physical page in the index is
able to support up to 128 GiB heaps, provided they aren't terribly
sparse. On 32-bit platforms, the index is only 128 bytes in size.
For #48409.
Change-Id: I72b7e74365046b18c64a6417224c5d85511194fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/399474 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Wed, 30 Mar 2022 22:10:49 +0000 (22:10 +0000)]
runtime: make the scavenger and allocator respect the memory limit
This change does everything necessary to make the memory allocator and
the scavenger respect the memory limit. In particular, it:
- Adds a second goal for the background scavenge that's based on the
memory limit, setting a target 5% below the limit to make sure it's
working hard when the application is close to it.
- Makes span allocation assist the scavenger if the next allocation is
about to put total memory use above the memory limit.
- Measures any scavenge assist time and adds it to GC assist time for
the sake of GC CPU limiting, to avoid a death spiral as a result of
scavenging too much.
All of these changes have a relatively small impact, but each is
intimately related and thus benefit from being done together.
For #48409.
Change-Id: I35517a752f74dd12a151dd620f102c77e095d3e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/397017 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Mon, 21 Mar 2022 21:27:06 +0000 (21:27 +0000)]
runtime: set the heap goal from the memory limit
This change makes the memory limit functional by including it in the
heap goal calculation. Specifically, we derive a heap goal from the
memory limit, and compare that to the GOGC-based goal. If the goal based
on the memory limit is lower, we prefer that.
To derive the memory limit goal, the heap goal calculation now takes
a few additional parameters as input. As a result, the heap goal, in the
presence of a memory limit, may change dynamically. The consequences of
this are that different parts of the runtime can have different views of
the heap goal; this is OK. What's important is that all of the runtime
is able to observe the correct heap goal for the moment it's doing
something that affects it, like anything that should trigger a GC cycle.
On the topic of triggering a GC cycle, this change also allows any
manually managed memory allocation from the page heap to trigger a GC.
So, specifically workbufs, unrolled GC scan programs, and goroutine
stacks. The reason for this is that now non-heap memory can effect the
trigger or the heap goal.
Most sources of non-heap memory only change slowly, like GC pointer
bitmaps, or change in response to explicit function calls like
GOMAXPROCS. Note also that unrolled GC scan programs and workbufs are
really only relevant during a GC cycle anyway, so they won't actually
ever trigger a GC. Our primary target here is goroutine stacks.
Goroutine stacks can increase quickly, and this is currently totally
independent of the GC cycle. Thus, if for example a goroutine begins to
recurse suddenly and deeply, then even though the heap goal and trigger
react, we might not notice until its too late. As a result, we need to
trigger a GC cycle.
We do this trigger in allocManual instead of in stackalloc because it's
far more general. We ultimately care about memory that's mapped
read/write and not returned to the OS, which is much more the domain of
the page heap than the stack allocator. Furthermore, there may be new
sources of memory manual allocation in the future (e.g. arenas) that
need to trigger a GC if necessary. As such, I'm inclined to leave the
trigger in allocManual as an extra defensive measure.
It's worth noting that because goroutine stacks do not behave quite as
predictably as other non-heap memory, there is the potential for the
heap goal to swing wildly. Fortunately, goroutine stacks that haven't
been set up to shrink by the last GC cycle will not shrink until after
the next one. This reduces the amount of possible churn in the heap goal
because it means that shrinkage only happens once per goroutine, per GC
cycle. After all the goroutines that should shrink did, then goroutine
stacks will only grow. The shrink mechanism is analagous to sweeping,
which is incremental and thus tends toward a steady amount of heap
memory used. As a result, in practice, I expect this to be a non-issue.
Note that if the memory limit is not set, this change should be a no-op.
For #48409.
Change-Id: Ie06d10175e5e36f9fb6450e26ed8acd3d30c681c
Reviewed-on: https://go-review.googlesource.com/c/go/+/394221
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Sun, 27 Mar 2022 20:52:52 +0000 (20:52 +0000)]
runtime: remove float64 multiplication in heap trigger compute path
As of the last CL, the heap trigger is computed as-needed. This means
that some of the niceties we assumed (that the float64 computations
don't matter because we're doing this rarely anyway) are no longer true.
While we're not exactly on a hot path right now, the trigger check still
happens often enough that it's a little too hot for comfort.
This change optimizes the computation by replacing the float64
multiplication with a shift and a constant integer multiplication.
I ran an allocation microbenchmark for an allocation size that would hit
this path often. CPU profiles seem to indicate this path was ~0.1% of
cycles (dwarfed by other costs, e.g. zeroing memory) even if all we're
doing is allocating, so the "optimization" here isn't particularly
important. However, since the code here is executed significantly more
frequently, and this change isn't particularly complicated, let's err
on the size of efficiency if we can help it.
Note that because of the way the constants are represented now, they're
ever so slightly different from before, so this change technically isn't
a total no-op. In practice however, it should be. These constants are
fuzzy and hand-picked anyway, so having them shift a little is unlikely
to make a significant change to the behavior of the GC.
For #48409.
Change-Id: Iabb2385920f7d891b25040226f35a3f31b7bf844
Reviewed-on: https://go-review.googlesource.com/c/go/+/397015
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 21 Mar 2022 21:27:06 +0000 (21:27 +0000)]
runtime: check the heap goal and trigger dynamically
As it stands, the heap goal and the trigger are set once by
gcController.commit, and then read out of gcController. However with the
coming memory limit we need the GC to be able to respond to changes in
non-heap memory. The simplest way of achieving this is to compute the
heap goal and its associated trigger dynamically.
In order to make this easier to implement, the GC trigger is now based
on the heap goal, as opposed to the status quo of computing both
simultaneously. In many cases we just want the heap goal anyway, not
both, but we definitely need the goal to compute the trigger, because
the trigger's bounds are entirely based on the goal (the initial runway
is not). A consequence of this is that we can't rely on the trigger to
enforce a minimum heap size anymore, and we need to lift that up
directly to the goal. Specifically, we need to lift up any part of the
calculation that *could* put the trigger ahead of the goal. Luckily this
is just the heap minimum and minimum sweep distance. In the first case,
the pacer may behave slightly differently, as the heap minimum is no
longer the minimum trigger, but the actual minimum heap goal. In the
second case it should be the same, as we ensure the additional runway
for sweeping is added to both the goal *and* the trigger, as before, by
computing that in gcControllerState.commit.
There's also another place we update the heap goal: if a GC starts and
we triggered beyond the goal, we always ensure there's some runway.
That calculation uses the current trigger, which violates the rule of
keeping the goal based on the trigger. Notice, however, that using the
precomputed trigger for this isn't even quite correct: due to a bug, or
something else, we might trigger a GC beyond the precomputed trigger.
So this change also adds a "triggered" field to gcControllerState that
tracks the point at which a GC actually triggered. This is independent
of the precomputed trigger, so it's fine for the heap goal calculation
to rely on it. It also turns out, there's more than just that one place
where we really should be using the actual trigger point, so this change
fixes those up too.
Also, because the heap minimum is set by the goal and not the trigger,
the maximum trigger calculation now happens *after* the goal is set, so
the maximum trigger actually does what I originally intended (and what
the comment says): at small heaps, the pacer picks 95% of the runway as
the maximum trigger. Currently, the pacer picks a small trigger based
on a not-yet-rounded-up heap goal, so the trigger gets rounded up to the
goal, and as per the "ensure there's some runway" check, the runway ends
up at always being 64 KiB. That check is supposed to be for exceptional
circumstances, not the status quo. There's a test introduced in the last
CL that needs to be updated to accomodate this slight change in
behavior.
So, this all sounds like a lot that changed, but what we're talking about
here are really, really tight corner cases that arise from situations
outside of our control, like pathologically bad behavior on the part of
an OS or CPU. Even in these corner cases, it's very unlikely that users
will notice any difference at all. What's more important, I think, is
that the pacer behaves more closely to what all the comments describe,
and what the original intent was.
Another note: at first, one might think that computing the heap goal and
trigger dynamically introduces some raciness, but not in this CL: the heap
goal and trigger are completely static.
Allocation outside of a GC cycle may now be a bit slower than before, as
the GC trigger check is now significantly more complex. However, note
that this executes basically just as often as gcController.revise, and
that makes up for a vanishingly small part of any CPU profile. The next
CL cleans up the floating point multiplications on this path
nonetheless, just to be safe.
For #48409.
Change-Id: I280f5ad607a86756d33fb8449ad08555cbee93f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/397014
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Thu, 7 Apr 2022 17:51:05 +0000 (17:51 +0000)]
runtime: rewrite pacer max trigger calculation
Currently the maximum trigger calculation is totally incorrect with
respect to the comment above it and its intent. This change rectifies
this mistake.
For #48409.
Change-Id: Ifef647040a8bdd304dd327695f5f315796a61a74
Reviewed-on: https://go-review.googlesource.com/c/go/+/398834
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Fri, 1 Apr 2022 22:34:45 +0000 (22:34 +0000)]
runtime: move inconsistent memstats into gcController
Fundamentally, all of these memstats exist to serve the runtime in
managing memory. For the sake of simpler testing, couple these stats
more tightly with the GC.
This CL was mostly done automatically. The fields had to be moved
manually, but the references to the fields were updated via
Michael Anthony Knyszek [Fri, 1 Apr 2022 18:15:24 +0000 (18:15 +0000)]
runtime: clean up inconsistent heap stats
The inconsistent heaps stats in memstats are a bit messy. Primarily,
heap_sys is non-orthogonal with heap_released and heap_inuse. In later
CLs, we're going to want heap_sys-heap_released-heap_inuse, so clean
this up by replacing heap_sys with an orthogonal metric: heapFree.
heapFree represents page heap memory that is free but not released.
I think this change also simplifies a lot of reasoning about these
stats; it's much clearer what they mean, and to obtain HeapSys for
memstats, we no longer need to do the strange subtraction from heap_sys
when allocating specifically non-heap memory from the page heap.
Because we're removing heap_sys, we need to replace it with a sysMemStat
for mem.go functions. In this case, heap_released is the most
appropriate because we increase it anyway (again, non-orthogonality). In
which case, it makes sense for heap_inuse, heap_released, and heapFree
to become more uniform, and to just represent them all as sysMemStats.
While we're here and messing with the types of heap_inuse and
heap_released, let's also fix their names (and last_heap_inuse's name)
up to the more modern Go convention of camelCase.
For #48409.
Change-Id: I87fcbf143b3e36b065c7faf9aa888d86bd11710b
Reviewed-on: https://go-review.googlesource.com/c/go/+/397677
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Tue, 15 Mar 2022 02:48:18 +0000 (02:48 +0000)]
runtime: track how much memory is mapped in the Ready state
This change adds a field to memstats called mappedReady that tracks how
much memory is in the Ready state at any given time. In essence, it's
the total memory usage by the Go runtime (with one exception which is
documented). Essentially, all memory mapped read/write that has either
been paged in or will soon.
To make tracking this not involve the many different stats that track
mapped memory, we track this statistic at a very low level. The downside
of tracking this statistic at such a low level is that it managed to
catch lots of situations where the runtime wasn't fully accounting for
memory. This change rectifies these situations by always accounting for
memory that's mapped in some way (i.e. always passing a sysMemStat to a
mem.go function), with *two* exceptions.
Rectifying these situations means also having the memory mapped during
testing being accounted for, so that tests (i.e. ReadMemStats) that
ultimately check mappedReady continue to work correctly without special
exceptions. We choose to simply account for this memory in other_sys.
Let's talk about the exceptions. The first is the arenas array for
finding heap arena metadata from an address is mapped as read/write in
one large chunk. It's tens of MiB in size. On systems with demand
paging, we assume that the whole thing isn't paged in at once (after
all, it maps to the whole address space, and it's exceedingly difficult
with today's technology to even broach having as much physical memory as
the total address space). On systems where we have to commit memory
manually, we use a two-level structure.
Now, the reason why this is an exception is because we have no mechanism
to track what memory is paged in, and we can't just account for the
entire thing, because that would *look* like an enormous overhead.
Furthermore, this structure is on a few really, really critical paths in
the runtime, so doing more explicit tracking isn't really an option. So,
we explicitly don't and call sysAllocOS to map this memory.
The second exception is that we call sysFree with no accounting to clean
up address space reservations, or otherwise to throw out mappings we
don't care about. In this case, also drop down to a lower level and call
sysFreeOS to explicitly avoid accounting.
The third exception is debuglog allocations. That is purely a debugging
facility and ideally we want it to have as small an impact on the
runtime as possible. If we include it in mappedReady calculations, it
could cause GC pacing shifts in future CLs, especailly if one increases
the debuglog buffer sizes as a one-off.
As of this CL, these are the only three places in the runtime that would
pass nil for a stat to any of the functions in mem.go. As a result, this
CL makes sysMemStats mandatory to facilitate better accounting in the
future. It's now much easier to grep and find out where accounting is
explicitly elided, because one doesn't have to follow the trail of
sysMemStat nil pointer values, and can just look at the function name.
For #48409.
Change-Id: I274eb467fc2603881717482214fddc47c9eaf218
Reviewed-on: https://go-review.googlesource.com/c/go/+/393402 Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 15 Feb 2022 00:22:20 +0000 (00:22 +0000)]
runtime: add byte count parser for GOMEMLIMIT
This change adds a parser for the GOMEMLIMIT environment variable's
input. This environment variable accepts a number followed by an
optional prefix expressing the unit. Acceptable units include
B, KiB, MiB, GiB, TiB, where *iB is a power-of-two byte unit.
For #48409.
Change-Id: I6a3b4c02b175bfcf9c4debee6118cf5dda93bb6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/393400
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Michael Knyszek [Sat, 2 Oct 2021 02:52:12 +0000 (22:52 -0400)]
runtime: add GC CPU utilization limiter
This change adds a GC CPU utilization limiter to the GC. It disables
assists to ensure GC CPU utilization remains under 50%. It uses a leaky
bucket mechanism that will only fill if GC CPU utilization exceeds 50%.
Once the bucket begins to overflow, GC assists are limited until the
bucket empties, at the risk of GC overshoot. The limiter is primarily
updated by assists. The scheduler may also update it, but only if the
GC is on and a few milliseconds have passed since the last update. This
second case exists to ensure that if the limiter is on, and no assists
are happening, we're still updating the limiter regularly.
The purpose of this limiter is to mitigate GC death spirals, opting to
use more memory instead.
This change turns the limiter on always. In practice, 50% overall GC CPU
utilization is very difficult to hit unless you're trying; even the most
allocation-heavy applications with complex heaps still need to do
something with that memory. Note that small GOGC values (i.e.
single-digit, or low teens) are more likely to trigger the limiter,
which means the GOGC tradeoff may no longer be respected. Even so, it
should still be relatively rare.
This change also introduces the feature flag for code to support the
memory limit feature.
For #48409.
Change-Id: Ia30f914e683e491a00900fd27868446c65e5d3c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/353989 Reviewed-by: Michael Pratt <mpratt@google.com>
Robert Griesemer [Thu, 21 Apr 2022 05:19:49 +0000 (22:19 -0700)]
cmd/compile/internal/syntax: accept all valid type parameter lists
Type parameter lists starting with the form [name *T|...] or
[name (X)|...] may look like an array length expression [x].
Only after parsing the entire initial expression and checking
whether the expression contains type elements or is followed
by a comma can we make the final decision.
This change simplifies the existing parsing strategy: instead
of trying to make an upfront decision with limited information
(which is insufficient), the parser now parses the start of a
type parameter list or array length specification as expression.
In a second step, if the expression can be split into a name
followed by a type element, or a name followed by an ordinary
expression which is succeeded by a comma, we assume a type
parameter list (because it can't be an array length).
In all other cases we assume an array length specification.
Fixes #49482.
Change-Id: I269b6291999bf60dc697d33d24a5635f01e065b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/402256 Reviewed-by: Benny Siegert <bsiegert@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Mon, 2 May 2022 11:57:07 +0000 (13:57 +0200)]
internal/poll, net, syscall: use accept4 on solaris
Solaris supports accept4 since version 11.4, see
https://docs.oracle.com/cd/E88353_01/html/E37843/accept4-3c.html
Use it in internal/poll.accept like on other platforms.
Change-Id: I3d9830a85e93bbbed60486247c2f91abc646371f
Reviewed-on: https://go-review.googlesource.com/c/go/+/403394 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Jorropo [Mon, 2 May 2022 11:50:36 +0000 (11:50 +0000)]
io: NopCloser forward WriterTo implementations if the reader supports it
This patch also include related fixes to net/http.
io_test.go don't test reading or WritingTo of the because the logic is simple.
NopCloser didn't even had direct tests before.
Fixes #51566
Change-Id: I1943ee2c20d0fe749f4d04177342ce6eca443efe
GitHub-Last-Rev: a6b9af4e945a6903735a74aa185e2d1c4c2e2cef
GitHub-Pull-Request: golang/go#52340
Reviewed-on: https://go-review.googlesource.com/c/go/+/400236
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Robert Findley [Mon, 2 May 2022 15:13:08 +0000 (11:13 -0400)]
go/types,types2: delay the check for conflicting struct field names
In #52529, we observed that checking types for duplicate fields and
methods during method collection can result in incorrect early expansion
of the base type. Fix this by delaying the check for duplicate fields.
Notably, we can't delay the check for duplicate methods as we must
preserve the invariant that added method names are unique.
After this change, it may be possible in the presence of errors to have
a type-checked type containing a method name that conflicts with a field
name. With the previous logic conflicting methods would have been
skipped. This is a change in behavior, but only for invalid code.
Preserving the existing behavior would likely require delaying method
collection, which could have more significant consequences.
As a result of this change, the compiler test fixedbugs/issue28268.go
started passing with types2, being previously marked as broken. The fix
was not actually related to the duplicate method error, but rather the
fact that we stopped reporting redundant errors on the calls to x.b()
and x.E(), because they are now (valid!) methods.
Fixes #52529
Change-Id: I850ce85c6ba76d79544f46bfd3deb8538d8c7d00
Reviewed-on: https://go-review.googlesource.com/c/go/+/403455 Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Mon, 2 May 2022 21:03:07 +0000 (14:03 -0700)]
bufio: clarify io.EOF behavior of Reader.Read
Fixes #52577
Change-Id: Idaff2604979f9a9c1c7d3140c8a5d218fcd27a56
Reviewed-on: https://go-review.googlesource.com/c/go/+/403594 Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Joe Tsai [Mon, 2 May 2022 18:40:57 +0000 (11:40 -0700)]
compress/flate: move idempotent close logic to compressor
The compressor methods already have logic for handling a sticky error.
Merge the logic from CL 136475 into that.
This slightly changes the error message to be more sensible
in the situation where it's returned by Flush.
Updates #27741
Change-Id: Ie34cf3164d0fa6bd0811175ca467dbbcb3be1395
Reviewed-on: https://go-review.googlesource.com/c/go/+/403514 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Paul E. Murphy [Mon, 11 Apr 2022 20:21:03 +0000 (15:21 -0500)]
internal/bytealg: improve PPC64 equal
Rewrite the vector loop to process 64B per iteration,
this greatly improves performance on POWER9/POWER10 for
large sizes.
Likewise, use a similar tricks for sizes >= 8 and <= 64.
And, rewrite small comparisons, it's a little slower for 1 byte,
but constant time for 1-7 bytes. Thus, it is increasingly faster
for 2-7B.
Benchmarks results below are from P8/P9 ppc64le (in that order),
several additional testcases have been added to test interesting
sizes. Likewise, the old variant was padded to the same code
size of the new variant to minimize layout related noise:
Austin Clements [Mon, 2 May 2022 18:54:22 +0000 (14:54 -0400)]
cmd/compile: fix loopreschedchecks for regabi
The loopreschedchecks pass (GOEXPERIMENT=preemptibleloops) had
bit-rotted in two ways because of the regabi experiment:
1. The call to goschedguarded was generating a pre-regabi StaticCall.
This CL updates it to construct a new-style StaticCall.
2. The mem finder did not account for tuples or results containing a
mem. This caused it to construct phis that were supposed to thread
the mem into the added blocks, but they could instead thread a
tuple or results containing a mem, causing things to go wrong
later. This CL updates the mem finder to add an op to select out
the mem if it finds the last live mem in a block is a tuple or
results. This isn't ideal since we'll deadcode out most of these,
but it's the easiest thing to do and this is just an experiment.
Tested by running the runtime tests. Ideally we'd have a real test for
this, but I don't think it's worth the effort for code that clearly
hasn't been enabled by anyone for at least a year.
Matthew Dempsky [Fri, 29 Apr 2022 18:58:03 +0000 (11:58 -0700)]
cmd/compile: simplify code from CL 398474
This CL:
1. extracts typecheck.LookupNum into a method on *types.Pkg, so that
it can be used with any package, not just types.LocalPkg,
2. adds a new helper function closureSym to generate symbols in the
appropriate package as needed within stencil.go, and
3. updates the existing typecheck.LookupNum+Name.SetSym code to call
closureSym instead.
No functional change (so no need to backport to Go 1.18), but a little
cleaner, and avoids polluting types.LocalPkg.Syms with symbols that we
won't end up using.
Updates #52117.
Change-Id: Ifc8a3b76a37c830125e9d494530d1f5b2e3e3e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/403197 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Xiaodong Liu [Wed, 30 Mar 2022 07:47:04 +0000 (15:47 +0800)]
cmd/internal/objabi: define Go relocation types for loong64
Contributors to the loong64 port are:
Weining Lu <luweining@loongson.cn>
Lei Wang <wanglei@loongson.cn>
Lingqin Gong <gonglingqin@loongson.cn>
Xiaolin Zhao <zhaoxiaolin@loongson.cn>
Meidan Li <limeidan@loongson.cn>
Xiaojuan Zhai <zhaixiaojuan@loongson.cn>
Qiyuan Pu <puqiyuan@loongson.cn>
Guoqi Chen <chenguoqi@loongson.cn>
This port has been updated to Go 1.15.6:
https://github.com/loongson/go
Updates #46229
Change-Id: I8d31b3cd827325aa0ff748ca8c0c0da6df6ed99f
Reviewed-on: https://go-review.googlesource.com/c/go/+/396734 Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Russ Cox [Thu, 27 Jan 2022 18:10:48 +0000 (13:10 -0500)]
all: use os/exec instead of internal/execabs
We added internal/execabs back in January 2021 in order to fix
a security problem caused by os/exec's handling of the current
directory. Now that os/exec has that code, internal/execabs is
superfluous and can be deleted.
This commit rewrites all the imports back to os/exec and
deletes internal/execabs.
For #43724.
Change-Id: Ib9736baf978be2afd42a1225e2ab3fd5d33d19df
Reviewed-on: https://go-review.googlesource.com/c/go/+/381375
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Wayne Zuo [Sun, 1 May 2022 07:56:20 +0000 (15:56 +0800)]
A+C: add Wayne Zuo (individual CLA)
Change-Id: I12fe0b7952a41f6d0f78f892d823244793745279
Reviewed-on: https://go-review.googlesource.com/c/go/+/403336
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Benny Siegert <bsiegert@gmail.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Russ Cox [Mon, 2 May 2022 15:12:25 +0000 (11:12 -0400)]
net/http: fix for recent go.mod update
cmd/internal/moddeps was failing.
Ran the commands it suggested:
% go mod tidy # to remove extraneous dependencies
% go mod vendor # to vendor dependencies
% go generate -run=bundle std # to regenerate bundled packages
% go generate syscall internal/syscall/... # to regenerate syscall packages
os/exec: return error when PATH lookup would use current directory
CL 381374 was reverted because x/sys/execabs broke.
This CL reapplies CL 381374, but adding a lookPathErr error
field back, for execabs to manipulate with reflect.
That field will just be a bit of scar tissue in this package forever,
to keep old code working with new toolchains.
CL 403256 fixes x/sys/execabs's test to be ready for the change.
Older versions of x/sys/execabs will keep working
(that is, will keep rejecting what they should reject),
but they will return a slightly different error from LookPath
without that CL, and the test fails because of the different
error text.
Baokun Lee [Mon, 20 Dec 2021 06:02:00 +0000 (14:02 +0800)]
internal/poll: clear completed Buffers to permit earlier collection
Updates #45163
Change-Id: I73a6f22715550e0e8b83fbd3ebec72ef019f153f
Reviewed-on: https://go-review.googlesource.com/c/go/+/373374
Run-TryBot: Lee Baokun <bk@golangcn.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Gregory Man [Thu, 20 Sep 2018 08:48:17 +0000 (11:48 +0300)]
compress/flate: return error on closed stream write
Previously flate.Writer allowed writes after Close, and this behavior
could lead to stream corruption.
Fixes #27741
Change-Id: Iee1ac69f8199232f693dba77b275f7078257b582
Reviewed-on: https://go-review.googlesource.com/c/go/+/136475
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Xiaodong Liu [Sun, 15 Aug 2021 07:32:16 +0000 (15:32 +0800)]
cmd/cgo: configure cgo tool for loong64
Define pointer and int type size for loong64
Add "-mabi=lp64d" argument to gcc
Contributors to the loong64 port are:
Weining Lu <luweining@loongson.cn>
Lei Wang <wanglei@loongson.cn>
Lingqin Gong <gonglingqin@loongson.cn>
Xiaolin Zhao <zhaoxiaolin@loongson.cn>
Meidan Li <limeidan@loongson.cn>
Xiaojuan Zhai <zhaixiaojuan@loongson.cn>
Qiyuan Pu <puqiyuan@loongson.cn>
Guoqi Chen <chenguoqi@loongson.cn>
This port has been updated to Go 1.15.6:
https://github.com/loongson/go
Updates #46229
Change-Id: I9699fd9af0112e72193ac24b736b85c580887a0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/342305 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Change-Id: Ia0b4a5963724ed92be27f557ad141335b389e97f
GitHub-Last-Rev: b0e72cb26de251865ef865bf92a6b8ff9dbf7b04
GitHub-Pull-Request: golang/go#52621
Reviewed-on: https://go-review.googlesource.com/c/go/+/403136 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Xiaodong Liu [Sun, 15 Aug 2021 07:57:43 +0000 (15:57 +0800)]
cmd/go/internal: configure go tool workflow for loong64
Contributors to the loong64 port are:
Weining Lu <luweining@loongson.cn>
Lei Wang <wanglei@loongson.cn>
Lingqin Gong <gonglingqin@loongson.cn>
Xiaolin Zhao <zhaoxiaolin@loongson.cn>
Meidan Li <limeidan@loongson.cn>
Xiaojuan Zhai <zhaixiaojuan@loongson.cn>
Qiyuan Pu <puqiyuan@loongson.cn>
Guoqi Chen <chenguoqi@loongson.cn>
This port has been updated to Go 1.15.6:
https://github.com/loongson/go
Updates #46229
Change-Id: I6b537a7d842b0683586917fe7ea7cd4d70d888de
Reviewed-on: https://go-review.googlesource.com/c/go/+/342308
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Xiaodong Liu [Sun, 15 Aug 2021 07:53:05 +0000 (15:53 +0800)]
cmd/dist: support dist tool for loong64
Contributors to the loong64 port are:
Weining Lu <luweining@loongson.cn>
Lei Wang <wanglei@loongson.cn>
Lingqin Gong <gonglingqin@loongson.cn>
Xiaolin Zhao <zhaoxiaolin@loongson.cn>
Meidan Li <limeidan@loongson.cn>
Xiaojuan Zhai <zhaixiaojuan@loongson.cn>
Qiyuan Pu <puqiyuan@loongson.cn>
Guoqi Chen <chenguoqi@loongson.cn>
This port has been updated to Go 1.15.6:
https://github.com/loongson/go
Updates #46229
Change-Id: I61dca43680d8e5bd3198a38577450a53f405a987
Reviewed-on: https://go-review.googlesource.com/c/go/+/342307 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
os/signal: scale back the solaris-amd64-oraclerel settle time
The settleTime is arbitrary. Ideally we should refactor the test to
avoid it (using subprocesses instead of sleeps to isolate tests from
each others' delayed signals), but as a shorter-term workaround let's
try scaling it back to match linux/ppc64 (the other builder that
empirically requires a longer settleTime).
Change-Id: I545c71ea3145280828ca4186aad036a6c02016ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/400635 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Keith Randall [Fri, 29 Apr 2022 04:18:49 +0000 (21:18 -0700)]
sync/atomic: use consistent first-store-in-progress marker
We need to use the same marker everywhere. My CL to rename the
marker (CL 241661) and the CL to add more uses of the marker
under the old name (CL 241678) weren't coordinated with each other.
Fixes #52612
Change-Id: I97023c0769e518491924ef457fe03bf64a2cefa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/403094
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Russ Cox [Thu, 27 Jan 2022 17:59:37 +0000 (12:59 -0500)]
os/exec: return error when PATH lookup would use current directory
Following discussion on #43724, change os/exec to take the
approach of golang.org/x/sys/execabs, refusing to respect
path entries mentioning relative paths by default.
Code that insists on being able to find executables in relative
directories in the path will need to add a couple lines to override the error.
See the updated package docs in exec.go for more details.
Matthew Dempsky [Fri, 29 Apr 2022 17:47:57 +0000 (10:47 -0700)]
cmd/compile: consistent unified IR handling of package unsafe
Within the unified IR export format, I was treating package unsafe as
a normal package, but expecting importers to correctly handle
deduplicating it against their respective representation of package
unsafe.
However, the surrounding importer logic differs slightly between
cmd/compile/internal/noder (which unified IR was originally
implemented against) and go/importer (which it was more recently
ported to). In particular, noder initializes its packages map as
`map[string]*types2.Package{"unsafe": types2.Unsafe}`, whereas
go/importer initializes it as just `make(map[string]*types.Package)`.
This CL makes them all consistent. In particular, it:
1. changes noder to initialize packages to an empty map to prevent
further latent issues from the discrepency,
2. adds the same special handling of package unsafe already present in
go/internal/gcimporter's unified IR reader to both of cmd/compile's
implementations, and
3. changes the unified IR writer to treat package unsafe as a builtin
package, to force that readers similarly handle it correctly.
Fixes #52623.
Change-Id: Ibbab9b0a1d2a52d4cc91b56c5df49deedf81295a
Reviewed-on: https://go-review.googlesource.com/c/go/+/403196
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
[dev.boringcrypto] cmd/compile: remove the awful boringcrypto kludge
CL 60271 introduced this “AwfulBoringCryptoKludge.”
iant approved that CL saying “As long as it stays out of master...”
Now that the rsa and ecdsa code uses boring.Cache, the
“boring unsafe.Pointer” fields are gone from the key structs, and this
code is no longer needed. So delete it.
With the kludge deleted, we are one step closer to being able to merge
dev.boringcrypto into master.
For #51940.
Change-Id: Ie549db14b0b699c306dded2a2163f18f31d45530
Reviewed-on: https://go-review.googlesource.com/c/go/+/395884 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
[dev.boringcrypto] crypto/ecdsa, crypto/rsa: use boring.Cache
In the original BoringCrypto port, ecdsa and rsa's public and private
keys added a 'boring unsafe.Pointer' field to cache the BoringCrypto
form of the key. This led to problems with code that “knew” the layout
of those structs and in particular that they had no unexported fields.
In response, as an awful kludge, I changed the compiler to pretend
that field did not exist when laying out reflect data. Because we want
to merge BoringCrypto in the main tree, we need a different solution.
Using boring.Cache is that solution.
In the original BoringCrypto port, ecdsa and rsa's public and private
keys added a 'boring unsafe.Pointer' field to cache the BoringCrypto
form of the key. This led to problems with code that “knew” the layout
of those structs and in particular that they had no unexported fields.
In response, as an awful kludge, I changed the compiler to pretend
that field did not exist when laying out reflect data. Because we want
to merge BoringCrypto in the main tree, we need a different solution.
The different solution is this CL's boring.Cache, which is a
concurrent, GC-aware map from unsafe.Pointer to unsafe.Pointer (if
generics were farther along we could use them nicely here, but I am
afraid of breaking tools that aren't ready to see generics in the
standard library yet).
More complex approaches are possible, but a simple, fixed-size hash
table is easy to make concurrent and should be fine.
This API was added only for BoringCrypto, never shipped in standard
Go. This API is also not compatible with the expected future evolution
of crypto/x509, as we move closer to host verifiers on macOS and Windows.
If we want to merge BoringCrypto into the main tree, it is best not to
have differing API. So instead of a hook set by crypto/tls, move the
actual check directly into crypto/x509, eliminating the need for
exposed API.
[dev.boringcrypto] crypto/..., go/build: align deps test with standard rules
One annoying difference between dev.boringcrypto and master is that
there is not a clear separation between low-level (math/big-free)
crypto and high-level crypto, because crypto/internal/boring imports
both encoding/asn1 and math/big.
This CL removes both those problematic imports and aligns the
dependency rules in the go/build test with the ones in the main
branch.
To remove encoding/asn1, the crypto/internal/boring APIs change to
accepting and returning encoded ASN.1, leaving crypto/ecdsa to do the
marshaling and unmarshaling, which it already contains code to do.
To remove math/big, the crypto/internal/boring package defines
type BigInt []uint, which is the same representation as a big.Int's
internal storage. The new package crypto/internal/boring/bbig provides
conversions between BigInt and *big.Int. The boring package can then
be in the low-level crypto set, and any package needing to use bignum
APIs (necessarily in the high-level crypto set) can import bbig to
convert.
To simplify everything we hide from the test the fact that
crypto/internal/boring imports cgo. Better to pretend it doesn't and
keep the prohibitions that other packages like crypto/aes must not use
cgo (outside of BoringCrypto).
[dev.boringcrypto] cmd/dist: default to use of boringcrypto
The dev.boringcrypto branch has historically forced use of boringcrypto
with no additional configuration flags. The previous CL undid that.
This CL redoes it, so that direct uses of dev.boringcrypto don't lapse
unexpectedly into not having boringcrypto enabled.
When dev.boringcrypto is merged into master, we will undo this change
as part of the merge, so that the only final difference between master
and dev.boringcrypto will be this CL.
For #51940.
Change-Id: I816593a0b30b4e71093a7da9451bae7807d7167e
Reviewed-on: https://go-review.googlesource.com/c/go/+/402597
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
[dev.boringcrypto] cmd/go: pass dependency syso to cgo too
Proposal #42477 asked for a way to apply conditional build tags
to syso files (which have no source code to hold //go:build lines).
We ended up suggesting that the standard answer should be to
put the syso in its own package and then import that package from
a source file that is itself conditionally compiled.
A followup comment on that issue pointed out a problem that I did
not understand until I tried to use this approach myself: the cgo
build fails by default, because the link step only uses syso files from
the current package. You have to override this explicitly by arranging
to pass a “ignore unresolved symbols” flag to the host linker.
Many users will not know how to do this.
(I don't know how to do this off the top of my head.)
If we want users to use this approach, we should make it work better.
This CL does that, by including the syso files from dependencies of
the current package in the link step.
For #51940.
Change-Id: I53a0371b2df17e39a000a645b7686daa6a98722d
Reviewed-on: https://go-review.googlesource.com/c/go/+/402596 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
[dev.boringcrypto] cmd: use notsha256 instead of md5, sha1, sha256
When we add GOEXPERIMENT=boringcrypto, the bootstrap process
will not converge if the compiler itself depends on the boringcrypto
cgo-based implementations of sha1 and sha256.
Using notsha256 avoids boringcrypto and makes bootstrap converge.
Removing md5 is not strictly necessary but it seemed worthwhile to
be consistent.
For #51940.
Change-Id: Iba649507e0964d1a49a1d16e463dd23c4e348f14
Reviewed-on: https://go-review.googlesource.com/c/go/+/402595 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
[dev.boringcrypto] cmd/internal/notsha256: add new package
Package notsha256 implements the NOTSHA256 hash,
defined as bitwise NOT of SHA-256.
It will be used from the Go compiler toolchain where an
arbitrary hash is needed and the code currently reaches
for MD5, SHA1, or SHA256. The problem with all of those
is that when we add GOEXPERIMENT=boringcrypto, the
bootstrap process will not converge if the compiler itself
depends on the boringcrypto cgo code.
Using notsha256 avoids boringcrypto.
It is possible that I don't fully understand the convergence
problem and that there is a way to make the compiler converge
when using cgo, but keeping cgo out of the compiler seems safest.
It also makes clear that (except for the hack in codesign)
the code using this package doesn't care which hash is used.
For #51940.
Change-Id: Ie7c661183eacf8413a9d2074c96cbb9361e125ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/402594 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Replaced github username with my real name and added to AUTHORS.
Change-Id: Id68c30eeb31b3f0b7cecc434462029843758f9f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/402794 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Ludi Rehak [Wed, 16 Mar 2022 17:22:11 +0000 (10:22 -0700)]
regexp/syntax: fix typo in comment
Fix typo in comment describing IsWordChar.
Change-Id: Ia283813cf5662e218ee6d0411fb0c1b1ad1021f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/393435
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Fri, 15 Apr 2022 16:23:06 +0000 (12:23 -0400)]
runtime: use saved LR when unwinding through morestack
On LR machine, consider F calling G calling H, which grows stack.
The stack looks like
...
G's frame:
... locals ...
saved LR = return PC in F <- SP points here at morestack
H's frame (to be created)
At morestack, we save
gp.sched.pc = H's morestack call
gp.sched.sp = H's entry SP (the arrow above)
gp.sched.lr = return PC in G
Currently, when unwinding through morestack (if _TraceJumpStack
is set), we switch PC and SP but not LR. We then have
frame.pc = H's morestack call
frame.sp = H's entry SP (the arrow above)
As LR is not set, we load it from stack at *sp, so
frame.lr = return PC in F
As the SP hasn't decremented at the morestack call,
frame.fp = frame.sp = H's entry SP
Unwinding a frame, we have
frame.pc = old frame.lr = return PC in F
frame.sp = old frame.fp = H's entry SP a.k.a. G's SP
The PC and SP don't match. The unwinding will go off if F and G
have different frame sizes.
Fix this by preserving the LR when switching stack.
Also add code to detect infinite loop in unwinding.
TODO: add some test. I can reproduce the infinite loop (or throw
with added check) but the frequency is low.
May fix #52116.
Change-Id: I6e1294f1c6e55f664c962767a1cf6c466a0c0eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/400575
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Eric Fang <eric.fang@arm.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Now, 1.17 is the least supported version, the compiler always write
type information when exporting function bodies. So we can get rid of
go117ExportTypes constant and all its conditional checking codes.
Change-Id: I9ac616509c30601e94f99426049d814328253395
Reviewed-on: https://go-review.googlesource.com/c/go/+/402974 Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are several tests in the runtime that need to force various
things to escape to the heap. This CL centralizes this functionality
into runtime.Escape, defined in export_test.
Michael Pratt [Fri, 4 Mar 2022 18:24:04 +0000 (13:24 -0500)]
runtime: differentiate "user" and "system" throws
"User" throws are throws due to some invariant broken by the application.
"System" throws are due to some invariant broken by the runtime,
environment, etc (i.e., not the fault of the application).
This CL sends "user" throws through the new fatal. Currently this
function is identical to throw, but with a different name to clearly
differentiate the throw type in the stack trace, and hopefully be a bit
more clear to users what it means.
This CL changes a few categories of throw to fatal:
1. Concurrent map read/write.
2. Deadlock detection.
3. Unlock of unlocked sync.Mutex.
4. Inconsistent results from syscall.AllThreadsSyscall.
"Thread exhaustion" and "out of memory" (usually address space full)
throws are additional throws that are arguably the fault of user code,
but I've left off for now because there is no specific invariant that
they have broken to get into these states.
Michael Pratt [Thu, 28 Apr 2022 15:44:28 +0000 (11:44 -0400)]
syscall: add //go:norace to RawSyscall
RawSyscall is used in a variety of rather unsafe conditions, such as
after fork in forkAndExecInChild1. Disable race instrumentation to avoid
calling TSAN in unsafe conditions.
For #51087
Change-Id: I47c35e6f0768c77ddab99010ea0404c45ad2f1da
Reviewed-on: https://go-review.googlesource.com/c/go/+/402914
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Filippo Valsorda [Sat, 30 Oct 2021 06:01:35 +0000 (02:01 -0400)]
crypto/elliptic: use generics for nistec-based curves
There was no way to use an interface because the methods on the Point
types return concrete Point values, as they should.
A couple somewhat minor annoyances:
- Allocations went up due to #48849. This is fine here, where
math/big causes allocations anyway, but would probably not be fine
in nistec itself.
- Carrying the newPoint/newGenerator functions around as a field is
a little weird, even if type-safe. It also means we have to make
what were functions methods so they can access newPoint to return
the zero value. This is #35966.
For #52182
Change-Id: I050f3a27f15d3f189818da80da9de0cba0548931
Reviewed-on: https://go-review.googlesource.com/c/go/+/360015 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Filippo Valsorda [Sat, 30 Oct 2021 04:27:51 +0000 (00:27 -0400)]
crypto/elliptic: refactor package structure
Not quite golang.org/wiki/TargetSpecific compliant, but almost.
The only substantial code change is in randFieldElement: it used to use
Params().BitSize instead of Params().N.BitLen(), which is semantically
incorrect, even if the two values are the same for all named curves.