]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: don't mark uintptr->unsafe.Pointer conversion unsafe points
authorCherry Mui <cherryyz@google.com>
Wed, 11 Jan 2023 23:56:31 +0000 (18:56 -0500)
committerCherry Mui <cherryyz@google.com>
Tue, 28 Feb 2023 23:42:44 +0000 (23:42 +0000)
In the past, we planned to implement asynchronous preemption using
precise register pointer maps. In this strategy, conversions between
unsafe.Pointer and uintptr would need to be marked as unsafe points,
as if a pointer value is temporarily converted to uintptr (and not
live otherwise), the GC would not be able to see it when scanning
the stack (and registers).

But now we actually implemented asynchronous preemption with inner
frame conservative scan. So even if a pointer value lives as an
integer the GC can still see it. There is no need to mark the
conversion as unsafe points. This allows more places to be
preempted, as well as for debugger to inject a call.

Fixes #57719.

Change-Id: I375ab820d8d74d122b565cf72ecc7cdb225dbc01
Reviewed-on: https://go-review.googlesource.com/c/go/+/461696
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>

src/cmd/compile/internal/liveness/plive.go

index a479badfd0763cd50ee3bdf613bb47c2abd58715..82f651a520c72890c2a57d7301993abb6736bac1 100644 (file)
@@ -589,66 +589,6 @@ func (lv *liveness) markUnsafePoints() {
                        }
                }
        }
-
-       // Find uintptr -> unsafe.Pointer conversions and flood
-       // unsafeness back to a call (which is always a safe point).
-       //
-       // Looking for the uintptr -> unsafe.Pointer conversion has a
-       // few advantages over looking for unsafe.Pointer -> uintptr
-       // conversions:
-       //
-       // 1. We avoid needlessly blocking safe-points for
-       // unsafe.Pointer -> uintptr conversions that never go back to
-       // a Pointer.
-       //
-       // 2. We don't have to detect calls to reflect.Value.Pointer,
-       // reflect.Value.UnsafeAddr, and reflect.Value.InterfaceData,
-       // which are implicit unsafe.Pointer -> uintptr conversions.
-       // We can't even reliably detect this if there's an indirect
-       // call to one of these methods.
-       //
-       // TODO: For trivial unsafe.Pointer arithmetic, it would be
-       // nice to only flood as far as the unsafe.Pointer -> uintptr
-       // conversion, but it's hard to know which argument of an Add
-       // or Sub to follow.
-       var flooded bitvec.BitVec
-       var flood func(b *ssa.Block, vi int)
-       flood = func(b *ssa.Block, vi int) {
-               if flooded.N == 0 {
-                       flooded = bitvec.New(int32(lv.f.NumBlocks()))
-               }
-               if flooded.Get(int32(b.ID)) {
-                       return
-               }
-               for i := vi - 1; i >= 0; i-- {
-                       v := b.Values[i]
-                       if v.Op.IsCall() {
-                               // Uintptrs must not contain live
-                               // pointers across calls, so stop
-                               // flooding.
-                               return
-                       }
-                       lv.unsafePoints.Set(int32(v.ID))
-               }
-               if vi == len(b.Values) {
-                       // We marked all values in this block, so no
-                       // need to flood this block again.
-                       flooded.Set(int32(b.ID))
-               }
-               for _, pred := range b.Preds {
-                       flood(pred.Block(), len(pred.Block().Values))
-               }
-       }
-       for _, b := range lv.f.Blocks {
-               for i, v := range b.Values {
-                       if !(v.Op == ssa.OpConvert && v.Type.IsPtrShaped()) {
-                               continue
-                       }
-                       // Flood the unsafe-ness of this backwards
-                       // until we hit a call.
-                       flood(b, i+1)
-               }
-       }
 }
 
 // Returns true for instructions that must have a stack map.