Commits

Anonymous committed de23a6a

runtime: fix potential memory corruption
Reinforce the guarantee that MSpan_EnsureSwept actually ensures that the span is swept.
I have not observed crashes related to this, but I do not see why it can't crash as well.

LGTM=rsc
R=golang-codereviews
CC=golang-codereviews, khr, rsc
https://codereview.appspot.com/67990043

Comments (0)

Files changed (2)

src/pkg/runtime/mgc0.c

 {
 	uint32 sg;
 
+	// Caller must disable preemption.
+	// Otherwise when this function returns the span can become unswept again
+	// (if GC is triggered on another goroutine).
+	if(m->locks == 0 && m->mallocing == 0)
+		runtime·throw("MSpan_EnsureSwept: m is not locked");
+
 	sg = runtime·mheap.sweepgen;
 	if(runtime·atomicload(&s->sweepgen) == sg)
 		return;
-	m->locks++;
 	if(runtime·cas(&s->sweepgen, sg-2, sg-1)) {
 		runtime·MSpan_Sweep(s);
-		m->locks--;
 		return;
 	}
-	m->locks--;
 	// unfortunate condition, and we don't have efficient means to wait
 	while(runtime·atomicload(&s->sweepgen) != sg)
 		runtime·osyield();  

src/pkg/runtime/mheap.c

 
 	// Ensure that the span is swept.
 	// GC accesses specials list w/o locks. And it's just much safer.
+	m->locks++;
 	runtime·MSpan_EnsureSwept(span);
 
 	offset = (uintptr)p - (span->start << PageShift);
 	while((x = *t) != nil) {
 		if(offset == x->offset && kind == x->kind) {
 			runtime·unlock(&span->specialLock);
+			m->locks--;
 			return false; // already exists
 		}
 		if(offset < x->offset || (offset == x->offset && kind < x->kind))
 	s->next = x;
 	*t = s;
 	runtime·unlock(&span->specialLock);
+	m->locks--;
 	return true;
 }
 
 
 	// Ensure that the span is swept.
 	// GC accesses specials list w/o locks. And it's just much safer.
+	m->locks++;
 	runtime·MSpan_EnsureSwept(span);
 
 	offset = (uintptr)p - (span->start << PageShift);
 		if(offset == s->offset && kind == s->kind) {
 			*t = s->next;
 			runtime·unlock(&span->specialLock);
+			m->locks--;
 			return s;
 		}
 		t = &s->next;
 	}
 	runtime·unlock(&span->specialLock);
+	m->locks--;
 	return nil;
 }
 
 	Special *s, **t, *list;
 	uintptr offset;
 
+	if(span->sweepgen != runtime·mheap.sweepgen)
+		runtime·throw("runtime: freeallspecials: unswept span");
 	// first, collect all specials into the list; then, free them
 	// this is required to not cause deadlock between span->specialLock and proflock
 	list = nil;