x/tools/go/analysis/passes/nilness: nilness tracking across functions

[RPGillespie6]

Proposal Details

nilness checks currently only seem to apply to the scope of a single function. I.e. it doesn't detect and flag this obvious nil pointer dereference:

package main

import (
	"fmt"
)

type Foo struct {
	Bar string
}

func printBar(f *Foo) {
	fmt.Println(f.Bar)
}

func main() {
	printBar(nil)
}

It would be beneficial to track nilness issues across function calls in order to flag scenarios such as this.

Maybe nilness isn't the right analyzer for this; basically I want to achieve the same level of safety as .NET's nullable static analyzer, but in go

[gabyhelp]

Similar Issues

• x/tools/gopls: add the nilness analysis #44627
• proposal: cmd/vet: enable nilness checker by default #59714

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

I don't think there is any API change here, so taking it out of the proposal process.

CC @golang/tools-team

[adonovan]

Nilness does only very simple intraprocedural analysis; it could certainly be extended to detect and record which parameters are "definitely dereferenced" on all control paths, such as f in your example, and then to treat calls to such functions as dereference operators. (Similarly it could detect and record which function results are always nil.)

Have you tried using Uber's nilaway? It is the closest existing thing to what you're asking for.

[RPGillespie6]

I have not heard of nilaway, thanks for the link! Looks like nilaway could indeed be a good complement to or even replacement for go vet nilness! Running against the dummy program above, I get:

/home/user/sandbox/main.go:12:14: error: Potential nil panic detected. Observed nil flow from source to dereference point:
        - sandbox/main.go:16:11: literal `nil` passed as arg `f` to `printBar()`
        - sandbox/main.go:12:14: function parameter `f` accessed field `Bar`

I think that meets the spirit of this issue. Feel free to close this issue if you don't want or plan to increase the scope of nilness analyzer to this level.

[adonovan]

Glad nilaway was useful, but I think increasing the scope of nilness analyzer as proposed would be a useful little project, so let's keep this open.

[timothy-king]

"definitely dereferenced" is probably pretty reasonable. Doing this across packages seems doable and helpful too. Requires Facts that track enough info to create future reports in other packages.

I suspect we will want to keep multiple hops out of scope. Example:

func g(x *int) { print(*x) }
func f(x *int) { g(x) }
func h() { f(nil) }

Otherwise we get into issues with needing to represent "x's nilness" in the function summary of f.

(Similarly it could detect and record which function results are always nil.)

This is probably less useful. But seems doable. Cycles do need to be dealt with though.

[adonovan]

Otherwise we get into issues with needing to represent "x's nilness" in the function summary of f.

Not sure I follow. The fact summarizing a function's "definitely dereferenced" disposition would hold a bit for each pointerlike word of each parameter. So for func f(struct{a, b, c *int}) there would be three bits, each tracked independently. Multiple hops doesn't seem particularly challenging; it's a classic interprocedural fixed-point problem.

[timothy-king]

it's a classic interprocedural fixed-point problem.

I am hoping to avoid a major rewrite (which I am not sure we have the bandwidth for). Exactly one hop is not too different than what is there today. It just needs a single pass over package functions to report within the functions and summarize calls + a simple worklist to check intraprocedural calls.

[adonovan]

I am hoping to avoid a major rewrite (which I am not sure we have the bandwidth for)

I didn't say it would be easy, but it would be fun. ;-)