[go] runtime: ensure that we don't scan noscan objects

[Keith Randall (Gerrit)]

Keith Randall submitted this change.

View Change

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: Keith Randall: Run TryBots Keith Randall: Looks good to me, but someone else must approve Michael Knyszek: Looks good to me, approved Gopher Robot: TryBots succeeded

runtime: ensure that we don't scan noscan objects

We claim to not maintain pointer bits for noscan objects. But in fact
we do, since whenever we switch a page from scannable to noscan, we
call heapBits.initSpan which zeroes the heap bits.

Switch to ensure that we never scan noscan objects. This ensures that
we don't depend on the ptrbits for noscan objects. That fixes a bug
in the 1-bit bitmap CL which depended on that fact.

Change-Id: I4e66f582605b53732f8fca310c1f6bd2892963cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/422435
Run-TryBot: Keith Randall <k...@golang.org>
Reviewed-by: Michael Knyszek <mkny...@google.com>
Reviewed-by: Keith Randall <k...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/runtime/mgcmark.go
M src/runtime/mheap.go
2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 68600be..5362ff0 100644
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -387,7 +387,9 @@
 				// Mark everything that can be reached from
 				// the object (but *not* the object itself or
 				// we'll never collect it).
-				scanobject(p, gcw)
+				if !s.spanclass.noscan() {
+					scanobject(p, gcw)
+				}
 
 				// The special itself is a root.
 				scanblock(uintptr(unsafe.Pointer(&spf.fn)), goarch.PtrSize, &oneptrmask[0], gcw, nil)
@@ -1271,22 +1273,14 @@
 	if n == 0 {
 		throw("scanobject n == 0")
 	}
+	if s.spanclass.noscan() {
+		throw("scanobject of a noscan object")
+	}
 
 	if n > maxObletBytes {
 		// Large object. Break into oblets for better
 		// parallelism and lower latency.
 		if b == s.base() {
-			// It's possible this is a noscan object (not
-			// from greyobject, but from other code
-			// paths), in which case we must *not* enqueue
-			// oblets since their bitmaps will be
-			// uninitialized.
-			if s.spanclass.noscan() {
-				// Bypass the whole scan.
-				gcw.bytesMarked += uint64(n)
-				return
-			}
-
 			// Enqueue the other oblets to scan later.
 			// Some oblets may be in b's scalar tail, but
 			// these will be marked as "no more pointers",
diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index b19a2ff..5d42976 100644
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -1862,12 +1862,14 @@
 		// situation where it's possible that markrootSpans
 		// has already run but mark termination hasn't yet.
 		if gcphase != _GCoff {
-			base, _, _ := findObject(uintptr(p), 0, 0)
+			base, span, _ := findObject(uintptr(p), 0, 0)
 			mp := acquirem()
 			gcw := &mp.p.ptr().gcw
 			// Mark everything reachable from the object
 			// so it's retained for the finalizer.
-			scanobject(base, gcw)
+			if !span.spanclass.noscan() {
+				scanobject(base, gcw)
+			}
 			// Mark the finalizer itself, since the
 			// special isn't part of the GC'd heap.
 			scanblock(uintptr(unsafe.Pointer(&s.fn)), goarch.PtrSize, &oneptrmask[0], gcw, nil)

To view, visit change 422435. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I4e66f582605b53732f8fca310c1f6bd2892963cb

Gerrit-Change-Number: 422435

Gerrit-PatchSet: 3

Gerrit-Owner: Keith Randall <k...@golang.org>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Keith Randall <k...@golang.org>

Gerrit-Reviewer: Keith Randall <k...@google.com>

Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>

Gerrit-MessageType: merged