[release-branch.go1.8] cmd/compile: add missing WBs for reflect.{Slice,String}Header...

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 <mdempsky@google.com>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index bf483f8416c118860faab62de85a22f8457499e6..a0c1cf15b9e5d6162cd68e200767b080ff7d908e 100644 (file)
--- 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 7c2e2ab442886c4f511300a77075ad968ec16fec..d080d2106619151c3903e9e5e5283d448ca27d91 100644 (file)
--- 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 (file)
index 0000000..b94b1d0
--- /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))
+}