From fe79c752686b52c1722315bdf15bb035f5377457 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 1 Mar 2017 21:46:08 -0800 Subject: [release-branch.go1.8] cmd/compile: add missing WBs for reflect.{Slice,String}Header.Data Fixes #19168. (*state).insertWBstore needed to be tweaked for backporting so that store reflect.{Slice,String}Header.Data stores still fallthrough and end the SSA block. This wasn't necessary at master because of CL 36834. Change-Id: I3f4fcc0b189c53819ac29ef8de86fdad76a17488 Reviewed-on: https://go-review.googlesource.com/39615 Run-TryBot: Matthew Dempsky Reviewed-by: Austin Clements --- src/cmd/compile/internal/gc/ssa.go | 9 ++++-- src/cmd/compile/internal/gc/walk.go | 41 ++++++++++++++++++++++---- test/fixedbugs/issue19168.go | 58 +++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 test/fixedbugs/issue19168.go diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index bf483f8416..a0c1cf15b9 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -3470,8 +3470,13 @@ func (s *state) insertWBstore(t *Type, left, right *ssa.Value, line int32, skip if s.WBLineno == 0 { s.WBLineno = left.Line } - s.storeTypeScalars(t, left, right, skip) - s.storeTypePtrsWB(t, left, right) + if t == Types[TUINTPTR] { + // Stores to reflect.{Slice,String}Header.Data. + s.vars[&memVar] = s.newValue3I(ssa.OpStoreWB, ssa.TypeMem, s.config.PtrSize, left, right, s.mem()) + } else { + s.storeTypeScalars(t, left, right, skip) + s.storeTypePtrsWB(t, left, right) + } // WB ops will be expanded to branches at writebarrier phase. // To make it easy, we put WB ops at the end of a block, so diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 7c2e2ab442..d080d21066 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -2071,6 +2071,29 @@ func isstack(n *Node) bool { return false } +// isReflectHeaderDataField reports whether l is an expression p.Data +// where p has type reflect.SliceHeader or reflect.StringHeader. +func isReflectHeaderDataField(l *Node) bool { + if l.Type != Types[TUINTPTR] { + return false + } + + var tsym *Sym + switch l.Op { + case ODOT: + tsym = l.Left.Type.Sym + case ODOTPTR: + tsym = l.Left.Type.Elem().Sym + default: + return false + } + + if tsym == nil || l.Sym.Name != "Data" || tsym.Pkg.Path != "reflect" { + return false + } + return tsym.Name == "SliceHeader" || tsym.Name == "StringHeader" +} + // Do we need a write barrier for the assignment l = r? func needwritebarrier(l *Node, r *Node) bool { if !use_writebarrier { @@ -2081,15 +2104,21 @@ func needwritebarrier(l *Node, r *Node) bool { return false } - // No write barrier for write of non-pointers. - dowidth(l.Type) - - if !haspointers(l.Type) { + // No write barrier for write to stack. + if isstack(l) { return false } - // No write barrier for write to stack. - if isstack(l) { + // Package unsafe's documentation says storing pointers into + // reflect.SliceHeader and reflect.StringHeader's Data fields + // is valid, even though they have type uintptr (#19168). + if isReflectHeaderDataField(l) { + return true + } + + // No write barrier for write of non-pointers. + dowidth(l.Type) + if !haspointers(l.Type) { return false } diff --git a/test/fixedbugs/issue19168.go b/test/fixedbugs/issue19168.go new file mode 100644 index 0000000000..b94b1d0a99 --- /dev/null +++ b/test/fixedbugs/issue19168.go @@ -0,0 +1,58 @@ +// errorcheck -0 -l -d=wb + +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +import ( + "reflect" + "unsafe" + + reflect2 "reflect" +) + +func sink(e interface{}) + +func a(hdr *reflect.SliceHeader, p *byte) { + hdr.Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" +} + +func b(hdr *reflect.StringHeader, p *byte) { + hdr.Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" +} + +func c(hdrs *[1]reflect.SliceHeader, p *byte) { + hdrs[0].Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" +} + +func d(hdr *struct{ s reflect.StringHeader }, p *byte) { + hdr.s.Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" +} + +func e(p *byte) (resHeap, resStack string) { + sink(&resHeap) + + hdr := (*reflect.StringHeader)(unsafe.Pointer(&resHeap)) + hdr.Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" + + // No write barrier for non-escaping stack vars. + hdr = (*reflect.StringHeader)(unsafe.Pointer(&resStack)) + hdr.Data = uintptr(unsafe.Pointer(p)) + + return +} + +func f(hdr *reflect2.SliceHeader, p *byte) { + hdr.Data = uintptr(unsafe.Pointer(p)) // ERROR "write barrier" +} + +type SliceHeader struct { + Data uintptr +} + +func g(hdr *SliceHeader, p *byte) { + // No write barrier for lookalike SliceHeader. + hdr.Data = uintptr(unsafe.Pointer(p)) +} -- cgit v1.2.3-54-g00ecf