Analysis check for leaked pointers to loop variable address

[Barisere Jonathan]

Hello everyone.

I have a question about a common programming error I have made several times. It regards the use of a pointer to a loop variable outside of the loop. Basically, we want to convert a slice, map, or channel of items (the source sequence) into a map, slice, or channel of pointers to respective items in the source sequence. But, rather than point to the actual values, we point to the loop variable.

I have written some code in a GitHub gist to demonstrate this mistake. I think this mistake can be caught by code analysis tools, but I don't know of any that catches these. If there is any, please let me know so that I can save my future self some unnecessary trouble. If there isn't, how can I make one (probably add it to vet)?

Here's the code sample, if you prefer to read it all here.

// escape_test.go

package main_test

import (
    "testing"
    "testing/quick"
    "unsafe"
)

func all(xs []*int, f func(x *int) bool) (result bool) {
    result = true
    for _, v := range xs {
        result = result && f(v)
    }
    return result
}

func Test_references_to_loop_variable_outside_the_loop_have_same_value(t *testing.T) {
    f := func(xs []int) bool {
        var loopVariableAddress uintptr
        var zeroOfUintptr uintptr
        var copies = make([]*int, 0, len(xs))
        for _, x := range xs {
            if loopVariableAddress == zeroOfUintptr {
                // Store the loop variable's address for later comparison.
                loopVariableAddress = uintptr(unsafe.Pointer(&x))
            }
            // Copy the address of x into a slice.
            copies = append(copies, &x)
            // The append statement above is most likely a mistake.
            // We probably mean
            // copies = append(copies, &xs[index]);
            // assuming `index` is the ignored loop index.
        }
        return all(copies, func(x *int) bool {
            // All values in `copies` are the same pointer address.
            return uintptr(unsafe.Pointer(x)) == loopVariableAddress && *x == *copies[0]
        })
    }

    if err := quick.Check(f, nil); err != nil {
        t.Fatal(err)
    }
}

[Jan Mercl]

On Mon, Mar 15, 2021 at 3:03 AM Barisere Jonathan
<barry.jo...@gmail.com> wrote:

> I think this mistake can be caught by code analysis tools, but I don't know of any that catches these.

IMO the problem is that while sometimes it's a mistake, in some other
cases it is not and it is perfectly good, reasonable code. Next to
impossible for a machine to decide which case it sees.

[Barisere Jonathan]

That's correct. I've found that it's most often a mistake when working with slices. But for maps, there is some use for it, such as relying on maps being unordered to select random values from the map, although there should be a better way to do that.

An analysis check for this should produce a warning. I'm betting that since it's an error in most of the cases I've come across, a warning will be useful most of the time.