diff options
Diffstat (limited to 'tools/checklocks')
| -rw-r--r-- | tools/checklocks/BUILD | 21 | ||||
| -rw-r--r-- | tools/checklocks/README.md | 157 | ||||
| -rw-r--r-- | tools/checklocks/analysis.go | 628 | ||||
| -rw-r--r-- | tools/checklocks/annotations.go | 129 | ||||
| -rw-r--r-- | tools/checklocks/checklocks.go | 154 | ||||
| -rw-r--r-- | tools/checklocks/facts.go | 614 | ||||
| -rw-r--r-- | tools/checklocks/state.go | 315 | ||||
| -rw-r--r-- | tools/checklocks/test/BUILD | 20 | ||||
| -rw-r--r-- | tools/checklocks/test/alignment.go | 51 | ||||
| -rw-r--r-- | tools/checklocks/test/atomics.go | 91 | ||||
| -rw-r--r-- | tools/checklocks/test/basics.go | 145 | ||||
| -rw-r--r-- | tools/checklocks/test/branches.go | 56 | ||||
| -rw-r--r-- | tools/checklocks/test/closures.go | 100 | ||||
| -rw-r--r-- | tools/checklocks/test/defer.go | 38 | ||||
| -rw-r--r-- | tools/checklocks/test/incompat.go | 54 | ||||
| -rw-r--r-- | tools/checklocks/test/methods.go | 117 | ||||
| -rw-r--r-- | tools/checklocks/test/parameters.go | 48 | ||||
| -rw-r--r-- | tools/checklocks/test/return.go | 61 | ||||
| -rw-r--r-- | tools/checklocks/test/test.go | 64 |
19 files changed, 0 insertions, 2863 deletions
diff --git a/tools/checklocks/BUILD b/tools/checklocks/BUILD deleted file mode 100644 index d23b7cde6..000000000 --- a/tools/checklocks/BUILD +++ /dev/null @@ -1,21 +0,0 @@ -load("//tools:defs.bzl", "go_library") - -package(licenses = ["notice"]) - -go_library( - name = "checklocks", - srcs = [ - "analysis.go", - "annotations.go", - "checklocks.go", - "facts.go", - "state.go", - ], - nogo = False, - visibility = ["//tools/nogo:__subpackages__"], - deps = [ - "@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", - ], -) diff --git a/tools/checklocks/README.md b/tools/checklocks/README.md deleted file mode 100644 index bd4beb649..000000000 --- a/tools/checklocks/README.md +++ /dev/null @@ -1,157 +0,0 @@ -# CheckLocks Analyzer - -<!--* freshness: { owner: 'gvisor-eng' reviewed: '2021-03-21' } *--> - -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 - -Individual struct members may be noted as requiring atomic access. These -annotations are of the form: - -```go -type foo struct { - // +checkatomic - bar int32 -} -``` - -This will ensure that all accesses to bar are atomic, with the exception of -operations on newly allocated objects. - -## Lock annotations - -Individual struct members may be protected by annotations that indicate locking -requirements for accessing members. 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. 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. - -## Mixed mode - -Some members may allow read-only atomic access, but be protected against writes -by a mutex. Generally, this imposes the following requirements: - -For a read, one of the following must be true: - -1. A lock held be held. -1. The access is atomic. - -For a write, both of the following must be true: - -1. The lock must be held. -1. The write must be atomic. - -In order to annotate a relevant field, simply apply *both* annotations from -above. For example: - -```go -type foo struct { - mu sync.Mutex - // +checklocks:mu - // +checkatomic - bar int32 -} -``` diff --git a/tools/checklocks/analysis.go b/tools/checklocks/analysis.go deleted file mode 100644 index d3fd797d0..000000000 --- a/tools/checklocks/analysis.go +++ /dev/null @@ -1,628 +0,0 @@ -// 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 checklocks - -import ( - "go/token" - "go/types" - "strings" - - "golang.org/x/tools/go/ssa" -) - -func gcd(a, b atomicAlignment) atomicAlignment { - for b != 0 { - a, b = b, a%b - } - return a -} - -// typeAlignment returns the type alignment for the given type. -func (pc *passContext) typeAlignment(pkg *types.Package, obj types.Object) atomicAlignment { - requiredOffset := atomicAlignment(1) - if pc.pass.ImportObjectFact(obj, &requiredOffset) { - return requiredOffset - } - - switch x := obj.Type().Underlying().(type) { - case *types.Struct: - fields := make([]*types.Var, x.NumFields()) - for i := 0; i < x.NumFields(); i++ { - fields[i] = x.Field(i) - } - offsets := pc.pass.TypesSizes.Offsetsof(fields) - for i := 0; i < x.NumFields(); i++ { - // Check the offset, and then assuming that this offset - // aligns with the offset for the broader type. - fieldRequired := pc.typeAlignment(pkg, fields[i]) - if offsets[i]%int64(fieldRequired) != 0 { - // The offset of this field is not compatible. - pc.maybeFail(fields[i].Pos(), "have alignment %d, need %d", offsets[i], fieldRequired) - } - // Ensure the requiredOffset is the LCM of the offset. - requiredOffset *= fieldRequired / gcd(requiredOffset, fieldRequired) - } - case *types.Array: - // Export direct alignment requirements. - if named, ok := x.Elem().(*types.Named); ok { - requiredOffset = pc.typeAlignment(pkg, named.Obj()) - } - default: - // Use the compiler's underlying alignment. - requiredOffset = atomicAlignment(pc.pass.TypesSizes.Alignof(obj.Type().Underlying())) - } - - if pkg == obj.Pkg() { - // Cache as an object fact, to subsequent calls. Note that we - // can only export object facts for the package that we are - // currently analyzing. There may be no exported facts for - // array types or alias types, for example. - pc.pass.ExportObjectFact(obj, &requiredOffset) - } - - return requiredOffset -} - -// checkTypeAlignment checks the alignment of the given type. -// -// This calls typeAlignment, which resolves all types recursively. This method -// should be called for all types individual to ensure full coverage. -func (pc *passContext) checkTypeAlignment(pkg *types.Package, typ *types.Named) { - _ = pc.typeAlignment(pkg, typ.Obj()) -} - -// checkAtomicCall checks for an atomic access. -// -// inst is the instruction analyzed, obj is used only for maybeFail. -// -// If mustBeAtomic is true, then we assert that the instruction *is* an atomic -// fucnction call. If it is false, then we assert that it is *not* an atomic -// dispatch. -// -// If readOnly is true, then only atomic read access are allowed. Note that -// readOnly is only meaningful if mustBeAtomic is set. -func (pc *passContext) checkAtomicCall(inst ssa.Instruction, obj types.Object, mustBeAtomic, readOnly bool) { - switch x := inst.(type) { - case *ssa.Call: - if x.Common().IsInvoke() { - if mustBeAtomic { - // This is an illegal interface dispatch. - pc.maybeFail(inst.Pos(), "dynamic dispatch with atomic-only field") - } - return - } - fn, ok := x.Common().Value.(*ssa.Function) - if !ok { - if mustBeAtomic { - // This is an illegal call to a non-static function. - pc.maybeFail(inst.Pos(), "dispatch to non-static function with atomic-only field") - } - return - } - pkg := fn.Package() - if pkg == nil { - if mustBeAtomic { - // This is a call to some shared wrapper function. - pc.maybeFail(inst.Pos(), "dispatch to shared function or wrapper") - } - return - } - var lff lockFunctionFacts // Check for exemption. - if obj := fn.Object(); obj != nil && pc.pass.ImportObjectFact(obj, &lff) && lff.Ignore { - return - } - if name := pkg.Pkg.Name(); name != "atomic" && name != "atomicbitops" { - if mustBeAtomic { - // This is an illegal call to a non-atomic package function. - pc.maybeFail(inst.Pos(), "dispatch to non-atomic function with atomic-only field") - } - return - } - if !mustBeAtomic { - // We are *not* expecting an atomic dispatch. - if _, ok := pc.forced[pc.positionKey(inst.Pos())]; !ok { - pc.maybeFail(inst.Pos(), "unexpected call to atomic function") - } - } - if !strings.HasPrefix(fn.Name(), "Load") && readOnly { - // We are not allowing any reads in this context. - if _, ok := pc.forced[pc.positionKey(inst.Pos())]; !ok { - pc.maybeFail(inst.Pos(), "unexpected call to atomic write function, is a lock missing?") - } - return - } - default: - if mustBeAtomic { - // This is something else entirely. - if _, ok := pc.forced[pc.positionKey(inst.Pos())]; !ok { - pc.maybeFail(inst.Pos(), "illegal use of atomic-only field by %T instruction", inst) - } - return - } - } -} - -func resolveStruct(typ types.Type) (*types.Struct, bool) { - structType, ok := typ.Underlying().(*types.Struct) - if ok { - return structType, true - } - ptrType, ok := typ.Underlying().(*types.Pointer) - if ok { - return resolveStruct(ptrType.Elem()) - } - return nil, false -} - -func findField(typ types.Type, field int) (types.Object, bool) { - structType, ok := resolveStruct(typ) - if !ok { - return nil, false - } - return structType.Field(field), true -} - -// instructionWithReferrers is a generalization over ssa.Field, ssa.FieldAddr. -type instructionWithReferrers interface { - ssa.Instruction - Referrers() *[]ssa.Instruction -} - -// checkFieldAccess checks the validity of a field access. -// -// This also enforces atomicity constraints for fields that must be accessed -// atomically. The parameter isWrite indicates whether this field is used -// downstream for a write operation. -func (pc *passContext) checkFieldAccess(inst instructionWithReferrers, structObj ssa.Value, field int, ls *lockState, isWrite bool) { - var ( - lff lockFieldFacts - lgf lockGuardFacts - guardsFound int - guardsHeld int - ) - - fieldObj, _ := findField(structObj.Type(), field) - pc.pass.ImportObjectFact(fieldObj, &lff) - pc.pass.ImportObjectFact(fieldObj, &lgf) - - for guardName, fl := range lgf.GuardedBy { - guardsFound++ - r := fl.resolve(structObj) - if _, ok := ls.isHeld(r); ok { - guardsHeld++ - continue - } - if _, ok := pc.forced[pc.positionKey(inst.Pos())]; ok { - // Mark this as locked, since it has been forced. - ls.lockField(r) - guardsHeld++ - continue - } - // Note that we may allow this if the disposition is atomic, - // and we are allowing atomic reads only. This will fall into - // the atomic disposition check below, which asserts that the - // access is atomic. Further, guardsHeld < guardsFound will be - // true for this case, so we require it to be read-only. - if lgf.AtomicDisposition != atomicRequired { - // There is no force key, no atomic access and no lock held. - pc.maybeFail(inst.Pos(), "invalid field access, %s must be locked when accessing %s (locks: %s)", guardName, fieldObj.Name(), ls.String()) - } - } - - // Check the atomic access for this field. - switch lgf.AtomicDisposition { - case atomicRequired: - // Check that this is used safely as an input. - readOnly := guardsHeld < guardsFound - if refs := inst.Referrers(); refs != nil { - for _, otherInst := range *refs { - pc.checkAtomicCall(otherInst, fieldObj, true, readOnly) - } - } - // Check that this is not otherwise written non-atomically, - // even if we do hold all the locks. - if isWrite { - pc.maybeFail(inst.Pos(), "non-atomic write of field %s, writes must still be atomic with locks held (locks: %s)", fieldObj.Name(), ls.String()) - } - case atomicDisallow: - // Check that this is *not* used atomically. - if refs := inst.Referrers(); refs != nil { - for _, otherInst := range *refs { - pc.checkAtomicCall(otherInst, fieldObj, false, false) - } - } - } -} - -func (pc *passContext) checkCall(call callCommon, ls *lockState) { - // See: https://godoc.org/golang.org/x/tools/go/ssa#CallCommon - // - // 1. "call" mode: when Method is nil (!IsInvoke), a CallCommon represents an ordinary - // function call of the value in Value, which may be a *Builtin, a *Function or any - // other value of kind 'func'. - // - // Value may be one of: - // (a) a *Function, indicating a statically dispatched call - // to a package-level function, an anonymous function, or - // a method of a named type. - // - // (b) a *MakeClosure, indicating an immediately applied - // function literal with free variables. - // - // (c) a *Builtin, indicating a statically dispatched call - // to a built-in function. - // - // (d) any other value, indicating a dynamically dispatched - // function call. - switch fn := call.Common().Value.(type) { - case *ssa.Function: - var lff lockFunctionFacts - if fn.Object() != nil { - pc.pass.ImportObjectFact(fn.Object(), &lff) - pc.checkFunctionCall(call, fn, &lff, ls) - } else { - // Anonymous functions have no facts, and cannot be - // annotated. We don't check for violations using the - // function facts, since they cannot exist. Instead, we - // do a fresh analysis using the current lock state. - fnls := ls.fork() - for i, arg := range call.Common().Args { - fnls.store(fn.Params[i], arg) - } - pc.checkFunction(call, fn, &lff, fnls, true /* force */) - } - case *ssa.MakeClosure: - // Note that creating and then invoking closures locally is - // allowed, but analysis of passing closures is done when - // checking individual instructions. - pc.checkClosure(call, fn, ls) - default: - return - } -} - -// postFunctionCallUpdate updates all conditions. -func (pc *passContext) postFunctionCallUpdate(call callCommon, lff *lockFunctionFacts, ls *lockState) { - // Release all locks not still held. - for fieldName, fg := range lff.HeldOnEntry { - if _, ok := lff.HeldOnExit[fieldName]; ok { - continue - } - r := fg.resolveCall(call.Common().Args, call.Value()) - if s, ok := ls.unlockField(r); !ok { - if _, ok := pc.forced[pc.positionKey(call.Pos())]; !ok { - pc.maybeFail(call.Pos(), "attempt to release %s (%s), but not held (locks: %s)", fieldName, s, ls.String()) - } - } - } - - // Update all held locks if acquired. - for fieldName, fg := range lff.HeldOnExit { - if _, ok := lff.HeldOnEntry[fieldName]; ok { - continue - } - // Acquire the lock per the annotation. - r := fg.resolveCall(call.Common().Args, call.Value()) - if s, ok := ls.lockField(r); !ok { - if _, ok := pc.forced[pc.positionKey(call.Pos())]; !ok { - pc.maybeFail(call.Pos(), "attempt to acquire %s (%s), but already held (locks: %s)", fieldName, s, ls.String()) - } - } - } -} - -// checkFunctionCall checks preconditions for function calls, and tracks the -// lock state by recording relevant calls to sync functions. Note that calls to -// atomic functions are tracked by checkFieldAccess by looking directly at the -// referrers (because ordering doesn't matter there, so we need not scan in -// instruction order). -func (pc *passContext) checkFunctionCall(call callCommon, fn *ssa.Function, lff *lockFunctionFacts, ls *lockState) { - // Check all guards required are held. - for fieldName, fg := range lff.HeldOnEntry { - r := fg.resolveCall(call.Common().Args, call.Value()) - if s, ok := ls.isHeld(r); !ok { - if _, ok := pc.forced[pc.positionKey(call.Pos())]; !ok { - pc.maybeFail(call.Pos(), "must hold %s (%s) to call %s, but not held (locks: %s)", fieldName, s, fn.Name(), ls.String()) - } else { - // Force the lock to be acquired. - ls.lockField(r) - } - } - } - - // Update all lock state accordingly. - pc.postFunctionCallUpdate(call, lff, ls) - - // Check if it's a method dispatch for something in the sync package. - // See: https://godoc.org/golang.org/x/tools/go/ssa#Function - if fn.Package() != nil && fn.Package().Pkg.Name() == "sync" && fn.Signature.Recv() != nil { - switch fn.Name() { - case "Lock", "RLock": - if s, ok := ls.lockField(resolvedValue{value: call.Common().Args[0], valid: true}); !ok { - if _, ok := pc.forced[pc.positionKey(call.Pos())]; !ok { - // Double locking a mutex that is already locked. - pc.maybeFail(call.Pos(), "%s already locked (locks: %s)", s, ls.String()) - } - } - case "Unlock", "RUnlock": - if s, ok := ls.unlockField(resolvedValue{value: call.Common().Args[0], valid: true}); !ok { - if _, ok := pc.forced[pc.positionKey(call.Pos())]; !ok { - // Unlocking something that is already unlocked. - pc.maybeFail(call.Pos(), "%s already unlocked (locks: %s)", s, ls.String()) - } - } - } - } -} - -// checkClosure forks the lock state, and creates a binding for the FreeVars of -// the closure. This allows the analysis to resolve the closure. -func (pc *passContext) checkClosure(call callCommon, fn *ssa.MakeClosure, ls *lockState) { - clls := ls.fork() - clfn := fn.Fn.(*ssa.Function) - for i, fv := range clfn.FreeVars { - clls.store(fv, fn.Bindings[i]) - } - - // Note that this is *not* a call to check function call, which checks - // against the function preconditions. Instead, this does a fresh - // analysis of the function from source code with a different state. - var nolff lockFunctionFacts - pc.checkFunction(call, clfn, &nolff, clls, true /* force */) -} - -// freshAlloc indicates that v has been allocated within the local scope. There -// is no lock checking done on objects that are freshly allocated. -func freshAlloc(v ssa.Value) bool { - switch x := v.(type) { - case *ssa.Alloc: - return true - case *ssa.FieldAddr: - return freshAlloc(x.X) - case *ssa.Field: - return freshAlloc(x.X) - case *ssa.IndexAddr: - return freshAlloc(x.X) - case *ssa.Index: - return freshAlloc(x.X) - case *ssa.Convert: - return freshAlloc(x.X) - case *ssa.ChangeType: - return freshAlloc(x.X) - default: - return false - } -} - -// isWrite indicates that this value is used as the addr field in a store. -// -// Note that this may still be used for a write. The return here is optimistic -// but sufficient for basic analysis. -func isWrite(v ssa.Value) bool { - refs := v.Referrers() - if refs == nil { - return false - } - for _, ref := range *refs { - if s, ok := ref.(*ssa.Store); ok && s.Addr == v { - return true - } - } - return false -} - -// callCommon is an ssa.Value that also implements Common. -type callCommon interface { - Pos() token.Pos - Common() *ssa.CallCommon - Value() *ssa.Call -} - -// checkInstruction checks the legality the single instruction based on the -// current lockState. -func (pc *passContext) checkInstruction(inst ssa.Instruction, ls *lockState) (*ssa.Return, *lockState) { - switch x := inst.(type) { - case *ssa.Store: - // Record that this value is holding this other value. This is - // because at the beginning of each ssa execution, there is a - // series of assignments of parameter values to alloc objects. - // This allows us to trace these back to the original - // parameters as aliases above. - // - // Note that this may overwrite an existing value in the lock - // state, but this is intentional. - ls.store(x.Addr, x.Val) - case *ssa.Field: - if !freshAlloc(x.X) { - pc.checkFieldAccess(x, x.X, x.Field, ls, false) - } - case *ssa.FieldAddr: - if !freshAlloc(x.X) { - pc.checkFieldAccess(x, x.X, x.Field, ls, isWrite(x)) - } - case *ssa.Call: - pc.checkCall(x, ls) - case *ssa.Defer: - ls.pushDefer(x) - case *ssa.RunDefers: - for d := ls.popDefer(); d != nil; d = ls.popDefer() { - pc.checkCall(d, ls) - } - case *ssa.MakeClosure: - refs := x.Referrers() - if refs == nil { - // This is strange, it's not used? Ignore this case, - // since it will probably be optimized away. - return nil, nil - } - hasNonCall := false - for _, ref := range *refs { - switch ref.(type) { - case *ssa.Call, *ssa.Defer: - // Analysis will be done on the call itself - // subsequently, including the lock state at - // the time of the call. - default: - // We need to analyze separately. Per below, - // this means that we'll analyze at closure - // construction time no zero assumptions about - // when it will be called. - hasNonCall = true - } - } - if !hasNonCall { - return nil, nil - } - // Analyze the closure without bindings. This means that we - // assume no lock facts or have any existing lock state. Only - // trivial closures are acceptable in this case. - clfn := x.Fn.(*ssa.Function) - var nolff lockFunctionFacts - pc.checkFunction(nil, clfn, &nolff, nil, false /* force */) - case *ssa.Return: - return x, ls // Valid return state. - } - return nil, nil -} - -// checkBasicBlock traverses the control flow graph starting at a set of given -// block and checks each instruction for allowed operations. -func (pc *passContext) checkBasicBlock(fn *ssa.Function, block *ssa.BasicBlock, lff *lockFunctionFacts, parent *lockState, seen map[*ssa.BasicBlock]*lockState) *lockState { - if oldLS, ok := seen[block]; ok && oldLS.isCompatible(parent) { - return nil - } - - // If the lock state is not compatible, then we need to do the - // recursive analysis to ensure that it is still sane. For example, the - // following is guaranteed to generate incompatible locking states: - // - // if foo { - // mu.Lock() - // } - // other stuff ... - // if foo { - // mu.Unlock() - // } - - var ( - rv *ssa.Return - rls *lockState - ) - - // Analyze this block. - seen[block] = parent - ls := parent.fork() - for _, inst := range block.Instrs { - rv, rls = pc.checkInstruction(inst, ls) - if rls != nil { - failed := false - // Validate held locks. - for fieldName, fg := range lff.HeldOnExit { - r := fg.resolveStatic(fn, rv) - if s, ok := rls.isHeld(r); !ok { - if _, ok := pc.forced[pc.positionKey(rv.Pos())]; !ok { - pc.maybeFail(rv.Pos(), "lock %s (%s) not held (locks: %s)", fieldName, s, rls.String()) - failed = true - } else { - // Force the lock to be acquired. - rls.lockField(r) - } - } - } - // Check for other locks, but only if the above didn't trip. - if !failed && rls.count() != len(lff.HeldOnExit) { - pc.maybeFail(rv.Pos(), "return with unexpected locks held (locks: %s)", rls.String()) - } - } - } - - // Analyze all successors. - for _, succ := range block.Succs { - // Collect possible return values, and make sure that the lock - // state aligns with any return value that we may have found - // above. Note that checkBasicBlock will recursively analyze - // the lock state to ensure that Releases and Acquires are - // respected. - if pls := pc.checkBasicBlock(fn, succ, lff, ls, seen); pls != nil { - if rls != nil && !rls.isCompatible(pls) { - if _, ok := pc.forced[pc.positionKey(fn.Pos())]; !ok { - pc.maybeFail(fn.Pos(), "incompatible return states (first: %s, second: %v)", rls.String(), pls.String()) - } - } - rls = pls - } - } - return rls -} - -// checkFunction checks a function invocation, typically starting with nil lockState. -func (pc *passContext) checkFunction(call callCommon, fn *ssa.Function, lff *lockFunctionFacts, parent *lockState, force bool) { - defer func() { - // Mark this function as checked. This is used by the top-level - // loop to ensure that all anonymous functions are scanned, if - // they are not explicitly invoked here. Note that this can - // happen if the anonymous functions are e.g. passed only as - // parameters or used to initialize some structure. - pc.functions[fn] = struct{}{} - }() - if _, ok := pc.functions[fn]; !force && ok { - // This function has already been analyzed at least once. - // That's all we permit for each function, although this may - // cause some anonymous functions to be analyzed in only one - // context. - return - } - - // If no return value is provided, then synthesize one. This is used - // below only to check against the locks preconditions, which may - // include return values. - if call == nil { - call = &ssa.Call{Call: ssa.CallCommon{Value: fn}} - } - - // Initialize ls with any preconditions that require locks to be held - // for the method to be invoked. Note that in the overwhleming majority - // of cases, parent will be nil. However, in the case of closures and - // anonymous functions, we may start with a non-nil lock state. - ls := parent.fork() - for fieldName, fg := range lff.HeldOnEntry { - // The first is the method object itself so we skip that when looking - // for receiver/function parameters. - r := fg.resolveStatic(fn, call.Value()) - if s, ok := ls.lockField(r); !ok { - // This can only happen if the same value is declared - // multiple times, and should be caught by the earlier - // fact scanning. Keep it here as a sanity check. - pc.maybeFail(fn.Pos(), "lock %s (%s) acquired multiple times (locks: %s)", fieldName, s, ls.String()) - } - } - - // Scan the blocks. - seen := make(map[*ssa.BasicBlock]*lockState) - if len(fn.Blocks) > 0 { - pc.checkBasicBlock(fn, fn.Blocks[0], lff, ls, seen) - } - - // Scan the recover block. - if fn.Recover != nil { - pc.checkBasicBlock(fn, fn.Recover, lff, ls, seen) - } - - // Update all lock state accordingly. This will be called only if we - // are doing inline analysis for e.g. an anonymous function. - if call != nil && parent != nil { - pc.postFunctionCallUpdate(call, lff, parent) - } -} diff --git a/tools/checklocks/annotations.go b/tools/checklocks/annotations.go deleted file mode 100644 index 371260980..000000000 --- a/tools/checklocks/annotations.go +++ /dev/null @@ -1,129 +0,0 @@ -// 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 checklocks - -import ( - "fmt" - - "go/token" - "strconv" - "strings" -) - -const ( - checkLocksAnnotation = "// +checklocks:" - checkLocksAcquires = "// +checklocksacquire:" - checkLocksReleases = "// +checklocksrelease:" - checkLocksIgnore = "// +checklocksignore" - checkLocksForce = "// +checklocksforce" - checkLocksFail = "// +checklocksfail" - checkAtomicAnnotation = "// +checkatomic" -) - -// failData indicates an expected failure. -type failData struct { - pos token.Pos - count int - seen int -} - -// positionKey is a simple position string. -type positionKey string - -// positionKey converts from a token.Pos to a key we can use to track failures -// as the position of the failure annotation is not the same as the position of -// the actual failure (different column/offsets). Hence we ignore these fields -// and only use the file/line numbers to track failures. -func (pc *passContext) positionKey(pos token.Pos) positionKey { - position := pc.pass.Fset.Position(pos) - return positionKey(fmt.Sprintf("%s:%d", position.Filename, position.Line)) -} - -// addFailures adds an expected failure. -func (pc *passContext) addFailures(pos token.Pos, s string) { - count := 1 - if len(s) > 0 && s[0] == ':' { - parsedCount, err := strconv.Atoi(s[1:]) - if err != nil { - pc.pass.Reportf(pos, "unable to parse failure annotation %q: %v", s[1:], err) - return - } - count = parsedCount - } - pc.failures[pc.positionKey(pos)] = &failData{ - pos: pos, - count: count, - } -} - -// addExemption adds an exemption. -func (pc *passContext) addExemption(pos token.Pos) { - pc.exemptions[pc.positionKey(pos)] = struct{}{} -} - -// addForce adds a force annotation. -func (pc *passContext) addForce(pos token.Pos) { - pc.forced[pc.positionKey(pos)] = struct{}{} -} - -// maybeFail checks a potential failure against a specific failure map. -func (pc *passContext) maybeFail(pos token.Pos, fmtStr string, args ...interface{}) { - if fd, ok := pc.failures[pc.positionKey(pos)]; ok { - fd.seen++ - return - } - if _, ok := pc.exemptions[pc.positionKey(pos)]; ok { - return // Ignored, not counted. - } - pc.pass.Reportf(pos, fmtStr, args...) -} - -// checkFailure checks for the expected failure counts. -func (pc *passContext) checkFailures() { - for _, fd := range pc.failures { - if fd.count != fd.seen { - // We are missing expect failures, report as much as possible. - pc.pass.Reportf(fd.pos, "got %d failures, want %d failures", fd.seen, fd.count) - } - } -} - -// extractAnnotations extracts annotations from text. -func (pc *passContext) extractAnnotations(s string, fns map[string]func(p string)) { - for prefix, fn := range fns { - if strings.HasPrefix(s, prefix) { - fn(s[len(prefix):]) - } - } -} - -// extractLineFailures extracts all line-based exceptions. -// -// Note that this applies only to individual line exemptions, and does not -// consider function-wide exemptions, or specific field exemptions, which are -// extracted separately as part of the saved facts for those objects. -func (pc *passContext) extractLineFailures() { - for _, f := range pc.pass.Files { - for _, cg := range f.Comments { - for _, c := range cg.List { - pc.extractAnnotations(c.Text, map[string]func(string){ - checkLocksFail: func(p string) { pc.addFailures(c.Pos(), p) }, - checkLocksIgnore: func(string) { pc.addExemption(c.Pos()) }, - checkLocksForce: func(string) { pc.addForce(c.Pos()) }, - }) - } - } - } -} diff --git a/tools/checklocks/checklocks.go b/tools/checklocks/checklocks.go deleted file mode 100644 index 180f8873f..000000000 --- a/tools/checklocks/checklocks.go +++ /dev/null @@ -1,154 +0,0 @@ -// 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 checklocks performs lock analysis to identify and flag unprotected -// access to annotated fields. -// -// For detailed usage refer to README.md in the same directory. -package checklocks - -import ( - "go/ast" - "go/token" - "go/types" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/buildssa" - "golang.org/x/tools/go/ssa" -) - -// Analyzer is the main entrypoint. -var Analyzer = &analysis.Analyzer{ - Name: "checklocks", - Doc: "checks lock preconditions on functions and fields", - Run: run, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, - FactTypes: []analysis.Fact{(*atomicAlignment)(nil), (*lockFieldFacts)(nil), (*lockGuardFacts)(nil), (*lockFunctionFacts)(nil)}, -} - -// passContext is a pass with additional expected failures. -type passContext struct { - pass *analysis.Pass - failures map[positionKey]*failData - exemptions map[positionKey]struct{} - forced map[positionKey]struct{} - functions map[*ssa.Function]struct{} -} - -// forAllTypes applies the given function over all types. -func (pc *passContext) forAllTypes(fn func(ts *ast.TypeSpec)) { - for _, f := range pc.pass.Files { - for _, decl := range f.Decls { - d, ok := decl.(*ast.GenDecl) - if !ok || d.Tok != token.TYPE { - continue - } - for _, gs := range d.Specs { - fn(gs.(*ast.TypeSpec)) - } - } - } -} - -// forAllFunctions applies the given function over all functions. -func (pc *passContext) forAllFunctions(fn func(fn *ast.FuncDecl)) { - for _, f := range pc.pass.Files { - for _, decl := range f.Decls { - d, ok := decl.(*ast.FuncDecl) - if !ok { - continue - } - fn(d) - } - } -} - -// run is the main entrypoint. -func run(pass *analysis.Pass) (interface{}, error) { - pc := &passContext{ - pass: pass, - failures: make(map[positionKey]*failData), - exemptions: make(map[positionKey]struct{}), - forced: make(map[positionKey]struct{}), - functions: make(map[*ssa.Function]struct{}), - } - - // Find all line failure annotations. - pc.extractLineFailures() - - // Find all struct declarations and export relevant facts. - pc.forAllTypes(func(ts *ast.TypeSpec) { - if ss, ok := ts.Type.(*ast.StructType); ok { - pc.exportLockFieldFacts(ts, ss) - } - }) - pc.forAllTypes(func(ts *ast.TypeSpec) { - if ss, ok := ts.Type.(*ast.StructType); ok { - pc.exportLockGuardFacts(ts, ss) - } - }) - - // Check all alignments. - pc.forAllTypes(func(ts *ast.TypeSpec) { - typ, ok := pass.TypesInfo.TypeOf(ts.Name).(*types.Named) - if !ok { - return - } - pc.checkTypeAlignment(pass.Pkg, typ) - }) - - // Find all function declarations and export relevant facts. - pc.forAllFunctions(func(fn *ast.FuncDecl) { - pc.exportFunctionFacts(fn) - }) - - // Scan all code looking for invalid accesses. - state := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA) - for _, fn := range state.SrcFuncs { - // Import function facts generated above. - // - // Note that anonymous(closures) functions do not have an - // object but do show up in the SSA. They can only be invoked - // by named functions in the package, and they are analyzing - // inline on every call. Thus we skip the analysis here. They - // will be hit on calls, or picked up in the pass below. - if obj := fn.Object(); obj == nil { - continue - } - var lff lockFunctionFacts - pc.pass.ImportObjectFact(fn.Object(), &lff) - - // Do we ignore this? - if lff.Ignore { - continue - } - - // Check the basic blocks in the function. - pc.checkFunction(nil, fn, &lff, nil, false /* force */) - } - for _, fn := range state.SrcFuncs { - // Ensure all anonymous functions are hit. They are not - // permitted to have any lock preconditions. - if obj := fn.Object(); obj != nil { - continue - } - var nolff lockFunctionFacts - pc.checkFunction(nil, fn, &nolff, nil, false /* force */) - } - - // Check for expected failures. - pc.checkFailures() - - return nil, nil -} diff --git a/tools/checklocks/facts.go b/tools/checklocks/facts.go deleted file mode 100644 index 1a43dbbe6..000000000 --- a/tools/checklocks/facts.go +++ /dev/null @@ -1,614 +0,0 @@ -// 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 checklocks - -import ( - "fmt" - "go/ast" - "go/token" - "go/types" - "regexp" - "strings" - - "golang.org/x/tools/go/ssa" -) - -// atomicAlignment is saved per type. -// -// This represents the alignment required for the type, which may -// be implied and imposed by other types within the aggregate type. -type atomicAlignment int - -// AFact implements analysis.Fact.AFact. -func (*atomicAlignment) AFact() {} - -// atomicDisposition is saved per field. -// -// This represents how the field must be accessed. It must either -// be non-atomic (default), atomic or ignored. -type atomicDisposition int - -const ( - atomicDisallow atomicDisposition = iota - atomicIgnore - atomicRequired -) - -// fieldList is a simple list of fields, used in two types below. -// -// Note that the integers in this list refer to one of two things: -// - A positive integer refers to a field index in a struct. -// - A negative integer refers to a field index in a struct, where -// that field is a pointer and must be subsequently resolved. -type fieldList []int - -// resolvedValue is an ssa.Value with additional fields. -// -// This can be resolved to a string as part of a lock state. -type resolvedValue struct { - value ssa.Value - valid bool - fieldList []int -} - -// findExtract finds a relevant extract. This must exist within the referrers -// to the call object. If this doesn't then the object which is locked is never -// consumed, and we should consider this a bug. -func findExtract(v ssa.Value, index int) (ssa.Value, bool) { - if refs := v.Referrers(); refs != nil { - for _, inst := range *refs { - if x, ok := inst.(*ssa.Extract); ok && x.Tuple == v && x.Index == index { - return inst.(ssa.Value), true - } - } - } - return nil, false -} - -// resolve resolves the given field list. -func (fl fieldList) resolve(v ssa.Value) (rv resolvedValue) { - return resolvedValue{ - value: v, - fieldList: fl, - valid: true, - } -} - -// valueAsString returns a string representing this value. -// -// This must align with how the string is generated in valueAsString. -func (rv resolvedValue) valueAsString(ls *lockState) string { - typ := rv.value.Type() - s := ls.valueAsString(rv.value) - for i, fieldNumber := range rv.fieldList { - switch { - case fieldNumber > 0: - field, ok := findField(typ, fieldNumber-1) - if !ok { - // This can't be resolved, return for debugging. - return fmt.Sprintf("{%s+%v}", s, rv.fieldList[i:]) - } - s = fmt.Sprintf("&(%s.%s)", s, field.Name()) - typ = field.Type() - case fieldNumber < 1: - field, ok := findField(typ, (-fieldNumber)-1) - if !ok { - // See above. - return fmt.Sprintf("{%s+%v}", s, rv.fieldList[i:]) - } - s = fmt.Sprintf("*(&(%s.%s))", s, field.Name()) - typ = field.Type() - } - } - return s -} - -// lockFieldFacts apply on every struct field. -type lockFieldFacts struct { - // IsMutex is true if the field is of type sync.Mutex. - IsMutex bool - - // IsRWMutex is true if the field is of type sync.RWMutex. - IsRWMutex bool - - // IsPointer indicates if the field is a pointer. - IsPointer bool - - // FieldNumber is the number of this field in the struct. - FieldNumber int -} - -// AFact implements analysis.Fact.AFact. -func (*lockFieldFacts) AFact() {} - -// lockGuardFacts contains guard information. -type lockGuardFacts struct { - // GuardedBy is the set of locks that are guarding this field. The key - // is the original annotation value, and the field list is the object - // traversal path. - GuardedBy map[string]fieldList - - // AtomicDisposition is the disposition for this field. Note that this - // can affect the interpretation of the GuardedBy field above, see the - // relevant comment. - AtomicDisposition atomicDisposition -} - -// AFact implements analysis.Fact.AFact. -func (*lockGuardFacts) AFact() {} - -// functionGuard is used by lockFunctionFacts, below. -type functionGuard struct { - // ParameterNumber is the index of the object that contains the - // guarding mutex. From this parameter, a walk is performed - // subsequently using the resolve method. - // - // Note that is ParameterNumber is beyond the size of parameters, then - // it may return to a return value. This applies only for the Acquires - // relation below. - ParameterNumber int - - // NeedsExtract is used in the case of a return value, and indicates - // that the field must be extracted from a tuple. - NeedsExtract bool - - // FieldList is the traversal path to the object. - FieldList fieldList -} - -// resolveReturn resolves a return value. -// -// Precondition: rv is either an ssa.Value, or an *ssa.Return. -func (fg *functionGuard) resolveReturn(rv interface{}, args int) resolvedValue { - if rv == nil { - // For defers and other objects, this may be nil. This is - // handled in state.go in the actual lock checking logic. - return resolvedValue{ - value: nil, - valid: false, - } - } - index := fg.ParameterNumber - args - // If this is a *ssa.Return object, i.e. we are analyzing the function - // and not the call site, then we can just pull the result directly. - if r, ok := rv.(*ssa.Return); ok { - return fg.FieldList.resolve(r.Results[index]) - } - if fg.NeedsExtract { - // Resolve on the extracted field, this is necessary if the - // type here is not an explicit return. Note that rv must be an - // ssa.Value, since it is not an *ssa.Return. - v, ok := findExtract(rv.(ssa.Value), index) - if !ok { - return resolvedValue{ - value: v, - valid: false, - } - } - return fg.FieldList.resolve(v) - } - if index != 0 { - // This should not happen, NeedsExtract should always be set. - panic("NeedsExtract is false, but return value index is non-zero") - } - // Resolve on the single return. - return fg.FieldList.resolve(rv.(ssa.Value)) -} - -// resolveStatic returns an ssa.Value representing the given field. -// -// Precondition: per resolveReturn. -func (fg *functionGuard) resolveStatic(fn *ssa.Function, rv interface{}) resolvedValue { - if fg.ParameterNumber >= len(fn.Params) { - return fg.resolveReturn(rv, len(fn.Params)) - } - return fg.FieldList.resolve(fn.Params[fg.ParameterNumber]) -} - -// resolveCall returns an ssa.Value representing the given field. -func (fg *functionGuard) resolveCall(args []ssa.Value, rv ssa.Value) resolvedValue { - if fg.ParameterNumber >= len(args) { - return fg.resolveReturn(rv, len(args)) - } - return fg.FieldList.resolve(args[fg.ParameterNumber]) -} - -// lockFunctionFacts apply on every method. -type lockFunctionFacts struct { - // HeldOnEntry tracks the names and number of parameter (including receiver) - // lockFuncfields that guard calls to this function. - // - // The key is the name specified in the checklocks annotation. e.g given - // the following code: - // - // ``` - // type A struct { - // mu sync.Mutex - // a int - // } - // - // // +checklocks:a.mu - // func xyz(a *A) {..} - // ``` - // - // '`+checklocks:a.mu' will result in an entry in this map as shown below. - // HeldOnEntry: {"a.mu" => {ParameterNumber: 0, FieldNumbers: {0}} - // - // Unlikely lockFieldFacts, there is no atomic interpretation. - HeldOnEntry map[string]functionGuard - - // HeldOnExit tracks the locks that are expected to be held on exit. - HeldOnExit map[string]functionGuard - - // Ignore means this function has local analysis ignores. - // - // This is not used outside the local package. - Ignore bool -} - -// AFact implements analysis.Fact.AFact. -func (*lockFunctionFacts) AFact() {} - -// checkGuard validates the guardName. -func (lff *lockFunctionFacts) checkGuard(pc *passContext, d *ast.FuncDecl, guardName string, allowReturn bool) (functionGuard, bool) { - if _, ok := lff.HeldOnEntry[guardName]; ok { - pc.maybeFail(d.Pos(), "annotation %s specified more than once, already required", guardName) - return functionGuard{}, false - } - if _, ok := lff.HeldOnExit[guardName]; ok { - pc.maybeFail(d.Pos(), "annotation %s specified more than once, already acquired", guardName) - return functionGuard{}, false - } - fg, ok := pc.findFunctionGuard(d, guardName, allowReturn) - return fg, ok -} - -// addGuardedBy adds a field to both HeldOnEntry and HeldOnExit. -func (lff *lockFunctionFacts) addGuardedBy(pc *passContext, d *ast.FuncDecl, guardName string) { - if fg, ok := lff.checkGuard(pc, d, guardName, false /* allowReturn */); ok { - if lff.HeldOnEntry == nil { - lff.HeldOnEntry = make(map[string]functionGuard) - } - if lff.HeldOnExit == nil { - lff.HeldOnExit = make(map[string]functionGuard) - } - lff.HeldOnEntry[guardName] = fg - lff.HeldOnExit[guardName] = fg - } -} - -// addAcquires adds a field to HeldOnExit. -func (lff *lockFunctionFacts) addAcquires(pc *passContext, d *ast.FuncDecl, guardName string) { - if fg, ok := lff.checkGuard(pc, d, guardName, true /* allowReturn */); ok { - if lff.HeldOnExit == nil { - lff.HeldOnExit = make(map[string]functionGuard) - } - lff.HeldOnExit[guardName] = fg - } -} - -// addReleases adds a field to HeldOnEntry. -func (lff *lockFunctionFacts) addReleases(pc *passContext, d *ast.FuncDecl, guardName string) { - if fg, ok := lff.checkGuard(pc, d, guardName, false /* allowReturn */); ok { - if lff.HeldOnEntry == nil { - lff.HeldOnEntry = make(map[string]functionGuard) - } - lff.HeldOnEntry[guardName] = fg - } -} - -// fieldListFor returns the fieldList for the given object. -func (pc *passContext) fieldListFor(pos token.Pos, fieldObj types.Object, index int, fieldName string, checkMutex bool) (int, bool) { - var lff lockFieldFacts - if !pc.pass.ImportObjectFact(fieldObj, &lff) { - // This should not happen: we export facts for all fields. - panic(fmt.Sprintf("no lockFieldFacts available for field %s", fieldName)) - } - // Check that it is indeed a mutex. - if checkMutex && !lff.IsMutex && !lff.IsRWMutex { - pc.maybeFail(pos, "field %s is not a mutex or an rwmutex", fieldName) - return 0, false - } - // Return the resolution path. - if lff.IsPointer { - return -(index + 1), true - } - return (index + 1), true -} - -// resolveOneField resolves a field in a single struct. -func (pc *passContext) resolveOneField(pos token.Pos, structType *types.Struct, fieldName string, checkMutex bool) (fl fieldList, fieldObj types.Object, ok bool) { - // Scan to match the next field. - for i := 0; i < structType.NumFields(); i++ { - fieldObj := structType.Field(i) - if fieldObj.Name() != fieldName { - continue - } - flOne, ok := pc.fieldListFor(pos, fieldObj, i, fieldName, checkMutex) - if !ok { - return nil, nil, false - } - fl = append(fl, flOne) - return fl, fieldObj, true - } - // Is this an embed? - for i := 0; i < structType.NumFields(); i++ { - fieldObj := structType.Field(i) - if !fieldObj.Embedded() { - continue - } - // Is this an embedded struct? - structType, ok := resolveStruct(fieldObj.Type()) - if !ok { - continue - } - // Need to check that there is a resolution path. If there is - // no resolution path that's not a failure: we just continue - // scanning the next embed to find a match. - flEmbed, okEmbed := pc.fieldListFor(pos, fieldObj, i, fieldName, false) - flCont, fieldObjCont, okCont := pc.resolveOneField(pos, structType, fieldName, checkMutex) - if okEmbed && okCont { - fl = append(fl, flEmbed) - fl = append(fl, flCont...) - return fl, fieldObjCont, true - } - } - pc.maybeFail(pos, "field %s does not exist", fieldName) - return nil, nil, false -} - -// resolveField resolves a set of fields given a string, such a 'a.b.c'. -// -// Note that this checks that the final element is a mutex of some kind, and -// will fail appropriately. -func (pc *passContext) resolveField(pos token.Pos, structType *types.Struct, parts []string) (fl fieldList, ok bool) { - for partNumber, fieldName := range parts { - flOne, fieldObj, ok := pc.resolveOneField(pos, structType, fieldName, partNumber >= len(parts)-1 /* checkMutex */) - if !ok { - // Error already reported. - return nil, false - } - fl = append(fl, flOne...) - if partNumber < len(parts)-1 { - // Traverse to the next type. - structType, ok = resolveStruct(fieldObj.Type()) - if !ok { - pc.maybeFail(pos, "invalid intermediate field %s", fieldName) - return fl, false - } - } - } - return fl, true -} - -var ( - mutexRE = regexp.MustCompile("((.*/)|^)sync.(CrossGoroutineMutex|Mutex)") - rwMutexRE = regexp.MustCompile("((.*/)|^)sync.(CrossGoroutineRWMutex|RWMutex)") -) - -// exportLockFieldFacts finds all struct fields that are mutexes, and ensures -// that they are annotated approperly. -// -// This information is consumed subsequently by exportLockGuardFacts, and this -// function must be called first on all structures. -func (pc *passContext) exportLockFieldFacts(ts *ast.TypeSpec, ss *ast.StructType) { - structType := pc.pass.TypesInfo.TypeOf(ts.Name).Underlying().(*types.Struct) - for i := range ss.Fields.List { - lff := &lockFieldFacts{ - FieldNumber: i, - } - // We use HasSuffix below because fieldType can be fully - // qualified with the package name eg for the gvisor sync - // package mutex fields have the type: - // "<package path>/sync/sync.Mutex" - fieldObj := structType.Field(i) - s := fieldObj.Type().String() - switch { - case mutexRE.MatchString(s): - lff.IsMutex = true - case rwMutexRE.MatchString(s): - lff.IsRWMutex = true - } - // Save whether this is a pointer. - _, lff.IsPointer = fieldObj.Type().Underlying().(*types.Pointer) - // We must always export the lockFieldFacts, since traversal - // can take place along any object in the struct. - pc.pass.ExportObjectFact(fieldObj, lff) - } -} - -// exportLockGuardFacts finds all relevant guard information for structures. -// -// This function requires exportLockFieldFacts be called first on all -// structures. -func (pc *passContext) exportLockGuardFacts(ts *ast.TypeSpec, ss *ast.StructType) { - structType := pc.pass.TypesInfo.TypeOf(ts.Name).Underlying().(*types.Struct) - for i, field := range ss.Fields.List { - if field.Doc == nil { - continue - } - var ( - lff lockFieldFacts - lgf lockGuardFacts - ) - pc.pass.ImportObjectFact(structType.Field(i), &lff) - fieldObj := structType.Field(i) - for _, l := range field.Doc.List { - pc.extractAnnotations(l.Text, map[string]func(string){ - checkAtomicAnnotation: func(string) { - switch lgf.AtomicDisposition { - case atomicRequired: - pc.maybeFail(fieldObj.Pos(), "annotation is redundant, already atomic required") - case atomicIgnore: - pc.maybeFail(fieldObj.Pos(), "annotation is contradictory, already atomic ignored") - } - lgf.AtomicDisposition = atomicRequired - }, - checkLocksIgnore: func(string) { - switch lgf.AtomicDisposition { - case atomicIgnore: - pc.maybeFail(fieldObj.Pos(), "annotation is redundant, already atomic ignored") - case atomicRequired: - pc.maybeFail(fieldObj.Pos(), "annotation is contradictory, already atomic required") - } - lgf.AtomicDisposition = atomicIgnore - }, - checkLocksAnnotation: func(guardName string) { - // Check for a duplicate annotation. - if _, ok := lgf.GuardedBy[guardName]; ok { - pc.maybeFail(fieldObj.Pos(), "annotation %s specified more than once", guardName) - return - } - fl, ok := pc.resolveField(fieldObj.Pos(), structType, strings.Split(guardName, ".")) - if ok { - // If we successfully resolved - // the field, then save it. - if lgf.GuardedBy == nil { - lgf.GuardedBy = make(map[string]fieldList) - } - lgf.GuardedBy[guardName] = fl - } - }, - }) - } - // Save only if there is something meaningful. - if len(lgf.GuardedBy) > 0 || lgf.AtomicDisposition != atomicDisallow { - pc.pass.ExportObjectFact(structType.Field(i), &lgf) - } - } -} - -// countFields gives an accurate field count, according for unnamed arguments -// and return values and the compact identifier format. -func countFields(fl []*ast.Field) (count int) { - for _, field := range fl { - if len(field.Names) == 0 { - count++ - continue - } - count += len(field.Names) - } - return -} - -// matchFieldList attempts to match the given field. -func (pc *passContext) matchFieldList(pos token.Pos, fl []*ast.Field, guardName string) (functionGuard, bool) { - parts := strings.Split(guardName, ".") - parameterName := parts[0] - parameterNumber := 0 - for _, field := range fl { - // See countFields, above. - if len(field.Names) == 0 { - parameterNumber++ - continue - } - for _, name := range field.Names { - if name.Name != parameterName { - parameterNumber++ - continue - } - ptrType, ok := pc.pass.TypesInfo.TypeOf(field.Type).Underlying().(*types.Pointer) - if !ok { - // Since mutexes cannot be copied we only care - // about parameters that are pointer types when - // checking for guards. - pc.maybeFail(pos, "parameter name %s does not refer to a pointer type", parameterName) - return functionGuard{}, false - } - structType, ok := ptrType.Elem().Underlying().(*types.Struct) - if !ok { - // Fields can only be in named structures. - pc.maybeFail(pos, "parameter name %s does not refer to a pointer to a struct", parameterName) - return functionGuard{}, false - } - fg := functionGuard{ - ParameterNumber: parameterNumber, - } - fl, ok := pc.resolveField(pos, structType, parts[1:]) - fg.FieldList = fl - return fg, ok // If ok is false, already failed. - } - } - return functionGuard{}, false -} - -// findFunctionGuard identifies the parameter number and field number for a -// particular string of the 'a.b'. -// -// This function will report any errors directly. -func (pc *passContext) findFunctionGuard(d *ast.FuncDecl, guardName string, allowReturn bool) (functionGuard, bool) { - var ( - parameterList []*ast.Field - returnList []*ast.Field - ) - if d.Recv != nil { - parameterList = append(parameterList, d.Recv.List...) - } - if d.Type.Params != nil { - parameterList = append(parameterList, d.Type.Params.List...) - } - if fg, ok := pc.matchFieldList(d.Pos(), parameterList, guardName); ok { - return fg, ok - } - if allowReturn { - if d.Type.Results != nil { - returnList = append(returnList, d.Type.Results.List...) - } - if fg, ok := pc.matchFieldList(d.Pos(), returnList, guardName); ok { - // Fix this up to apply to the return value, as noted - // in fg.ParameterNumber. For the ssa analysis, we must - // record whether this has multiple results, since - // *ssa.Call indicates: "The Call instruction yields - // the function result if there is exactly one. - // Otherwise it returns a tuple, the components of - // which are accessed via Extract." - fg.ParameterNumber += countFields(parameterList) - fg.NeedsExtract = countFields(returnList) > 1 - return fg, ok - } - } - // We never saw a matching parameter. - pc.maybeFail(d.Pos(), "annotation %s does not have a matching parameter", guardName) - return functionGuard{}, false -} - -// exportFunctionFacts exports relevant function findings. -func (pc *passContext) exportFunctionFacts(d *ast.FuncDecl) { - if d.Doc == nil || d.Doc.List == nil { - return - } - var lff lockFunctionFacts - for _, l := range d.Doc.List { - pc.extractAnnotations(l.Text, map[string]func(string){ - checkLocksIgnore: func(string) { - // Note that this applies to all atomic - // analysis as well. There is no provided way - // to selectively ignore only lock analysis or - // atomic analysis, as we expect this use to be - // extremely rare. - lff.Ignore = true - }, - checkLocksAnnotation: func(guardName string) { lff.addGuardedBy(pc, d, guardName) }, - checkLocksAcquires: func(guardName string) { lff.addAcquires(pc, d, guardName) }, - checkLocksReleases: func(guardName string) { lff.addReleases(pc, d, guardName) }, - }) - } - - // Export the function facts if there is anything to save. - if lff.Ignore || len(lff.HeldOnEntry) > 0 || len(lff.HeldOnExit) > 0 { - funcObj := pc.pass.TypesInfo.Defs[d.Name].(*types.Func) - pc.pass.ExportObjectFact(funcObj, &lff) - } -} diff --git a/tools/checklocks/state.go b/tools/checklocks/state.go deleted file mode 100644 index 57061a32e..000000000 --- a/tools/checklocks/state.go +++ /dev/null @@ -1,315 +0,0 @@ -// 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 checklocks - -import ( - "fmt" - "go/token" - "go/types" - "strings" - "sync/atomic" - - "golang.org/x/tools/go/ssa" -) - -// lockState tracks the locking state and aliases. -type lockState struct { - // lockedMutexes is used to track which mutexes in a given struct are - // currently locked. Note that most of the heavy lifting is done by - // valueAsString below, which maps to specific structure fields, etc. - lockedMutexes []string - - // stored stores values that have been stored in memory, bound to - // FreeVars or passed as Parameterse. - stored map[ssa.Value]ssa.Value - - // used is a temporary map, used only for valueAsString. It prevents - // multiple use of the same memory location. - used map[ssa.Value]struct{} - - // defers are the stack of defers that have been pushed. - defers []*ssa.Defer - - // refs indicates the number of references on this structure. If it's - // greater than one, we will do copy-on-write. - refs *int32 -} - -// newLockState makes a new lockState. -func newLockState() *lockState { - refs := int32(1) // Not shared. - return &lockState{ - lockedMutexes: make([]string, 0), - used: make(map[ssa.Value]struct{}), - stored: make(map[ssa.Value]ssa.Value), - defers: make([]*ssa.Defer, 0), - refs: &refs, - } -} - -// fork forks the locking state. When a lockState is forked, any modifications -// will cause maps to be copied. -func (l *lockState) fork() *lockState { - if l == nil { - return newLockState() - } - atomic.AddInt32(l.refs, 1) - return &lockState{ - lockedMutexes: l.lockedMutexes, - used: make(map[ssa.Value]struct{}), - stored: l.stored, - defers: l.defers, - refs: l.refs, - } -} - -// modify indicates that this state will be modified. -func (l *lockState) modify() { - if atomic.LoadInt32(l.refs) > 1 { - // Copy the lockedMutexes. - lm := make([]string, len(l.lockedMutexes)) - copy(lm, l.lockedMutexes) - l.lockedMutexes = lm - - // Copy the stored values. - s := make(map[ssa.Value]ssa.Value) - for k, v := range l.stored { - s[k] = v - } - l.stored = s - - // Reset the used values. - l.used = make(map[ssa.Value]struct{}) - - // Copy the defers. - ds := make([]*ssa.Defer, len(l.defers)) - copy(ds, l.defers) - l.defers = ds - - // Drop our reference. - atomic.AddInt32(l.refs, -1) - newRefs := int32(1) // Not shared. - l.refs = &newRefs - } -} - -// isHeld indicates whether the field is held is not. -func (l *lockState) isHeld(rv resolvedValue) (string, bool) { - if !rv.valid { - return rv.valueAsString(l), false - } - s := rv.valueAsString(l) - for _, k := range l.lockedMutexes { - if k == s { - return s, true - } - } - return s, false -} - -// lockField locks the given field. -// -// If false is returned, the field was already locked. -func (l *lockState) lockField(rv resolvedValue) (string, bool) { - if !rv.valid { - return rv.valueAsString(l), false - } - s := rv.valueAsString(l) - for _, k := range l.lockedMutexes { - if k == s { - return s, false - } - } - l.modify() - l.lockedMutexes = append(l.lockedMutexes, s) - return s, true -} - -// unlockField unlocks the given field. -// -// If false is returned, the field was not locked. -func (l *lockState) unlockField(rv resolvedValue) (string, bool) { - if !rv.valid { - return rv.valueAsString(l), false - } - s := rv.valueAsString(l) - for i, k := range l.lockedMutexes { - if k == s { - // Copy the last lock in and truncate. - l.modify() - l.lockedMutexes[i] = l.lockedMutexes[len(l.lockedMutexes)-1] - l.lockedMutexes = l.lockedMutexes[:len(l.lockedMutexes)-1] - return s, true - } - } - return s, false -} - -// store records an alias. -func (l *lockState) store(addr ssa.Value, v ssa.Value) { - l.modify() - l.stored[addr] = v -} - -// isSubset indicates other holds all the locks held by l. -func (l *lockState) isSubset(other *lockState) bool { - held := 0 // Number in l, held by other. - for _, k := range l.lockedMutexes { - for _, ok := range other.lockedMutexes { - if k == ok { - held++ - break - } - } - } - return held >= len(l.lockedMutexes) -} - -// count indicates the number of locks held. -func (l *lockState) count() int { - return len(l.lockedMutexes) -} - -// isCompatible returns true if the states are compatible. -func (l *lockState) isCompatible(other *lockState) bool { - return l.isSubset(other) && other.isSubset(l) -} - -// elemType is a type that implements the Elem function. -type elemType interface { - Elem() types.Type -} - -// valueAsString returns a string for a given value. -// -// This decomposes the value into the simplest possible representation in terms -// of parameters, free variables and globals. During resolution, stored values -// may be transferred, as well as bound free variables. -// -// Nil may not be passed here. -func (l *lockState) valueAsString(v ssa.Value) string { - switch x := v.(type) { - case *ssa.Parameter: - // Was this provided as a paramter for a local anonymous - // function invocation? - v, ok := l.stored[x] - if ok { - return l.valueAsString(v) - } - return fmt.Sprintf("{param:%s}", x.Name()) - case *ssa.Global: - return fmt.Sprintf("{global:%s}", x.Name()) - case *ssa.FreeVar: - // Attempt to resolve this, in case we are being invoked in a - // scope where all the variables are bound. - v, ok := l.stored[x] - if ok { - // The FreeVar is typically bound to a location, so we - // check what's been stored there. Note that the second - // may map to the same FreeVar, which we can check. - stored, ok := l.stored[v] - if ok { - return l.valueAsString(stored) - } - } - return fmt.Sprintf("{freevar:%s}", x.Name()) - case *ssa.Convert: - // Just disregard conversion. - return l.valueAsString(x.X) - case *ssa.ChangeType: - // Ditto, disregard. - return l.valueAsString(x.X) - case *ssa.UnOp: - if x.Op != token.MUL { - break - } - // Is this loading a free variable? If yes, then this can be - // resolved in the original isAlias function. - if fv, ok := x.X.(*ssa.FreeVar); ok { - return l.valueAsString(fv) - } - // Should be try to resolve via a memory address? This needs to - // be done since a memory location can hold its own value. - if _, ok := l.used[x.X]; !ok { - // Check if we know what the accessed location holds. - // This is used to disambiguate memory locations. - v, ok := l.stored[x.X] - if ok { - l.used[x.X] = struct{}{} - defer func() { delete(l.used, x.X) }() - return l.valueAsString(v) - } - } - // x.X.Type is pointer. We must construct this type - // dynamically, since the ssa.Value could be synthetic. - return fmt.Sprintf("*(%s)", l.valueAsString(x.X)) - case *ssa.Field: - structType, ok := resolveStruct(x.X.Type()) - if !ok { - // This should not happen. - panic(fmt.Sprintf("structType not available for struct: %#v", x.X)) - } - fieldObj := structType.Field(x.Field) - return fmt.Sprintf("%s.%s", l.valueAsString(x.X), fieldObj.Name()) - case *ssa.FieldAddr: - structType, ok := resolveStruct(x.X.Type()) - if !ok { - // This should not happen. - panic(fmt.Sprintf("structType not available for struct: %#v", x.X)) - } - fieldObj := structType.Field(x.Field) - return fmt.Sprintf("&(%s.%s)", l.valueAsString(x.X), fieldObj.Name()) - case *ssa.Index: - return fmt.Sprintf("%s[%s]", l.valueAsString(x.X), l.valueAsString(x.Index)) - case *ssa.IndexAddr: - return fmt.Sprintf("&(%s[%s])", l.valueAsString(x.X), l.valueAsString(x.Index)) - case *ssa.Lookup: - return fmt.Sprintf("%s[%s]", l.valueAsString(x.X), l.valueAsString(x.Index)) - case *ssa.Extract: - return fmt.Sprintf("%s[%d]", l.valueAsString(x.Tuple), x.Index) - } - - // In the case of any other type (e.g. this may be an alloc, a return - // value, etc.), just return the literal pointer value to the Value. - // This will be unique within the ssa graph, and so if two values are - // equal, they are from the same type. - return fmt.Sprintf("{%T:%p}", v, v) -} - -// String returns the full lock state. -func (l *lockState) String() string { - if l.count() == 0 { - return "no locks held" - } - return strings.Join(l.lockedMutexes, ",") -} - -// pushDefer pushes a defer onto the stack. -func (l *lockState) pushDefer(d *ssa.Defer) { - l.modify() - l.defers = append(l.defers, d) -} - -// popDefer pops a defer from the stack. -func (l *lockState) popDefer() *ssa.Defer { - // Does not technically modify the underlying slice. - count := len(l.defers) - if count == 0 { - return nil - } - d := l.defers[count-1] - l.defers = l.defers[:count-1] - return d -} diff --git a/tools/checklocks/test/BUILD b/tools/checklocks/test/BUILD deleted file mode 100644 index 966bbac22..000000000 --- a/tools/checklocks/test/BUILD +++ /dev/null @@ -1,20 +0,0 @@ -load("//tools:defs.bzl", "go_library") - -package(licenses = ["notice"]) - -go_library( - name = "test", - srcs = [ - "alignment.go", - "atomics.go", - "basics.go", - "branches.go", - "closures.go", - "defer.go", - "incompat.go", - "methods.go", - "parameters.go", - "return.go", - "test.go", - ], -) diff --git a/tools/checklocks/test/alignment.go b/tools/checklocks/test/alignment.go deleted file mode 100644 index cd857ff73..000000000 --- a/tools/checklocks/test/alignment.go +++ /dev/null @@ -1,51 +0,0 @@ -// 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 - -type alignedStruct32 struct { - v int32 -} - -type alignedStruct64 struct { - v int64 -} - -type alignedStructGood struct { - v0 alignedStruct32 - v1 alignedStruct32 - v2 alignedStruct64 -} - -type alignedStructGoodArray0 struct { - v0 [3]alignedStruct32 - v1 [3]alignedStruct32 - v2 alignedStruct64 -} - -type alignedStructGoodArray1 [16]alignedStructGood - -type alignedStructBad struct { - v0 alignedStruct32 - v1 alignedStruct64 - v2 alignedStruct32 -} - -type alignedStructBadArray0 struct { - v0 [3]alignedStruct32 - v1 [2]alignedStruct64 - v2 [1]alignedStruct32 -} - -type alignedStructBadArray1 [16]alignedStructBad diff --git a/tools/checklocks/test/atomics.go b/tools/checklocks/test/atomics.go deleted file mode 100644 index 8e060d8a2..000000000 --- a/tools/checklocks/test/atomics.go +++ /dev/null @@ -1,91 +0,0 @@ -// 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 - -import ( - "sync" - "sync/atomic" -) - -type atomicStruct struct { - accessedNormally int32 - - // +checkatomic - accessedAtomically int32 - - // +checklocksignore - ignored int32 -} - -func testNormalAccess(tc *atomicStruct, v chan int32, p chan *int32) { - v <- tc.accessedNormally - p <- &tc.accessedNormally -} - -func testAtomicAccess(tc *atomicStruct, v chan int32) { - v <- atomic.LoadInt32(&tc.accessedAtomically) -} - -func testAtomicAccessInvalid(tc *atomicStruct, v chan int32) { - v <- atomic.LoadInt32(&tc.accessedNormally) // +checklocksfail -} - -func testNormalAccessInvalid(tc *atomicStruct, v chan int32, p chan *int32) { - v <- tc.accessedAtomically // +checklocksfail - p <- &tc.accessedAtomically // +checklocksfail -} - -func testIgnored(tc *atomicStruct, v chan int32, p chan *int32) { - v <- atomic.LoadInt32(&tc.ignored) - v <- tc.ignored - p <- &tc.ignored -} - -type atomicMixedStruct struct { - mu sync.Mutex - - // +checkatomic - // +checklocks:mu - accessedMixed int32 -} - -func testAtomicMixedValidRead(tc *atomicMixedStruct, v chan int32) { - v <- atomic.LoadInt32(&tc.accessedMixed) -} - -func testAtomicMixedInvalidRead(tc *atomicMixedStruct, v chan int32, p chan *int32) { - v <- tc.accessedMixed // +checklocksfail - p <- &tc.accessedMixed // +checklocksfail -} - -func testAtomicMixedValidLockedWrite(tc *atomicMixedStruct, v chan int32, p chan *int32) { - tc.mu.Lock() - atomic.StoreInt32(&tc.accessedMixed, 1) - tc.mu.Unlock() -} - -func testAtomicMixedInvalidLockedWrite(tc *atomicMixedStruct, v chan int32, p chan *int32) { - tc.mu.Lock() - tc.accessedMixed = 1 // +checklocksfail:2 - tc.mu.Unlock() -} - -func testAtomicMixedInvalidAtomicWrite(tc *atomicMixedStruct, v chan int32, p chan *int32) { - atomic.StoreInt32(&tc.accessedMixed, 1) // +checklocksfail -} - -func testAtomicMixedInvalidWrite(tc *atomicMixedStruct, v chan int32, p chan *int32) { - tc.accessedMixed = 1 // +checklocksfail:2 -} diff --git a/tools/checklocks/test/basics.go b/tools/checklocks/test/basics.go deleted file mode 100644 index 7a773171f..000000000 --- a/tools/checklocks/test/basics.go +++ /dev/null @@ -1,145 +0,0 @@ -// 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 - -import ( - "sync" -) - -func testLockedAccessValid(tc *oneGuardStruct) { - tc.mu.Lock() - tc.guardedField = 1 - tc.mu.Unlock() -} - -func testLockedAccessIgnore(tc *oneGuardStruct) { - tc.mu.Lock() - tc.unguardedField = 1 - tc.mu.Unlock() -} - -func testUnlockedAccessInvalidWrite(tc *oneGuardStruct) { - tc.guardedField = 2 // +checklocksfail -} - -func testUnlockedAccessInvalidRead(tc *oneGuardStruct) { - x := tc.guardedField // +checklocksfail - _ = x -} - -func testUnlockedAccessValid(tc *oneGuardStruct) { - tc.unguardedField = 2 -} - -func testCallValidAccess(tc *oneGuardStruct) { - callValidAccess(tc) -} - -func callValidAccess(tc *oneGuardStruct) { - tc.mu.Lock() - tc.guardedField = 1 - tc.mu.Unlock() -} - -func testCallValueMixup(tc *oneGuardStruct) { - callValueMixup(tc, tc) -} - -func callValueMixup(tc1, tc2 *oneGuardStruct) { - tc1.mu.Lock() - tc2.guardedField = 2 // +checklocksfail - tc1.mu.Unlock() -} - -func testCallPreconditionsInvalid(tc *oneGuardStruct) { - callPreconditions(tc) // +checklocksfail -} - -func testCallPreconditionsValid(tc *oneGuardStruct) { - tc.mu.Lock() - callPreconditions(tc) - tc.mu.Unlock() -} - -// +checklocks:tc.mu -func callPreconditions(tc *oneGuardStruct) { - tc.guardedField = 1 -} - -type nestedFieldsStruct struct { - mu sync.Mutex - - // +checklocks:mu - nestedStruct struct { - nested1 int - nested2 int - } -} - -func testNestedGuardValid(tc *nestedFieldsStruct) { - tc.mu.Lock() - tc.nestedStruct.nested1 = 1 - tc.nestedStruct.nested2 = 2 - tc.mu.Unlock() -} - -func testNestedGuardInvalid(tc *nestedFieldsStruct) { - tc.nestedStruct.nested1 = 1 // +checklocksfail -} - -type rwGuardStruct struct { - rwMu sync.RWMutex - - // +checklocks:rwMu - guardedField int -} - -func testRWValidRead(tc *rwGuardStruct) { - tc.rwMu.Lock() - tc.guardedField = 1 - tc.rwMu.Unlock() -} - -func testRWValidWrite(tc *rwGuardStruct) { - tc.rwMu.RLock() - tc.guardedField = 2 - tc.rwMu.RUnlock() -} - -func testRWInvalidWrite(tc *rwGuardStruct) { - tc.guardedField = 3 // +checklocksfail -} - -func testRWInvalidRead(tc *rwGuardStruct) { - x := tc.guardedField + 3 // +checklocksfail - _ = x -} - -func testTwoLocksDoubleGuardStructValid(tc *twoLocksDoubleGuardStruct) { - tc.mu.Lock() - tc.secondMu.Lock() - tc.doubleGuardedField = 1 - tc.secondMu.Unlock() -} - -func testTwoLocksDoubleGuardStructOnlyOne(tc *twoLocksDoubleGuardStruct) { - tc.mu.Lock() - tc.doubleGuardedField = 2 // +checklocksfail - tc.mu.Unlock() -} - -func testTwoLocksDoubleGuardStructInvalid(tc *twoLocksDoubleGuardStruct) { - tc.doubleGuardedField = 3 // +checklocksfail:2 -} diff --git a/tools/checklocks/test/branches.go b/tools/checklocks/test/branches.go deleted file mode 100644 index 81fec29e5..000000000 --- a/tools/checklocks/test/branches.go +++ /dev/null @@ -1,56 +0,0 @@ -// 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 - -import ( - "math/rand" -) - -func testInconsistentReturn(tc *oneGuardStruct) { // +checklocksfail - if x := rand.Intn(10); x%2 == 1 { - tc.mu.Lock() - } -} - -func testConsistentBranching(tc *oneGuardStruct) { - x := rand.Intn(10) - if x%2 == 1 { - tc.mu.Lock() - } else { - tc.mu.Lock() - } - tc.guardedField = 1 - if x%2 == 1 { - tc.mu.Unlock() - } else { - tc.mu.Unlock() - } -} - -func testInconsistentBranching(tc *oneGuardStruct) { // +checklocksfail:2 - // We traverse the control flow graph in all consistent ways. We cannot - // determine however, that the first if block and second if block will - // evaluate to the same condition. Therefore, there are two consistent - // paths through this code, and two inconsistent paths. Either way, the - // guardedField should be also marked as an invalid access. - x := rand.Intn(10) - if x%2 == 1 { - tc.mu.Lock() - } - tc.guardedField = 1 // +checklocksfail - if x%2 == 1 { - tc.mu.Unlock() // +checklocksforce - } -} diff --git a/tools/checklocks/test/closures.go b/tools/checklocks/test/closures.go deleted file mode 100644 index 7da87540a..000000000 --- a/tools/checklocks/test/closures.go +++ /dev/null @@ -1,100 +0,0 @@ -// 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 - -func testClosureInvalid(tc *oneGuardStruct) { - // This is expected to fail. - callClosure(func() { - tc.guardedField = 1 // +checklocksfail - }) -} - -func testClosureUnsupported(tc *oneGuardStruct) { - // Locked outside the closure, so may or may not be valid. This cannot - // be handled and we should explicitly fail. This can't be handled - // because of the call through callClosure, below, which means the - // closure will actually be passed as a value somewhere. - tc.mu.Lock() - callClosure(func() { - tc.guardedField = 1 // +checklocksfail - }) - tc.mu.Unlock() -} - -func testClosureValid(tc *oneGuardStruct) { - // All locking happens within the closure. This should not present a - // problem for analysis. - callClosure(func() { - tc.mu.Lock() - tc.guardedField = 1 - tc.mu.Unlock() - }) -} - -func testClosureInline(tc *oneGuardStruct) { - // If the closure is being dispatching inline only, then we should be - // able to analyze this call and give it a thumbs up. - tc.mu.Lock() - func() { - tc.guardedField = 1 - }() - tc.mu.Unlock() -} - -func testAnonymousInvalid(tc *oneGuardStruct) { - // Invalid, as per testClosureInvalid above. - callAnonymous(func(tc *oneGuardStruct) { - tc.guardedField = 1 // +checklocksfail - }, tc) -} - -func testAnonymousUnsupported(tc *oneGuardStruct) { - // Not supportable, as per testClosureUnsupported above. - tc.mu.Lock() - callAnonymous(func(tc *oneGuardStruct) { - tc.guardedField = 1 // +checklocksfail - }, tc) - tc.mu.Unlock() -} - -func testAnonymousValid(tc *oneGuardStruct) { - // Valid, as per testClosureValid above. - callAnonymous(func(tc *oneGuardStruct) { - tc.mu.Lock() - tc.guardedField = 1 - tc.mu.Unlock() - }, tc) -} - -func testAnonymousInline(tc *oneGuardStruct) { - // Unlike the closure case, we are able to dynamically infer the set of - // preconditions for the function dispatch and assert that this is - // a valid call. - tc.mu.Lock() - func(tc *oneGuardStruct) { - tc.guardedField = 1 - }(tc) - tc.mu.Unlock() -} - -//go:noinline -func callClosure(fn func()) { - fn() -} - -//go:noinline -func callAnonymous(fn func(*oneGuardStruct), tc *oneGuardStruct) { - fn(tc) -} diff --git a/tools/checklocks/test/defer.go b/tools/checklocks/test/defer.go deleted file mode 100644 index 6e574e5eb..000000000 --- a/tools/checklocks/test/defer.go +++ /dev/null @@ -1,38 +0,0 @@ -// 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 - -func testDeferValidUnlock(tc *oneGuardStruct) { - tc.mu.Lock() - tc.guardedField = 1 - defer tc.mu.Unlock() -} - -func testDeferValidAccess(tc *oneGuardStruct) { - tc.mu.Lock() - defer func() { - tc.guardedField = 1 - tc.mu.Unlock() - }() -} - -func testDeferInvalidAccess(tc *oneGuardStruct) { - tc.mu.Lock() - defer func() { - // N.B. Executed after tc.mu.Unlock(). - tc.guardedField = 1 // +checklocksfail - }() - tc.mu.Unlock() -} diff --git a/tools/checklocks/test/incompat.go b/tools/checklocks/test/incompat.go deleted file mode 100644 index b39bc66c1..000000000 --- a/tools/checklocks/test/incompat.go +++ /dev/null @@ -1,54 +0,0 @@ -// 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 - -import ( - "sync" -) - -// unsupportedLockerStruct verifies that trying to annotate a field that is not a -// sync.Mutex or sync.RWMutex results in a failure. -type unsupportedLockerStruct struct { - mu sync.Locker - - // +checklocks:mu - x int // +checklocksfail -} - -// badFieldsStruct verifies that refering invalid fields fails. -type badFieldsStruct struct { - // +checklocks:mu - x int // +checklocksfail -} - -// redundantStruct verifies that redundant annotations fail. -type redundantStruct struct { - mu sync.Mutex - - // +checklocks:mu - // +checklocks:mu - x int // +checklocksfail -} - -// conflictsStruct verifies that conflicting annotations fail. -type conflictsStruct struct { - // +checkatomicignore - // +checkatomic - x int // +checklocksfail - - // +checkatomic - // +checkatomicignore - y int // +checklocksfail -} diff --git a/tools/checklocks/test/methods.go b/tools/checklocks/test/methods.go deleted file mode 100644 index 72e26fca6..000000000 --- a/tools/checklocks/test/methods.go +++ /dev/null @@ -1,117 +0,0 @@ -// 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 - -import ( - "sync" -) - -type testMethods struct { - mu sync.Mutex - - // +checklocks:mu - guardedField int -} - -func (t *testMethods) methodValid() { - t.mu.Lock() - t.guardedField = 1 - t.mu.Unlock() -} - -func (t *testMethods) methodInvalid() { - t.guardedField = 2 // +checklocksfail -} - -// +checklocks:t.mu -func (t *testMethods) MethodLocked(a, b, c int) { - t.guardedField = 3 -} - -// +checklocksignore -func (t *testMethods) methodIgnore() { - t.guardedField = 2 -} - -func testMethodCallsValid(tc *testMethods) { - tc.methodValid() -} - -func testMethodCallsValidPreconditions(tc *testMethods) { - tc.mu.Lock() - tc.MethodLocked(1, 2, 3) - tc.mu.Unlock() -} - -func testMethodCallsInvalid(tc *testMethods) { - tc.MethodLocked(4, 5, 6) // +checklocksfail -} - -func testMultipleParameters(tc1, tc2, tc3 *testMethods) { - tc1.mu.Lock() - tc1.guardedField = 1 - tc2.guardedField = 2 // +checklocksfail - tc3.guardedField = 3 // +checklocksfail - tc1.mu.Unlock() -} - -type testMethodsWithParameters struct { - mu sync.Mutex - - // +checklocks:mu - guardedField int -} - -type ptrToTestMethodsWithParameters *testMethodsWithParameters - -// +checklocks:t.mu -// +checklocks:a.mu -func (t *testMethodsWithParameters) methodLockedWithParameters(a *testMethodsWithParameters, b *testMethodsWithParameters) { - t.guardedField = a.guardedField - b.guardedField = a.guardedField // +checklocksfail -} - -// +checklocks:t.mu -// +checklocks:a.mu -// +checklocks:b.mu -func (t *testMethodsWithParameters) methodLockedWithPtrType(a *testMethodsWithParameters, b ptrToTestMethodsWithParameters) { - t.guardedField = a.guardedField - b.guardedField = a.guardedField -} - -// +checklocks:a.mu -func standaloneFunctionWithGuard(a *testMethodsWithParameters) { - a.guardedField = 1 - a.mu.Unlock() - a.guardedField = 1 // +checklocksfail -} - -type testMethodsWithEmbedded struct { - mu sync.Mutex - - // +checklocks:mu - guardedField int - p *testMethodsWithParameters -} - -// +checklocks:t.mu -func (t *testMethodsWithEmbedded) DoLocked(a, b *testMethodsWithParameters) { - t.guardedField = 1 - a.mu.Lock() - b.mu.Lock() - t.p.methodLockedWithParameters(a, b) // +checklocksfail - a.mu.Unlock() - b.mu.Unlock() -} diff --git a/tools/checklocks/test/parameters.go b/tools/checklocks/test/parameters.go deleted file mode 100644 index 5b9e664b6..000000000 --- a/tools/checklocks/test/parameters.go +++ /dev/null @@ -1,48 +0,0 @@ -// 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 - -func testParameterPassingbyAddrValid(tc *oneGuardStruct) { - tc.mu.Lock() - nestedWithGuardByAddr(&tc.guardedField, &tc.unguardedField) - tc.mu.Unlock() -} - -func testParameterPassingByAddrInalid(tc *oneGuardStruct) { - nestedWithGuardByAddr(&tc.guardedField, &tc.unguardedField) // +checklocksfail -} - -func testParameterPassingByValueValid(tc *oneGuardStruct) { - tc.mu.Lock() - nestedWithGuardByValue(tc.guardedField, tc.unguardedField) - tc.mu.Unlock() -} - -func testParameterPassingByValueInalid(tc *oneGuardStruct) { - 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 -} diff --git a/tools/checklocks/test/return.go b/tools/checklocks/test/return.go deleted file mode 100644 index 47c7b6773..000000000 --- a/tools/checklocks/test/return.go +++ /dev/null @@ -1,61 +0,0 @@ -// 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 - -// +checklocks:tc.mu -func testReturnInvalidGuard() (tc *oneGuardStruct) { // +checklocksfail - return new(oneGuardStruct) -} - -// +checklocksrelease:tc.mu -func testReturnInvalidRelease() (tc *oneGuardStruct) { // +checklocksfail - return new(oneGuardStruct) -} - -// +checklocksacquire:tc.mu -func testReturnInvalidAcquire() (tc *oneGuardStruct) { - return new(oneGuardStruct) // +checklocksfail -} - -// +checklocksacquire:tc.mu -func testReturnValidAcquire() (tc *oneGuardStruct) { - tc = new(oneGuardStruct) - tc.mu.Lock() - return tc -} - -func testReturnAcquireCall() { - tc := testReturnValidAcquire() - tc.guardedField = 1 - tc.mu.Unlock() -} - -// +checklocksacquire:tc.val.mu -// +checklocksacquire:tc.ptr.mu -func testReturnValidNestedAcquire() (tc *nestedGuardStruct) { - tc = new(nestedGuardStruct) - tc.ptr = new(oneGuardStruct) - tc.val.mu.Lock() - tc.ptr.mu.Lock() - return tc -} - -func testReturnNestedAcquireCall() { - tc := testReturnValidNestedAcquire() - tc.val.guardedField = 1 - tc.ptr.guardedField = 1 - tc.val.mu.Unlock() - tc.ptr.mu.Unlock() -} diff --git a/tools/checklocks/test/test.go b/tools/checklocks/test/test.go deleted file mode 100644 index cbf6b1635..000000000 --- a/tools/checklocks/test/test.go +++ /dev/null @@ -1,64 +0,0 @@ -// 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. -// -// Tests are all compilation tests in separate files. -package test - -import ( - "sync" -) - -// oneGuardStruct has one guarded field. -type oneGuardStruct struct { - mu sync.Mutex - // +checklocks:mu - guardedField int - unguardedField int -} - -// twoGuardStruct has two guarded fields. -type twoGuardStruct struct { - mu sync.Mutex - // +checklocks:mu - guardedField1 int - // +checklocks:mu - guardedField2 int -} - -// twoLocksStruct has two locks and two fields. -type twoLocksStruct struct { - mu sync.Mutex - secondMu sync.Mutex - // +checklocks:mu - guardedField1 int - // +checklocks:secondMu - guardedField2 int -} - -// twoLocksDoubleGuardStruct has two locks and a single field with two guards. -type twoLocksDoubleGuardStruct struct { - mu sync.Mutex - secondMu sync.Mutex - // +checklocks:mu - // +checklocks:secondMu - doubleGuardedField int -} - -// nestedGuardStruct nests oneGuardStruct fields. -type nestedGuardStruct struct { - val oneGuardStruct - ptr *oneGuardStruct -} |