| Tim Windelschmidt | 1f4590b | 2025-07-29 23:05:36 +0200 | [diff] [blame^] | 1 | // Copyright 2018 The Go Authors. All rights reserved. |
| 2 | // Use of this source code is governed by a BSD-style |
| 3 | // license that can be found in the LICENSE file. |
| 4 | |
| 5 | // Portions of this file are additionally subject to the following |
| 6 | // license and copyright. |
| 7 | // |
| 8 | // Copyright 2021 The Cockroach Authors. |
| 9 | // SPDX-License-Identifier: Apache-2.0 |
| 10 | |
| 11 | // Package nilness inspects the control-flow graph of an SSA function |
| 12 | // and reports errors such as nil pointer dereferences and degenerate |
| 13 | // nil pointer comparisons. |
| 14 | package nilness |
| 15 | |
| 16 | import ( |
| 17 | "fmt" |
| 18 | "go/token" |
| 19 | "go/types" |
| 20 | |
| 21 | "golang.org/x/tools/go/analysis" |
| 22 | "golang.org/x/tools/go/analysis/passes/buildssa" |
| 23 | "golang.org/x/tools/go/ssa" |
| 24 | ) |
| 25 | |
| 26 | const doc = `check for redundant or impossible nil comparisons |
| 27 | |
| 28 | The nilness checker inspects the control-flow graph of each function in |
| 29 | a package and reports nil pointer dereferences, degenerate nil |
| 30 | pointers, and panics with nil values. A degenerate comparison is of the form |
| 31 | x==nil or x!=nil where x is statically known to be nil or non-nil. These are |
| 32 | often a mistake, especially in control flow related to errors. Panics with nil |
| 33 | values are checked because they are not detectable by |
| 34 | |
| 35 | if r := recover(); r != nil { |
| 36 | |
| 37 | This check reports conditions such as: |
| 38 | |
| 39 | if f == nil { // impossible condition (f is a function) |
| 40 | } |
| 41 | |
| 42 | and: |
| 43 | |
| 44 | p := &v |
| 45 | ... |
| 46 | if p != nil { // tautological condition |
| 47 | } |
| 48 | |
| 49 | and: |
| 50 | |
| 51 | if p == nil { |
| 52 | print(*p) // nil dereference |
| 53 | } |
| 54 | |
| 55 | and: |
| 56 | |
| 57 | if p == nil { |
| 58 | panic(p) |
| 59 | } |
| 60 | ` |
| 61 | |
| 62 | type argExtractor func(c *ssa.CallCommon) ssa.Value |
| 63 | |
| 64 | type analyzerConfig struct { |
| 65 | // reportDegenerateConditions controls the reporting of "impossible" |
| 66 | // and "tautological" comparisons. While many of these are in error, |
| 67 | // it also catches nil checks where the current control flow graph |
| 68 | // can't produce nil but where we might want to guard against changes |
| 69 | // in the future. It also catches a number of compound conditionals |
| 70 | // where the "tautological" check produces more readable code. |
| 71 | reportDegenerateIfConditions bool |
| 72 | // argExtractors is a map from the full name of a function to |
| 73 | // a func that returns the argument from a Call that should |
| 74 | // not be passed provably nil values. |
| 75 | argExtractors map[string]argExtractor |
| 76 | } |
| 77 | |
| 78 | var crdbConfig = analyzerConfig{ |
| 79 | reportDegenerateIfConditions: false, |
| 80 | argExtractors: map[string]argExtractor{}, |
| 81 | } |
| 82 | |
| 83 | var defaultConfig = analyzerConfig{ |
| 84 | reportDegenerateIfConditions: true, |
| 85 | } |
| 86 | |
| 87 | // Analyzer defines a pass that checks for uses of provably nil |
| 88 | // values that were likely in error with custom configuration |
| 89 | // suitable for the CRDB codebase. |
| 90 | var Analyzer = &analysis.Analyzer{ |
| 91 | Name: "nilness", |
| 92 | Doc: doc, |
| 93 | Run: crdbConfig.run, |
| 94 | Requires: []*analysis.Analyzer{buildssa.Analyzer}, |
| 95 | } |
| 96 | |
| 97 | // TestAnalyzer defines a pass that checks for uses of provably nil values |
| 98 | // that were likely in error. |
| 99 | var TestAnalyzer = &analysis.Analyzer{ |
| 100 | Name: "nilness", |
| 101 | Doc: doc, |
| 102 | Run: defaultConfig.run, |
| 103 | Requires: []*analysis.Analyzer{buildssa.Analyzer}, |
| 104 | } |
| 105 | |
| 106 | func (a *analyzerConfig) run(pass *analysis.Pass) (interface{}, error) { |
| 107 | ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) |
| 108 | for _, fn := range ssainput.SrcFuncs { |
| 109 | a.runFunc(pass, fn) |
| 110 | } |
| 111 | return nil, nil |
| 112 | } |
| 113 | |
| 114 | func (a *analyzerConfig) runFunc(pass *analysis.Pass, fn *ssa.Function) { |
| 115 | reportf := func(category string, pos token.Pos, format string, args ...interface{}) { |
| 116 | pass.Report(analysis.Diagnostic{ |
| 117 | Pos: pos, |
| 118 | Category: category, |
| 119 | Message: fmt.Sprintf(format, args...), |
| 120 | }) |
| 121 | } |
| 122 | |
| 123 | // notNil reports an error if v is provably nil. |
| 124 | notNil := func(stack []fact, instr ssa.Instruction, v ssa.Value, descr string) { |
| 125 | if nilnessOf(stack, v) == isnil { |
| 126 | reportf("nilderef", instr.Pos(), "nil dereference in "+descr) |
| 127 | } |
| 128 | } |
| 129 | |
| 130 | // notNilArg reports an error if v is provably nil. |
| 131 | notNilArg := func(stack []fact, instr ssa.Instruction, v ssa.Value, descr string) { |
| 132 | if nilnessOf(stack, v) == isnil { |
| 133 | reportf("nilarg", instr.Pos(), "nil argument to "+descr) |
| 134 | } |
| 135 | } |
| 136 | |
| 137 | // visit visits reachable blocks of the CFG in dominance order, |
| 138 | // maintaining a stack of dominating nilness facts. |
| 139 | // |
| 140 | // By traversing the dom tree, we can pop facts off the stack as |
| 141 | // soon as we've visited a subtree. Had we traversed the CFG, |
| 142 | // we would need to retain the set of facts for each block. |
| 143 | seen := make([]bool, len(fn.Blocks)) // seen[i] means visit should ignore block i |
| 144 | var visit func(b *ssa.BasicBlock, stack []fact) |
| 145 | visit = func(b *ssa.BasicBlock, stack []fact) { |
| 146 | if seen[b.Index] { |
| 147 | return |
| 148 | } |
| 149 | seen[b.Index] = true |
| 150 | |
| 151 | // Report nil dereferences. |
| 152 | for _, instr := range b.Instrs { |
| 153 | switch instr := instr.(type) { |
| 154 | case ssa.CallInstruction: |
| 155 | call := instr.Common() |
| 156 | notNil(stack, instr, call.Value, call.Description()) |
| 157 | |
| 158 | switch f := call.Value.(type) { |
| 159 | case *ssa.Function: |
| 160 | if tf, ok := f.Object().(*types.Func); ok { |
| 161 | if extract, ok := a.argExtractors[tf.FullName()]; ok { |
| 162 | notNilArg(stack, instr, extract(call), tf.FullName()) |
| 163 | } |
| 164 | } |
| 165 | } |
| 166 | case *ssa.FieldAddr: |
| 167 | notNil(stack, instr, instr.X, "field selection") |
| 168 | case *ssa.IndexAddr: |
| 169 | notNil(stack, instr, instr.X, "index operation") |
| 170 | case *ssa.MapUpdate: |
| 171 | notNil(stack, instr, instr.Map, "map update") |
| 172 | case *ssa.Slice: |
| 173 | // A nilcheck occurs in ptr[:] iff ptr is a pointer to an array. |
| 174 | if _, ok := instr.X.Type().Underlying().(*types.Pointer); ok { |
| 175 | notNil(stack, instr, instr.X, "slice operation") |
| 176 | } |
| 177 | case *ssa.Store: |
| 178 | notNil(stack, instr, instr.Addr, "store") |
| 179 | case *ssa.TypeAssert: |
| 180 | if !instr.CommaOk { |
| 181 | notNil(stack, instr, instr.X, "type assertion") |
| 182 | } |
| 183 | case *ssa.UnOp: |
| 184 | if instr.Op == token.MUL { // *X |
| 185 | notNil(stack, instr, instr.X, "load") |
| 186 | } |
| 187 | } |
| 188 | } |
| 189 | |
| 190 | // Look for panics with nil value |
| 191 | for _, instr := range b.Instrs { |
| 192 | switch instr := instr.(type) { |
| 193 | case *ssa.Panic: |
| 194 | if nilnessOf(stack, instr.X) == isnil { |
| 195 | reportf("nilpanic", instr.Pos(), "panic with nil value") |
| 196 | } |
| 197 | } |
| 198 | } |
| 199 | |
| 200 | // For nil comparison blocks, report an error if the condition |
| 201 | // is degenerate, and push a nilness fact on the stack when |
| 202 | // visiting its true and false successor blocks. |
| 203 | if binop, tsucc, fsucc := eq(b); binop != nil { |
| 204 | xnil := nilnessOf(stack, binop.X) |
| 205 | ynil := nilnessOf(stack, binop.Y) |
| 206 | |
| 207 | if ynil != unknown && xnil != unknown && (xnil == isnil || ynil == isnil) { |
| 208 | // Degenerate condition: |
| 209 | // the nilness of both operands is known, |
| 210 | // and at least one of them is nil. |
| 211 | if a.reportDegenerateIfConditions { |
| 212 | var adj string |
| 213 | if (xnil == ynil) == (binop.Op == token.EQL) { |
| 214 | adj = "tautological" |
| 215 | } else { |
| 216 | adj = "impossible" |
| 217 | } |
| 218 | reportf("cond", binop.Pos(), "%s condition: %s %s %s", adj, xnil, binop.Op, ynil) |
| 219 | } |
| 220 | |
| 221 | // If tsucc's or fsucc's sole incoming edge is impossible, |
| 222 | // it is unreachable. Prune traversal of it and |
| 223 | // all the blocks it dominates. |
| 224 | // (We could be more precise with full dataflow |
| 225 | // analysis of control-flow joins.) |
| 226 | var skip *ssa.BasicBlock |
| 227 | if xnil == ynil { |
| 228 | skip = fsucc |
| 229 | } else { |
| 230 | skip = tsucc |
| 231 | } |
| 232 | for _, d := range b.Dominees() { |
| 233 | if d == skip && len(d.Preds) == 1 { |
| 234 | continue |
| 235 | } |
| 236 | visit(d, stack) |
| 237 | } |
| 238 | return |
| 239 | } |
| 240 | |
| 241 | // "if x == nil" or "if nil == y" condition; x, y are unknown. |
| 242 | if xnil == isnil || ynil == isnil { |
| 243 | var newFacts facts |
| 244 | if xnil == isnil { |
| 245 | // x is nil, y is unknown: |
| 246 | // t successor learns y is nil. |
| 247 | newFacts = expandFacts(fact{binop.Y, isnil}) |
| 248 | } else { |
| 249 | // x is nil, y is unknown: |
| 250 | // t successor learns x is nil. |
| 251 | newFacts = expandFacts(fact{binop.X, isnil}) |
| 252 | } |
| 253 | |
| 254 | for _, d := range b.Dominees() { |
| 255 | // Successor blocks learn a fact |
| 256 | // only at non-critical edges. |
| 257 | // (We could do be more precise with full dataflow |
| 258 | // analysis of control-flow joins.) |
| 259 | s := stack |
| 260 | if len(d.Preds) == 1 { |
| 261 | if d == tsucc { |
| 262 | s = append(s, newFacts...) |
| 263 | } else if d == fsucc { |
| 264 | s = append(s, newFacts.negate()...) |
| 265 | } |
| 266 | } |
| 267 | visit(d, s) |
| 268 | } |
| 269 | return |
| 270 | } |
| 271 | } |
| 272 | |
| 273 | for _, d := range b.Dominees() { |
| 274 | visit(d, stack) |
| 275 | } |
| 276 | } |
| 277 | |
| 278 | // Visit the entry block. No need to visit fn.Recover. |
| 279 | if fn.Blocks != nil { |
| 280 | visit(fn.Blocks[0], make([]fact, 0, 20)) // 20 is plenty |
| 281 | } |
| 282 | } |
| 283 | |
| 284 | // A fact records that a block is dominated |
| 285 | // by the condition v == nil or v != nil. |
| 286 | type fact struct { |
| 287 | value ssa.Value |
| 288 | nilness nilness |
| 289 | } |
| 290 | |
| 291 | func (f fact) negate() fact { return fact{f.value, -f.nilness} } |
| 292 | |
| 293 | type nilness int |
| 294 | |
| 295 | const ( |
| 296 | isnonnil = -1 |
| 297 | unknown nilness = 0 |
| 298 | isnil = 1 |
| 299 | ) |
| 300 | |
| 301 | var nilnessStrings = []string{"non-nil", "unknown", "nil"} |
| 302 | |
| 303 | func (n nilness) String() string { return nilnessStrings[n+1] } |
| 304 | |
| 305 | // nilnessOf reports whether v is definitely nil, definitely not nil, |
| 306 | // or unknown given the dominating stack of facts. |
| 307 | func nilnessOf(stack []fact, v ssa.Value) nilness { |
| 308 | switch v := v.(type) { |
| 309 | // unwrap ChangeInterface values recursively, to detect if underlying |
| 310 | // values have any facts recorded or are otherwise known with regard to nilness. |
| 311 | // |
| 312 | // This work must be in addition to expanding facts about |
| 313 | // ChangeInterfaces during inference/fact gathering because this covers |
| 314 | // cases where the nilness of a value is intrinsic, rather than based |
| 315 | // on inferred facts, such as a zero value interface variable. That |
| 316 | // said, this work alone would only inform us when facts are about |
| 317 | // underlying values, rather than outer values, when the analysis is |
| 318 | // transitive in both directions. |
| 319 | case *ssa.ChangeInterface: |
| 320 | if underlying := nilnessOf(stack, v.X); underlying != unknown { |
| 321 | return underlying |
| 322 | } |
| 323 | } |
| 324 | |
| 325 | // Is value intrinsically nil or non-nil? |
| 326 | switch v := v.(type) { |
| 327 | case *ssa.Alloc, |
| 328 | *ssa.FieldAddr, |
| 329 | *ssa.FreeVar, |
| 330 | *ssa.Function, |
| 331 | *ssa.Global, |
| 332 | *ssa.IndexAddr, |
| 333 | *ssa.MakeChan, |
| 334 | *ssa.MakeClosure, |
| 335 | *ssa.MakeInterface, |
| 336 | *ssa.MakeMap, |
| 337 | *ssa.MakeSlice: |
| 338 | return isnonnil |
| 339 | case *ssa.Const: |
| 340 | if v.IsNil() { |
| 341 | return isnil |
| 342 | } |
| 343 | return isnonnil |
| 344 | } |
| 345 | |
| 346 | // Search dominating control-flow facts. |
| 347 | for _, f := range stack { |
| 348 | if f.value == v { |
| 349 | return f.nilness |
| 350 | } |
| 351 | } |
| 352 | return unknown |
| 353 | } |
| 354 | |
| 355 | // If b ends with an equality comparison, eq returns the operation and |
| 356 | // its true (equal) and false (not equal) successors. |
| 357 | func eq(b *ssa.BasicBlock) (op *ssa.BinOp, tsucc, fsucc *ssa.BasicBlock) { |
| 358 | if If, ok := b.Instrs[len(b.Instrs)-1].(*ssa.If); ok { |
| 359 | if binop, ok := If.Cond.(*ssa.BinOp); ok { |
| 360 | switch binop.Op { |
| 361 | case token.EQL: |
| 362 | return binop, b.Succs[0], b.Succs[1] |
| 363 | case token.NEQ: |
| 364 | return binop, b.Succs[1], b.Succs[0] |
| 365 | } |
| 366 | } |
| 367 | } |
| 368 | return nil, nil, nil |
| 369 | } |
| 370 | |
| 371 | // expandFacts takes a single fact and returns the set of facts that can be |
| 372 | // known about it or any of its related values. Some operations, like |
| 373 | // ChangeInterface, have transitive nilness, such that if you know the |
| 374 | // underlying value is nil, you also know the value itself is nil, and vice |
| 375 | // versa. This operation allows callers to match on any of the related values |
| 376 | // in analyzes, rather than just the one form of the value that happened to |
| 377 | // appear in a comparison. |
| 378 | // |
| 379 | // This work must be in addition to unwrapping values within nilnessOf because |
| 380 | // while this work helps give facts about transitively known values based on |
| 381 | // inferred facts, the recursive check within nilnessOf covers cases where |
| 382 | // nilness facts are intrinsic to the underlying value, such as a zero value |
| 383 | // interface variables. |
| 384 | // |
| 385 | // ChangeInterface is the only expansion currently supported, but others, like |
| 386 | // Slice, could be added. At this time, this tool does not check slice |
| 387 | // operations in a way this expansion could help. See |
| 388 | // https://play.golang.org/p/mGqXEp7w4fR for an example. |
| 389 | func expandFacts(f fact) []fact { |
| 390 | ff := []fact{f} |
| 391 | |
| 392 | Loop: |
| 393 | for { |
| 394 | switch v := f.value.(type) { |
| 395 | case *ssa.ChangeInterface: |
| 396 | f = fact{v.X, f.nilness} |
| 397 | ff = append(ff, f) |
| 398 | default: |
| 399 | break Loop |
| 400 | } |
| 401 | } |
| 402 | |
| 403 | return ff |
| 404 | } |
| 405 | |
| 406 | type facts []fact |
| 407 | |
| 408 | func (ff facts) negate() facts { |
| 409 | nn := make([]fact, len(ff)) |
| 410 | for i, f := range ff { |
| 411 | nn[i] = f.negate() |
| 412 | } |
| 413 | return nn |
| 414 | } |
| 415 | |
| 416 | var firstArg = func(c *ssa.CallCommon) ssa.Value { return c.Args[0] } |