Merge release-20201019.0-34-g9ca66ec59 (automated)

6 files changed, 95 insertions, 130 deletions

diff --git a/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go b/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go
index 9431c1506..2f0dd126e 100644
--- a/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go
+++ b/pkg/sentry/fsimpl/proc/fd_dir_inode_refs.go
@@ -2,11 +2,9 @@ package proc
 
 import (
 	"fmt"
-	"runtime"
 	"sync/atomic"
 
-	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
+	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
 // ownerType is used to customize logging. Note that we use a pointer to T so
@@ -19,11 +17,6 @@ var fdDirInodeownerType *fdDirInode
 // Note that the number of references is actually refCount + 1 so that a default
 // zero-value Refs object contains one reference.
 //
-// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in
-// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount.
-// This will allow us to add stack trace information to the leak messages
-// without growing the size of Refs.
-//
 // +stateify savable
 type fdDirInodeRefs struct {
 	// refCount is composed of two fields:
@@ -36,24 +29,16 @@ type fdDirInodeRefs struct {
 	refCount int64
 }
 
-func (r *fdDirInodeRefs) finalize() {
-	var note string
-	switch refs_vfs1.GetLeakMode() {
-	case refs_vfs1.NoLeakChecking:
-		return
-	case refs_vfs1.UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, fdDirInodeownerType, n)
+// EnableLeakCheck enables reference leak checking on r.
+func (r *fdDirInodeRefs) EnableLeakCheck() {
+	if refsvfs2.LeakCheckEnabled() {
+		refsvfs2.Register(r, fmt.Sprintf("%T", fdDirInodeownerType))
 	}
 }
 
-// EnableLeakCheck checks for reference leaks when Refs gets garbage collected.
-func (r *fdDirInodeRefs) EnableLeakCheck() {
-	if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking {
-		runtime.SetFinalizer(r, (*fdDirInodeRefs).finalize)
-	}
+// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+func (r *fdDirInodeRefs) LeakMessage() string {
+	return fmt.Sprintf("%T %p: reference count of %d instead of 0", fdDirInodeownerType, r, r.ReadRefs())
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -68,7 +53,7 @@ func (r *fdDirInodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *fdDirInodeRefs) IncRef() {
 	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, fdDirInodeownerType))
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, fdDirInodeownerType))
 	}
 }
 
@@ -110,9 +95,18 @@ func (r *fdDirInodeRefs) DecRef(destroy func()) {
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, fdDirInodeownerType))
 
 	case v == -1:
+		if refsvfs2.LeakCheckEnabled() {
+			refsvfs2.Unregister(r, fmt.Sprintf("%T", fdDirInodeownerType))
+		}
 
 		if destroy != nil {
 			destroy()
 		}
 	}
 }
+
+func (r *fdDirInodeRefs) afterLoad() {
+	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+		r.EnableLeakCheck()
+	}
+}
diff --git a/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go b/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go
index 872b20eb0..2065c97b4 100644
--- a/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go
+++ b/pkg/sentry/fsimpl/proc/fd_info_dir_inode_refs.go
@@ -2,11 +2,9 @@ package proc
 
 import (
 	"fmt"
-	"runtime"
 	"sync/atomic"
 
-	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
+	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
 // ownerType is used to customize logging. Note that we use a pointer to T so
@@ -19,11 +17,6 @@ var fdInfoDirInodeownerType *fdInfoDirInode
 // Note that the number of references is actually refCount + 1 so that a default
 // zero-value Refs object contains one reference.
 //
-// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in
-// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount.
-// This will allow us to add stack trace information to the leak messages
-// without growing the size of Refs.
-//
 // +stateify savable
 type fdInfoDirInodeRefs struct {
 	// refCount is composed of two fields:
@@ -36,24 +29,16 @@ type fdInfoDirInodeRefs struct {
 	refCount int64
 }
 
-func (r *fdInfoDirInodeRefs) finalize() {
-	var note string
-	switch refs_vfs1.GetLeakMode() {
-	case refs_vfs1.NoLeakChecking:
-		return
-	case refs_vfs1.UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, fdInfoDirInodeownerType, n)
+// EnableLeakCheck enables reference leak checking on r.
+func (r *fdInfoDirInodeRefs) EnableLeakCheck() {
+	if refsvfs2.LeakCheckEnabled() {
+		refsvfs2.Register(r, fmt.Sprintf("%T", fdInfoDirInodeownerType))
 	}
 }
 
-// EnableLeakCheck checks for reference leaks when Refs gets garbage collected.
-func (r *fdInfoDirInodeRefs) EnableLeakCheck() {
-	if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking {
-		runtime.SetFinalizer(r, (*fdInfoDirInodeRefs).finalize)
-	}
+// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+func (r *fdInfoDirInodeRefs) LeakMessage() string {
+	return fmt.Sprintf("%T %p: reference count of %d instead of 0", fdInfoDirInodeownerType, r, r.ReadRefs())
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -68,7 +53,7 @@ func (r *fdInfoDirInodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *fdInfoDirInodeRefs) IncRef() {
 	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, fdInfoDirInodeownerType))
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, fdInfoDirInodeownerType))
 	}
 }
 
@@ -110,9 +95,18 @@ func (r *fdInfoDirInodeRefs) DecRef(destroy func()) {
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, fdInfoDirInodeownerType))
 
 	case v == -1:
+		if refsvfs2.LeakCheckEnabled() {
+			refsvfs2.Unregister(r, fmt.Sprintf("%T", fdInfoDirInodeownerType))
+		}
 
 		if destroy != nil {
 			destroy()
 		}
 	}
 }
+
+func (r *fdInfoDirInodeRefs) afterLoad() {
+	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+		r.EnableLeakCheck()
+	}
+}
diff --git a/pkg/sentry/fsimpl/proc/proc_state_autogen.go b/pkg/sentry/fsimpl/proc/proc_state_autogen.go
index e17a2a13c..1b5e98012 100644
--- a/pkg/sentry/fsimpl/proc/proc_state_autogen.go
+++ b/pkg/sentry/fsimpl/proc/proc_state_autogen.go
@@ -23,10 +23,9 @@ func (r *fdDirInodeRefs) StateSave(stateSinkObject state.Sink) {
 	stateSinkObject.Save(0, &r.refCount)
 }
 
-func (r *fdDirInodeRefs) afterLoad() {}
-
 func (r *fdDirInodeRefs) StateLoad(stateSourceObject state.Source) {
 	stateSourceObject.Load(0, &r.refCount)
+	stateSourceObject.AfterLoad(r.afterLoad)
 }
 
 func (r *fdInfoDirInodeRefs) StateTypeName() string {
@@ -46,10 +45,9 @@ func (r *fdInfoDirInodeRefs) StateSave(stateSinkObject state.Sink) {
 	stateSinkObject.Save(0, &r.refCount)
 }
 
-func (r *fdInfoDirInodeRefs) afterLoad() {}
-
 func (r *fdInfoDirInodeRefs) StateLoad(stateSourceObject state.Source) {
 	stateSourceObject.Load(0, &r.refCount)
+	stateSourceObject.AfterLoad(r.afterLoad)
 }
 
 func (ft *FilesystemType) StateTypeName() string {
@@ -267,10 +265,9 @@ func (r *subtasksInodeRefs) StateSave(stateSinkObject state.Sink) {
 	stateSinkObject.Save(0, &r.refCount)
 }
 
-func (r *subtasksInodeRefs) afterLoad() {}
-
 func (r *subtasksInodeRefs) StateLoad(stateSourceObject state.Source) {
 	stateSourceObject.Load(0, &r.refCount)
+	stateSourceObject.AfterLoad(r.afterLoad)
 }
 
 func (i *taskInode) StateTypeName() string {
@@ -1101,10 +1098,9 @@ func (r *taskInodeRefs) StateSave(stateSinkObject state.Sink) {
 	stateSinkObject.Save(0, &r.refCount)
 }
 
-func (r *taskInodeRefs) afterLoad() {}
-
 func (r *taskInodeRefs) StateLoad(stateSourceObject state.Source) {
 	stateSourceObject.Load(0, &r.refCount)
+	stateSourceObject.AfterLoad(r.afterLoad)
 }
 
 func (n *ifinet6) StateTypeName() string {
@@ -1747,10 +1743,9 @@ func (r *tasksInodeRefs) StateSave(stateSinkObject state.Sink) {
 	stateSinkObject.Save(0, &r.refCount)
 }
 
-func (r *tasksInodeRefs) afterLoad() {}
-
 func (r *tasksInodeRefs) StateLoad(stateSourceObject state.Source) {
 	stateSourceObject.Load(0, &r.refCount)
+	stateSourceObject.AfterLoad(r.afterLoad)
 }
 
 func (t *tcpMemDir) StateTypeName() string {
diff --git a/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go b/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go
index c6d9b3522..c4c0baf31 100644
--- a/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go
+++ b/pkg/sentry/fsimpl/proc/subtasks_inode_refs.go
@@ -2,11 +2,9 @@ package proc
 
 import (
 	"fmt"
-	"runtime"
 	"sync/atomic"
 
-	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
+	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
 // ownerType is used to customize logging. Note that we use a pointer to T so
@@ -19,11 +17,6 @@ var subtasksInodeownerType *subtasksInode
 // Note that the number of references is actually refCount + 1 so that a default
 // zero-value Refs object contains one reference.
 //
-// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in
-// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount.
-// This will allow us to add stack trace information to the leak messages
-// without growing the size of Refs.
-//
 // +stateify savable
 type subtasksInodeRefs struct {
 	// refCount is composed of two fields:
@@ -36,24 +29,16 @@ type subtasksInodeRefs struct {
 	refCount int64
 }
 
-func (r *subtasksInodeRefs) finalize() {
-	var note string
-	switch refs_vfs1.GetLeakMode() {
-	case refs_vfs1.NoLeakChecking:
-		return
-	case refs_vfs1.UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, subtasksInodeownerType, n)
+// EnableLeakCheck enables reference leak checking on r.
+func (r *subtasksInodeRefs) EnableLeakCheck() {
+	if refsvfs2.LeakCheckEnabled() {
+		refsvfs2.Register(r, fmt.Sprintf("%T", subtasksInodeownerType))
 	}
 }
 
-// EnableLeakCheck checks for reference leaks when Refs gets garbage collected.
-func (r *subtasksInodeRefs) EnableLeakCheck() {
-	if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking {
-		runtime.SetFinalizer(r, (*subtasksInodeRefs).finalize)
-	}
+// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+func (r *subtasksInodeRefs) LeakMessage() string {
+	return fmt.Sprintf("%T %p: reference count of %d instead of 0", subtasksInodeownerType, r, r.ReadRefs())
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -68,7 +53,7 @@ func (r *subtasksInodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *subtasksInodeRefs) IncRef() {
 	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, subtasksInodeownerType))
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, subtasksInodeownerType))
 	}
 }
 
@@ -110,9 +95,18 @@ func (r *subtasksInodeRefs) DecRef(destroy func()) {
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, subtasksInodeownerType))
 
 	case v == -1:
+		if refsvfs2.LeakCheckEnabled() {
+			refsvfs2.Unregister(r, fmt.Sprintf("%T", subtasksInodeownerType))
+		}
 
 		if destroy != nil {
 			destroy()
 		}
 	}
 }
+
+func (r *subtasksInodeRefs) afterLoad() {
+	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+		r.EnableLeakCheck()
+	}
+}
diff --git a/pkg/sentry/fsimpl/proc/task_inode_refs.go b/pkg/sentry/fsimpl/proc/task_inode_refs.go
index 714488450..67638f6ae 100644
--- a/pkg/sentry/fsimpl/proc/task_inode_refs.go
+++ b/pkg/sentry/fsimpl/proc/task_inode_refs.go
@@ -2,11 +2,9 @@ package proc
 
 import (
 	"fmt"
-	"runtime"
 	"sync/atomic"
 
-	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
+	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
 // ownerType is used to customize logging. Note that we use a pointer to T so
@@ -19,11 +17,6 @@ var taskInodeownerType *taskInode
 // Note that the number of references is actually refCount + 1 so that a default
 // zero-value Refs object contains one reference.
 //
-// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in
-// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount.
-// This will allow us to add stack trace information to the leak messages
-// without growing the size of Refs.
-//
 // +stateify savable
 type taskInodeRefs struct {
 	// refCount is composed of two fields:
@@ -36,24 +29,16 @@ type taskInodeRefs struct {
 	refCount int64
 }
 
-func (r *taskInodeRefs) finalize() {
-	var note string
-	switch refs_vfs1.GetLeakMode() {
-	case refs_vfs1.NoLeakChecking:
-		return
-	case refs_vfs1.UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, taskInodeownerType, n)
+// EnableLeakCheck enables reference leak checking on r.
+func (r *taskInodeRefs) EnableLeakCheck() {
+	if refsvfs2.LeakCheckEnabled() {
+		refsvfs2.Register(r, fmt.Sprintf("%T", taskInodeownerType))
 	}
 }
 
-// EnableLeakCheck checks for reference leaks when Refs gets garbage collected.
-func (r *taskInodeRefs) EnableLeakCheck() {
-	if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking {
-		runtime.SetFinalizer(r, (*taskInodeRefs).finalize)
-	}
+// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+func (r *taskInodeRefs) LeakMessage() string {
+	return fmt.Sprintf("%T %p: reference count of %d instead of 0", taskInodeownerType, r, r.ReadRefs())
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -68,7 +53,7 @@ func (r *taskInodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *taskInodeRefs) IncRef() {
 	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, taskInodeownerType))
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, taskInodeownerType))
 	}
 }
 
@@ -110,9 +95,18 @@ func (r *taskInodeRefs) DecRef(destroy func()) {
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, taskInodeownerType))
 
 	case v == -1:
+		if refsvfs2.LeakCheckEnabled() {
+			refsvfs2.Unregister(r, fmt.Sprintf("%T", taskInodeownerType))
+		}
 
 		if destroy != nil {
 			destroy()
 		}
 	}
 }
+
+func (r *taskInodeRefs) afterLoad() {
+	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+		r.EnableLeakCheck()
+	}
+}
diff --git a/pkg/sentry/fsimpl/proc/tasks_inode_refs.go b/pkg/sentry/fsimpl/proc/tasks_inode_refs.go
index 22d9cc488..b882335d7 100644
--- a/pkg/sentry/fsimpl/proc/tasks_inode_refs.go
+++ b/pkg/sentry/fsimpl/proc/tasks_inode_refs.go
@@ -2,11 +2,9 @@ package proc
 
 import (
 	"fmt"
-	"runtime"
 	"sync/atomic"
 
-	"gvisor.dev/gvisor/pkg/log"
-	refs_vfs1 "gvisor.dev/gvisor/pkg/refs"
+	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
 // ownerType is used to customize logging. Note that we use a pointer to T so
@@ -19,11 +17,6 @@ var tasksInodeownerType *tasksInode
 // Note that the number of references is actually refCount + 1 so that a default
 // zero-value Refs object contains one reference.
 //
-// TODO(gvisor.dev/issue/1486): Store stack traces when leak check is enabled in
-// a map with 16-bit hashes, and store the hash in the top 16 bits of refCount.
-// This will allow us to add stack trace information to the leak messages
-// without growing the size of Refs.
-//
 // +stateify savable
 type tasksInodeRefs struct {
 	// refCount is composed of two fields:
@@ -36,24 +29,16 @@ type tasksInodeRefs struct {
 	refCount int64
 }
 
-func (r *tasksInodeRefs) finalize() {
-	var note string
-	switch refs_vfs1.GetLeakMode() {
-	case refs_vfs1.NoLeakChecking:
-		return
-	case refs_vfs1.UninitializedLeakChecking:
-		note = "(Leak checker uninitialized): "
-	}
-	if n := r.ReadRefs(); n != 0 {
-		log.Warningf("%sRefs %p owned by %T garbage collected with ref count of %d (want 0)", note, r, tasksInodeownerType, n)
+// EnableLeakCheck enables reference leak checking on r.
+func (r *tasksInodeRefs) EnableLeakCheck() {
+	if refsvfs2.LeakCheckEnabled() {
+		refsvfs2.Register(r, fmt.Sprintf("%T", tasksInodeownerType))
 	}
 }
 
-// EnableLeakCheck checks for reference leaks when Refs gets garbage collected.
-func (r *tasksInodeRefs) EnableLeakCheck() {
-	if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking {
-		runtime.SetFinalizer(r, (*tasksInodeRefs).finalize)
-	}
+// LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
+func (r *tasksInodeRefs) LeakMessage() string {
+	return fmt.Sprintf("%T %p: reference count of %d instead of 0", tasksInodeownerType, r, r.ReadRefs())
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -68,7 +53,7 @@ func (r *tasksInodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *tasksInodeRefs) IncRef() {
 	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive ref count %p owned by %T", r, tasksInodeownerType))
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, tasksInodeownerType))
 	}
 }
 
@@ -110,9 +95,18 @@ func (r *tasksInodeRefs) DecRef(destroy func()) {
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, tasksInodeownerType))
 
 	case v == -1:
+		if refsvfs2.LeakCheckEnabled() {
+			refsvfs2.Unregister(r, fmt.Sprintf("%T", tasksInodeownerType))
+		}
 
 		if destroy != nil {
 			destroy()
 		}
 	}
 }
+
+func (r *tasksInodeRefs) afterLoad() {
+	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+		r.EnableLeakCheck()
+	}
+}