Rename WARN* functions to BugTraceback*, and optimize them for inlining.

pkg/log/BUILD

@@ -8,12 +8,12 @@ package(
 go_library(
     name = "log",
     srcs = [
+        "bug.go",
         "glog.go",
         "json.go",
         "json_k8s.go",
         "log.go",
         "rate_limited.go",
-        "warn_on.go",
     ],
     marshal = False,
     stateify = False,
@@ -31,9 +31,9 @@ go_test(
     name = "log_test",
     size = "small",
     srcs = [
+        "bug_test.go",
         "json_test.go",
         "log_test.go",
-        "warn_on_test.go",
     ],
     library = ":log",
 )

pkg/log/warn_on.go → pkg/log/bug.go

@@ -22,13 +22,22 @@ import (
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
+// This file contains helper functions analogous to the Linux kernel's WARN*
+// macros. Should be used for non-fatal errors that should be treated as bugs
+// none the less.
+
 const (
 	warnFmtStr         = "WARNING: BUG on %s:%d\n"
 	warnUnknownLineStr = "WARNING: BUG on unknown line\n"
 	catchAllMagic      = "runtime.Caller failed"
 )
 
-func warn(caller int, msg string) {
+//go:noinline
+func reportBugErr(caller int, err error) {
+	reportBug(caller+1, err.Error(), nil)
+}
+
+func reportBug(caller int, msg string, vars []any) {
 	var b strings.Builder
 	if _, file, line, ok := runtime.Caller(caller); ok {
 		b.WriteString(fmt.Sprintf(warnFmtStr, file, line))
@@ -37,7 +46,11 @@ func warn(caller int, msg string) {
 	}
 	b.WriteByte('\n')
 	if len(msg) > 0 {
-		b.WriteString(msg)
+		if len(vars) > 0 {
+			b.WriteString(fmt.Sprintf(msg, vars...))
+		} else {
+			b.WriteString(msg)
+		}
 		b.WriteByte('\n')
 	}
 	TracebackAll(b.String())
@@ -50,7 +63,12 @@ var (
 	warnedSet map[string]struct{}
 )
 
-func warnOnce(caller int, msg string) {
+//go:noinline
+func reportBugErrOnce(caller int, err error) {
+	reportBugOnce(caller+1, err.Error(), nil)
+}
+
+func reportBugOnce(caller int, msg string, vars []any) {
 	var b strings.Builder
 	if _, file, line, ok := runtime.Caller(caller); ok {
 		key := fmt.Sprintf("%s:%d", file, line)
@@ -62,7 +80,11 @@ func warnOnce(caller int, msg string) {
 			b.WriteString(fmt.Sprintf(warnFmtStr, file, line))
 			b.WriteByte('\n')
 			if len(msg) > 0 {
-				b.WriteString(msg)
+				if len(vars) > 0 {
+					b.WriteString(fmt.Sprintf(msg, vars...))
+				} else {
+					b.WriteString(msg)
+				}
 				b.WriteByte('\n')
 			}
 
@@ -79,7 +101,11 @@ func warnOnce(caller int, msg string) {
 			b.WriteString(warnUnknownLineStr)
 			b.WriteByte('\n')
 			if len(msg) > 0 {
-				b.WriteString(msg)
+				if len(vars) > 0 {
+					b.WriteString(fmt.Sprintf(msg, vars...))
+				} else {
+					b.WriteString(msg)
+				}
 				b.WriteByte('\n')
 			}
 
@@ -89,94 +115,47 @@ func warnOnce(caller int, msg string) {
 	}
 }
 
-// WARN serves the same purpose as the Linux kernel's WARN macro. Use it
-// for reporting abnormal bugs encountered at runtime that should be fixed.
-//
-// Will print out the full Go stacktrace at time of invocation.
+// BugTraceback will report a bug with a traceback of all goroutines if the
+// error isn't nil. Use it for reporting abnormal bugs encountered at runtime
+// that should be fixed.
 //
 // Do not use this for bad user input. Errors reported by this function should
 // not be fatal.
-func WARN(cond bool, s string, a ...any) {
-	if !cond {
-		return
+func BugTraceback(err error) {
+	if err != nil {
+		reportBugErr(2, err)
 	}
-	msg := fmt.Sprintf(s, a...)
-	warn(2, msg)
 }
 
-// WARN_ON serves the same purpose as the Linux kernel's WARN_ON macro. Use it
-// for reporting abnormal bugs encountered at runtime that should be fixed.
-//
-// Will print out the full Go stacktrace at time of invocation.
-//
-// Do not use this for bad user input. Errors reported by this function should
-// not be fatal.
-func WARN_ON(cond bool) {
-	if !cond {
-		return
-	}
-	warn(2, "")
-}
-
-// WARN_ERR is a more Go-friendly version of the typical Linux WARN* macros. If
-// the error isn't nil, it will report the error prefixed with the standard WARN
-// string. Use it for reporting abnormal bugs encountered at runtime that
-// should be fixed.
-//
-// Will print out the full Go stacktrace at time of invocation.
-//
-// Do not use this for bad user input. Errors reported by this function should
-// not be fatal.
-func WARN_ERR(err error) {
-	if err == nil {
-		return
-	}
-	warn(2, err.Error())
-}
-
-// WARN_ONCE serves the same purpose as the Linux kernel's WARN_ONCE macro.
+// BugTracebackf will report a bug with a traceback of all goroutines.
 // Use it for reporting abnormal bugs encountered at runtime that should be
 // fixed.
 //
-// Will print out the full Go stacktrace at time of invocation.
-//
 // Do not use this for bad user input. Errors reported by this function should
 // not be fatal.
-func WARN_ONCE(cond bool, s string, a ...any) {
-	if !cond {
-		return
-	}
-	msg := fmt.Sprintf(s, a...)
-	warnOnce(2, msg)
+func BugTracebackf(s string, a ...any) {
+	reportBug(2, s, a)
 }
 
-// WARN_ON_ONCE serves the same purpose as the Linux kernel's WARN_ON_ONCE macro.
-// Use it for reporting abnormal bugs encountered at runtime that should be
-// fixed.
-//
-// Will print out the full Go stacktrace at time of invocation.
+// BugTracebackOnce will report a bug with a traceback of all goroutines if the
+// error isn't nil. Use it for reporting abnormal bugs encountered at runtime
+// that should be fixed. If called multiple time from same invocation, will only
+// print once.
 //
 // Do not use this for bad user input. Errors reported by this function should
 // not be fatal.
-func WARN_ON_ONCE(cond bool) {
-	if !cond {
-		return
+func BugTracebackOnce(err error) {
+	if err != nil {
+		reportBugErrOnce(2, err)
 	}
-	warnOnce(2, "")
 }
 
-// WARN_ERR_ONCE is a more Go-friendly version of the typical Linux WARN* macros.
-// If the error isn't nil, it will report the error prefixed with the standard
-// WARN string. Use it for reporting abnormal bugs encountered at runtime that
-// should be fixed.
-//
-// Will print out the full Go stacktrace at time of invocation.
+// BugTracebackfOnce will report a bug with a traceback of all goroutines.
+// Use it for reporting abnormal bugs encountered at runtime that should be
+// fixed. If called multiple time from same invocation, will only print once.
 //
 // Do not use this for bad user input. Errors reported by this function should
 // not be fatal.
-func WARN_ERR_ONCE(err error) {
-	if err == nil {
-		return
-	}
-	warnOnce(2, err.Error())
+func BugTracebackfOnce(s string, a ...any) {
+	reportBugOnce(2, s, a)
 }

pkg/log/warn_on_test.go → pkg/log/bug_test.go

@@ -20,6 +20,8 @@ import (
 	"testing"
 )
 
+const expectFile = "pkg/log/bug_test.go"
+
 func TestWarnOn(t *testing.T) {
 	tw := &testWriter{}
 	e := GoogleEmitter{Writer: &Writer{Next: tw}}
@@ -33,104 +35,84 @@ func TestWarnOn(t *testing.T) {
 
 	testCases := map[string]func(t *testing.T){
 		"testConditionControlsPrint": func(t *testing.T) {
-			WARN_ON(false)
+			BugTraceback(nil)
 			if len(tw.lines) > 0 {
-				t.Errorf("WARN_ON printed when it shouldn't have")
+				t.Errorf("BugTraceback printed when it shouldn't have")
 			}
 
-			WARN_ON(true)
+			BugTraceback(fmt.Errorf("error"))
 			if len(tw.lines) == 0 {
-				t.Errorf("WARN_ON didn't print anything when it should have")
+				t.Errorf("BugTraceback didn't print anything when it should have")
 			}
 		},
 		"testStringFormat": func(t *testing.T) {
-			expectFile := "pkg/log/warn_on_test.go"
-			// Don't try to match the line to make this test less
-			// brittle to somebody accidentally sneezing on this file.
-			expectStr := strings.SplitN(warnFmtStr, "%", 2)[0]
-
-			WARN_ON(true)
-
-			if len(tw.lines) == 0 {
-				t.Errorf("WARN_ON didn't print anything when it should have")
-			}
-			if !strings.Contains(tw.lines[0], expectFile) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
-			}
-			if !strings.Contains(tw.lines[0], expectStr) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr, tw.lines[0])
-			}
-		},
-		"testCustomFormat": func(t *testing.T) {
-			expectFile := "pkg/log/warn_on_test.go"
 			expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0]
 			expectStr2 := "This is just a test warning"
-			WARN(true, "This is just a test warning")
+			BugTracebackf("This is just a test warning: %s", "with another string")
 
 			if len(tw.lines) == 0 {
-				t.Errorf("WARN_ON didn't print anything when it should have")
+				t.Errorf("BugTracebackf didn't print anything when it should have")
 			}
 			if !strings.Contains(tw.lines[0], expectFile) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
+				t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
 			}
 			if !strings.Contains(tw.lines[0], expectStr1) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
+				t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
 			}
 			if !strings.Contains(tw.lines[0], expectStr2) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
+				t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
 			}
 		},
 		"testWarnErr": func(t *testing.T) {
-			expectFile := "pkg/log/warn_on_test.go"
 			expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0]
 			expectStr2 := "My little error string"
 			var err error
-			WARN_ERR(err)
+			BugTraceback(err)
 			if len(tw.lines) > 0 {
-				t.Errorf("WARN_ON printed when it shouldn't have")
+				t.Errorf("BugTraceback printed when it shouldn't have")
 			}
 
 			err = fmt.Errorf("My little error string")
-			WARN_ERR(err)
+			BugTraceback(err)
 			if len(tw.lines) == 0 {
-				t.Errorf("WARN_ON didn't print anything when it should have")
+				t.Errorf("BugTraceback didn't print anything when it should have")
 			}
 			if !strings.Contains(tw.lines[0], expectFile) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
+				t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
 			}
 			if !strings.Contains(tw.lines[0], expectStr1) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
+				t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
 			}
 			if !strings.Contains(tw.lines[0], expectStr2) {
-				t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
+				t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
 			}
 		},
 		"testWarnOnceOnlyPrintsOnce": func(t *testing.T) {
 			testHelperFunc := func() {
-				WARN_ON_ONCE(true)
+				BugTracebackfOnce("error")
 			}
 
 			testHelperFunc()
 			if len(tw.lines) == 0 {
-				t.Errorf("WarnOnOnce didn't print anything when it should have")
+				t.Errorf("BugTracebackfOnce didn't print anything when it should have")
 			}
 			tw.clear()
 
 			testHelperFunc()
 			if len(tw.lines) > 0 {
-				t.Errorf("WarnOnOnce printed out a warning a second time when it shouldn't have")
+				t.Errorf("BugTracebackfOnce printed out a warning a second time when it shouldn't have")
 			}
 		},
 		"testWarnOnceDoesntClobberOthers": func(t *testing.T) {
-			WARN_ON_ONCE(true)
+			BugTracebackfOnce("error")
 			if len(tw.lines) == 0 {
-				t.Errorf("First WarnOnOnce didn't print anything when it should have")
+				t.Errorf("First BugTracebackfOnce didn't print anything when it should have")
 			}
 			tw.clear()
 
-			WARN_ON_ONCE(true)
+			BugTracebackfOnce("error")
 			if len(tw.lines) == 0 {
-				t.Errorf("Second WarnOnOnce didn't print anything when it should have")
+				t.Errorf("Second BugTracebackfOnce didn't print anything when it should have")
 			}
 		},
 	}