From 0f9d71c42812ad989f3b01df8db9b9356bba6c2c Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 26 Oct 2015 09:55:02 -0400 Subject: [PATCH] go/pointer: fix bug in constraint generation of "for _, v := range map" + regression test Change-Id: I9ec28f222e14af0cb737494e3c48e5423cdcf236 Reviewed-on: https://go-review.googlesource.com/16294 Reviewed-by: David Crawshaw --- go/pointer/gen.go | 23 ++++++++++++++++++++++- go/pointer/testdata/maps.go | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/go/pointer/gen.go b/go/pointer/gen.go index 43849a2b..89d28732 100644 --- a/go/pointer/gen.go +++ b/go/pointer/gen.go @@ -1050,11 +1050,32 @@ func (a *analysis) genInstr(cgn *cgnode, instr ssa.Instruction) { // Assumes that Next is always directly applied to a Range result. theMap := instr.Iter.(*ssa.Range).X tMap := theMap.Type().Underlying().(*types.Map) + ksize := a.sizeof(tMap.Key()) vsize := a.sizeof(tMap.Elem()) + // The k/v components of the Next tuple may each be invalid. + tTuple := instr.Type().(*types.Tuple) + // Load from the map's (k,v) into the tuple's (ok, k, v). - a.genLoad(cgn, a.valueNode(instr)+1, theMap, 0, ksize+vsize) + osrc := uint32(0) // offset within map object + odst := uint32(1) // offset within tuple (initially just after 'ok bool') + sz := uint32(0) // amount to copy + + // Is key valid? + if tTuple.At(1).Type() != tInvalid { + sz += ksize + } else { + odst += ksize + osrc += ksize + } + + // Is value valid? + if tTuple.At(2).Type() != tInvalid { + sz += vsize + } + + a.genLoad(cgn, a.valueNode(instr)+nodeid(odst), theMap, osrc, sz) } case *ssa.Lookup: diff --git a/go/pointer/testdata/maps.go b/go/pointer/testdata/maps.go index 6f3751d7..67293045 100644 --- a/go/pointer/testdata/maps.go +++ b/go/pointer/testdata/maps.go @@ -45,7 +45,30 @@ func maps2() { print(m2[nil]) // @pointsto main.c } +var g int + +func maps3() { + // Regression test for a constraint generation bug for map range + // loops in which the key is unused: the (ok, k, v) tuple + // returned by ssa.Next may have type 'invalid' for the k and/or + // v components, so copying the map key or value may cause + // miswiring if the key has >1 components. In the worst case, + // this causes a crash. The test below used to report that + // pts(v) includes not just main.g but new(float64) too, which + // is ill-typed. + + // sizeof(K) > 1, abstractly + type K struct{ a, b *float64 } + k := K{new(float64), nil} + m := map[K]*int{k: &g} + + for _, v := range m { + print(v) // @pointsto main.g + } +} + func main() { maps1() maps2() + maps3() }