netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Clean documentation and add go vet support for checklocks.

Browse Source 
This makes it easier to iterate with checklocks. This pattern will be
duplicated with more complex analyzers.

Updates #7038

PiperOrigin-RevId: 426317618
This commit is contained in:
Adin Scannell
2022-02-03 22:26:47 -08:00
committed by gVisor bot
parent b8689e0586
commit dce4528a1a
8 changed files with 256 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BUILD
+1
View File
@@ -118,6 +118,7 @@ go_path(
"//runsc/cli",
"//shim/cli",
"//webhook/pkg/cli",
"//tools/checklocks",
# Packages that are not dependencies of the above.
"//pkg/sentry/kernel/memevent",
go.mod
+7 -3
View File
@@ -24,7 +24,7 @@ require (
github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2
github.com/vishvananda/netlink v1.0.1-0.20190930145447-2ec5bdc52b86
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654
golang.org/x/time v0.0.0-20191024005414-555d28b269f0
google.golang.org/grpc v1.42.0-dev.0.20211020220737-f00baa6c3c84
google.golang.org/protobuf v1.27.1
@@ -57,11 +57,15 @@ require (
github.com/opencontainers/runc v1.0.0-rc90 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/vishvananda/netns v0.0.0-20211101163701-50045581ed74 // indirect
github.com/yuin/goldmark v1.4.1 // indirect
go.opencensus.io v0.23.0 // indirect
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 // indirect
golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420 // indirect
golang.org/x/mod v0.5.1 // indirect
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f // indirect
golang.org/x/oauth2 v0.0.0-20211005180243-6b3c2da341f1 // indirect
golang.org/x/text v0.3.6 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/tools v0.1.9 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20210722135532-667f2b7c528f // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
go.sum
+12
View File
@@ -377,6 +377,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
github.com/yuin/goldmark v1.4.1 h1:/vn0k+RBvwlxEmP5E7SZMqNxPhfMVFEJiykr15/0XKM=
github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
@@ -430,6 +432,8 @@ golang.org/x/mod v0.1.1-0.20191107180719-034126e5016b/go.mod h1:QqPTAvyqsEbceGzB
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.5.1 h1:OJxoQ/rynoF0dcCdI7cLPktw/hR2cueqYfjm43oqK38=
golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -467,6 +471,8 @@ golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwY
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420 h1:a8jGStKg0XqKDlKqjLrXn0ioF5MH36pT7Z0BRTqLhbk=
golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f h1:OfiFi4JbukWwe3lzw+xunroH1mnC1e2Gy5cxNJApiSY=
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -535,6 +541,8 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac h1:oN6lz7iLW/YC7un8pq+9bOLyXrprv2+DKfkJY+2LJJw=
golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -545,6 +553,8 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 h1:/5xXl8Y5W96D+TtHSlonuFqGHIWVuyCkGJLwGh9JJFs=
@@ -595,6 +605,8 @@ golang.org/x/tools v0.0.0-20200804011535-6c149bb5ef0d/go.mod h1:njjCfa9FT2d7l9Bc
golang.org/x/tools v0.0.0-20200825202427-b303f430e36d/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.9 h1:j9KsMiaP1c3B0OTQGth0/k+miLGTgLsAFUCrF2vLcF8=
golang.org/x/tools v0.1.9/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
tools/checklocks/BUILD
+5 -1
View File
@@ -12,7 +12,11 @@ go_library(
"state.go",
],
nogo = False,
visibility = ["//tools/nogo:__subpackages__"],
visibility = [
"//:__pkg__",
"//tools/checklocks/cmd:__subpackages__",
"//tools/nogo:__subpackages__",
],
deps = [
"@org_golang_x_tools//go/analysis:go_default_library",
"@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library",
tools/checklocks/README.md
+197 -70
View File
@@ -1,14 +1,38 @@
# CheckLocks Analyzer
<!--* freshness: { owner: 'gvisor-eng' reviewed: '2021-10-20' } *-->
<!--* freshness: { owner: 'gvisor-eng' reviewed: '2022-02-02' } *-->
Checklocks is an analyzer for lock and atomic constraints. The analyzer relies
on explicit annotations to identify fields that should be checked for access.
## Atomic annotations
## Installation and Usage
The analyzer is integrated into the gVisor `nogo` framework. It automatically
applies to all code in this repository.
For external usage and to iterate quickly, it may be used as part of `go vet`.
You can install the tool separately via:
```sh
go install gvisor.dev/gvisor/tools/checklocks/cmd/checklocks@go
```
And, if installed to the default path, run it via:
```sh
go vet -vettool=$HOME/go/bin/checklocks ./...
```
## Annotations
This analyzer supports annotations for atomic access and lock enforcement, in
order to allow for mixed semantics. These are first described separately, then
the combination is discussed.
### Atomic Access Enforcement
Individual struct members may be noted as requiring atomic access. These
annotations are of the form:
annotations are of the form `+checkatomic`, for example:
```go
type foo struct {
@@ -18,22 +42,24 @@ type foo struct {
```
This will ensure that all accesses to bar are atomic, with the exception of
operations on newly allocated objects.
operations on newly allocated objects (when detectable).
## Lock annotations
## Lock Enforcement
Individual struct members may be protected by annotations that indicate locking
requirements for accessing members. These annotations are of the form:
requirements for accessing members. These annotations are of the form
`+checklocks`, for example:
```go
type foo struct {
mu sync.Mutex
// +checklocks:mu
bar int
foo int // No annotation on foo means it's not guarded by mu.
secondMu sync.Mutex
secondMu sync.RWMutex
// Multiple annotations indicate that both must be held but the
// checker does not assert any lock ordering.
@@ -43,89 +69,107 @@ type foo struct {
}
```
The checklocks annotation may also apply to functions. For example:
These semantics are enforcable on `sync.Mutex`, `sync.RWMutex` and `sync.Locker`
fields. Semantics with respect to reading and writing are automatically detected
and enforced. If an access is read-only, then the lock need only be held as a
read lock, in the case of an `sync.RWMutex`.
The locks must be resolvable within the scope of the declaration. This means the
lock must refer to one of:
* A struct-local lock (e.g. mu).
* A lock resolvable from the local struct (e.g. fieldX.mu).
* A global lock (e.g. globalMu).
* A lock resolvable from a global struct (e.g. globalX.mu).
Like atomic access enforcement, checks may be elided on newly allocated objects.
### Function Annotations
The `+checklocks` annotation may apply to functions. For example:
```go
// +checklocks:f.mu
func (f *foo) doThingLocked() { }
```
This will check that the "f.mu" is locked for any calls, where possible.
The field provided in the `+checklocks` annotation must be resolvable as one of:
In case of functions which initialize structs that may have annotations one can
use the following annotation on the function to disable reporting by the lock
checker. The lock checker will still track any mutexes acquired or released but
won't report any failures for this function for unguarded field access.
* A parameter, receiver or return value (e.g. mu).
* A lock resolvable from a parameter, receiver or return value (e.g. f.mu).
* A global lock (e.g. globalMu).
* A lock resolvable from a global struct (e.g. globalX.mu).
This annotation will ensure that the given lock is held for all calls, and all
analysis of this function will assume that this is the case.
Additional variants of the `+checklocks` annotation are supported for functions:
* `+checklocksread`: This enforces that at least a read lock is held. Note
that this assumption will apply locally, so accesses and function calls will
assume that only a read lock is available.
* `+checklocksacquire`: This enforces that the given lock is *not* held on
entry, but it will be held on exit. This assertion will be checked locally
and applied to the caller's lock state.
* `+checklocksrelease`: This enforces that the given lock is held on entry,
and will be release on exit. This assertion is checked locally and applied
to the caller's lock state.
* `+checklocksacquireread`: A read variant of `+checklocksacquire`.
* `+checklocksreleaseread`: A read variant of `+checklocksrelease`.
* `+checklocksalias:a.b.c=x.y`: For parameters with complex relationships,
this annotation can be used to specify that the `a.b.c` lock is equivalent
to the `x.y` state. That is, any operation on either of these locks applies
to both, and any assertions that can be made about either applies to both.
For examples of these cases see the tests.
#### Anonymous Functions and Closures
Anonymous functions and closures cannot be annotated.
If anonymous functions and closures are bound and invoked within a single scope,
the analysis will happen with the available lock state. For example, the
following will not report any violations:
```go
// +checklocks:ignore
func newXXX() *X {
...
func foo(ts *testStruct) {
x := func() {
ts.guardedField = 1
}
ts.mu.Lock()
x() // We know the context x is being invoked.
ts.mu.Unlock()
}
```
***The checker treats both 'sync.Mutex' and 'sync.RWMutex' identically, i.e, as
a sync.Mutex. The checker does not distinguish between read locks vs. exclusive
locks and treats all locks as exclusive locks***.
This pattern often applies to defer usage, which allows defered functions to be
fully analyzed with the lock state at time of execution.
For cases the checker is able to correctly handle today please see test/test.go.
The checklocks check also flags any invalid annotations where the mutex
annotation refers either to something that is not a 'sync.Mutex' or
'sync.RWMutex' or where the field does not exist at all. This will prevent the
annotations from becoming stale over time as fields are renamed, etc.
## Lock suggestions
Based on locks held during field access, the analyzer will suggest annotations.
These can be ignored with the standard `+checklocksignore` annotation.
The annotation will be generated when the lock is held the vast majority of the
time the field is accessed. Note that it is possible for this frequency to be
greater than 100%, if the lock is held multiple times. For example:
However, if a closure is passed to another function, the anonymous function
backing that closure will be analyzed assuming no available lock state. For
example, the following will report violations:
```go
func foo(ts1 *testStruct, ts2 *testStruct) {
ts1.Lock()
ts2.Lock()
ts1.guardedField = 1 // 200% locks held.
ts1.Unlock()
ts2.Unlock()
}
```
## Currently not supported
1. Anonymous functions are not correctly evaluated. The analyzer does not
currently support specifying annotations on anonymous functions as a result
evaluation of a function that accesses protected fields will fail.
```go
type A struct {
mu sync.Mutex
// +checklocks:mu
x int
}
func abc() {
var a A
f := func() { a.x = 1 } <=== This line will be flagged by analyzer
a.mu.Lock()
func runFunc(f func()) {
f()
a.mu.Unlock()
}
func foo(ts *testStruct) {
x := func() {
ts.guardedField = 1
}
ts.mu.Lock()
runFunc(x) // We can't know what will happen with x.
ts.mu.Unlock()
}
```
### Explicitly Not Supported
Since x cannot be annotated, this may require use of the force annotation used
below. However, if anonymous functions and closures require annotations, there
may be an opportunity to split them into named functions for improved analysis
and debuggability, and avoid the need to use force annotations.
1. The checker will not support guards on anything other than the cases
described above. For example, global mutexes cannot be referred to by
checklocks. Only struct members can be used.
2. The checker will not support checking for lock ordering violations.
## Mixed mode
### Mixed Atomic Access and Lock Enforcement
Some members may allow read-only atomic access, but be protected against writes
by a mutex. Generally, this imposes the following requirements:
@@ -151,3 +195,86 @@ type foo struct {
bar int32
}
```
This enforces that the preconditions above are upheld.
## Ignoring and Forcing
From time to time, it may be necessary to ignore results produced by the
analyzer. These can be disabled on a per-field, per-function or per-line basis.
For fields, only lock suggestions may be ignored. See below for details.
For functions, the `+checklocksignore` annotation can be applied. This prevents
any local analysis from taking place. Note that the other annotations can still
be applied to the function, which will enforce assertions in caller analysis.
For example:
```go
// +checklocks:ts.mu
// +checklocksignore
func foo(ts *testStruct) {
ts.guardedField = 1
}
```
For individual lines, the `+checklocksforce` annotation can be applied after the
statement. This does not simply ignore the line, rather it *forces* the
necessary assertion to become true. For example, if a lock must be held, this
annotation will mark that lock as held for all subsequent lines. For example:
```go
func foo(ts *testStruct) {
ts.guardedField = 1 // +checklocksforce: don't care about locking.
}
```
In general, both annotations should be highly discouraged. It should be possible
to avoid their use by factoring functions in such a way that annotations can be
applied consistently and without the need for ignoring and forcing.
## Testing
Tests can be built using the `+checklocksfail` annotation. When applied after a
statement, these will generate a report if the line does *not* fail an
assertion. For example:
```go
func foo(ts *testStruct) {
ts.guardedField = 1 // +checklocksfail: violation.
}
```
These annotations are primarily useful for analyzer development and testing.
## Suggestions
Based on locks held during field access, the analyzer may suggest annotations.
These can be ignored with the `+checklocksignore` annotation on fields.
```go
type foo struct {
mu sync.Mutex
// +checklocksignore: mu is not required, it just happens to be held always.
bar int32
}
```
The annotation will be generated when the lock is held the vast majority of the
time the field is accessed. Note that it is possible for this frequency to be
greater than 100%, if the lock is held multiple times. For example:
```go
func foo(ts1 *testStruct, ts2 *testStruct) {
ts1.Lock()
ts2.Lock()
ts1.guardedField = 1 // 200% locks held.
ts1.Unlock()
ts2.Unlock()
}
```
It should be expected that this annotation is also rare. If the field is not
protected by the mutex, it suggests that the critical section could be made
smaller by restructuring the code or the structure instead of applying the
ignore annotation.
tools/checklocks/cmd/checklocks/BUILD
+10
View File
@@ -0,0 +1,10 @@
load("//tools:defs.bzl", "go_binary")
go_binary(
name = "checklocks",
srcs = ["main.go"],
deps = [
"//tools/checklocks",
"@org_golang_x_tools//go/analysis/singlechecker:go_default_library",
],
)
tools/checklocks/cmd/checklocks/main.go
+23
View File
@@ -0,0 +1,23 @@
// Copyright 2020 The gVisor Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// Binary checklocks is a `vettool` for `go vet`.
package main
import (
"golang.org/x/tools/go/analysis/singlechecker"
"gvisor.dev/gvisor/tools/checklocks"
)
func main() { singlechecker.Main(checklocks.Analyzer) }
tools/go_branch.sh
+1 -1
View File
@@ -116,7 +116,7 @@ EOF
# There are a few solitary files that can get left behind due to the way bazel
# constructs the gopath target. Note that we don't find all Go files here
# because they may correspond to unused templates, etc.
declare -ar binaries=( "runsc" "shim" "webhook" )
declare -ar binaries=( "runsc" "shim" "webhook" "tools/checklocks/cmd/checklocks" )
for target in "${binaries[@]}"; do
mkdir -p "${target}"
cp "${repo_orig}/${target}"/*.go "${target}/"