gordonklaus / ineffassign

Detect ineffectual assignments in Go code.
MIT License
394 stars 22 forks source link

False positive with slice pointing to an array #16

Closed dolmen closed 7 years ago

dolmen commented 7 years ago

This program triggers a false positive reported by ineffassign: main.go:9:3: ineffectual assignment to iv. iv and ivSlice point to the same data so this is not a ineffective assignment.

package main

import "fmt"

func main() {
    var iv [16]byte
    ivSlice := iv[:]
    for i := range ivSlice {
        iv = [16]byte{} // Reset
        ivSlice[i] = 'A'
        fmt.Printf("%x\n", ivSlice)
    }
}
gordonklaus commented 7 years ago

Nice find, thanks for the report.

(Note to self: To fix ineffassign, I would conservatively mark any variable that is the target of a slice operation as "escaping" (meaning it should never be reported as an ineffectual assignment). This could end up silencing some true positives, but I suspect not many.)

dolmen commented 7 years ago

I think that the case to consider here is that any array variable from which a slice is built will escape analysis from that point. Because the array data may be modified directly via the array but also through the slice (or through any other slice derived from that slice, or any function that takes the slice as a parameter).

dolmen commented 7 years ago

This is in fact the same case of escape as with pointers:

package main

func main() {
    var i int
    p := &i
    f(p)
    i = 0
    f(p)

    var arr [1]int
    s := arr[:]
    g(s)
    arr = [1]int{}
    g(s)
}

func f(*int)  {}
func g([]int) {}

f can modify i through p, so as soon as a pointer to i is taken, i can't be statically tracked. g can modify arr through s, so as soon as a slice to arr is taken, arr can't be statically tracked.