We're about to switch to having significantly fewer maps in the
liveness map, so switch from a dense representation to a sparse
representation.
Passes toolstash-check.
For #36365.
Change-Id: Icb17bd6ace17667a280bc5fba4039cae3020a8d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230543
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These are necessarily deeply non-preemptible, so there's no point in
emitting stack maps for them. We already mark them as unsafe points,
so this only affects the runtime, since user code does not emit stack
maps at unsafe points. SSAGenState.PrepareCall also excludes them when
it's sanity checking call stack maps.
Right now this only drops a handful of unnecessary stack maps from the
runtime, but we're about to start emitting stack maps only at calls
for user code, too. At that point, this will matter much more.
For #36365.
Change-Id: Ib3abfedfddc8e724d933a064fa4d573500627990
Reviewed-on: https://go-review.googlesource.com/c/go/+/230542
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The compiler currently conflates whether a Value has a stack map with
whether it's an unsafe point. For the most part, unsafe-points don't
have stack maps, so this is mostly fine, but call instructions can be
both an unsafe-point *and* have a stack map. For example, none of the
instructions in a nosplit function should be preemptible, but calls
must still have stack maps in case the called function grows the stack
or get preempted.
Currently, the compiler can't distinguish this case, so calls in
nosplit functions are marked as safe-points just because they have
stack maps. This is particularly problematic if a nosplit function
calls another nosplit function, since this can introduce a preemption
point where there should be none.
We realized this was a problem for split-stack prologues a while back,
and CL 207349 changed the encoding of unsafe-points to use the
register map index instead of the stack map index so we could record
both a stack map and an unsafe-point at the same instruction. But this
was never extended into the compiler.
This CL fixes this problem in the compiler. We make LivenessIndex
slightly more abstract by separating unsafe-point marks from stack and
register map indexes. We map this to the PCDATA encoding later when
producing Progs. This isn't enough to fix the whole problem for
nosplit functions, because obj still adds prologues and marks those as
preemptible, but it's a step in the right direction.
I checked this CL by comparing maps before and after this change in
the runtime and net/http. In net/http, unsafe-points match exactly; at
anything that isn't an unsafe-point, both the stack and register maps
are unchanged by this CL. In the runtime, at every point that was a
safe-point before this change, the stack maps agree (and mostly the
runtime doesn't have register maps at all now). In both, all CALLs
(except write barrier calls) have stack maps.
For #36365.
Change-Id: I066628938b02e78be5c81a6614295bcf7cc566c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/230541
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, this function conflates two (easily conflated!) concepts:
whether a Value is a safe-point and whether it has a stack map. In
particular, call Values may not be a safe-point, but may need a stack
map anyway in case the called function grows the stack.
Hence, rename this function to "hasStackMap", since that's really what
it represents.
For #36365.
Change-Id: I89839de0be8db3be3f0d3a7fb5fcf0b0b6ebc98a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230540
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
PanicBounds and PanicExtend are lowered to runtime calls (with a
non-Go ABI), but are not currently marked as calls. Since liveness
analysis only emits stack maps at calls in the runtime, this means
these panic call sites in the runtime won't get a stack map. These
almost immediately turn into throws in the runtime, but there's still
a chance they'll try to grow the stack first, which would lead to a
different panic.
To fix this, mark these operations as calls.
Outside the runtime, we currently emit stack maps for everything that
isn't an unsafe-point, so these panic calls get stack maps by default.
However, we're about to move to emitting stack maps only at call
sites, at which point this will start to matter outside the runtime as
well.
I confirmed that this has no effect on anything but PCDATA/FUNCDATA in
runtime and net/http.
For #36365.
Change-Id: Ic5bb463fd152cc320c815dc04cf62005261ae169
Reviewed-on: https://go-review.googlesource.com/c/go/+/230539
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
A debugger can inject a call at almost any PC, which causes
significant complications with stack scanning and growth. Currently,
the runtime solves this using precise stack maps and register maps at
nearly all PCs, but these extra maps require roughly 5% of the binary.
These extra maps were originally considered worth this space because
they were intended to be used for non-cooperative preemption, but are
now used only for debug call injection.
This CL switches from using precise maps to instead using conservative
frame scanning, much like how non-cooperative preemption works. When a
call is injected, the runtime flushes all potential pointer registers
to the stack, and then treats that frame as well as the interrupted
frame conservatively.
The limitation of conservative frame scanning is that we cannot grow
the goroutine stack. That's doable because the previous CL switched to
performing debug calls on a new goroutine, where they are free to grow
the stack.
With this CL, there are no remaining uses of precise register maps
(though we still use the unsafe-point information that's encoded in
the register map PCDATA stream), and stack maps are only used at call
sites.
For #36365.
Change-Id: Ie217b6711f3741ccc437552d8ff88f961a73cee0
Reviewed-on: https://go-review.googlesource.com/c/go/+/229300
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, when a debugger injects a call, that call happens on the
goroutine where the debugger injected it. However, this requires
significant runtime complexity that we're about to remove.
To prepare for this, this CL switches to a different approach that
leaves the interrupted goroutine parked and runs the debug call on a
new goroutine. When the debug call returns, it resumes the original
goroutine.
This should be essentially transparent to debuggers. It follows the
exact same call injection protocol and ensures the whole protocol
executes indivisibly on a single OS thread. The only difference is
that the current G and stack now change part way through the protocol.
For #36365.
Change-Id: I68463bfd73cbee06cfc49999606410a59dd8f653
Reviewed-on: https://go-review.googlesource.com/c/go/+/229299
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, newproc1 allocates, initializes, and schedules a new
goroutine. We're about to change debug call injection in a way that
will need to create a new goroutine without immediately scheduling it.
To prepare for that, make scheduling the responsibility of newproc1's
caller. Currently, there's exactly one caller (newproc), so this
simply shifts that responsibility.
For #36365.
Change-Id: Idacd06b63e738982e840fe995d891bfd377ce23b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229298
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Rusage.Maxrss is in bytes on Darwin but in KiB on Linux. Fix this
discrepancy so it's always in bytes.
Change-Id: Ic714abc3276566b8fe5e30565072092212610854
Reviewed-on: https://go-review.googlesource.com/c/go/+/230979
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
golang.org/cl/229763 removed the documentation of requirements of
the function passed to FieldsFunc. The current implementation does
not require functions to return consistent results but this had not
been the case for previous implementations.
Add the requirement for consistent results back to the documentation
to allow for future implementations to be more allocation efficient
for an output with more than 32 fields. This is possible with a two
pass algorithm first determining the number of fields used to allocate
the output slice and then splitting the input into fields.
While at it align the documentation of bytes.FieldsFunc with
strings.FieldFunc.
Fixes#38630
Change-Id: Iabbf9ca3dff0daa41f4ec930a21a3dd98e19f122
Reviewed-on: https://go-review.googlesource.com/c/go/+/230797
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
When then go command is run with -trimpath, it will now use
-fdebug-prefix-map when invoking the C compiler (if supported) to
replace the source root directory with a dummy root directory.
This should prevent source directories from appearing either literally
or in compressed DWARF in linked binaries.
Updates #36072
Change-Id: Iedd08d5e886f81e981f11248a1be4ed4f58bdd29
Reviewed-on: https://go-review.googlesource.com/c/go/+/212101
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This allows writing
// F does a thing.
//go:noinline
func F()
without the //go:noinline or other directive (such as //line)
ending up looking like extra words in the doc comment.
Fixes#37974.
Change-Id: Ic738d72802cc2fa448f7633915e7126d2f76d8ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/224737
Reviewed-by: Robert Griesemer <gri@golang.org>
When parsing go.mod files found via file-path replacements, it's safer to
use lockedfile.Read instead of ioutil.ReadFile, in case of overwriting by
other concurrent go commands.
Change-Id: I7dcac3bb5ada84bee1eb634b39f813c461ef103a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230838
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Previously, non-standard attributes in Name.Names were being
omitted when printed using Name.String(). Now, any non-standard
attributes that would not already be printed in Name.String()
are being added temporarily to Name.ExtraNames to be printed.
Fixes#33094Fixes#23069
Change-Id: Id9829c20968e16db7194549f69c0eb5985044944
Reviewed-on: https://go-review.googlesource.com/c/go/+/229864
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Per suggestion in CL 202578, this CL drops the purego build tag used
within this package.
Change-Id: I33626c73d6602e321528544ee601741f7e634c1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230677
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Adds a few instructions to ppc64enc.s that were missing from the
previous update.
Change-Id: Ieafce39e905cdf4da3bfb00fdd5a39ab28089cb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/230437
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Once the symbol is compressed, we will not need the uncompressed
symbol content. Free its memory.
Linking cmd/compile,
name old live-B new live-B delta
Dwarfcompress_GC 42.7M ± 0% 37.9M ± 0% -11.31% (p=0.008 n=5+5)
Change-Id: Ib6cc73832946d158ff4f5b4f31be9c35ba7cf103
Reviewed-on: https://go-review.googlesource.com/c/go/+/230859
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Non functional, but required by the majority of the architectures.
Change-Id: I57601016c28ce665a9d434e283a1db8bded9b133
Reviewed-on: https://go-review.googlesource.com/c/go/+/230858
Run-TryBot: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
After LoadFull, we'll be using sym.Symbols mostly. We still need
the loader information for symbol index mappings and name
lookups, but not much else. Free some memory.
Linking cmd/compile,
name old time/op new time/op delta
Loadlibfull_GC 44.5M ± 0% 35.8M ± 0% -19.66% (p=0.008 n=5+5)
Archive_GC 46.4M ± 0% 37.6M ± 0% -18.89% (p=0.008 n=5+5)
Linking cmd/compile with external linking,
name old time/op new time/op delta
Loadlibfull_GC 82.5M ± 0% 57.4M ± 0% -30.41% (p=0.008 n=5+5)
Archive_GC 86.8M ± 0% 61.7M ± 0% -28.90% (p=0.008 n=5+5)
Change-Id: I6859d488172ef8968918b86de527fbfed6832ebf
Reviewed-on: https://go-review.googlesource.com/c/go/+/230300
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
When applying relocations, we need to resolve ABI aliases.
relocsym does that. Architecture-specific archreloc also needs to
do that. The old code doesn't do that since ABI aliases are
resolved in loadlibfull, or, in the old linker, in a much earlier
stage. We don't do this in the new linker, as we want to avoid
mutating relocations.
While here, move R_CONST and R_GOTOFF handling to generic code.
They appear on several architectures and the handling are same.
Should fix 386-clang and *bsd-386 builds.
Change-Id: I6681c94f0327555d6cf329d0a518c88848773671
Reviewed-on: https://go-review.googlesource.com/c/go/+/230857
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Support external linking for the new reloc pass as well, and
enable it on AMD64 and 386.
Change-Id: Ia71aec3d7c14e9d661e0748d2e988f29f220d1e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230308
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Remove 'Sub' field from sym.Symbol, replacing uses (those downstream of
loadlibfull) with loader method calls.
NB: removing the Outer field will have to wait for now; it is accessed
in archreloc methods that don't have access to link ctxt or loader
currently.
Change-Id: I2abe5906fc169c64b2ab7d5ad213619bea5a17c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/230617
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This updates the PPC64.rules file to use the MOD instructions
that are available in power9. Prior to power9 this is done
using a longer sequence with multiply and divide.
Included in this change is removal of the REM* opcode variations
that set the CC or OV bits since their settings are based
on the DIV and are not appropriate for the REM.
Change-Id: Iceed9ce33e128e1911c15592ee674276ce8ba3fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/229761
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Prior to this commit, NewXxx could panic when passed an image.Rectangle
with one of width or height being negative. But it might not panic if
both were negative, because (bpp * w * h) could still be positive. After
this commit, it will panic if both are negative.
With overflow, NewXxx might not have panicked if (bpp * w * h), the
length passed to "make([]uint8, length)", was still non-negative (after
truncation), but even if w and h were valid (non-negative), the overall
byte slice wasn't long enough. Iterating over the pixels would possibly
panic later with index out of bounds. This change moves the panic
earlier, closer to where the mistake is.
Change-Id: I011feb2d53515fc3f0fe72bb6c23b3953772c577
Reviewed-on: https://go-review.googlesource.com/c/go/+/230220
Reviewed-by: Rob Pike <r@golang.org>
Convert some optimizations rules to strongly-typed versions. Similar to
CL 230338, this CL only converts rules that need no additional changes
(i.e: only need to change '->' to '=>').
This CL covers the rules from line 800 - 1219.
Passes toolstash-check
Change-Id: I94181a809fa38918b78301f1c0c680b7a8ab552f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230738
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Only enabled for AMD64 when internal linking for now.
Change-Id: I2aa9ee47c0f7413ea7bbcdd31b8317c14220bba3
Reviewed-on: https://go-review.googlesource.com/c/go/+/230302
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Previously, looking up only IPv4 or IPv6 addresses was only possible
with DefaultResolver via ResolveIPAddr. Add this functionality to the
Resolver type with a new method, LookupIP. This largely brings Resolver
functionally to parity with the global functions. The name LookupIP is
used over ResolveIPAddr to be consistent with the other Resolver
methods.
There are two main benefits to (*Resolver).LookupIP over
(*Resolver).LookupHost. First is an ergonomic benefit. Wanting a
specific family of address is common enough to justify a method, evident
by the existence of ResolveIPAddr. Second, this opens the possibility of
not performing unnecessary DNS requests when only a specific family of
addresses are needed. This optimization is left to follow up work.
Updates #30452
Change-Id: I241f61019588022a39738f8920b0ddba900cecdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/228641
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allocate the runcases slice on the stack if the number
of select cases is small (up to 4).
Found while looking at production profiles of common
proto based RPC server framework code in Google which do
not have a large number of cases.
name old time/op new time/op delta
Select/1 147ns ± 2% 120ns ± 6% -18.32% (p=0.000 n=7+10)
Select/4 316ns ± 5% 249ns ± 2% -21.23% (p=0.000 n=10+10)
Select/8 516ns ± 3% 515ns ± 3% ~ (p=0.858 n=10+9)
name old alloc/op new alloc/op delta
Select/1 96.0B ± 0% 64.0B ± 0% -33.33% (p=0.000 n=10+10)
Select/4 336B ± 0% 208B ± 0% -38.10% (p=0.000 n=10+10)
Select/8 672B ± 0% 672B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Select/1 4.00 ± 0% 3.00 ± 0% -25.00% (p=0.000 n=10+10)
Select/4 7.00 ± 0% 6.00 ± 0% -14.29% (p=0.000 n=10+10)
Select/8 11.0 ± 0% 11.0 ± 0% ~ (all equal)
Change-Id: I1687e74fc8e86606a27f03fa8a561bcfb68775d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/230657
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Opening a connection with Connect should still create a derived
context with a timeout because some clients will not use a timeout
and the connection pool may open a connection asynchronously.
Likewise, if a connection close makes a network operation it should
provide some type of sane timeout for the operation.
Fixes#38185
Change-Id: I9b7ce2996c81c486170dcc84b12672a99610fa27
Reviewed-on: https://go-review.googlesource.com/c/go/+/230438
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
CL 220499 started marking readonly syms as SRODATA earlier,
so we can use that in the writebarrier pass now.
Passes toolstash-check.
Change-Id: Ic4d49714b8bffbe03c8e9a75ca96df4475bae732
Reviewed-on: https://go-review.googlesource.com/c/go/+/230559
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Remove the "ElfSym" and "LocalElfSym" fields from sym.Symbol,
replacing uses with loader method calls as needed.
Change-Id: I3828f13203ece2bdc03eeb09ab37a5c94e21a726
Reviewed-on: https://go-review.googlesource.com/c/go/+/230462
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Remove sym.Symbol 'Unit' field, replacing accesses to the field with
calls into the loader instead.
Change-Id: Ia1abd4c3d93036705dd624a49cb3d9cbe6a5188b
Reviewed-on: https://go-review.googlesource.com/c/go/+/230307
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Add new get/set methods to the loader for recording the ELF symbol
index for a given loader symbol. These are map-based, since it is
expected that many/most symbols will not need an ELF symbol table
entry.
Change-Id: I1102c3637775515ccc6650118e8b059468a2c3ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/230461
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove the 'Gotype' field from sym.Symbol, as it is now no longer
used. Store the loader.Sym for a symbol as a field in sym.Symbol
("SymIdx"). Then remove sym.Symbol 'File' field, and replace the field
accesses in question with calls into the loader instead.
Change-Id: I01c5504425006b8d3fe77fac2b69a86e198c7a5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230304
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
None of the users of genasmsym are doing anything with the Gotype
field of sym.Symbol, so remove that param from the callback function.
Change-Id: Ie902c4cdbcc6b68d353daf5ce21a99012161a946
Reviewed-on: https://go-review.googlesource.com/c/go/+/230545
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The span set data structure may leak blocks due to a race in the logic
to check whether it's safe to free a block. The simplest example of this
race is between two poppers:
1. Popper A claims slot spanSetEntries-2.
2. Popper B claims slot spanSetEntries-1.
3. Popper A gets descheduled before it subtracts from block.used.
4. Popper B subtracts from block.used, sees that claimed
spanSetEntries-1, but also that block.used != 0, so it returns.
5. Popper A comes back and subtracts from block.used, but it didn't
claim spanSetEntries-1 so it also returns.
The spine is left with a stale block pointer and the block later gets
overwritten by pushes, never to be re-used again.
The problem here is that we designate the claimer of slot
spanSetEntries-1 to be the one who frees the block, but that may not be
the thread that actually does the last subtraction from block.used.
Fixing this problem is tricky, and the fundamental problem there is that
block.used is not stable: it may be observed to be zero, but that
doesn't necessarily mean you're the last popper!
Do something simpler: keep a counter of how many pops have happened to a
given block instead of block.used. This counter monotonically increases
when a pop is _completely done_. Because this counter is monotonically
increasing, and only increases when a popper is done, then we know for
sure whichever popper is the last to increase it (i.e. its value is
spanSetBlockEntries) is also the last popper in the block. Because the
race described above still exists, the last popper may not be the one
which claimed the last slot in the block, but we know for certain nobody
else is popping from that block anymore so we can safely free it.
Finally, because pops serialize with pushes to the same slot, we need
not worry about concurrent pushers at all.
Updates #37487.
Change-Id: I6697219372774c8ca7d8ee6895eaa230a64ce9e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/230497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make Wasm more like other architectures, writing data sections to
heap in Asmb instead of Asmb2. Then we can remove the
copy-on-write logic in applying relocations.
Change-Id: I26d5315ea9fba032fe4bdb9b5c7fe483611c4373
Reviewed-on: https://go-review.googlesource.com/c/go/+/230465
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
