From 54dffda2b6f967d216b59fcbda116c74b07c4990 Mon Sep 17 00:00:00 2001 From: Jan Ziak <0xe2.0x9a.0x9b@gmail.com> Date: Tue, 19 Mar 2013 22:17:39 +0100 Subject: [PATCH] runtime: prevent garbage collection during hashmap insertion Inserting a key-value pair into a hashmap storing keys or values indirectly can cause the garbage collector to find the hashmap in an inconsistent state. Fixes #5074. R=golang-dev, minux.ma, rsc CC=golang-dev https://golang.org/cl/7913043 --- src/pkg/runtime/gc_test.go | 15 +++++++++++++++ src/pkg/runtime/hashmap.c | 6 ++++-- src/pkg/runtime/mgc0.c | 16 ++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/pkg/runtime/gc_test.go b/src/pkg/runtime/gc_test.go index e1e1b1d015..3475339bfe 100644 --- a/src/pkg/runtime/gc_test.go +++ b/src/pkg/runtime/gc_test.go @@ -7,6 +7,7 @@ package runtime_test import ( "os" "runtime" + "runtime/debug" "testing" ) @@ -82,3 +83,17 @@ func TestGcDeepNesting(t *testing.T) { t.Fail() } } + +func TestGcHashmapIndirection(t *testing.T) { + defer debug.SetGCPercent(debug.SetGCPercent(1)) + runtime.GC() + type T struct { + a [256]int + } + m := make(map[T]T) + for i := 0; i < 2000; i++ { + var a T + a.a[0] = i + m[a] = T{} + } +} diff --git a/src/pkg/runtime/hashmap.c b/src/pkg/runtime/hashmap.c index 37111daa90..dc5dfb82f5 100644 --- a/src/pkg/runtime/hashmap.c +++ b/src/pkg/runtime/hashmap.c @@ -1016,10 +1016,12 @@ runtime·mapassign(MapType *t, Hmap *h, byte *ak, byte *av) res = nil; hit = hash_insert(t, h, ak, (void**)&res); if(!hit) { + // Need to pass dogc=0 to runtime·mallocgc because the garbage collector + // is assuming that all hashmaps are in a consistent state. if(h->flag & IndirectKey) - *(void**)res = runtime·mal(t->key->size); + *(void**)res = runtime·mallocgc(t->key->size, 0, 0, 1); if(h->flag & IndirectVal) - *(void**)(res+h->valoff) = runtime·mal(t->elem->size); + *(void**)(res+h->valoff) = runtime·mallocgc(t->elem->size, 0, 0, 1); } t->key->alg->copy(t->key->size, hash_keyptr(h, res), ak); t->elem->alg->copy(t->elem->size, hash_valptr(h, res), av); diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c index ce362934c9..a79c22ef95 100644 --- a/src/pkg/runtime/mgc0.c +++ b/src/pkg/runtime/mgc0.c @@ -932,14 +932,26 @@ scanblock(Workbuf *wbuf, Obj *wp, uintptr nobj, bool keepworking) if(!(mapkey_kind & KindNoPointers) || d.indirectkey) { if(!d.indirectkey) *objbufpos++ = (Obj){d.key_data, mapkey_size, mapkey_ti}; - else + else { + if(Debug) { + obj = *(void**)d.key_data; + if(!(arena_start <= obj && obj < arena_used)) + runtime·throw("scanblock: inconsistent hashmap"); + } *ptrbufpos++ = (PtrTarget){*(void**)d.key_data, mapkey_ti}; + } } if(!(mapval_kind & KindNoPointers) || d.indirectval) { if(!d.indirectval) *objbufpos++ = (Obj){d.val_data, mapval_size, mapval_ti}; - else + else { + if(Debug) { + obj = *(void**)d.val_data; + if(!(arena_start <= obj && obj < arena_used)) + runtime·throw("scanblock: inconsistent hashmap"); + } *ptrbufpos++ = (PtrTarget){*(void**)d.val_data, mapval_ti}; + } } } }