From d74342e66d800f8c93b1c6ade5ad9b4f1b60ca1c Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 09:40:21 +0000
Subject: [PATCH 1/6] testutils: import nilness pass from go/tools

This imports the nilness check from golang.org/x/tools at commit
3e1cb95235af786c5d1f0deffbd0b0646bf00024 with minor modifications to
pass our linter.

The purpose of importing this linter into the source tree rather than
depending on it is so that we can make local modifications to it in
future commits.

Release note: None
---
 pkg/BUILD.bazel                               |   1 +
 pkg/testutils/lint/passes/nilness/BUILD.bazel |  24 ++
 pkg/testutils/lint/passes/nilness/nilness.go  | 368 ++++++++++++++++++
 .../lint/passes/nilness/nilness_test.go       |  30 ++
 .../lint/passes/nilness/testdata/src/a/a.go   | 177 +++++++++
 vendor                                        |   2 +-
 6 files changed, 601 insertions(+), 1 deletion(-)
 create mode 100644 pkg/testutils/lint/passes/nilness/BUILD.bazel
 create mode 100644 pkg/testutils/lint/passes/nilness/nilness.go
 create mode 100644 pkg/testutils/lint/passes/nilness/nilness_test.go
 create mode 100644 pkg/testutils/lint/passes/nilness/testdata/src/a/a.go

diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
index bc23c01639f3..deab1d1a473f 100644
--- a/pkg/BUILD.bazel
+++ b/pkg/BUILD.bazel
@@ -264,6 +264,7 @@ ALL_TESTS = [
     "//pkg/testutils/lint/passes/forbiddenmethod:forbiddenmethod_test",
     "//pkg/testutils/lint/passes/hash:hash_test",
     "//pkg/testutils/lint/passes/leaktestcall:leaktestcall_test",
+    "//pkg/testutils/lint/passes/nilness:nilness_test",
     "//pkg/testutils/lint/passes/nocopy:nocopy_test",
     "//pkg/testutils/lint/passes/passesutil:passesutil_test",
     "//pkg/testutils/lint/passes/returnerrcheck:returnerrcheck_test",
diff --git a/pkg/testutils/lint/passes/nilness/BUILD.bazel b/pkg/testutils/lint/passes/nilness/BUILD.bazel
new file mode 100644
index 000000000000..728a4e56e181
--- /dev/null
+++ b/pkg/testutils/lint/passes/nilness/BUILD.bazel
@@ -0,0 +1,24 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "nilness",
+    srcs = ["nilness.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/nilness",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis",
+        "@org_golang_x_tools//go/analysis/passes/buildssa",
+        "@org_golang_x_tools//go/ssa",
+    ],
+)
+
+go_test(
+    name = "nilness_test",
+    srcs = ["nilness_test.go"],
+    data = glob(["testdata/**"]),
+    tags = ["broken_in_bazel"],
+    deps = [
+        ":nilness",
+        "@org_golang_x_tools//go/analysis/analysistest",
+    ],
+)
diff --git a/pkg/testutils/lint/passes/nilness/nilness.go b/pkg/testutils/lint/passes/nilness/nilness.go
new file mode 100644
index 000000000000..e5234057a61e
--- /dev/null
+++ b/pkg/testutils/lint/passes/nilness/nilness.go
@@ -0,0 +1,368 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Portions of this file are additionally subject to the following
+// license and copyright.
+//
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package nilness inspects the control-flow graph of an SSA function
+// and reports errors such as nil pointer dereferences and degenerate
+// nil pointer comparisons.
+package nilness
+
+import (
+	"fmt"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/ssa"
+)
+
+const doc = `check for redundant or impossible nil comparisons
+
+The nilness checker inspects the control-flow graph of each function in
+a package and reports nil pointer dereferences, degenerate nil
+pointers, and panics with nil values. A degenerate comparison is of the form
+x==nil or x!=nil where x is statically known to be nil or non-nil. These are
+often a mistake, especially in control flow related to errors. Panics with nil
+values are checked because they are not detectable by
+
+	if r := recover(); r != nil {
+
+This check reports conditions such as:
+
+	if f == nil { // impossible condition (f is a function)
+	}
+
+and:
+
+	p := &v
+	...
+	if p != nil { // tautological condition
+	}
+
+and:
+
+	if p == nil {
+		print(*p) // nil dereference
+	}
+
+and:
+
+	if p == nil {
+		panic(p)
+	}
+`
+
+// Analyzer defines a pass that checks for uses of provably nil values
+// that were likely in error.
+var Analyzer = &analysis.Analyzer{
+	Name:     "nilness",
+	Doc:      doc,
+	Run:      run,
+	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+	for _, fn := range ssainput.SrcFuncs {
+		runFunc(pass, fn)
+	}
+	return nil, nil
+}
+
+func runFunc(pass *analysis.Pass, fn *ssa.Function) {
+	reportf := func(category string, pos token.Pos, format string, args ...interface{}) {
+		pass.Report(analysis.Diagnostic{
+			Pos:      pos,
+			Category: category,
+			Message:  fmt.Sprintf(format, args...),
+		})
+	}
+
+	// notNil reports an error if v is provably nil.
+	notNil := func(stack []fact, instr ssa.Instruction, v ssa.Value, descr string) {
+		if nilnessOf(stack, v) == isnil {
+			reportf("nilderef", instr.Pos(), "nil dereference in "+descr)
+		}
+	}
+
+	// visit visits reachable blocks of the CFG in dominance order,
+	// maintaining a stack of dominating nilness facts.
+	//
+	// By traversing the dom tree, we can pop facts off the stack as
+	// soon as we've visited a subtree.  Had we traversed the CFG,
+	// we would need to retain the set of facts for each block.
+	seen := make([]bool, len(fn.Blocks)) // seen[i] means visit should ignore block i
+	var visit func(b *ssa.BasicBlock, stack []fact)
+	visit = func(b *ssa.BasicBlock, stack []fact) {
+		if seen[b.Index] {
+			return
+		}
+		seen[b.Index] = true
+
+		// Report nil dereferences.
+		for _, instr := range b.Instrs {
+			switch instr := instr.(type) {
+			case ssa.CallInstruction:
+				notNil(stack, instr, instr.Common().Value,
+					instr.Common().Description())
+			case *ssa.FieldAddr:
+				notNil(stack, instr, instr.X, "field selection")
+			case *ssa.IndexAddr:
+				notNil(stack, instr, instr.X, "index operation")
+			case *ssa.MapUpdate:
+				notNil(stack, instr, instr.Map, "map update")
+			case *ssa.Slice:
+				// A nilcheck occurs in ptr[:] iff ptr is a pointer to an array.
+				if _, ok := instr.X.Type().Underlying().(*types.Pointer); ok {
+					notNil(stack, instr, instr.X, "slice operation")
+				}
+			case *ssa.Store:
+				notNil(stack, instr, instr.Addr, "store")
+			case *ssa.TypeAssert:
+				if !instr.CommaOk {
+					notNil(stack, instr, instr.X, "type assertion")
+				}
+			case *ssa.UnOp:
+				if instr.Op == token.MUL { // *X
+					notNil(stack, instr, instr.X, "load")
+				}
+			}
+		}
+
+		// Look for panics with nil value
+		for _, instr := range b.Instrs {
+			switch instr := instr.(type) {
+			case *ssa.Panic:
+				if nilnessOf(stack, instr.X) == isnil {
+					reportf("nilpanic", instr.Pos(), "panic with nil value")
+				}
+			}
+		}
+
+		// For nil comparison blocks, report an error if the condition
+		// is degenerate, and push a nilness fact on the stack when
+		// visiting its true and false successor blocks.
+		if binop, tsucc, fsucc := eq(b); binop != nil {
+			xnil := nilnessOf(stack, binop.X)
+			ynil := nilnessOf(stack, binop.Y)
+
+			if ynil != unknown && xnil != unknown && (xnil == isnil || ynil == isnil) {
+				// Degenerate condition:
+				// the nilness of both operands is known,
+				// and at least one of them is nil.
+				var adj string
+				if (xnil == ynil) == (binop.Op == token.EQL) {
+					adj = "tautological"
+				} else {
+					adj = "impossible"
+				}
+				reportf("cond", binop.Pos(), "%s condition: %s %s %s", adj, xnil, binop.Op, ynil)
+
+				// If tsucc's or fsucc's sole incoming edge is impossible,
+				// it is unreachable.  Prune traversal of it and
+				// all the blocks it dominates.
+				// (We could be more precise with full dataflow
+				// analysis of control-flow joins.)
+				var skip *ssa.BasicBlock
+				if xnil == ynil {
+					skip = fsucc
+				} else {
+					skip = tsucc
+				}
+				for _, d := range b.Dominees() {
+					if d == skip && len(d.Preds) == 1 {
+						continue
+					}
+					visit(d, stack)
+				}
+				return
+			}
+
+			// "if x == nil" or "if nil == y" condition; x, y are unknown.
+			if xnil == isnil || ynil == isnil {
+				var newFacts facts
+				if xnil == isnil {
+					// x is nil, y is unknown:
+					// t successor learns y is nil.
+					newFacts = expandFacts(fact{binop.Y, isnil})
+				} else {
+					// x is nil, y is unknown:
+					// t successor learns x is nil.
+					newFacts = expandFacts(fact{binop.X, isnil})
+				}
+
+				for _, d := range b.Dominees() {
+					// Successor blocks learn a fact
+					// only at non-critical edges.
+					// (We could do be more precise with full dataflow
+					// analysis of control-flow joins.)
+					s := stack
+					if len(d.Preds) == 1 {
+						if d == tsucc {
+							s = append(s, newFacts...)
+						} else if d == fsucc {
+							s = append(s, newFacts.negate()...)
+						}
+					}
+					visit(d, s)
+				}
+				return
+			}
+		}
+
+		for _, d := range b.Dominees() {
+			visit(d, stack)
+		}
+	}
+
+	// Visit the entry block.  No need to visit fn.Recover.
+	if fn.Blocks != nil {
+		visit(fn.Blocks[0], make([]fact, 0, 20)) // 20 is plenty
+	}
+}
+
+// A fact records that a block is dominated
+// by the condition v == nil or v != nil.
+type fact struct {
+	value   ssa.Value
+	nilness nilness
+}
+
+func (f fact) negate() fact { return fact{f.value, -f.nilness} }
+
+type nilness int
+
+const (
+	isnonnil         = -1
+	unknown  nilness = 0
+	isnil            = 1
+)
+
+var nilnessStrings = []string{"non-nil", "unknown", "nil"}
+
+func (n nilness) String() string { return nilnessStrings[n+1] }
+
+// nilnessOf reports whether v is definitely nil, definitely not nil,
+// or unknown given the dominating stack of facts.
+func nilnessOf(stack []fact, v ssa.Value) nilness {
+	switch v := v.(type) {
+	// unwrap ChangeInterface values recursively, to detect if underlying
+	// values have any facts recorded or are otherwise known with regard to nilness.
+	//
+	// This work must be in addition to expanding facts about
+	// ChangeInterfaces during inference/fact gathering because this covers
+	// cases where the nilness of a value is intrinsic, rather than based
+	// on inferred facts, such as a zero value interface variable. That
+	// said, this work alone would only inform us when facts are about
+	// underlying values, rather than outer values, when the analysis is
+	// transitive in both directions.
+	case *ssa.ChangeInterface:
+		if underlying := nilnessOf(stack, v.X); underlying != unknown {
+			return underlying
+		}
+	}
+
+	// Is value intrinsically nil or non-nil?
+	switch v := v.(type) {
+	case *ssa.Alloc,
+		*ssa.FieldAddr,
+		*ssa.FreeVar,
+		*ssa.Function,
+		*ssa.Global,
+		*ssa.IndexAddr,
+		*ssa.MakeChan,
+		*ssa.MakeClosure,
+		*ssa.MakeInterface,
+		*ssa.MakeMap,
+		*ssa.MakeSlice:
+		return isnonnil
+	case *ssa.Const:
+		if v.IsNil() {
+			return isnil
+		}
+		return isnonnil
+	}
+
+	// Search dominating control-flow facts.
+	for _, f := range stack {
+		if f.value == v {
+			return f.nilness
+		}
+	}
+	return unknown
+}
+
+// If b ends with an equality comparison, eq returns the operation and
+// its true (equal) and false (not equal) successors.
+func eq(b *ssa.BasicBlock) (op *ssa.BinOp, tsucc, fsucc *ssa.BasicBlock) {
+	if If, ok := b.Instrs[len(b.Instrs)-1].(*ssa.If); ok {
+		if binop, ok := If.Cond.(*ssa.BinOp); ok {
+			switch binop.Op {
+			case token.EQL:
+				return binop, b.Succs[0], b.Succs[1]
+			case token.NEQ:
+				return binop, b.Succs[1], b.Succs[0]
+			}
+		}
+	}
+	return nil, nil, nil
+}
+
+// expandFacts takes a single fact and returns the set of facts that can be
+// known about it or any of its related values. Some operations, like
+// ChangeInterface, have transitive nilness, such that if you know the
+// underlying value is nil, you also know the value itself is nil, and vice
+// versa. This operation allows callers to match on any of the related values
+// in analyzes, rather than just the one form of the value that happened to
+// appear in a comparison.
+//
+// This work must be in addition to unwrapping values within nilnessOf because
+// while this work helps give facts about transitively known values based on
+// inferred facts, the recursive check within nilnessOf covers cases where
+// nilness facts are intrinsic to the underlying value, such as a zero value
+// interface variables.
+//
+// ChangeInterface is the only expansion currently supported, but others, like
+// Slice, could be added. At this time, this tool does not check slice
+// operations in a way this expansion could help. See
+// https://play.golang.org/p/mGqXEp7w4fR for an example.
+func expandFacts(f fact) []fact {
+	ff := []fact{f}
+
+Loop:
+	for {
+		switch v := f.value.(type) {
+		case *ssa.ChangeInterface:
+			f = fact{v.X, f.nilness}
+			ff = append(ff, f)
+		default:
+			break Loop
+		}
+	}
+
+	return ff
+}
+
+type facts []fact
+
+func (ff facts) negate() facts {
+	nn := make([]fact, len(ff))
+	for i, f := range ff {
+		nn[i] = f.negate()
+	}
+	return nn
+}
diff --git a/pkg/testutils/lint/passes/nilness/nilness_test.go b/pkg/testutils/lint/passes/nilness/nilness_test.go
new file mode 100644
index 000000000000..761063a3d7a1
--- /dev/null
+++ b/pkg/testutils/lint/passes/nilness/nilness_test.go
@@ -0,0 +1,30 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Portions of this file are additionally subject to the following
+// license and copyright.
+//
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package nilness_test
+
+import (
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/nilness"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, nilness.Analyzer, "a")
+}
diff --git a/pkg/testutils/lint/passes/nilness/testdata/src/a/a.go b/pkg/testutils/lint/passes/nilness/testdata/src/a/a.go
new file mode 100644
index 000000000000..2af1336b0ccb
--- /dev/null
+++ b/pkg/testutils/lint/passes/nilness/testdata/src/a/a.go
@@ -0,0 +1,177 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Portions of this file are additionally subject to the following
+// license and copyright.
+//
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package a
+
+type X struct{ f, g int }
+
+func f(x, y *X) {
+	if x == nil {
+		print(x.f) // want "nil dereference in field selection"
+	} else {
+		print(x.f)
+	}
+
+	if x == nil {
+		if nil != y {
+			print(1)
+			panic(0)
+		}
+		x.f = 1 // want "nil dereference in field selection"
+		y.f = 1 // want "nil dereference in field selection"
+	}
+
+	var f func()
+	if f == nil { // want "tautological condition: nil == nil"
+		go f() // want "nil dereference in dynamic function call"
+	} else {
+		// This block is unreachable,
+		// so we don't report an error for the
+		// nil dereference in the call.
+		defer f()
+	}
+}
+
+func f2(ptr *[3]int, i interface{}) {
+	if ptr != nil {
+		print(ptr[:])
+		*ptr = [3]int{}
+		print(*ptr)
+	} else {
+		print(ptr[:])   // want "nil dereference in slice operation"
+		*ptr = [3]int{} // want "nil dereference in store"
+		print(*ptr)     // want "nil dereference in load"
+
+		if ptr != nil { // want "impossible condition: nil != nil"
+			// Dominated by ptr==nil and ptr!=nil,
+			// this block is unreachable.
+			// We do not report errors within it.
+			print(*ptr)
+		}
+	}
+
+	if i != nil {
+		print(i.(interface{ f() }))
+	} else {
+		print(i.(interface{ f() })) // want "nil dereference in type assertion"
+	}
+}
+
+func g() error
+
+func f3() error {
+	err := g()
+	if err != nil {
+		return err
+	}
+	if err != nil && err.Error() == "foo" { // want "impossible condition: nil != nil"
+		print(0)
+	}
+	ch := make(chan int)
+	if ch == nil { // want "impossible condition: non-nil == nil"
+		print(0)
+	}
+	if ch != nil { // want "tautological condition: non-nil != nil"
+		print(0)
+	}
+	return nil
+}
+
+func h(err error, b bool) {
+	if err != nil && b {
+		return
+	} else if err != nil {
+		panic(err)
+	}
+}
+
+func i(*int) error {
+	for {
+		if err := g(); err != nil {
+			return err
+		}
+	}
+}
+
+func f4(x *X) {
+	if x == nil {
+		panic(x)
+	}
+}
+
+func f5(x *X) {
+	panic(nil) // want "panic with nil value"
+}
+
+func f6(x *X) {
+	var err error
+	panic(err) // want "panic with nil value"
+}
+
+func f7() {
+	x, err := bad()
+	if err != nil {
+		panic(0)
+	}
+	if x == nil {
+		panic(err) // want "panic with nil value"
+	}
+}
+
+func bad() (*X, error) {
+	return nil, nil
+}
+
+func f8() {
+	var e error
+	v, _ := e.(interface{})
+	print(v)
+}
+
+func f9(x interface {
+	a()
+	b()
+	c()
+}) {
+
+	x.b() // we don't catch this panic because we don't have any facts yet
+	xx := interface {
+		a()
+		b()
+	}(x)
+	if xx != nil {
+		return
+	}
+	x.c()  // want "nil dereference in dynamic method call"
+	xx.b() // want "nil dereference in dynamic method call"
+	xxx := interface{ a() }(xx)
+	xxx.a() // want "nil dereference in dynamic method call"
+
+	if unknown() {
+		panic(x) // want "panic with nil value"
+	}
+	if unknown() {
+		panic(xx) // want "panic with nil value"
+	}
+	if unknown() {
+		panic(xxx) // want "panic with nil value"
+	}
+}
+
+func unknown() bool {
+	return false
+}
diff --git a/vendor b/vendor
index 23f0fdc8662c..239554abe5b7 160000
--- a/vendor
+++ b/vendor
@@ -1 +1 @@
-Subproject commit 23f0fdc8662c5dc3fbb3a5c73bb7408bd71a7be6
+Subproject commit 239554abe5b72fedd92c2b3c6e37d512288efa8b

From daeaaf2b896097fc7551daaf686eae3f74d8d947 Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 10:22:43 +0000
Subject: [PATCH 2/6] testutils: check for nil arguments to error.With-style
 functions

These functions return nil if the first argument provided is nil.
While this behaviour is often useful, passing an always-nil value to
such a function is typically an indication of a bug.

Release note: None
---
 pkg/testutils/lint/passes/nilness/nilness.go | 42 +++++++++++++++++++-
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/pkg/testutils/lint/passes/nilness/nilness.go b/pkg/testutils/lint/passes/nilness/nilness.go
index e5234057a61e..8fe4a423eda9 100644
--- a/pkg/testutils/lint/passes/nilness/nilness.go
+++ b/pkg/testutils/lint/passes/nilness/nilness.go
@@ -99,6 +99,13 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
 		}
 	}
 
+	// notNilArg reports an error if v is provably nil.
+	notNilArg := func(stack []fact, instr ssa.Instruction, v ssa.Value, descr string) {
+		if nilnessOf(stack, v) == isnil {
+			reportf("nilarg", instr.Pos(), "nil argument to "+descr)
+		}
+	}
+
 	// visit visits reachable blocks of the CFG in dominance order,
 	// maintaining a stack of dominating nilness facts.
 	//
@@ -117,8 +124,17 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
 		for _, instr := range b.Instrs {
 			switch instr := instr.(type) {
 			case ssa.CallInstruction:
-				notNil(stack, instr, instr.Common().Value,
-					instr.Common().Description())
+				call := instr.Common()
+				notNil(stack, instr, call.Value, call.Description())
+
+				switch f := call.Value.(type) {
+				case *ssa.Function:
+					if tf, ok := f.Object().(*types.Func); ok {
+						if extract, ok := argExtractors[tf.FullName()]; ok {
+							notNilArg(stack, instr, extract(call), tf.FullName())
+						}
+					}
+				}
 			case *ssa.FieldAddr:
 				notNil(stack, instr, instr.X, "field selection")
 			case *ssa.IndexAddr:
@@ -366,3 +382,25 @@ func (ff facts) negate() facts {
 	}
 	return nn
 }
+
+var firstArg = func(c *ssa.CallCommon) ssa.Value { return c.Args[0] }
+
+var argExtractors = map[string]func(c *ssa.CallCommon) ssa.Value{
+	"github.com/cockroachdb/errors.Handled":                          firstArg,
+	"github.com/cockroachdb/errors.HandledWithMessage":               firstArg,
+	"github.com/cockroachdb/errors.NewAssertionErrorWithWrappedErrf": firstArg,
+	"github.com/cockroachdb/errors.Mark":                             firstArg,
+	"github.com/cockroachdb/errors.WithContextTags":                  firstArg,
+	"github.com/cockroachdb/errors.WithDetail":                       firstArg,
+	"github.com/cockroachdb/errors.WithDetailf":                      firstArg,
+	"github.com/cockroachdb/errors.WithHint":                         firstArg,
+	"github.com/cockroachdb/errors.WithHintf":                        firstArg,
+	"github.com/cockroachdb/errors.WithMessage":                      firstArg,
+	"github.com/cockroachdb/errors.WithMessagef":                     firstArg,
+	"github.com/cockroachdb/errors.WithStack":                        firstArg,
+	"github.com/cockroachdb/errors.WithStackDepth":                   firstArg,
+	"github.com/cockroachdb/errors.Wrap":                             firstArg,
+	"github.com/cockroachdb/errors.Wrapf":                            firstArg,
+	"github.com/cockroachdb/errors.WrapWithDepth":                    firstArg,
+	"github.com/cockroachdb/errors.WrapWithDepthf":                   firstArg,
+}

From 7faefb883bbcad225ee7bcc89f519e182fa28d6b Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 12:25:35 +0000
Subject: [PATCH 3/6] testutils: disable "degenerate if" reporting in nilness
 pass

The "degenerate" cases that the nilness pass found covered a number of
cases where the "tautological" check made the code a bit more readable
or cases where a nil check might be tautological now, but could easily
change in the future.

Release note: None
---
 pkg/testutils/lint/passes/nilness/nilness.go | 97 +++++++++++++-------
 1 file changed, 66 insertions(+), 31 deletions(-)

diff --git a/pkg/testutils/lint/passes/nilness/nilness.go b/pkg/testutils/lint/passes/nilness/nilness.go
index 8fe4a423eda9..3dafd905489f 100644
--- a/pkg/testutils/lint/passes/nilness/nilness.go
+++ b/pkg/testutils/lint/passes/nilness/nilness.go
@@ -66,24 +66,77 @@ and:
 	}
 `
 
+type argExtractor func(c *ssa.CallCommon) ssa.Value
+
+type analyzerConfig struct {
+	// reportDegenerateConditions controls the reporting of "impossible"
+	// and "tautological" comparisons. While many of these are in error,
+	// it also catches nil checks where the current control flow graph
+	// can't produce nil but where we might want to guard against changes
+	// in the future. It also catches a number of compound conditionals
+	// where the "tautological" check produces more readable code.
+	reportDegenerateIfConditions bool
+	// argExtractors is a map from the full name of a function to
+	// a func that returns the argument from a Call that should
+	// not be passed provably nil values.
+	argExtractors map[string]argExtractor
+}
+
+var crdbConfig = analyzerConfig{
+	reportDegenerateIfConditions: false,
+	argExtractors: map[string]argExtractor{
+		"github.com/cockroachdb/errors.Handled":                          firstArg,
+		"github.com/cockroachdb/errors.HandledWithMessage":               firstArg,
+		"github.com/cockroachdb/errors.NewAssertionErrorWithWrappedErrf": firstArg,
+		"github.com/cockroachdb/errors.Mark":                             firstArg,
+		"github.com/cockroachdb/errors.WithContextTags":                  firstArg,
+		"github.com/cockroachdb/errors.WithDetail":                       firstArg,
+		"github.com/cockroachdb/errors.WithDetailf":                      firstArg,
+		"github.com/cockroachdb/errors.WithHint":                         firstArg,
+		"github.com/cockroachdb/errors.WithHintf":                        firstArg,
+		"github.com/cockroachdb/errors.WithMessage":                      firstArg,
+		"github.com/cockroachdb/errors.WithMessagef":                     firstArg,
+		"github.com/cockroachdb/errors.WithStack":                        firstArg,
+		"github.com/cockroachdb/errors.WithStackDepth":                   firstArg,
+		"github.com/cockroachdb/errors.Wrap":                             firstArg,
+		"github.com/cockroachdb/errors.Wrapf":                            firstArg,
+		"github.com/cockroachdb/errors.WrapWithDepth":                    firstArg,
+		"github.com/cockroachdb/errors.WrapWithDepthf":                   firstArg,
+	},
+}
+
+var defaultConfig = analyzerConfig{
+	reportDegenerateIfConditions: true,
+}
+
+// CRDBAnalyzer defines a pass that checks for uses of provably nil
+// values that were likely in error with custom configuruation
+// suitable for the CRDB codebase.
+var CRDBAnalyzer = &analysis.Analyzer{
+	Name:     "nilness",
+	Doc:      doc,
+	Run:      crdbConfig.run,
+	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+}
+
 // Analyzer defines a pass that checks for uses of provably nil values
 // that were likely in error.
 var Analyzer = &analysis.Analyzer{
 	Name:     "nilness",
 	Doc:      doc,
-	Run:      run,
+	Run:      defaultConfig.run,
 	Requires: []*analysis.Analyzer{buildssa.Analyzer},
 }
 
-func run(pass *analysis.Pass) (interface{}, error) {
+func (a *analyzerConfig) run(pass *analysis.Pass) (interface{}, error) {
 	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 	for _, fn := range ssainput.SrcFuncs {
-		runFunc(pass, fn)
+		a.runFunc(pass, fn)
 	}
 	return nil, nil
 }
 
-func runFunc(pass *analysis.Pass, fn *ssa.Function) {
+func (a *analyzerConfig) runFunc(pass *analysis.Pass, fn *ssa.Function) {
 	reportf := func(category string, pos token.Pos, format string, args ...interface{}) {
 		pass.Report(analysis.Diagnostic{
 			Pos:      pos,
@@ -130,7 +183,7 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
 				switch f := call.Value.(type) {
 				case *ssa.Function:
 					if tf, ok := f.Object().(*types.Func); ok {
-						if extract, ok := argExtractors[tf.FullName()]; ok {
+						if extract, ok := a.argExtractors[tf.FullName()]; ok {
 							notNilArg(stack, instr, extract(call), tf.FullName())
 						}
 					}
@@ -180,13 +233,15 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
 				// Degenerate condition:
 				// the nilness of both operands is known,
 				// and at least one of them is nil.
-				var adj string
-				if (xnil == ynil) == (binop.Op == token.EQL) {
-					adj = "tautological"
-				} else {
-					adj = "impossible"
+				if a.reportDegenerateIfConditions {
+					var adj string
+					if (xnil == ynil) == (binop.Op == token.EQL) {
+						adj = "tautological"
+					} else {
+						adj = "impossible"
+					}
+					reportf("cond", binop.Pos(), "%s condition: %s %s %s", adj, xnil, binop.Op, ynil)
 				}
-				reportf("cond", binop.Pos(), "%s condition: %s %s %s", adj, xnil, binop.Op, ynil)
 
 				// If tsucc's or fsucc's sole incoming edge is impossible,
 				// it is unreachable.  Prune traversal of it and
@@ -384,23 +439,3 @@ func (ff facts) negate() facts {
 }
 
 var firstArg = func(c *ssa.CallCommon) ssa.Value { return c.Args[0] }
-
-var argExtractors = map[string]func(c *ssa.CallCommon) ssa.Value{
-	"github.com/cockroachdb/errors.Handled":                          firstArg,
-	"github.com/cockroachdb/errors.HandledWithMessage":               firstArg,
-	"github.com/cockroachdb/errors.NewAssertionErrorWithWrappedErrf": firstArg,
-	"github.com/cockroachdb/errors.Mark":                             firstArg,
-	"github.com/cockroachdb/errors.WithContextTags":                  firstArg,
-	"github.com/cockroachdb/errors.WithDetail":                       firstArg,
-	"github.com/cockroachdb/errors.WithDetailf":                      firstArg,
-	"github.com/cockroachdb/errors.WithHint":                         firstArg,
-	"github.com/cockroachdb/errors.WithHintf":                        firstArg,
-	"github.com/cockroachdb/errors.WithMessage":                      firstArg,
-	"github.com/cockroachdb/errors.WithMessagef":                     firstArg,
-	"github.com/cockroachdb/errors.WithStack":                        firstArg,
-	"github.com/cockroachdb/errors.WithStackDepth":                   firstArg,
-	"github.com/cockroachdb/errors.Wrap":                             firstArg,
-	"github.com/cockroachdb/errors.Wrapf":                            firstArg,
-	"github.com/cockroachdb/errors.WrapWithDepth":                    firstArg,
-	"github.com/cockroachdb/errors.WrapWithDepthf":                   firstArg,
-}

From 8ddf70e0aa0679dda3f1402bc4e2e6f566cdc6fd Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 11:03:20 +0000
Subject: [PATCH 4/6] jobs,storage: don't call errors.Wrap-style functions with
 nil errors

These calls to errors.Wrap-style functions were passing errors that
would always be nil, which means that they would be returning a nil
error rather than the non-nil error that the author likely expected.

Release note: None
---
 pkg/jobs/job_scheduler.go                 | 4 ++--
 pkg/jobs/registry.go                      | 5 ++---
 pkg/storage/cloudimpl/workload_storage.go | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/pkg/jobs/job_scheduler.go b/pkg/jobs/job_scheduler.go
index c60b9b7c7b98..eee7f9dc2f03 100644
--- a/pkg/jobs/job_scheduler.go
+++ b/pkg/jobs/job_scheduler.go
@@ -88,7 +88,7 @@ SELECT
 FROM %s S
 WHERE next_run < %s
 ORDER BY random()
-%s 
+%s
 FOR UPDATE`, env.SystemJobsTableName(), CreatedByScheduledJobs,
 		StatusSucceeded, StatusCanceled, StatusFailed,
 		env.ScheduledJobsTableName(), env.NowExpr(), limitClause)
@@ -313,7 +313,7 @@ func (s *jobScheduler) executeSchedules(
 			return s.processSchedule(ctx, schedule, numRunning, stats, txn)
 		}); processErr != nil {
 			if errors.HasType(processErr, (*savePointError)(nil)) {
-				return errors.Wrapf(err, "savepoint error for schedule %d", schedule.ScheduleID())
+				return errors.Wrapf(processErr, "savepoint error for schedule %d", schedule.ScheduleID())
 			}
 
 			// Failed to process schedule.
diff --git a/pkg/jobs/registry.go b/pkg/jobs/registry.go
index 1e38a3fb3872..3b795d2b9cf9 100644
--- a/pkg/jobs/registry.go
+++ b/pkg/jobs/registry.go
@@ -1200,7 +1200,7 @@ func (r *Registry) stepThroughStateMachine(
 		jm.ResumeFailed.Inc(1)
 		if sErr := (*InvalidStatusError)(nil); errors.As(err, &sErr) {
 			if sErr.status != StatusCancelRequested && sErr.status != StatusPauseRequested {
-				return errors.NewAssertionErrorWithWrappedErrf(jobErr,
+				return errors.NewAssertionErrorWithWrappedErrf(sErr,
 					"job %d: unexpected status %s provided for a running job", job.ID(), sErr.status)
 			}
 			return sErr
@@ -1281,8 +1281,7 @@ func (r *Registry) stepThroughStateMachine(
 			errors.Wrapf(err, "job %d: cannot be reverted, manual cleanup may be required", job.ID()))
 	case StatusFailed:
 		if jobErr == nil {
-			return errors.NewAssertionErrorWithWrappedErrf(jobErr,
-				"job %d: has StatusFailed but no error was provided", job.ID())
+			return errors.AssertionFailedf("job %d: has StatusFailed but no error was provided", job.ID())
 		}
 		if err := job.failed(ctx, nil /* txn */, jobErr, nil /* fn */); err != nil {
 			// If we can't transactionally mark the job as failed then it will be
diff --git a/pkg/storage/cloudimpl/workload_storage.go b/pkg/storage/cloudimpl/workload_storage.go
index 13a481e26fa0..15865f0dbba6 100644
--- a/pkg/storage/cloudimpl/workload_storage.go
+++ b/pkg/storage/cloudimpl/workload_storage.go
@@ -76,7 +76,7 @@ func makeWorkloadStorage(
 		}
 	}
 	if s.table.Name == `` {
-		return nil, errors.Wrapf(err, `unknown table %s for generator %s`, conf.Table, meta.Name)
+		return nil, errors.Errorf(`unknown table %s for generator %s`, conf.Table, meta.Name)
 	}
 	return s, nil
 }

From cf1672f6e7731d5948e38fa968c0fef1e17ef2e0 Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 12:22:25 +0000
Subject: [PATCH 5/6] multiple: fix uses of provably nil errors

These cases were found by the nilness linter. Most appear to be err
checks that were simply copy and pasted but were no longer needed.

Release note: None
---
 pkg/cmd/roachtest/cluster.go      |  2 +-
 pkg/cmd/smithtest/main.go         |  3 ---
 pkg/geo/geomfn/linestring.go      |  4 ----
 pkg/server/init.go                | 12 +++++-------
 pkg/server/init_handshake.go      | 10 ++++------
 pkg/sql/alter_sequence.go         |  3 ---
 pkg/sql/alter_table.go            |  3 ---
 pkg/sql/opt/optbuilder/orderby.go |  4 ----
 pkg/sql/pgwire/conn.go            |  3 ++-
 pkg/sql/sem/tree/create.go        |  4 ----
 pkg/util/encoding/encoding.go     |  3 ---
 pkg/util/json/encoded.go          |  8 ++++----
 12 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go
index b0de67d5be4d..af347bf3bb85 100644
--- a/pkg/cmd/roachtest/cluster.go
+++ b/pkg/cmd/roachtest/cluster.go
@@ -1698,7 +1698,7 @@ WHERE t.status NOT IN ('RANGE_CONSISTENT', 'RANGE_INDETERMINATE')`)
 	for rows.Next() {
 		var rangeID int32
 		var prettyKey, status, detail string
-		if scanErr := rows.Scan(&rangeID, &prettyKey, &status, &detail); err != nil {
+		if scanErr := rows.Scan(&rangeID, &prettyKey, &status, &detail); scanErr != nil {
 			return scanErr
 		}
 		finalErr = errors.CombineErrors(finalErr,
diff --git a/pkg/cmd/smithtest/main.go b/pkg/cmd/smithtest/main.go
index fedaa0686147..3d7f2d3754e9 100644
--- a/pkg/cmd/smithtest/main.go
+++ b/pkg/cmd/smithtest/main.go
@@ -188,9 +188,6 @@ func (s WorkerSetup) run(ctx context.Context, rnd *rand.Rand) error {
 				return errors.Wrap(err, "connector error")
 			}
 			db = gosql.OpenDB(connector)
-			if err != nil {
-				return errors.Wrap(err, "connect")
-			}
 			fmt.Println("connected to", match[1])
 			break
 		}
diff --git a/pkg/geo/geomfn/linestring.go b/pkg/geo/geomfn/linestring.go
index 526ca72ec9d3..4ce9d12d8366 100644
--- a/pkg/geo/geomfn/linestring.go
+++ b/pkg/geo/geomfn/linestring.go
@@ -91,10 +91,6 @@ func LineLocatePoint(line geo.Geometry, point geo.Geometry) (float64, error) {
 	}
 
 	p := closestT.(*geom.Point)
-	if err != nil {
-		return 0, err
-	}
-
 	// build new line segment to the closest point we found
 	lineSegment := geom.NewLineString(geom.XY).MustSetCoords([]geom.Coord{lineString.Coord(0), p.Coords()})
 
diff --git a/pkg/server/init.go b/pkg/server/init.go
index 777fffcbb217..b41f67096e74 100644
--- a/pkg/server/init.go
+++ b/pkg/server/init.go
@@ -272,13 +272,11 @@ func (s *initServer) ServeAndWait(
 					return nil, false, err
 				}
 
-				if err != nil {
-					// We expect the join RPC to blindly retry on all
-					// "connection" errors save for one above. If we're
-					// here, we failed to initialize our first store after a
-					// successful join attempt.
-					return nil, false, errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error: %v", err)
-				}
+				// We expect the join RPC to blindly retry on all
+				// "connection" errors save for one above. If we're
+				// here, we failed to initialize our first store after a
+				// successful join attempt.
+				return nil, false, errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error: %v", err)
 			}
 
 			state := result.state
diff --git a/pkg/server/init_handshake.go b/pkg/server/init_handshake.go
index 5642ee29ba2e..7919cd823d0f 100644
--- a/pkg/server/init_handshake.go
+++ b/pkg/server/init_handshake.go
@@ -348,15 +348,13 @@ func (t *tlsInitHandshaker) getPeerCACert(
 		return nodeHostnameAndCA{}, err
 	}
 
-	// Read and validate server provided ack.
-	// HMAC(hostname + server CA public certificate, secretToken)
-	var msg nodeHostnameAndCA
-	if err != nil {
-		return nodeHostnameAndCA{}, err
-	}
 	if res.StatusCode != 200 {
 		return nodeHostnameAndCA{}, errors.Errorf("unexpected error returned from peer: HTTP %d", res.StatusCode)
 	}
+
+	// Read and validate server provided ack.
+	// HMAC(hostname + server CA public certificate, secretToken)
+	var msg nodeHostnameAndCA
 	if err := json.NewDecoder(res.Body).Decode(&msg); err != nil {
 		return nodeHostnameAndCA{}, err
 	}
diff --git a/pkg/sql/alter_sequence.go b/pkg/sql/alter_sequence.go
index cdf80c11ea48..e3e55a33d3ad 100644
--- a/pkg/sql/alter_sequence.go
+++ b/pkg/sql/alter_sequence.go
@@ -74,9 +74,6 @@ func (n *alterSequenceNode) startExec(params runParams) error {
 	}
 	opts := desc.SequenceOpts
 	seqValueKey := params.p.ExecCfg().Codec.SequenceKey(uint32(desc.ID))
-	if err != nil {
-		return err
-	}
 
 	getSequenceValue := func() (int64, error) {
 		kv, err := params.p.txn.Get(params.ctx, seqValueKey)
diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
index db419db4419e..543058dc8b8e 100644
--- a/pkg/sql/alter_table.go
+++ b/pkg/sql/alter_table.go
@@ -620,9 +620,6 @@ func (n *alterTableNode) startExec(params runParams) error {
 				descriptorChanged = true
 			}
 
-			if err != nil {
-				return err
-			}
 			if err := params.p.removeColumnComment(params.ctx, n.tableDesc.ID, colToDrop.GetID()); err != nil {
 				return err
 			}
diff --git a/pkg/sql/opt/optbuilder/orderby.go b/pkg/sql/opt/optbuilder/orderby.go
index 2bb7a12ea9fa..1010897959eb 100644
--- a/pkg/sql/opt/optbuilder/orderby.go
+++ b/pkg/sql/opt/optbuilder/orderby.go
@@ -133,10 +133,6 @@ func (b *Builder) analyzeOrderByIndex(
 	for i, n := 0, index.KeyColumnCount(); i < n; i++ {
 		// Columns which are indexable are always orderable.
 		col := index.Column(i)
-		if err != nil {
-			panic(err)
-		}
-
 		desc := col.Descending
 
 		// DESC inverts the order of the index.
diff --git a/pkg/sql/pgwire/conn.go b/pkg/sql/pgwire/conn.go
index 0bc2d7349088..969d51a57f49 100644
--- a/pkg/sql/pgwire/conn.go
+++ b/pkg/sql/pgwire/conn.go
@@ -288,7 +288,8 @@ func (c *conn) serveImpl(
 		// we only need the minimum to make pgx happy.
 		var err error
 		for param, value := range testingStatusReportParams {
-			if err := c.sendParamStatus(param, value); err != nil {
+			err = c.sendParamStatus(param, value)
+			if err != nil {
 				break
 			}
 		}
diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go
index 6df2e1aa909f..0eeaea135bf9 100644
--- a/pkg/sql/sem/tree/create.go
+++ b/pkg/sql/sem/tree/create.go
@@ -1656,10 +1656,6 @@ func (o KVOptions) ToRoleOptions(
 				}
 			} else {
 				strFn, err := typeAsStringOrNull(ro.Value, op)
-				if err != nil {
-					return nil, err
-				}
-
 				if err != nil {
 					return nil, err
 				}
diff --git a/pkg/util/encoding/encoding.go b/pkg/util/encoding/encoding.go
index e3b6c60bbaaa..1840db456d71 100644
--- a/pkg/util/encoding/encoding.go
+++ b/pkg/util/encoding/encoding.go
@@ -1196,9 +1196,6 @@ func EncodeGeoDescending(b []byte, curveIndex uint64, so *geopb.SpatialObject) (
 	}
 	n := len(b)
 	b = encodeBytesAscendingWithTerminator(b, data, ascendingGeoEscapes.escapedTerm)
-	if err != nil {
-		return nil, err
-	}
 	onesComplement(b[n:])
 	return b, nil
 }
diff --git a/pkg/util/json/encoded.go b/pkg/util/json/encoded.go
index 5b65cce21eae..e9f316bb784a 100644
--- a/pkg/util/json/encoded.go
+++ b/pkg/util/json/encoded.go
@@ -428,18 +428,18 @@ func (j *jsonEncoded) FetchValKey(key string) (JSON, error) {
 		// or maybe there's something fancier we could do if we know the locations
 		// of the offsets by strategically positioning our binary search guesses to
 		// land on them.
-		var err error
+		var searchErr error
 		i := sort.Search(j.containerLen, func(idx int) bool {
 			data, _, err := j.objectGetNthDataRange(idx)
 			if err != nil {
+				searchErr = err
 				return false
 			}
 			return string(data) >= key
 		})
-		if err != nil {
-			return nil, err
+		if searchErr != nil {
+			return nil, searchErr
 		}
-
 		// The sort.Search API implies that we have to double-check if the key we
 		// landed on is the one we were searching for in the first place.
 		if i >= j.containerLen {

From 84fdb06ef8b16777cfd3e1c73fc330ad4a1ea2c6 Mon Sep 17 00:00:00 2001
From: Steven Danna <danna@cockroachlabs.com>
Date: Mon, 8 Mar 2021 12:32:44 +0000
Subject: [PATCH 6/6] roachvet: add nilness linter

The nilness linter checks for errant uses of provably nil values. The
most useful of these checks so far as been its ability to find
errors.Wrapf-style functions that are always being passed nil errors.

Release note: None
---
 pkg/cmd/roachvet/BUILD.bazel    | 1 +
 pkg/cmd/roachvet/main.go        | 2 ++
 pkg/testutils/lint/lint_test.go | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/pkg/cmd/roachvet/BUILD.bazel b/pkg/cmd/roachvet/BUILD.bazel
index 17d9f8057fc3..8936679e1f70 100644
--- a/pkg/cmd/roachvet/BUILD.bazel
+++ b/pkg/cmd/roachvet/BUILD.bazel
@@ -11,6 +11,7 @@ go_library(
         "//pkg/testutils/lint/passes/forbiddenmethod",
         "//pkg/testutils/lint/passes/hash",
         "//pkg/testutils/lint/passes/leaktestcall",
+        "//pkg/testutils/lint/passes/nilness",
         "//pkg/testutils/lint/passes/nocopy",
         "//pkg/testutils/lint/passes/returnerrcheck",
         "//pkg/testutils/lint/passes/timer",
diff --git a/pkg/cmd/roachvet/main.go b/pkg/cmd/roachvet/main.go
index a7e7b96ada17..3030a6b5800a 100644
--- a/pkg/cmd/roachvet/main.go
+++ b/pkg/cmd/roachvet/main.go
@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/forbiddenmethod"
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/hash"
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/leaktestcall"
+	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/nilness"
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/nocopy"
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/returnerrcheck"
 	"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/timer"
@@ -63,6 +64,7 @@ func main() {
 		unconvert.Analyzer,
 		fmtsafe.Analyzer,
 		errcmp.Analyzer,
+		nilness.CRDBAnalyzer,
 	)
 
 	// Standard go vet analyzers:
diff --git a/pkg/testutils/lint/lint_test.go b/pkg/testutils/lint/lint_test.go
index c966875c9ccd..8b4fd2016d05 100644
--- a/pkg/testutils/lint/lint_test.go
+++ b/pkg/testutils/lint/lint_test.go
@@ -2048,6 +2048,8 @@ func TestLint(t *testing.T) {
 			// roachtest is not collecting redactable logs so we don't care
 			// about printf hygiene there as much.
 			stream.GrepNot(`pkg/cmd/roachtest/log\.go:.*format argument is not a constant expression`),
+			// We purposefully produce nil dereferences in this file to test crash conditions
+			stream.GrepNot(`pkg/util/log/logcrash/crash_reporting_test\.go:.*nil dereference in type assertion`),
 		}
 
 		const vetTool = "roachvet"