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 <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 68600be4e71cbc6f738bf0124d3aa523e8a3a676..5362ff0132774fe5ab9418c8f6eaea48326fff2f 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -387,7 +387,9 @@ func markrootSpans(gcw *gcWork, shard int) {
                                // 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 @@ func scanobject(b uintptr, gcw *gcWork) {
        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 b19a2ff4088c51b66e257b31554d1fc63a01bc79..5d4297617d40c01d5a11e3952e38a809da841e61 100644 (file)
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -1862,12 +1862,14 @@ func addfinalizer(p unsafe.Pointer, f *funcval, nret uintptr, fint *_type, ot *p
                // 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)