Interesting, I actually ran into some odd behavior with this function that
I'm not sure how to explain. Consider the following test:
import (
"reflect"
"testing"
"unsafe"
)
func sliceToStringUnsafe(v []byte) string {
var s string
sh := (*reflect.StringHeader)(unsafe.Pointer(&s))
sh.Data = uintptr(unsafe.Pointer(&v[0]))
sh.Len = len(v)
return s
}
func foo() string {
v := []byte{"foobarbaz"}
s := sliceToStringUnsafe(v)
return s
}
func TestFoo(t *testing.T) string {
foo := foo()
require.Equal(t, "foobarbaz", foo)
}
If I run this test with `go test` it passes. However, if I run the test
with `go test -covermode atomic` it fails and foo contains random data.
Perhaps the `covermode atomic` affects how the compiler performs escape
analysis?
On Saturday, March 25, 2017 at 12:43:38 PM UTC-4, Keith Randall wrote:
>
> That is a worry, but escape analysis can handle this case. It understands
> flow of pointers through unsafe.Pointer and uintptr. As a consequence, in
> this example it forces the array to be allocated on the heap.
>
> The runtime has on occasion a need to hide values from escape analysis.
> It has to do some weird math on the uintptr to hide the escape. It uses:
>
> // noescape hides a pointer from escape analysis. noescape is
> // the identity function but escape analysis doesn't think the
> // output depends on the input. noescape is inlined and currently
> // compiles down to zero instructions.
> // USE CAREFULLY!
> //go:nosplit
> func noescape(p unsafe.Pointer) unsafe.Pointer {
> x := uintptr(p)
> return unsafe.Pointer(x ^ 0)
> }
>
>
> On Friday, March 24, 2017 at 11:58:31 AM UTC-7, Jerome Froelich wrote:
>>
>> 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
>>> <[email protected]> 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 <[email protected]>
>>>
>>> >> wrote:
>>> >> > On Thu, Mar 23, 2017 at 9:16 AM, Caleb Spare <[email protected]>
>>> 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 [email protected].
>>> > 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 [email protected].
For more options, visit https://groups.google.com/d/optout.
