Mix checklocks and atomic analyzers.

This change makes the checklocks analyzer considerable more powerful, adding:
* The ability to traverse complex structures, e.g. to have multiple nested
  fields as part of the annotation.
* The ability to resolve simple anonymous functions and closures, and perform
  lock analysis across these invocations. This does not apply to closures that
  are passed elsewhere, since it is not possible to know the context in which
  they might be invoked.
* The ability to annotate return values in addition to receivers and other
  parameters, with the same complex structures noted above.
* Ignoring locking semantics for "fresh" objects, i.e. objects that are
  allocated in the local frame (typically a new-style function).
* Sanity checking of locking state across block transitions and returns, to
  ensure that no unexpected locks are held.

Note that initially, most of these findings are excluded by a comprehensive
nogo.yaml. The findings that are included are fundamental lock violations.
The changes here should be relatively low risk, minor refactorings to either
include necessary annotations to simplify the code structure (in general
removing closures in favor of methods) so that the analyzer can be easily
track the lock state.

This change additional includes two changes to nogo itself:
* Sanity checking of all types to ensure that the binary and ast-derived
  types have a consistent objectpath, to prevent the bug above from occurring
  silently (and causing much confusion). This also requires a trick in
  order to ensure that serialized facts are consumable downstream. This can
  be removed with https://go-review.googlesource.com/c/tools/+/331789 merged.
* A minor refactoring to isolation the objdump settings in its own package.
  This was originally used to implement the sanity check above, but this
  information is now being passed another way. The minor refactor is preserved
  however, since it cleans up the code slightly and is minimal risk.

PiperOrigin-RevId: 382613300

12 files changed, 789 insertions, 314 deletions

diff --git a/tools/checklocks/test/BUILD b/tools/checklocks/test/BUILD
index b055e71d9..966bbac22 100644
--- a/tools/checklocks/test/BUILD
+++ b/tools/checklocks/test/BUILD
@@ -4,5 +4,17 @@ package(licenses = ["notice"])
 
 go_library(
     name = "test",
-    srcs = ["test.go"],
+    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
new file mode 100644
index 000000000..cd857ff73
--- /dev/null
+++ b/tools/checklocks/test/alignment.go
@@ -0,0 +1,51 @@
+// 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
new file mode 100644
index 000000000..8e060d8a2
--- /dev/null
+++ b/tools/checklocks/test/atomics.go
@@ -0,0 +1,91 @@
+// 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
new file mode 100644
index 000000000..7a773171f
--- /dev/null
+++ b/tools/checklocks/test/basics.go
@@ -0,0 +1,145 @@
+// 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
new file mode 100644
index 000000000..81fec29e5
--- /dev/null
+++ b/tools/checklocks/test/branches.go
@@ -0,0 +1,56 @@
+// 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
new file mode 100644
index 000000000..7da87540a
--- /dev/null
+++ b/tools/checklocks/test/closures.go
@@ -0,0 +1,100 @@
+// 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
new file mode 100644
index 000000000..6e574e5eb
--- /dev/null
+++ b/tools/checklocks/test/defer.go
@@ -0,0 +1,38 @@
+// 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
new file mode 100644
index 000000000..b39bc66c1
--- /dev/null
+++ b/tools/checklocks/test/incompat.go
@@ -0,0 +1,54 @@
+// 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
new file mode 100644
index 000000000..72e26fca6
--- /dev/null
+++ b/tools/checklocks/test/methods.go
@@ -0,0 +1,117 @@
+// 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
new file mode 100644
index 000000000..5b9e664b6
--- /dev/null
+++ b/tools/checklocks/test/parameters.go
@@ -0,0 +1,48 @@
+// 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
new file mode 100644
index 000000000..47c7b6773
--- /dev/null
+++ b/tools/checklocks/test/return.go
@@ -0,0 +1,61 @@
+// 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
index 05693c183..cbf6b1635 100644
--- a/tools/checklocks/test/test.go
+++ b/tools/checklocks/test/test.go
@@ -13,99 +13,24 @@
 // limitations under the License.
 
 // Package test is a test package.
+//
+// Tests are all compilation tests in separate files.
 package test
 
 import (
-	"math/rand"
 	"sync"
 )
 
-type oneGuarded struct {
+// oneGuardStruct has one guarded field.
+type oneGuardStruct struct {
 	mu sync.Mutex
 	// +checklocks:mu
-	guardedField int
-
+	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 {
+// twoGuardStruct has two guarded fields.
+type twoGuardStruct struct {
 	mu sync.Mutex
 	// +checklocks:mu
 	guardedField1 int
@@ -113,250 +38,27 @@ type twoGuarded struct {
 	guardedField2 int
 }
 
-type twoLocks struct {
+// twoLocksStruct has two locks and two fields.
+type twoLocksStruct struct {
 	mu       sync.Mutex
 	secondMu sync.Mutex
-
 	// +checklocks:mu
 	guardedField1 int
 	// +checklocks:secondMu
 	guardedField2 int
 }
 
-type twoLocksDoubleGuard struct {
+// 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
 }
 
-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
+// nestedGuardStruct nests oneGuardStruct fields.
+type nestedGuardStruct struct {
+	val oneGuardStruct
+	ptr *oneGuardStruct
 }