]> Cypherpunks repositories - gostls13.git/commit
cmd/compile: fix unsafe-points with stack maps
authorAustin Clements <austin@google.com>
Tue, 21 Apr 2020 18:23:04 +0000 (14:23 -0400)
committerAustin Clements <austin@google.com>
Wed, 29 Apr 2020 21:29:17 +0000 (21:29 +0000)
commitfaafdf5115c994ff6d5ab3fe2eaf70ee47186f54
treec302b89b5434b183979142d4e7bbcf6ad2b1da1d
parenta6deafaf9e6262e1a9fa390e2cc0b6df1977828c
cmd/compile: fix unsafe-points with stack maps

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>
src/cmd/compile/internal/gc/gsubr.go
src/cmd/compile/internal/gc/plive.go
src/cmd/compile/internal/gc/ssa.go
src/cmd/internal/objabi/funcdata.go