Skip to content

Commit

Permalink
golang/go#40701: avoid comp lit usage of reflect.{String,Slice}Header
Browse files Browse the repository at this point in the history
See golang/go#40701 example three.
  • Loading branch information
twmb committed Sep 2, 2020
1 parent a57a9e6 commit 610077f
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions murmur.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ func (d *digest) Reset() {

func strslice(slice []byte) string {
var str string
*(*reflect.StringHeader)(unsafe.Pointer(&str)) = reflect.StringHeader{
Data: ((*reflect.SliceHeader)(unsafe.Pointer(&slice))).Data,
Len: len(slice),
}
strhdr := (*reflect.StringHeader)(unsafe.Pointer(&str))
strhdr.Data = ((*reflect.SliceHeader)(unsafe.Pointer(&slice))).Data
strhdr.Len = len(slice)
return str
}

2 comments on commit 610077f

@twmb
Copy link
Owner Author

@twmb twmb commented on 610077f Sep 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this may not be problematic; the third example is more about how the function takes (*byte, int) and converts that to a slice. The compiler cannot detect that the new slice is escaping.

Consider the input:

func make_slice() []byte {
    x := []byte("asdf") // will live on the stack unless it escapes
    return c(&x[0], len(x)) // c doesn't detect &x[0] leaking
}

this would be invalid (if c were the example 3 func).

However, to be even more super safe, may as well update to avoid dereffing the header at all.

@twmb
Copy link
Owner Author

@twmb twmb commented on 610077f Oct 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears this change was still necessary, as without this change, this is the unsafe pattern 1 as documented in https://github.com/jlauinger/go-safer.

Please sign in to comment.