Skip to content

Commit dd8aad0

Browse files
jakebaileyjakebailey
authored
Lint writes to protected record kind fields (#64791)
1 parent 996ab71 commit dd8aad0

19 files changed

Lines changed: 1096 additions & 35 deletions

File tree

‎tools/customlint/plugin.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
2020
cleanupAnalyzer,
2121
caseBodyAnalyzer,
2222
forbidParentAccessAnalyzer,
23+
protectedFieldAnalyzer,
2324
shadowAnalyzer,
2425
typedPathsAnalyzer,
2526
unexportedAPIAnalyzer,

‎tools/customlint/protectedfield.go‎

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
package customlint
2+
3+
import (
4+
"go/ast"
5+
"go/token"
6+
"go/types"
7+
"reflect"
8+
9+
"golang.org/x/tools/go/analysis"
10+
"golang.org/x/tools/go/analysis/passes/inspect"
11+
"golang.org/x/tools/go/ast/inspector"
12+
)
13+
14+
// protectedFieldAnalyzer guards fields tagged `customlint:"protected"`, such as
15+
// the kind headers that unsafe casts trust to identify the enclosing allocation.
16+
// Writing such a field, or copying a struct that contains one, can make a value
17+
// claim to be a larger record than its storage holds. Intentional writes are
18+
// suppressed with //nolint:customlint.
19+
var protectedFieldAnalyzer = &analysis.Analyzer{
20+
Name: "protectedfield",
21+
Doc: "forbids writes to protected fields and by-value copies of structs containing them",
22+
Requires: []*analysis.Analyzer{
23+
inspect.Analyzer,
24+
},
25+
Run: func(pass *analysis.Pass) (any, error) {
26+
return (&protectedFieldPass{pass: pass, contains: make(map[types.Type]bool)}).run()
27+
},
28+
}
29+
30+
type protectedFieldPass struct {
31+
pass *analysis.Pass
32+
contains map[types.Type]bool
33+
}
34+
35+
func (p *protectedFieldPass) run() (any, error) {
36+
in := p.pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
37+
38+
for c := range in.Root().Preorder(
39+
(*ast.AssignStmt)(nil),
40+
(*ast.IncDecStmt)(nil),
41+
(*ast.RangeStmt)(nil),
42+
(*ast.UnaryExpr)(nil),
43+
(*ast.CompositeLit)(nil),
44+
(*ast.FuncType)(nil),
45+
(*ast.FuncDecl)(nil),
46+
) {
47+
switch node := c.Node().(type) {
48+
case *ast.AssignStmt:
49+
for _, lhs := range node.Lhs {
50+
p.checkWrite(lhs, "write to")
51+
}
52+
case *ast.IncDecStmt:
53+
p.checkWrite(node.X, "write to")
54+
case *ast.RangeStmt:
55+
if node.Key != nil {
56+
p.checkWrite(node.Key, "write to")
57+
}
58+
if node.Value != nil {
59+
p.checkWrite(node.Value, "write to")
60+
}
61+
case *ast.UnaryExpr:
62+
if node.Op == token.AND {
63+
p.checkWrite(node.X, "address of")
64+
}
65+
case *ast.CompositeLit:
66+
p.checkLiteral(node)
67+
case *ast.FuncType:
68+
p.checkFields(node.Params, "parameter")
69+
p.checkFields(node.Results, "result")
70+
case *ast.FuncDecl:
71+
p.checkFields(node.Recv, "receiver")
72+
}
73+
}
74+
75+
for id, obj := range p.pass.TypesInfo.Defs {
76+
if v, ok := obj.(*types.Var); ok && (v.Kind() == types.PackageVar || v.Kind() == types.LocalVar) && p.containsProtected(v.Type()) {
77+
p.pass.Reportf(id.Pos(), "variable %s holds %s by value, which contains a protected field", id.Name, v.Type())
78+
}
79+
}
80+
81+
for c := range in.Root().Preorder((*ast.StarExpr)(nil), (*ast.IndexExpr)(nil), (*ast.CallExpr)(nil), (*ast.CompositeLit)(nil), (*ast.SelectorExpr)(nil)) {
82+
expr := c.Node().(ast.Expr)
83+
tv, ok := p.pass.TypesInfo.Types[expr]
84+
if !ok || !tv.IsValue() || !p.containsProtected(tv.Type) || p.allowedValue(c) {
85+
continue
86+
}
87+
p.pass.Reportf(expr.Pos(), "by-value copy of %s, which contains a protected field", tv.Type)
88+
}
89+
90+
return nil, nil
91+
}
92+
93+
func (p *protectedFieldPass) checkWrite(expr ast.Expr, what string) {
94+
sel, ok := ast.Unparen(expr).(*ast.SelectorExpr)
95+
if !ok {
96+
return
97+
}
98+
selection := p.pass.TypesInfo.Selections[sel]
99+
if selection == nil || selection.Kind() != types.FieldVal {
100+
return
101+
}
102+
typ := selection.Recv()
103+
var tag string
104+
for _, index := range selection.Index() {
105+
if ptr, ok := typ.Underlying().(*types.Pointer); ok {
106+
typ = ptr.Elem()
107+
}
108+
st := typ.Underlying().(*types.Struct)
109+
tag = st.Tag(index)
110+
typ = st.Field(index).Type()
111+
}
112+
if isProtectedTag(tag) {
113+
p.pass.Reportf(sel.Sel.Pos(), "%s protected field %s", what, sel.Sel.Name)
114+
}
115+
}
116+
117+
func (p *protectedFieldPass) checkLiteral(lit *ast.CompositeLit) {
118+
typ := p.pass.TypesInfo.TypeOf(lit)
119+
// Elided literals in []*T or map[K]*T composites have type *T.
120+
if ptr, ok := typ.Underlying().(*types.Pointer); ok {
121+
typ = ptr.Elem()
122+
}
123+
st, ok := typ.Underlying().(*types.Struct)
124+
if !ok {
125+
return
126+
}
127+
for i, elt := range lit.Elts {
128+
index := i
129+
if kv, ok := elt.(*ast.KeyValueExpr); ok {
130+
key, ok := kv.Key.(*ast.Ident)
131+
if !ok {
132+
continue
133+
}
134+
index = -1
135+
for j := range st.NumFields() {
136+
if st.Field(j).Name() == key.Name {
137+
index = j
138+
break
139+
}
140+
}
141+
if index < 0 {
142+
continue
143+
}
144+
}
145+
if index < st.NumFields() && isProtectedTag(st.Tag(index)) {
146+
p.pass.Reportf(elt.Pos(), "write to protected field %s", st.Field(index).Name())
147+
}
148+
}
149+
}
150+
151+
func (p *protectedFieldPass) checkFields(fields *ast.FieldList, what string) {
152+
if fields == nil {
153+
return
154+
}
155+
for _, field := range fields.List {
156+
if typ := p.pass.TypesInfo.TypeOf(field.Type); typ != nil && p.containsProtected(typ) {
157+
p.pass.Reportf(field.Type.Pos(), "%s holds %s by value, which contains a protected field", what, typ)
158+
}
159+
}
160+
}
161+
162+
// allowedValue reports whether a record-valued expression is used without
163+
// copying it: selecting a field or method, taking its address, indexing it, or
164+
// as an operand of unsafe.Sizeof, Alignof, or Offsetof.
165+
func (p *protectedFieldPass) allowedValue(c inspector.Cursor) bool {
166+
child := c.Node()
167+
for parent := c.Parent(); ; parent = parent.Parent() {
168+
switch node := parent.Node().(type) {
169+
case *ast.ParenExpr:
170+
child = node
171+
continue
172+
case *ast.SelectorExpr:
173+
return node.X == child
174+
case *ast.UnaryExpr:
175+
return node.Op == token.AND
176+
case *ast.IndexExpr:
177+
return node.X == child
178+
case *ast.SliceExpr:
179+
return node.X == child
180+
}
181+
for call := range parent.Enclosing((*ast.CallExpr)(nil)) {
182+
if p.isUnsafeLayoutCall(call.Node().(*ast.CallExpr)) {
183+
return true
184+
}
185+
}
186+
return false
187+
}
188+
}
189+
190+
func (p *protectedFieldPass) isUnsafeLayoutCall(call *ast.CallExpr) bool {
191+
sel, ok := ast.Unparen(call.Fun).(*ast.SelectorExpr)
192+
if !ok {
193+
return false
194+
}
195+
builtin, ok := p.pass.TypesInfo.Uses[sel.Sel].(*types.Builtin)
196+
return ok && (builtin.Name() == "Sizeof" || builtin.Name() == "Alignof" || builtin.Name() == "Offsetof")
197+
}
198+
199+
func (p *protectedFieldPass) containsProtected(typ types.Type) bool {
200+
if result, ok := p.contains[typ]; ok {
201+
return result
202+
}
203+
result := false
204+
switch t := typ.Underlying().(type) {
205+
case *types.Struct:
206+
for i := range t.NumFields() {
207+
if isProtectedTag(t.Tag(i)) || p.containsProtected(t.Field(i).Type()) {
208+
result = true
209+
break
210+
}
211+
}
212+
case *types.Array:
213+
result = p.containsProtected(t.Elem())
214+
}
215+
p.contains[typ] = result
216+
return result
217+
}
218+
219+
func isProtectedTag(tag string) bool {
220+
return reflect.StructTag(tag).Get("customlint") == "protected"
221+
}

‎tools/customlint/testdata/protectedfield/consumer/consumer.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package consumer
2+
3+
import "testdata/protectedfield/record"
4+
5+
type Container struct {
6+
header *record.Header
7+
small record.Small
8+
}
9+
10+
func writes(h *record.Header, s *record.Small, d *record.Defined) {
11+
h.Kind = record.KindLarge
12+
s.Kind = record.KindLarge
13+
s.Header.Kind++
14+
d.Kind = record.KindLarge
15+
h.Kind, h.Flags = record.KindLarge, 1
16+
_ = &h.Kind
17+
h.Flags = 1
18+
_ = &h.Flags
19+
_ = h.Kind
20+
for _, h.Kind = range []record.Kind{record.KindLarge} {
21+
}
22+
for h.Kind = range record.Kind(2) {
23+
}
24+
var kinds []record.Kind
25+
for _, kind := range kinds {
26+
_ = kind
27+
}
28+
}
29+
30+
func literals() {
31+
_ = &record.Header{Kind: record.KindLarge}
32+
_ = &record.Header{record.KindLarge, 1}
33+
_ = &record.Header{Flags: 1}
34+
_ = &record.Small{}
35+
_ = &record.Alias{Kind: record.KindSmall}
36+
_ = []*record.Header{{Kind: record.KindLarge}}
37+
_ = []*record.Header{{record.KindLarge, 1}}
38+
_ = map[int]*record.Header{0: {Kind: record.KindLarge}}
39+
_ = []*record.Header{{Flags: 1}}
40+
}
41+
42+
func copies(h *record.Header, s *record.Small, hs []record.Header) {
43+
*h = *record.NewSmall(1)
44+
small := *s
45+
var local record.Header
46+
_ = record.Small{}
47+
_ = hs[0]
48+
_ = &hs[0]
49+
_ = hs[0].Flags
50+
_ = (*h).Flags
51+
_ = &(*h)
52+
_ = s.Header
53+
_ = &s.Header
54+
for _, v := range hs {
55+
_ = v
56+
}
57+
_, _ = small, local
58+
_ = h.AsLarge()
59+
}
60+
61+
func param(h record.Header) {}
62+
63+
func result() (h *record.Header) { return nil }
64+
65+
func valueResult() record.Small { return record.Small{} }

0 commit comments

Comments
 (0)