It just occurred to me that there may actually be a problem with the 
conversion functions in the example. Specifically, since the Go 
compiler can allocate byte slices on the heap if they do not escape, the 
following function may be invalid:

func foo() string {
        v := []uint64{1,2,3}
        s := sliceToStringUnsafe(v)
return s
}

Admittedly, this is a contrived example but I think it can illustrate the 
issue. As noted on this previous thread 
<https://groups.google.com/forum/#!topic/golang-nuts/KdbtOqNK6JQ> the 
compiler will allocate slices on the heap if it can prove they do not 
escape. Consequently, in the function above, the data for v can be 
allocated on the stack. In such a case, since sliceToStringUnsafe only 
adjusts pointers, the string it returns will point to this stack-allocated 
data. However, once foo returns, this data is no longer valid and can be 
overwritten by subsequent functions that are pushed on the stack.

On Thursday, March 23, 2017 at 9:05:48 PM UTC-4, Caleb Spare wrote:
>
> Filed: https://github.com/golang/go/issues/19687 
>
> On Thu, Mar 23, 2017 at 4:41 PM, 'Keith Randall' via golang-nuts 
> <golan...@googlegroups.com <javascript:>> wrote: 
> > 
> > 
> > On Thursday, March 23, 2017 at 4:24:40 PM UTC-7, Caleb Spare wrote: 
> >> 
> >> That's very good to know. Thanks, Ian. 
> >> 
> >> Unfortunately if I use this KeepAlive-based fix, p escapes and so the 
> >> function now allocates. I guess I'll stick with the original version 
> >> from my first email. 
> >> 
> >> Does this indicate a shortcoming of either compiler support for 
> >> KeepAlive or escape analysis in general? 
> >> 
> > 
> > KeepAlive shouldn't be making things escape.  If that is happening you 
> > should file a bug for it. 
> > The definition is: 
> > 
> > //go:noinline 
> > func KeepAlive(interface{}) {} 
> > 
> > which should be pretty easy to analyze :) 
> > 
> >> Caleb 
> >> 
> >> On Thu, Mar 23, 2017 at 10:26 AM, Ian Lance Taylor <ia...@golang.org> 
> >> wrote: 
> >> > On Thu, Mar 23, 2017 at 9:16 AM, Caleb Spare <ces...@gmail.com> 
> wrote: 
> >> >> 
> >> >> Brief follow-up: does the seeming validity of the code rely at all 
> on 
> >> >> the fact that the indicated line is written as a single line? What 
> if, 
> >> >> instead, a *StringHeader var were extracted? 
> >> >> 
> >> >> func stringToSliceUnsafe(s string) []uint64 { 
> >> >>         var v []uint64 
> >> >>         h := (*reflect.StringHeader)(unsafe.Pointer(&s)) // <-- 
> >> >>         sh := (*reflect.SliceHeader)(unsafe.Pointer(&v)) 
> >> >>         sh.Data = h.Data 
> >> >>         sh.Len = h.Len >> 3 
> >> >>         sh.Cap = h.Len >> 3 
> >> >>         return v 
> >> >> } 
> >> >> 
> >> >> (Play link: https://play.golang.org/p/BmGtYTsGNY) 
> >> >> 
> >> >> Does h keep s alive? A strict reading of rule 6 doesn't seem to say 
> >> >> that keeping a *StringHeader or *SliceHeader around keeps the 
> >> >> underlying string/slice alive (but it's sort of implied by the rule 
> 6 
> >> >> example code, which doesn't refer to s after converting it to a 
> >> >> *StringHeader). 
> >> > 
> >> > That is an interesting point.  I don't think there is anything 
> keeping 
> >> > s alive here.  I think this isn't quite the same as the example in 
> the 
> >> > docs, because that example is assuming that you are doing to use s 
> >> > after setting the fields--why else would you be doing that?  In this 
> >> > case it does seem theoretically possible that s could be freed 
> between 
> >> > the assignment to h and the use of h.Data.  With the current and 
> >> > foreseeable toolchains it's a purely theoretical problem, since there 
> >> > is no point there where the goroutine could be preempted and the fact 
> >> > that s is no longer referenced be detected.  But as a theoretical 
> >> > problem it does seem real.  One fix would be something like 
> >> >     p := &s 
> >> >     h := (*reflect.StringHeader)(unsafe.Pointer(p)) 
> >> >     sh := (*reflect.SliceHeader)(unsafe.Pointer(&v)) 
> >> >     sh.Data = h.Data 
> >> >     sh.Len = ... 
> >> >     sh.Cap = ... 
> >> >     runtime.KeepAlive(p) 
> >> > 
> >> > Ian 
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> Groups 
> > "golang-nuts" group. 
> > To unsubscribe from this group and stop receiving emails from it, send 
> an 
> > email to golang-nuts...@googlegroups.com <javascript:>. 
> > For more options, visit https://groups.google.com/d/optout. 
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to