false positive for heap-allocated sql.Rows accessed in two different functions

[stevendanna]

The following function:

> nl -ba repro.go
     1	package repro
     2	
     3	import "database/sql"
     4	
     5	var db *sql.DB
     6	
     7	func issueN() error {
     8		var rows *sql.Rows
     9		func() {
    10			var err error
    11			rows, err = db.Query("select 1")
    12			if err != nil {
    13				panic(err)
    14			}
    15		}()
    16		for rows.Next() {
    17	
    18		}
    19		return rows.Err()
    20	}

produces the following false positive:

> go vet -vettool ~/rowserrcheck ./repro.go
# command-line-arguments
./repro.go:11:23: rows.Err must be checked

The ssadump for the reproducing module:

# Name: command-line-arguments.issueN$1
# Package: command-line-arguments
# Location: /home/ssd/rowserrcheck/repro.go:9:2
# Parent: issueN
# Free variables:
#   0:	rows **database/sql.Rows
func issueN$1():
0:                                                                entry P:0 S:2
	t0 = *db                                               *database/sql.DB
	t1 = (*database/sql.DB).Query(t0, "select 1":string, nil:[]interface{}...) (*database/sql.Rows, error)
	t2 = extract t1 #0                                   *database/sql.Rows
	*rows = t2
	t3 = extract t1 #1                                                error
	t4 = t3 != nil:error                                               bool
	if t4 goto 1 else 2
1:                                                              if.then P:1 S:0
	t5 = change interface interface{} <- error (t3)             interface{}
	panic t5
2:                                                              if.done P:1 S:0
	return

# Name: command-line-arguments.issueN
# Package: command-line-arguments
# Location: /home/ssd/rowserrcheck/repro.go:7:6
func issueN() error:
0:                                                                entry P:0 S:1
	t0 = new *database/sql.Rows (rows)                  **database/sql.Rows
	t1 = make closure issueN$1 [t0]                                  func()
	t2 = t1()                                                            ()
	jump 2
1:                                                             for.done P:1 S:0
	t3 = *t0                                             *database/sql.Rows
	t4 = (*database/sql.Rows).Err(t3)                                 error
	return t4
2:                                                             for.loop P:2 S:2
	t5 = *t0                                             *database/sql.Rows
	t6 = (*database/sql.Rows).Next(t5)                                 bool
	if t6 goto 2 else 1

# Name: command-line-arguments.init
# Package: command-line-arguments
# Synthetic: package initializer
func init():
0:                                                                entry P:0 S:2
	t0 = *init$guard                                                   bool
	if t0 goto 2 else 1
1:                                                           init.start P:1 S:1
	*init$guard = true:bool
	t1 = database/sql.init()                                             ()
	jump 2
2:                                                            init.done P:2 S:0
	return

This isn't fixed by #17

[stevendanna]

I looked into this one a bit and the main thing is that when a sql.Rows is assigned to a FreeVar the linter currently doesn't know how to follow it.

One possible thing to do is that when we hit a FreeVar inside a closure, to find the related binding and then check the references on that as well (and since you might be nested, I suppose that would have to happen recursively).