1 package sa5011
2 3 import (
4 "go/types"
5 6 "honnef.co/go/tools/analysis/lint"
7 "honnef.co/go/tools/analysis/report"
8 "honnef.co/go/tools/go/ir"
9 "honnef.co/go/tools/go/types/typeutil"
10 "honnef.co/go/tools/internal/passes/buildir"
11 12 "golang.org/x/tools/go/analysis"
13 )
14 15 var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
16 Analyzer: &analysis.Analyzer{
17 Name: "SA5011",
18 Run: run,
19 Requires: []*analysis.Analyzer{buildir.Analyzer},
20 },
21 Doc: &lint.RawDocumentation{
22 Title: `Possible nil pointer dereference`,
23 24 Text: `A pointer is being dereferenced unconditionally, while
25 also being checked against nil in another place. This suggests that
26 the pointer may be nil and dereferencing it may panic. This is
27 commonly a result of improperly ordered code or missing return
28 statements. Consider the following examples:
29 30 func fn(x *int) {
31 fmt.Println(*x)
32 33 // This nil check is equally important for the previous dereference
34 if x != nil {
35 foo(*x)
36 }
37 }
38 39 func TestFoo(t *testing.T) {
40 x := compute()
41 if x == nil {
42 t.Errorf("nil pointer received")
43 }
44 45 // t.Errorf does not abort the test, so if x is nil, the next line will panic.
46 foo(*x)
47 }
48 49 Staticcheck tries to deduce which functions abort control flow.
50 For example, it is aware that a function will not continue
51 execution after a call to \'panic\' or \'log.Fatal\'. However, sometimes
52 this detection fails, in particular in the presence of
53 conditionals. Consider the following example:
54 55 func Log(msg string, level int) {
56 fmt.Println(msg)
57 if level == levelFatal {
58 os.Exit(1)
59 }
60 }
61 62 func Fatal(msg string) {
63 Log(msg, levelFatal)
64 }
65 66 func fn(x *int) {
67 if x == nil {
68 Fatal("unexpected nil pointer")
69 }
70 fmt.Println(*x)
71 }
72 73 Staticcheck will flag the dereference of \'x\', even though it is perfectly
74 safe. Staticcheck is not able to deduce that a call to
75 Fatal will exit the program. For the time being, the easiest
76 workaround is to modify the definition of Fatal like so:
77 78 func Fatal(msg string) {
79 Log(msg, levelFatal)
80 panic("unreachable")
81 }
82 83 We also hard-code functions from common logging packages such as
84 logrus. Please file an issue if we're missing support for a
85 popular package.`,
86 Since: "2020.1",
87 Severity: lint.SeverityWarning,
88 MergeIf: lint.MergeIfAny,
89 },
90 })
91 92 var Analyzer = SCAnalyzer.Analyzer
93 94 func run(pass *analysis.Pass) (interface{}, error) {
95 // This is an extremely trivial check that doesn't try to reason
96 // about control flow. That is, phis and sigmas do not propagate
97 // any information. As such, we can flag this:
98 //
99 // _ = *x
100 // if x == nil { return }
101 //
102 // but we cannot flag this:
103 //
104 // if x == nil { println(x) }
105 // _ = *x
106 //
107 // but we can flag this, because the if's body doesn't use x:
108 //
109 // if x == nil { println("this is bad") }
110 // _ = *x
111 //
112 // nor many other variations of conditional uses of or assignments to x.
113 //
114 // However, even this trivial implementation finds plenty of
115 // real-world bugs, such as dereference before nil pointer check,
116 // or using t.Error instead of t.Fatal when encountering nil
117 // pointers.
118 //
119 // On the flip side, our naive implementation avoids false positives in branches, such as
120 //
121 // if x != nil { _ = *x }
122 //
123 // due to the same lack of propagating information through sigma
124 // nodes. x inside the branch will be independent of the x in the
125 // nil pointer check.
126 //
127 //
128 // We could implement a more powerful check, but then we'd be
129 // getting false positives instead of false negatives because
130 // we're incapable of deducing relationships between variables.
131 // For example, a function might return a pointer and an error,
132 // and the error being nil guarantees that the pointer is not nil.
133 // Depending on the surrounding code, the pointer may still end up
134 // being checked against nil in one place, and guarded by a check
135 // on the error in another, which would lead to us marking some
136 // loads as unsafe.
137 //
138 // Unfortunately, simply hard-coding the relationship between
139 // return values wouldn't eliminate all false positives, either.
140 // Many other more subtle relationships exist. An abridged example
141 // from real code:
142 //
143 // if a == nil && b == nil { return }
144 // c := fn(a)
145 // if c != "" { _ = *a }
146 //
147 // where `fn` is guaranteed to return a non-empty string if a
148 // isn't nil.
149 //
150 // We choose to err on the side of false negatives.
151 152 isNilConst := func(v ir.Value) bool {
153 if typeutil.IsPointerLike(v.Type()) {
154 if k, ok := v.(*ir.Const); ok {
155 return k.IsNil()
156 }
157 }
158 return false
159 }
160 161 for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
162 maybeNil := map[ir.Value]ir.Instruction{}
163 for _, b := range fn.Blocks {
164 for _, instr := range b.Instrs {
165 // Originally we looked at all ir.BinOp, but that would lead to calls like 'assert(x != nil)' causing false positives.
166 // Restrict ourselves to actual if statements, as these are more likely to affect control flow in a way we can observe.
167 if instr, ok := instr.(*ir.If); ok {
168 if cond, ok := instr.Cond.(*ir.BinOp); ok {
169 if isNilConst(cond.X) {
170 maybeNil[cond.Y] = cond
171 }
172 if isNilConst(cond.Y) {
173 maybeNil[cond.X] = cond
174 }
175 }
176 }
177 }
178 }
179 180 for _, b := range fn.Blocks {
181 for _, instr := range b.Instrs {
182 var ptr ir.Value
183 switch instr := instr.(type) {
184 case *ir.Load:
185 ptr = instr.X
186 case *ir.Store:
187 ptr = instr.Addr
188 case *ir.IndexAddr:
189 ptr = instr.X
190 if typeutil.All(ptr.Type(), func(term *types.Term) bool {
191 if _, ok := term.Type().Underlying().(*types.Slice); ok {
192 return true
193 }
194 return false
195 }) {
196 // indexing a nil slice does not cause a nil pointer panic
197 //
198 // Note: This also works around the bad lowering of range loops over slices
199 // (https://github.com/dominikh/go-tools/issues/1053)
200 continue
201 }
202 case *ir.FieldAddr:
203 ptr = instr.X
204 }
205 if ptr != nil {
206 switch ptr.(type) {
207 case *ir.Alloc, *ir.FieldAddr, *ir.IndexAddr:
208 // these cannot be nil
209 continue
210 }
211 if r, ok := maybeNil[ptr]; ok {
212 report.Report(pass, instr, "possible nil pointer dereference",
213 report.Related(r, "this check suggests that the pointer can be nil"))
214 }
215 }
216 }
217 }
218 }
219 220 return nil, nil
221 }
222