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

Add checklocks analyzer.

Browse Source 
This validates that struct fields if annotated with "// checklocks:mu" where
"mu" is a mutex field in the same struct then access to the field is only
done with "mu" locked.

All types that are guarded by a mutex must be annotated with

// +checklocks:<mutex field name>

For more details please refer to README.md.

PiperOrigin-RevId: 360729328
This commit is contained in:
Bhasker Hariharan
2021-03-03 12:24:21 -08:00
committed by gVisor bot
parent 80bc67c268
commit 3e69f5d088
18 changed files with 1437 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nogo.yaml
+45
View File
@@ -77,6 +77,51 @@ analyzers:
external: # Enabled.
cgocall:
external: # Enabled.
checklocks:
internal:
exclude:
- "^-$" # b/181776900: analyzer fails on buildkite
- pkg/sentry/fs/dirent.go # unsupported usage.
- pkg/sentry/fs/fsutil/inode_cached.go # unsupported usage.
- pkg/sentry/fs/gofer/inode_state.go # unsupported usage.
- pkg/sentry/fs/gofer/session.go # unsupported usage.
- pkg/sentry/fs/ramfs/dir.go # unsupported usage.
- pkg/sentry/fsimpl/fuse/connection.go # unsupported usage.
- pkg/sentry/fsimpl/kernfs/filesystem.go # unsupported usage.
- pkg/sentry/fsimpl/kernfs/inode_impl_util.go # unsupported usage.
- pkg/sentry/fsimpl/fuse/dev_test.go # unsupported usage.
- pkg/sentry/fsimpl/gofer/filesystem.go # unsupported usage.
- pkg/sentry/fsimpl/gofer/gofer.go # unsupported usage.
- pkg/sentry/fsimpl/gofer/regular_file.go # unsupported usage.
- pkg/sentry/fsimpl/gofer/special_file.go # unsupported usage.
- pkg/sentry/fsimpl/gofer/symlink.go # unsupported usage.
- pkg/sentry/fsimpl/overlay/copy_up.go # unsupported usage.
- pkg/sentry/fsimpl/overlay/filesystem.go # unsupported usage.
- pkg/sentry/fsimpl/tmpfs/filesystem.go # unsupported usage.
- pkg/sentry/fsimpl/verity/filesystem.go # unsupported usage.
- pkg/sentry/kernel/futex/futex.go # unsupported usage.
- pkg/sentry/kernel/pipe/vfs.go # unsupported usage.
- pkg/sentry/mm/syscalls.go # unsupported usage.
- pkg/sentry/kernel/fd_table.go # unsupported usage.
- pkg/sentry/kernel/ptrace.go # unsupported usage.
- pkg/sentry/time/calibrated_clock_test.go # unsupported usage.
- pkg/sentry/kernel/task_context.go # unsupported usage.
- pkg/sentry/pgalloc/pgalloc.go # unsupported usage.
- pkg/sentry/socket/unix/transport/connectioned.go # unsupported usage.
- pkg/sentry/vfs/dentry.go # unsupported usage.
- pkg/tcpip/network/internal/ip/generic_multicast_protocol_test.go # unsupported usage.
- pkg/tcpip/stack/conntrack.go # unsupported usage.
- pkg/tcpip/transport/packet/endpoint_state.go # unsupported usage.
- pkg/tcpip/transport/raw/endpoint_state.go # unsupported usage.
- pkg/tcpip/transport/icmp/endpoint.go # unsupported usage.
- pkg/tcpip/transport/icmp/endpoint_state.go # unsupported usage.
- pkg/tcpip/transport/tcp/accept.go # unsupported usage.
- pkg/tcpip/transport/tcp/connect.go # unsupported usage.
- pkg/tcpip/transport/tcp/dispatcher.go # unsupported usage (TryLock)
- pkg/tcpip/transport/tcp/endpoint.go # unsupported usage.
- pkg/tcpip/transport/tcp/endpoint_state.go # unsupported usage.
- pkg/tcpip/transport/udp/endpoint.go # unsupported usage (defer unlock in anonymous function)
- pkg/tcpip/transport/udp/endpoint_state.go # unsupported usage (missing nested mutex annotation support)
shadow: # Disable for now.
generated:
exclude: [".*"]
pkg/sentry/fs/copy_up.go
+6 -5
View File
@@ -303,17 +303,18 @@ func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error {
// Take a reference on the upper Inode (transferred to
// next.Inode.overlay.upper) and make new translations use it.
next.Inode.overlay.dataMu.Lock()
overlay := next.Inode.overlay
overlay.dataMu.Lock()
childUpperInode.IncRef()
next.Inode.overlay.upper = childUpperInode
next.Inode.overlay.dataMu.Unlock()
overlay.upper = childUpperInode
overlay.dataMu.Unlock()
// Invalidate existing translations through the lower Inode.
next.Inode.overlay.mappings.InvalidateAll(memmap.InvalidateOpts{})
overlay.mappings.InvalidateAll(memmap.InvalidateOpts{})
// Remove existing memory mappings from the lower Inode.
if lowerMappable != nil {
for seg := next.Inode.overlay.mappings.FirstSegment(); seg.Ok(); seg = seg.NextSegment() {
for seg := overlay.mappings.FirstSegment(); seg.Ok(); seg = seg.NextSegment() {
for m := range seg.Value() {
lowerMappable.RemoveMapping(ctx, m.MappingSpace, m.AddrRange, seg.Start(), m.Writable)
}
pkg/sentry/fs/gofer/inode_state.go
+1
View File
@@ -109,6 +109,7 @@ func (i *inodeFileState) loadLoading(_ struct{}) {
}
// afterLoad is invoked by stateify.
// +checklocks:i.loading
func (i *inodeFileState) afterLoad() {
load := func() (err error) {
// See comment on i.loading().
pkg/sentry/fsimpl/overlay/filesystem.go
+3
View File
@@ -85,6 +85,8 @@ func putDentrySlice(ds *[]*dentry) {
// but dentry slices are allocated lazily, and it's much easier to say "defer
// fs.renameMuRUnlockAndCheckDrop(&ds)" than "defer func() {
// fs.renameMuRUnlockAndCheckDrop(ds) }()" to work around this.
//
// +checklocks:fs.renameMu
func (fs *filesystem) renameMuRUnlockAndCheckDrop(ctx context.Context, dsp **[]*dentry) {
fs.renameMu.RUnlock()
if *dsp == nil {
@@ -110,6 +112,7 @@ func (fs *filesystem) renameMuRUnlockAndCheckDrop(ctx context.Context, dsp **[]*
putDentrySlice(*dsp)
}
// +checklocks:fs.renameMu
func (fs *filesystem) renameMuUnlockAndCheckDrop(ctx context.Context, ds **[]*dentry) {
if *ds == nil {
fs.renameMu.Unlock()
pkg/sentry/pgalloc/pgalloc.go
+63 -61
View File
@@ -876,6 +876,7 @@ func (f *MemoryFile) UpdateUsage() error {
// in bs, sets committed[i] to 1 if the page is committed and 0 otherwise.
//
// Precondition: f.mu must be held; it may be unlocked and reacquired.
// +checklocks:f.mu
func (f *MemoryFile) updateUsageLocked(currentUsage uint64, checkCommitted func(bs []byte, committed []byte) error) error {
// Track if anything changed to elide the merge. In the common case, we
// expect all segments to be committed and no merge to occur.
@@ -925,72 +926,73 @@ func (f *MemoryFile) updateUsageLocked(currentUsage uint64, checkCommitted func(
r := seg.Range()
var checkErr error
err := f.forEachMappingSlice(r, func(s []byte) {
if checkErr != nil {
return
}
// Ensure that we have sufficient buffer for the call
// (one byte per page). The length of each slice must
// be page-aligned.
bufLen := len(s) / usermem.PageSize
if len(buf) < bufLen {
buf = make([]byte, bufLen)
}
// Query for new pages in core.
// NOTE(b/165896008): mincore (which is passed as checkCommitted)
// by f.UpdateUsage() might take a really long time. So unlock f.mu
// while checkCommitted runs.
f.mu.Unlock()
err := checkCommitted(s, buf)
f.mu.Lock()
if err != nil {
checkErr = err
return
}
// Scan each page and switch out segments.
seg := f.usage.LowerBoundSegment(r.Start)
for i := 0; i < bufLen; {
if buf[i]&0x1 == 0 {
i++
continue
err := f.forEachMappingSlice(r,
func(s []byte) {
if checkErr != nil {
return
}
// Scan to the end of this committed range.
j := i + 1
for ; j < bufLen; j++ {
if buf[j]&0x1 == 0 {
break
// Ensure that we have sufficient buffer for the call
// (one byte per page). The length of each slice must
// be page-aligned.
bufLen := len(s) / usermem.PageSize
if len(buf) < bufLen {
buf = make([]byte, bufLen)
}
// Query for new pages in core.
// NOTE(b/165896008): mincore (which is passed as checkCommitted)
// by f.UpdateUsage() might take a really long time. So unlock f.mu
// while checkCommitted runs.
f.mu.Unlock()
err := checkCommitted(s, buf)
f.mu.Lock()
if err != nil {
checkErr = err
return
}
// Scan each page and switch out segments.
seg := f.usage.LowerBoundSegment(r.Start)
for i := 0; i < bufLen; {
if buf[i]&0x1 == 0 {
i++
continue
}
}
committedFR := memmap.FileRange{
Start: r.Start + uint64(i*usermem.PageSize),
End: r.Start + uint64(j*usermem.PageSize),
}
// Advance seg to committedFR.Start.
for seg.Ok() && seg.End() < committedFR.Start {
seg = seg.NextSegment()
}
// Mark pages overlapping committedFR as committed.
for seg.Ok() && seg.Start() < committedFR.End {
if seg.ValuePtr().canCommit() {
seg = f.usage.Isolate(seg, committedFR)
seg.ValuePtr().knownCommitted = true
amount := seg.Range().Length()
usage.MemoryAccounting.Inc(amount, seg.ValuePtr().kind)
f.usageExpected += amount
changedAny = true
// Scan to the end of this committed range.
j := i + 1
for ; j < bufLen; j++ {
if buf[j]&0x1 == 0 {
break
}
}
seg = seg.NextSegment()
committedFR := memmap.FileRange{
Start: r.Start + uint64(i*usermem.PageSize),
End: r.Start + uint64(j*usermem.PageSize),
}
// Advance seg to committedFR.Start.
for seg.Ok() && seg.End() < committedFR.Start {
seg = seg.NextSegment()
}
// Mark pages overlapping committedFR as committed.
for seg.Ok() && seg.Start() < committedFR.End {
if seg.ValuePtr().canCommit() {
seg = f.usage.Isolate(seg, committedFR)
seg.ValuePtr().knownCommitted = true
amount := seg.Range().Length()
usage.MemoryAccounting.Inc(amount, seg.ValuePtr().kind)
f.usageExpected += amount
changedAny = true
}
seg = seg.NextSegment()
}
// Continue scanning for committed pages.
i = j + 1
}
// Continue scanning for committed pages.
i = j + 1
}
// Advance r.Start.
r.Start += uint64(len(s))
})
// Advance r.Start.
r.Start += uint64(len(s))
})
if checkErr != nil {
return checkErr
}
pkg/sentry/vfs/dentry.go
+8
View File
@@ -211,12 +211,14 @@ func (vfs *VirtualFilesystem) PrepareDeleteDentry(mntns *MountNamespace, d *Dent
// AbortDeleteDentry must be called after PrepareDeleteDentry if the deletion
// fails.
// +checklocks:d.mu
func (vfs *VirtualFilesystem) AbortDeleteDentry(d *Dentry) {
d.mu.Unlock()
}
// CommitDeleteDentry must be called after PrepareDeleteDentry if the deletion
// succeeds.
// +checklocks:d.mu
func (vfs *VirtualFilesystem) CommitDeleteDentry(ctx context.Context, d *Dentry) {
d.dead = true
d.mu.Unlock()
@@ -270,6 +272,8 @@ func (vfs *VirtualFilesystem) PrepareRenameDentry(mntns *MountNamespace, from, t
// AbortRenameDentry must be called after PrepareRenameDentry if the rename
// fails.
// +checklocks:from.mu
// +checklocks:to.mu
func (vfs *VirtualFilesystem) AbortRenameDentry(from, to *Dentry) {
from.mu.Unlock()
if to != nil {
@@ -282,6 +286,8 @@ func (vfs *VirtualFilesystem) AbortRenameDentry(from, to *Dentry) {
// that was replaced by from.
//
// Preconditions: PrepareRenameDentry was previously called on from and to.
// +checklocks:from.mu
// +checklocks:to.mu
func (vfs *VirtualFilesystem) CommitRenameReplaceDentry(ctx context.Context, from, to *Dentry) {
from.mu.Unlock()
if to != nil {
@@ -297,6 +303,8 @@ func (vfs *VirtualFilesystem) CommitRenameReplaceDentry(ctx context.Context, fro
// from and to are exchanged by rename(RENAME_EXCHANGE).
//
// Preconditions: PrepareRenameDentry was previously called on from and to.
// +checklocks:from.mu
// +checklocks:to.mu
func (vfs *VirtualFilesystem) CommitRenameExchangeDentry(from, to *Dentry) {
from.mu.Unlock()
to.mu.Unlock()
pkg/sentry/vfs/mount.go
+9 -8
View File
@@ -217,20 +217,21 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
return err
}
vfs.mountMu.Lock()
vd.dentry.mu.Lock()
vdDentry := vd.dentry
vdDentry.mu.Lock()
for {
if vd.dentry.dead {
vd.dentry.mu.Unlock()
if vdDentry.dead {
vdDentry.mu.Unlock()
vfs.mountMu.Unlock()
vd.DecRef(ctx)
return syserror.ENOENT
}
// vd might have been mounted over between vfs.GetDentryAt() and
// vfs.mountMu.Lock().
if !vd.dentry.isMounted() {
if !vdDentry.isMounted() {
break
}
nextmnt := vfs.mounts.Lookup(vd.mount, vd.dentry)
nextmnt := vfs.mounts.Lookup(vd.mount, vdDentry)
if nextmnt == nil {
break
}
@@ -243,13 +244,13 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
}
// This can't fail since we're holding vfs.mountMu.
nextmnt.root.IncRef()
vd.dentry.mu.Unlock()
vdDentry.mu.Unlock()
vd.DecRef(ctx)
vd = VirtualDentry{
mount: nextmnt,
dentry: nextmnt.root,
}
vd.dentry.mu.Lock()
vdDentry.mu.Lock()
}
// TODO(gvisor.dev/issue/1035): Linux requires that either both the mount
// point and the mount root are directories, or neither are, and returns
@@ -258,7 +259,7 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
vfs.mounts.seq.BeginWrite()
vfs.connectLocked(mnt, vd, mntns)
vfs.mounts.seq.EndWrite()
vd.dentry.mu.Unlock()
vdDentry.mu.Unlock()
vfs.mountMu.Unlock()
return nil
}
pkg/sync/mutex_unsafe.go
+1
View File
@@ -72,6 +72,7 @@ func (m *Mutex) Lock() {
// Preconditions:
// * m is locked.
// * m was locked by this goroutine.
// +checklocksignore
func (m *Mutex) Unlock() {
noteUnlock(unsafe.Pointer(m))
m.m.Unlock()
pkg/sync/rwmutex_unsafe.go
+6
View File
@@ -105,6 +105,7 @@ func (rw *CrossGoroutineRWMutex) RUnlock() {
// TryLock locks rw for writing. It returns true if it succeeds and false
// otherwise. It does not block.
// +checklocksignore
func (rw *CrossGoroutineRWMutex) TryLock() bool {
if RaceEnabled {
RaceDisable()
@@ -155,6 +156,7 @@ func (rw *CrossGoroutineRWMutex) Lock() {
//
// Preconditions:
// * rw is locked for writing.
// +checklocksignore
func (rw *CrossGoroutineRWMutex) Unlock() {
if RaceEnabled {
RaceRelease(unsafe.Pointer(&rw.writerSem))
@@ -181,6 +183,7 @@ func (rw *CrossGoroutineRWMutex) Unlock() {
//
// Preconditions:
// * rw is locked for writing.
// +checklocksignore
func (rw *CrossGoroutineRWMutex) DowngradeLock() {
if RaceEnabled {
RaceRelease(unsafe.Pointer(&rw.readerSem))
@@ -250,6 +253,7 @@ func (rw *RWMutex) RLock() {
// Preconditions:
// * rw is locked for reading.
// * rw was locked by this goroutine.
// +checklocksignore
func (rw *RWMutex) RUnlock() {
rw.m.RUnlock()
noteUnlock(unsafe.Pointer(rw))
@@ -279,6 +283,7 @@ func (rw *RWMutex) Lock() {
// Preconditions:
// * rw is locked for writing.
// * rw was locked by this goroutine.
// +checklocksignore
func (rw *RWMutex) Unlock() {
rw.m.Unlock()
noteUnlock(unsafe.Pointer(rw))
@@ -288,6 +293,7 @@ func (rw *RWMutex) Unlock() {
//
// Preconditions:
// * rw is locked for writing.
// +checklocksignore
func (rw *RWMutex) DowngradeLock() {
// No note change for DowngradeLock.
rw.m.DowngradeLock()
pkg/tcpip/transport/tcp/endpoint.go
+2
View File
@@ -760,6 +760,7 @@ func (e *endpoint) LockUser() {
// protocol goroutine altogether.
//
// Precondition: e.LockUser() must have been called before calling e.UnlockUser()
// +checklocks:e.mu
func (e *endpoint) UnlockUser() {
// Lock segment queue before checking so that we avoid a race where
// segments can be queued between the time we check if queue is empty
@@ -800,6 +801,7 @@ func (e *endpoint) StopWork() {
}
// ResumeWork resumes packet processing. Only to be used in tests.
// +checklocks:e.mu
func (e *endpoint) ResumeWork() {
e.mu.Unlock()
}
pkg/tcpip/transport/tcp/endpoint_state.go
+1
View File
@@ -25,6 +25,7 @@ import (
"gvisor.dev/gvisor/pkg/tcpip/stack"
)
// +checklocks:e.mu
func (e *endpoint) drainSegmentLocked() {
// Drain only up to once.
if e.drainDone != nil {
tools/checklocks/BUILD
+16
View File
@@ -0,0 +1,16 @@
load("//tools:defs.bzl", "go_library")
package(licenses = ["notice"])
go_library(
name = "checklocks",
srcs = ["checklocks.go"],
nogo = False,
visibility = ["//tools/nogo:__subpackages__"],
deps = [
"//pkg/log",
"@org_golang_x_tools//go/analysis:go_default_library",
"@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library",
"@org_golang_x_tools//go/ssa:go_default_library",
],
)
tools/checklocks/README.md
+142
View File
@@ -0,0 +1,142 @@
# CheckLocks Analyzer
<!--* freshness: { owner: 'gvisor-eng' reviewed: '2020-10-05' } *-->
Checklocks is a nogo analyzer that at compile time uses Go's static analysis
tools to identify and flag cases where a field that is guarded by a mutex in the
same struct is accessed outside of a mutex lock.
The analyzer relies on explicit '// +checklocks:<mutex-name>' kind of
annotations to identify fields that should be checked for access.
Individual struct members may be protected by annotations that indicate how they
must be accessed. These annotations are of the form:
```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
// Multiple annotations indicate that both must be held but the
// checker does not assert any lock ordering.
// +checklocks:secondMu
// +checklocks:mu
foobar int
}
```
The checklocks annotation may also 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.
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.
```go
// +checklocks:ignore
func newXXX() *X {
...
}
```
***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***.
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.
# Currently not supported
1. The analyzer does not correctly handle deferred functions. e.g The following
code is not correctly checked by the analyzer. The defer call is never
evaluated. As a result if the lock was to be say unlocked twice via deferred
functions it would not be caught by the analyzer.
Similarly deferred anonymous functions are not evaluated either.
```go
type A struct {
mu sync.Mutex
// +checklocks:mu
x int
}
func abc() {
var a A
a.mu.Lock()
defer a.mu.Unlock()
defer a.mu.Unlock()
a.x = 1
}
```
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()
f()
a.mu.Unlock()
}
```
# Explicitly Not Supported
1. Checking for embedded mutexes as sync.Locker rather than directly as
'sync.Mutex'. In other words, the checker will not track mutex Lock and
Unlock() methods where the mutex is behind an interface dispatch.
An example that we won't handle is shown below (this in fact will fail to
build):
```go
type A struct {
mu sync.Locker
// +checklocks:mu
x int
}
func abc() {
mu sync.Mutex
a := A{mu: &mu}
a.x = 1 // This won't be flagged by copylocks checker.
}
```
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.
tools/checklocks/checklocks.go
+761
View File
File diff suppressed because it is too large Load Diff
tools/checklocks/test/BUILD
+8
View File
@@ -0,0 +1,8 @@
load("//tools:defs.bzl", "go_library")
package(licenses = ["notice"])
go_library(
name = "test",
srcs = ["test.go"],
)
tools/checklocks/test/test.go
+362
View File
@@ -0,0 +1,362 @@
// 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.
// Package test is a test package.
package test
import (
"math/rand"
"sync"
)
type oneGuarded struct {
mu sync.Mutex
// +checklocks:mu
guardedField int
unguardedField int
}
func testAccessOne() {
var tc oneGuarded
// Valid access
tc.mu.Lock()
tc.guardedField = 1
tc.unguardedField = 1
tc.mu.Unlock()
// Valid access as unguarded field is not protected by mu.
tc.unguardedField = 2
// Invalid access
tc.guardedField = 2 // +checklocksfail
// Invalid read of a guarded field.
x := tc.guardedField // +checklocksfail
_ = x
}
func testFunctionCallsNoParameters() {
// Couple of regular function calls with no parameters.
funcCallWithValidAccess()
funcCallWithInvalidAccess()
}
func funcCallWithValidAccess() {
var tc2 oneGuarded
// Valid tc2 access
tc2.mu.Lock()
tc2.guardedField = 1
tc2.mu.Unlock()
}
func funcCallWithInvalidAccess() {
var tc oneGuarded
var tc2 oneGuarded
// Invalid access, wrong mutex is held.
tc.mu.Lock()
tc2.guardedField = 2 // +checklocksfail
tc.mu.Unlock()
}
func testParameterPassing() {
var tc oneGuarded
// Valid call where a guardedField is passed to a function as a parameter.
tc.mu.Lock()
nestedWithGuardByAddr(&tc.guardedField, &tc.unguardedField)
tc.mu.Unlock()
// Invalid call where a guardedField is passed to a function as a parameter
// without holding locks.
nestedWithGuardByAddr(&tc.guardedField, &tc.unguardedField) // +checklocksfail
// Valid call where a guardedField is passed to a function as a parameter.
tc.mu.Lock()
nestedWithGuardByValue(tc.guardedField, tc.unguardedField)
tc.mu.Unlock()
// Invalid call where a guardedField is passed to a function as a parameter
// without holding locks.
nestedWithGuardByValue(tc.guardedField, tc.unguardedField) // +checklocksfail
}
func nestedWithGuardByAddr(guardedField, unguardedField *int) {
*guardedField = 4
*unguardedField = 5
}
func nestedWithGuardByValue(guardedField, unguardedField int) {
// read the fields to keep SA4009 static analyzer happy.
_ = guardedField
_ = unguardedField
guardedField = 4
unguardedField = 5
}
type twoGuarded struct {
mu sync.Mutex
// +checklocks:mu
guardedField1 int
// +checklocks:mu
guardedField2 int
}
type twoLocks struct {
mu sync.Mutex
secondMu sync.Mutex
// +checklocks:mu
guardedField1 int
// +checklocks:secondMu
guardedField2 int
}
type twoLocksDoubleGuard struct {
mu sync.Mutex
secondMu sync.Mutex
// +checklocks:mu
// +checklocks:secondMu
doubleGuardedField int
}
func testTwoLocksDoubleGuard() {
var tc twoLocksDoubleGuard
// Double guarded field
tc.mu.Lock()
tc.secondMu.Lock()
tc.doubleGuardedField = 1
tc.secondMu.Unlock()
// This should fail as we released the secondMu.
tc.doubleGuardedField = 2 // +checklocksfail
tc.mu.Unlock()
// This should fail as well as now we are not holding any locks.
//
// This line triggers two failures one for each mutex, hence the 2 after
// fail.
tc.doubleGuardedField = 3 // +checklocksfail:2
}
type rwGuarded struct {
rwMu sync.RWMutex
// +checklocks:rwMu
rwGuardedField int
}
func testRWGuarded() {
var tc rwGuarded
// Assignment w/ exclusive lock should pass.
tc.rwMu.Lock()
tc.rwGuardedField = 1
tc.rwMu.Unlock()
// Assignment w/ RWLock should pass as we don't differentiate between
// Lock/RLock.
tc.rwMu.RLock()
tc.rwGuardedField = 2
tc.rwMu.RUnlock()
// Assignment w/o hold Lock() should fail.
tc.rwGuardedField = 3 // +checklocksfail
// Reading w/o holding lock should fail.
x := tc.rwGuardedField + 3 // +checklocksfail
_ = x
}
type nestedFields struct {
mu sync.Mutex
// +checklocks:mu
nestedStruct struct {
nested1 int
nested2 int
}
}
func testNestedStructGuards() {
var tc nestedFields
// Valid access with mu held.
tc.mu.Lock()
tc.nestedStruct.nested1 = 1
tc.nestedStruct.nested2 = 2
tc.mu.Unlock()
// Invalid access to nested1 wihout holding mu.
tc.nestedStruct.nested1 = 1 // +checklocksfail
}
type testCaseMethods struct {
mu sync.Mutex
// +checklocks:mu
guardedField int
}
func (t *testCaseMethods) Method() {
// Valid access
t.mu.Lock()
t.guardedField = 1
t.mu.Unlock()
// invalid access
t.guardedField = 2 // +checklocksfail
}
// +checklocks:t.mu
func (t *testCaseMethods) MethodLocked(a, b, c int) {
t.guardedField = 3
}
// +checklocksignore
func (t *testCaseMethods) IgnoredMethod() {
// Invalid access but should not fail as the function is annotated
// with "// +checklocksignore"
t.guardedField = 2
}
func testMethodCalls() {
var tc2 testCaseMethods
// Valid use, tc2.Method acquires lock.
tc2.Method()
// Valid access tc2.mu is held before calling tc2.MethodLocked.
tc2.mu.Lock()
tc2.MethodLocked(1, 2, 3)
tc2.mu.Unlock()
// Invalid access no locks are being held.
tc2.MethodLocked(4, 5, 6) // +checklocksfail
}
type noMutex struct {
f int
g int
}
func (n noMutex) method() {
n.f = 1
n.f = n.g
}
func testNoMutex() {
var n noMutex
n.method()
}
func testMultiple() {
var tc1, tc2, tc3 testCaseMethods
tc1.mu.Lock()
// Valid access we are holding tc1's lock.
tc1.guardedField = 1
// Invalid access we are not holding tc2 or tc3's lock.
tc2.guardedField = 2 // +checklocksfail
tc3.guardedField = 3 // +checklocksfail
tc1.mu.Unlock()
}
func testConditionalBranchingLocks() {
var tc2 testCaseMethods
x := rand.Intn(10)
if x%2 == 1 {
tc2.mu.Lock()
}
// This is invalid access as tc2.mu is not held if we never entered
// the if block.
tc2.guardedField = 1 // +checklocksfail
var tc3 testCaseMethods
if x%2 == 1 {
tc3.mu.Lock()
} else {
tc3.mu.Lock()
}
// This is valid as tc3.mu is held in if and else blocks.
tc3.guardedField = 1
}
type testMethodWithParams struct {
mu sync.Mutex
// +checklocks:mu
guardedField int
}
type ptrToTestMethodWithParams *testMethodWithParams
// +checklocks:t.mu
// +checklocks:a.mu
func (t *testMethodWithParams) methodLockedWithParams(a *testMethodWithParams, b *testMethodWithParams) {
t.guardedField = a.guardedField
b.guardedField = a.guardedField // +checklocksfail
}
// +checklocks:t.mu
// +checklocks:a.mu
// +checklocks:b.mu
func (t *testMethodWithParams) methodLockedWithPtrType(a *testMethodWithParams, b ptrToTestMethodWithParams) {
t.guardedField = a.guardedField
b.guardedField = a.guardedField
}
// +checklocks:a.mu
func standaloneFunctionWithGuard(a *testMethodWithParams) {
a.guardedField = 1
a.mu.Unlock()
a.guardedField = 1 // +checklocksfail
}
type testMethodWithEmbedded struct {
mu sync.Mutex
// +checklocks:mu
guardedField int
p *testMethodWithParams
}
// +checklocks:t.mu
func (t *testMethodWithEmbedded) DoLocked() {
var a, b testMethodWithParams
t.guardedField = 1
a.mu.Lock()
b.mu.Lock()
t.p.methodLockedWithParams(&a, &b) // +checklocksfail
a.mu.Unlock()
b.mu.Unlock()
}
// UnsupportedLockerExample is a test that verifies that trying to annotate a
// field that is not a sync.Mutex/RWMutex results in a failure.
type UnsupportedLockerExample struct {
mu sync.Locker
// +checklocks:mu
x int // +checklocksfail
}
func abc() {
var mu sync.Mutex
a := UnsupportedLockerExample{mu: &mu}
a.x = 1
}
tools/nogo/BUILD
+1
View File
@@ -35,6 +35,7 @@ go_library(
visibility = ["//:sandbox"],
deps = [
"//tools/checkescape",
"//tools/checklocks",
"//tools/checkunsafe",
"@co_honnef_go_tools//staticcheck:go_default_library",
"@co_honnef_go_tools//stylecheck:go_default_library",
tools/nogo/analyzers.go
+2
View File
@@ -47,6 +47,7 @@ import (
"honnef.co/go/tools/stylecheck"
"gvisor.dev/gvisor/tools/checkescape"
"gvisor.dev/gvisor/tools/checklocks"
"gvisor.dev/gvisor/tools/checkunsafe"
)
@@ -79,6 +80,7 @@ var AllAnalyzers = []*analysis.Analyzer{
unusedresult.Analyzer,
checkescape.Analyzer,
checkunsafe.Analyzer,
checklocks.Analyzer,
}
// EscapeAnalyzers is a list of escape-related analyzers.