diff options
author | Matthew Dempsky <mdempsky@google.com> | 2017-03-01 21:46:08 -0800 |
---|---|---|
committer | Matthew Dempsky <mdempsky@google.com> | 2017-04-05 19:28:00 +0000 |
commit | fe79c752686b52c1722315bdf15bb035f5377457 (patch) | |
tree | 1b2b76d0a6181a342c27c541b64f1014d1363cf6 | |
parent | d7989b784e685eb51af1601af0f8a91d3bd53e5d (diff) | |
download | go-fe79c752686b52c1722315bdf15bb035f5377457.tar.gz go-fe79c752686b52c1722315bdf15bb035f5377457.zip |
[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 <mdempsky@google.com>
Reviewed-by: Austin Clements <austin@google.com>
-rw-r--r-- | src/cmd/compile/internal/gc/ssa.go | 9 | ||||
-rw-r--r-- | src/cmd/compile/internal/gc/walk.go | 41 | ||||
-rw-r--r-- | test/fixedbugs/issue19168.go | 58 |
3 files changed, 100 insertions, 8 deletions
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)) +} |