Removed bitmask and switched to tristate pointers (thank you @mknyszek).

Reset leaked goroutines to waiting when restarting GC cycle.

Author: VladSaiocUber

src/runtime/chan.go

@@ -263,11 +263,11 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
 	}
 	// No stack splits between assigning elem and enqueuing mysg
 	// on gp.waiting where copystack can find it.
-	mysg.elem = ep
+	mysg.elem.set(ep)
 	mysg.waitlink = nil
 	mysg.g = gp
 	mysg.isSelect = false
-	mysg.c = c
+	mysg.c.set(c)
 	gp.waiting = mysg
 	gp.param = nil
 	c.sendq.enqueue(mysg)
@@ -298,7 +298,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
 	if mysg.releasetime > 0 {
 		blockevent(mysg.releasetime-t0, 2)
 	}
-	mysg.c = nil
+	mysg.c.set(nil)
 	releaseSudog(mysg)
 	if closed {
 		if c.closed == 0 {
@@ -336,9 +336,9 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
 			c.sendx = c.recvx // c.sendx = (c.sendx+1) % c.dataqsiz
 		}
 	}
-	if sg.elem != nil {
+	if sg.elem.get() != nil {
 		sendDirect(c.elemtype, sg, ep)
-		sg.elem = nil
+		sg.elem.set(nil)
 	}
 	gp := sg.g
 	unlockf()
@@ -395,7 +395,7 @@ func sendDirect(t *_type, sg *sudog, src unsafe.Pointer) {
 	// Once we read sg.elem out of sg, it will no longer
 	// be updated if the destination's stack gets copied (shrunk).
 	// So make sure that no preemption points can happen between read & use.
-	dst := sg.elem
+	dst := sg.elem.get()
 	typeBitsBulkBarrier(t, uintptr(dst), uintptr(src), t.Size_)
 	// No need for cgo write barrier checks because dst is always
 	// Go memory.
@@ -406,7 +406,7 @@ func recvDirect(t *_type, sg *sudog, dst unsafe.Pointer) {
 	// dst is on our stack or the heap, src is on another stack.
 	// The channel is locked, so src will not move during this
 	// operation.
-	src := sg.elem
+	src := sg.elem.get()
 	typeBitsBulkBarrier(t, uintptr(dst), uintptr(src), t.Size_)
 	memmove(dst, src, t.Size_)
 }
@@ -441,9 +441,9 @@ func closechan(c *hchan) {
 		if sg == nil {
 			break
 		}
-		if sg.elem != nil {
-			typedmemclr(c.elemtype, sg.elem)
-			sg.elem = nil
+		if sg.elem.get() != nil {
+			typedmemclr(c.elemtype, sg.elem.get())
+			sg.elem.set(nil)
 		}
 		if sg.releasetime != 0 {
 			sg.releasetime = cputicks()
@@ -463,7 +463,7 @@ func closechan(c *hchan) {
 		if sg == nil {
 			break
 		}
-		sg.elem = nil
+		sg.elem.set(nil)
 		if sg.releasetime != 0 {
 			sg.releasetime = cputicks()
 		}
@@ -642,13 +642,13 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
 	}
 	// No stack splits between assigning elem and enqueuing mysg
 	// on gp.waiting where copystack can find it.
-	mysg.elem = ep
+	mysg.elem.set(ep)
 	mysg.waitlink = nil
 	gp.waiting = mysg
 
 	mysg.g = gp
 	mysg.isSelect = false
-	mysg.c = c
+	mysg.c.set(c)
 	gp.param = nil
 	c.recvq.enqueue(mysg)
 	if c.timer != nil {
@@ -680,7 +680,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
 	}
 	success := mysg.success
 	gp.param = nil
-	mysg.c = nil
+	mysg.c.set(nil)
 	releaseSudog(mysg)
 	return true, success
 }
@@ -727,14 +727,14 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
 			typedmemmove(c.elemtype, ep, qp)
 		}
 		// copy data from sender to queue
-		typedmemmove(c.elemtype, qp, sg.elem)
+		typedmemmove(c.elemtype, qp, sg.elem.get())
 		c.recvx++
 		if c.recvx == c.dataqsiz {
 			c.recvx = 0
 		}
 		c.sendx = c.recvx // c.sendx = (c.sendx+1) % c.dataqsiz
 	}
-	sg.elem = nil
+	sg.elem.set(nil)
 	gp := sg.g
 	unlockf()
 	gp.param = unsafe.Pointer(sg)

src/runtime/mgc.go

@@ -816,6 +816,12 @@ func gcStart(trigger gcTrigger) {
 		schedEnableUser(false)
 	}
 
+	if work.goroutineLeakFinder.pending.Load() {
+		work.goroutineLeakFinder.enabled = true
+		work.goroutineLeakFinder.pending.Store(false)
+		gcUntrackSyncObjects()
+	}
+
 	// Enter concurrent mark phase and enable
 	// write barriers.
 	//
@@ -832,11 +838,6 @@ func gcStart(trigger gcTrigger) {
 	// possible.
 	setGCPhase(_GCmark)
 
-	if work.goroutineLeakFinder.pending.Load() {
-		work.goroutineLeakFinder.enabled = true
-		work.goroutineLeakFinder.pending.Store(false)
-	}
-
 	gcBgMarkPrepare() // Must happen before assists are enabled.
 	gcPrepareMarkRoots()
 
@@ -1110,8 +1111,6 @@ top:
 func (gp *g) checkIfMaybeRunnable() bool {
 	// Unmask the goroutine address to ensure we are not
 	// dereferencing a masked address.
-	gp = gp.unmask()
-
 	switch gp.waitreason {
 	case waitReasonSelectNoCases,
 		waitReasonChanSendNilChan,
@@ -1125,7 +1124,7 @@ func (gp *g) checkIfMaybeRunnable() bool {
 		// Cycle all through all *sudog to check whether
 		// the goroutine is waiting on a marked channel.
 		for sg := gp.waiting; sg != nil; sg = sg.waitlink {
-			if isMarkedOrNotInHeap(unsafe.Pointer(sg.c)) {
+			if isMarkedOrNotInHeap(unsafe.Pointer(sg.c.get())) {
 				return true
 			}
 		}
@@ -1139,26 +1138,12 @@ func (gp *g) checkIfMaybeRunnable() bool {
 		// check if the synchronization primitive attached to the sudog is marked.
 		if gp.waiting != nil {
 			// Unmask the sema address and check if it's marked.
-			return isMarkedOrNotInHeap(gcUnmask(gp.waiting.elem))
+			return isMarkedOrNotInHeap(gp.waiting.elem.get())
 		}
 	}
 	return true
 }
 
-// unmask returns a *g object with an unmasked address.
-//
-//go:nosplit
-func (gp *g) unmask() *g {
-	return (*g)(gcUnmask(unsafe.Pointer(gp)))
-}
-
-// mask returns a *g object with a masked address.
-//
-//go:nosplit
-func (gp *g) mask() *g {
-	return (*g)(gcMask(unsafe.Pointer(gp)))
-}
-
 // Check to see if more blocked but marked goroutines exist;
 // if so add them into root set and increment work.markrootJobs accordingly
 // return true if we need to run another phase of markroots; return false otherwise
@@ -1171,16 +1156,14 @@ func gcDiscoverMoreStackRoots() {
 
 	// Reorder goroutine list
 	for vIndex < ivIndex {
-		gp := work.stackRoots[vIndex]
-		if gp.checkIfMaybeRunnable() {
-			work.stackRoots[vIndex] = gp
+		if work.stackRoots[vIndex].checkIfMaybeRunnable() {
 			vIndex = vIndex + 1
 			continue
 		}
 		for ivIndex = ivIndex - 1; ivIndex != vIndex; ivIndex = ivIndex - 1 {
-			if swapGp := work.stackRoots[ivIndex]; swapGp.checkIfMaybeRunnable() {
-				work.stackRoots[ivIndex] = gp
-				work.stackRoots[vIndex] = swapGp.unmask()
+			if gp := work.stackRoots[ivIndex]; gp.checkIfMaybeRunnable() {
+				work.stackRoots[ivIndex] = work.stackRoots[vIndex]
+				work.stackRoots[vIndex] = gp
 				vIndex = vIndex + 1
 				break
 			}
@@ -1197,6 +1180,35 @@ func gcDiscoverMoreStackRoots() {
 	}
 }
 
+// getSyncObjectsUnreachable scans allgs and sets the elem and c fields of all sudogs to
+// an untrackable pointer. This prevents the GC from marking these objects as live in memory
+// by following these pointers when runnning deadlock detection.
+func gcUntrackSyncObjects() {
+	assertWorldStopped()
+
+	forEachGRace(func(gp *g) {
+		for sg := gp.waiting; sg != nil; sg = sg.waitlink {
+			sg.elem.untrack()
+			sg.c.untrack()
+		}
+	})
+}
+
+// gcRestoreSyncObjects restores the elem and c fields of all sudogs to their original values.
+// Should be invoked after the goroutine leak detection phase.
+//
+//go:nosplit
+func gcRestoreSyncObjects() {
+	assertWorldStopped()
+
+	forEachGRace(func(gp *g) {
+		for sg := gp.waiting; sg != nil; sg = sg.waitlink {
+			sg.elem.track()
+			sg.c.track()
+		}
+	})
+}
+
 // findGoleaks scans the remaining stackRoots and marks any which are
 // blocked over exclusively unreachable concurrency primitives as leaked (deadlocked).
 // Returns true if the goroutine leak check was performed (or unnecessary).
@@ -1216,7 +1228,7 @@ func findGoleaks() bool {
 	// Make sure these are pushed to the runnable set and ready to be marked.
 	var foundMoreWork bool
 	for i := work.nLiveStackRoots; i < work.nStackRoots; i++ {
-		gp := work.stackRoots[i].unmask()
+		gp := work.stackRoots[i]
 		if readgstatus(gp) == _Gwaiting && !gp.checkIfMaybeRunnable() {
 			// Blocking unrunnable goroutines will be skipped.
 			continue
@@ -1237,7 +1249,7 @@ func findGoleaks() bool {
 
 	// For the remaining goroutines, mark them as unreachable and leaked.
 	for i := work.nLiveStackRoots; i < work.nStackRoots; i++ {
-		gp := work.stackRoots[i].unmask()
+		gp := work.stackRoots[i]
 		casgstatus(gp, _Gwaiting, _Gleaked)
 		fn := findfunc(gp.startpc)
 		if fn.valid() {
@@ -1247,7 +1259,6 @@ func findGoleaks() bool {
 		}
 		traceback(gp.sched.pc, gp.sched.sp, gp.sched.lr, gp)
 		println()
-		work.stackRoots[i] = gp
 	}
 	// Put the remaining roots as ready for marking and drain them.
 	work.markrootJobs += uint32(work.nStackRoots - work.nLiveStackRoots)
@@ -1407,6 +1418,11 @@ func gcMarkTermination(stw worldStop) {
 		throw("non-concurrent sweep failed to drain all sweep queues")
 	}
 
+	if work.goroutineLeakFinder.enabled {
+		// Restore the elem and c fields of all sudogs to their original values.
+		gcRestoreSyncObjects()
+	}
+
 	systemstack(func() {
 		// Pull the GC out of goroutine leak detection mode.
 		work.goroutineLeakFinder.enabled = false

src/runtime/mgcmark.go

@@ -51,31 +51,8 @@ const (
 	// Must be a multiple of the pageInUse bitmap element size and
 	// must also evenly divide pagesPerArena.
 	pagesPerSpanRoot = 512
-
-	gcUndoBitMask = uintptr(uintptrMask >> 2) // This constant reserves some bits of the address space for the GC to use in order to mask addresses.
-	gcBitMask     = ^gcUndoBitMask            // This flips every bit in gcUndoBitMask of uinptr width
 )
 
-// gcMask masks addresses that should not be automatically marked during the GC.
-//
-//go:nosplit
-func gcMask(p unsafe.Pointer) unsafe.Pointer {
-	if goexperiment.GoroutineLeakFinderGC {
-		return unsafe.Pointer(uintptr(p) | gcBitMask)
-	}
-	return p
-}
-
-// gcUnmask undoes the bit-mask applied to a pointer.
-//
-//go:nosplit
-func gcUnmask(p unsafe.Pointer) unsafe.Pointer {
-	if goexperiment.GoroutineLeakFinderGC {
-		return unsafe.Pointer(uintptr(p) & gcUndoBitMask)
-	}
-	return p
-}
-
 // internalBlocked returns true if the goroutine is blocked due to an
 // internal (non-leaking) waitReason, e.g. waiting for the netpoller or garbage collector.
 // Such goroutines are never leak detection candidates according to the GC.
@@ -96,19 +73,24 @@ func (gp *g) internalBlocked() bool {
 func allGsSnapshotSortedForGC() ([]*g, int) {
 	assertWorldStoppedOrLockHeld(&allglock)
 
+	// Reset the status of leaked goroutines in order to improve
+	// the precision of goroutine leak detection.
+	for _, gp := range allgs {
+		gp.atomicstatus.CompareAndSwap(_Gleaked, _Gwaiting)
+	}
+
 	allgsSorted := make([]*g, len(allgs))
 
 	// Indices cutting off runnable and blocked Gs.
 	var currIndex, blockedIndex = 0, len(allgsSorted) - 1
 	for _, gp := range allgs {
-		gp = gp.unmask()
 		// not sure if we need atomic load because we are stopping the world,
 		// but do it just to be safe for now
 		if status := readgstatus(gp); status != _Gwaiting || gp.internalBlocked() {
 			allgsSorted[currIndex] = gp
 			currIndex++
 		} else {
-			allgsSorted[blockedIndex] = gp.mask()
+			allgsSorted[blockedIndex] = gp
 			blockedIndex--
 		}
 	}
@@ -1617,8 +1599,7 @@ func scanobject(b uintptr, gcw *gcWork) {
 		// At this point we have extracted the next potential pointer.
 		// Quickly filter out nil and pointers back to the current object.
 		// The GC will skip masked addresses if GoroutineLeakFinderGC is enabled.
-		if obj != 0 && obj-b >= n &&
-			(!goexperiment.GoroutineLeakFinderGC || obj <= gcUndoBitMask) {
+		if obj != 0 && obj-b >= n {
 			// Test if obj points into the Go heap and, if so,
 			// mark the object.
 			//