Skip to content

Commit 2099bc1

Browse files
konstantin-s-bogomgvisor-bot
authored andcommitted
Rename WARN* functions to BugTraceback*, and optimize them for inlining.
The rename is todo with following a more Go-style naming convention. To this end, the "default" BugTraceback will be the one that takes a single error parameter. The simple "cond" variant will likely not be widely used and has been removed. As to inlining the BugTraceback(err) variant seems to require a hack from pkg/sync/gate_unsafe.go:Enter where there is a noinline helper, so as to make it check err == nil before making an expensive err.Error() call. PiperOrigin-RevId: 791443090
1 parent 5917d21 commit 2099bc1

File tree

3 files changed

+79
-118
lines changed

3 files changed

+79
-118
lines changed

pkg/log/BUILD

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ package(
88
go_library(
99
name = "log",
1010
srcs = [
11+
"bug.go",
1112
"glog.go",
1213
"json.go",
1314
"json_k8s.go",
1415
"log.go",
1516
"rate_limited.go",
16-
"warn_on.go",
1717
],
1818
marshal = False,
1919
stateify = False,
@@ -31,9 +31,9 @@ go_test(
3131
name = "log_test",
3232
size = "small",
3333
srcs = [
34+
"bug_test.go",
3435
"json_test.go",
3536
"log_test.go",
36-
"warn_on_test.go",
3737
],
3838
library = ":log",
3939
)

pkg/log/warn_on.go renamed to pkg/log/bug.go

Lines changed: 52 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,22 @@ import (
2222
"gvisor.dev/gvisor/pkg/sync"
2323
)
2424

25+
// This file contains helper functions analogous to the Linux kernel's WARN*
26+
// macros. Should be used for non-fatal errors that should be treated as bugs
27+
// none the less.
28+
2529
const (
2630
warnFmtStr = "WARNING: BUG on %s:%d\n"
2731
warnUnknownLineStr = "WARNING: BUG on unknown line\n"
2832
catchAllMagic = "runtime.Caller failed"
2933
)
3034

31-
func warn(caller int, msg string) {
35+
//go:noinline
36+
func reportBugErr(caller int, err error) {
37+
reportBug(caller+1, err.Error(), nil)
38+
}
39+
40+
func reportBug(caller int, msg string, vars []any) {
3241
var b strings.Builder
3342
if _, file, line, ok := runtime.Caller(caller); ok {
3443
b.WriteString(fmt.Sprintf(warnFmtStr, file, line))
@@ -37,7 +46,11 @@ func warn(caller int, msg string) {
3746
}
3847
b.WriteByte('\n')
3948
if len(msg) > 0 {
40-
b.WriteString(msg)
49+
if len(vars) > 0 {
50+
b.WriteString(fmt.Sprintf(msg, vars...))
51+
} else {
52+
b.WriteString(msg)
53+
}
4154
b.WriteByte('\n')
4255
}
4356
TracebackAll(b.String())
@@ -50,7 +63,12 @@ var (
5063
warnedSet map[string]struct{}
5164
)
5265

53-
func warnOnce(caller int, msg string) {
66+
//go:noinline
67+
func reportBugErrOnce(caller int, err error) {
68+
reportBugOnce(caller+1, err.Error(), nil)
69+
}
70+
71+
func reportBugOnce(caller int, msg string, vars []any) {
5472
var b strings.Builder
5573
if _, file, line, ok := runtime.Caller(caller); ok {
5674
key := fmt.Sprintf("%s:%d", file, line)
@@ -62,7 +80,11 @@ func warnOnce(caller int, msg string) {
6280
b.WriteString(fmt.Sprintf(warnFmtStr, file, line))
6381
b.WriteByte('\n')
6482
if len(msg) > 0 {
65-
b.WriteString(msg)
83+
if len(vars) > 0 {
84+
b.WriteString(fmt.Sprintf(msg, vars...))
85+
} else {
86+
b.WriteString(msg)
87+
}
6688
b.WriteByte('\n')
6789
}
6890

@@ -79,7 +101,11 @@ func warnOnce(caller int, msg string) {
79101
b.WriteString(warnUnknownLineStr)
80102
b.WriteByte('\n')
81103
if len(msg) > 0 {
82-
b.WriteString(msg)
104+
if len(vars) > 0 {
105+
b.WriteString(fmt.Sprintf(msg, vars...))
106+
} else {
107+
b.WriteString(msg)
108+
}
83109
b.WriteByte('\n')
84110
}
85111

@@ -89,94 +115,47 @@ func warnOnce(caller int, msg string) {
89115
}
90116
}
91117

92-
// WARN serves the same purpose as the Linux kernel's WARN macro. Use it
93-
// for reporting abnormal bugs encountered at runtime that should be fixed.
94-
//
95-
// Will print out the full Go stacktrace at time of invocation.
118+
// BugTraceback will report a bug with a traceback of all goroutines if the
119+
// error isn't nil. Use it for reporting abnormal bugs encountered at runtime
120+
// that should be fixed.
96121
//
97122
// Do not use this for bad user input. Errors reported by this function should
98123
// not be fatal.
99-
func WARN(cond bool, s string, a ...any) {
100-
if !cond {
101-
return
124+
func BugTraceback(err error) {
125+
if err != nil {
126+
reportBugErr(2, err)
102127
}
103-
msg := fmt.Sprintf(s, a...)
104-
warn(2, msg)
105128
}
106129

107-
// WARN_ON serves the same purpose as the Linux kernel's WARN_ON macro. Use it
108-
// for reporting abnormal bugs encountered at runtime that should be fixed.
109-
//
110-
// Will print out the full Go stacktrace at time of invocation.
111-
//
112-
// Do not use this for bad user input. Errors reported by this function should
113-
// not be fatal.
114-
func WARN_ON(cond bool) {
115-
if !cond {
116-
return
117-
}
118-
warn(2, "")
119-
}
120-
121-
// WARN_ERR is a more Go-friendly version of the typical Linux WARN* macros. If
122-
// the error isn't nil, it will report the error prefixed with the standard WARN
123-
// string. Use it for reporting abnormal bugs encountered at runtime that
124-
// should be fixed.
125-
//
126-
// Will print out the full Go stacktrace at time of invocation.
127-
//
128-
// Do not use this for bad user input. Errors reported by this function should
129-
// not be fatal.
130-
func WARN_ERR(err error) {
131-
if err == nil {
132-
return
133-
}
134-
warn(2, err.Error())
135-
}
136-
137-
// WARN_ONCE serves the same purpose as the Linux kernel's WARN_ONCE macro.
130+
// BugTracebackf will report a bug with a traceback of all goroutines.
138131
// Use it for reporting abnormal bugs encountered at runtime that should be
139132
// fixed.
140133
//
141-
// Will print out the full Go stacktrace at time of invocation.
142-
//
143134
// Do not use this for bad user input. Errors reported by this function should
144135
// not be fatal.
145-
func WARN_ONCE(cond bool, s string, a ...any) {
146-
if !cond {
147-
return
148-
}
149-
msg := fmt.Sprintf(s, a...)
150-
warnOnce(2, msg)
136+
func BugTracebackf(s string, a ...any) {
137+
reportBug(2, s, a)
151138
}
152139

153-
// WARN_ON_ONCE serves the same purpose as the Linux kernel's WARN_ON_ONCE macro.
154-
// Use it for reporting abnormal bugs encountered at runtime that should be
155-
// fixed.
156-
//
157-
// Will print out the full Go stacktrace at time of invocation.
140+
// BugTracebackOnce will report a bug with a traceback of all goroutines if the
141+
// error isn't nil. Use it for reporting abnormal bugs encountered at runtime
142+
// that should be fixed. If called multiple time from same invocation, will only
143+
// print once.
158144
//
159145
// Do not use this for bad user input. Errors reported by this function should
160146
// not be fatal.
161-
func WARN_ON_ONCE(cond bool) {
162-
if !cond {
163-
return
147+
func BugTracebackOnce(err error) {
148+
if err != nil {
149+
reportBugErrOnce(2, err)
164150
}
165-
warnOnce(2, "")
166151
}
167152

168-
// WARN_ERR_ONCE is a more Go-friendly version of the typical Linux WARN* macros.
169-
// If the error isn't nil, it will report the error prefixed with the standard
170-
// WARN string. Use it for reporting abnormal bugs encountered at runtime that
171-
// should be fixed.
172-
//
173-
// Will print out the full Go stacktrace at time of invocation.
153+
// BugTracebackfOnce will report a bug with a traceback of all goroutines.
154+
// Use it for reporting abnormal bugs encountered at runtime that should be
155+
// fixed. If called multiple time from same invocation, will only print once.
174156
//
175157
// Do not use this for bad user input. Errors reported by this function should
176158
// not be fatal.
177-
func WARN_ERR_ONCE(err error) {
178-
if err == nil {
179-
return
180-
}
181-
warnOnce(2, err.Error())
159+
func BugTracebackfOnce(s string, a ...any) {
160+
reportBugOnce(2, s, a)
182161
}

pkg/log/warn_on_test.go renamed to pkg/log/bug_test.go

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"testing"
2121
)
2222

23+
const expectFile = "pkg/log/bug_test.go"
24+
2325
func TestWarnOn(t *testing.T) {
2426
tw := &testWriter{}
2527
e := GoogleEmitter{Writer: &Writer{Next: tw}}
@@ -33,104 +35,84 @@ func TestWarnOn(t *testing.T) {
3335

3436
testCases := map[string]func(t *testing.T){
3537
"testConditionControlsPrint": func(t *testing.T) {
36-
WARN_ON(false)
38+
BugTraceback(nil)
3739
if len(tw.lines) > 0 {
38-
t.Errorf("WARN_ON printed when it shouldn't have")
40+
t.Errorf("BugTraceback printed when it shouldn't have")
3941
}
4042

41-
WARN_ON(true)
43+
BugTraceback(fmt.Errorf("error"))
4244
if len(tw.lines) == 0 {
43-
t.Errorf("WARN_ON didn't print anything when it should have")
45+
t.Errorf("BugTraceback didn't print anything when it should have")
4446
}
4547
},
4648
"testStringFormat": func(t *testing.T) {
47-
expectFile := "pkg/log/warn_on_test.go"
48-
// Don't try to match the line to make this test less
49-
// brittle to somebody accidentally sneezing on this file.
50-
expectStr := strings.SplitN(warnFmtStr, "%", 2)[0]
51-
52-
WARN_ON(true)
53-
54-
if len(tw.lines) == 0 {
55-
t.Errorf("WARN_ON didn't print anything when it should have")
56-
}
57-
if !strings.Contains(tw.lines[0], expectFile) {
58-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
59-
}
60-
if !strings.Contains(tw.lines[0], expectStr) {
61-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr, tw.lines[0])
62-
}
63-
},
64-
"testCustomFormat": func(t *testing.T) {
65-
expectFile := "pkg/log/warn_on_test.go"
6649
expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0]
6750
expectStr2 := "This is just a test warning"
68-
WARN(true, "This is just a test warning")
51+
BugTracebackf("This is just a test warning: %s", "with another string")
6952

7053
if len(tw.lines) == 0 {
71-
t.Errorf("WARN_ON didn't print anything when it should have")
54+
t.Errorf("BugTracebackf didn't print anything when it should have")
7255
}
7356
if !strings.Contains(tw.lines[0], expectFile) {
74-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
57+
t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
7558
}
7659
if !strings.Contains(tw.lines[0], expectStr1) {
77-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
60+
t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
7861
}
7962
if !strings.Contains(tw.lines[0], expectStr2) {
80-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
63+
t.Errorf("BugTracebackf didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
8164
}
8265
},
8366
"testWarnErr": func(t *testing.T) {
84-
expectFile := "pkg/log/warn_on_test.go"
8567
expectStr1 := strings.SplitN(warnFmtStr, "%", 2)[0]
8668
expectStr2 := "My little error string"
8769
var err error
88-
WARN_ERR(err)
70+
BugTraceback(err)
8971
if len(tw.lines) > 0 {
90-
t.Errorf("WARN_ON printed when it shouldn't have")
72+
t.Errorf("BugTraceback printed when it shouldn't have")
9173
}
9274

9375
err = fmt.Errorf("My little error string")
94-
WARN_ERR(err)
76+
BugTraceback(err)
9577
if len(tw.lines) == 0 {
96-
t.Errorf("WARN_ON didn't print anything when it should have")
78+
t.Errorf("BugTraceback didn't print anything when it should have")
9779
}
9880
if !strings.Contains(tw.lines[0], expectFile) {
99-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
81+
t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectFile, tw.lines[0])
10082
}
10183
if !strings.Contains(tw.lines[0], expectStr1) {
102-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
84+
t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr1, tw.lines[0])
10385
}
10486
if !strings.Contains(tw.lines[0], expectStr2) {
105-
t.Errorf("WARN_ON didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
87+
t.Errorf("BugTraceback didn't contain expected output, expected: '%s', got: '%s'", expectStr2, tw.lines[0])
10688
}
10789
},
10890
"testWarnOnceOnlyPrintsOnce": func(t *testing.T) {
10991
testHelperFunc := func() {
110-
WARN_ON_ONCE(true)
92+
BugTracebackfOnce("error")
11193
}
11294

11395
testHelperFunc()
11496
if len(tw.lines) == 0 {
115-
t.Errorf("WarnOnOnce didn't print anything when it should have")
97+
t.Errorf("BugTracebackfOnce didn't print anything when it should have")
11698
}
11799
tw.clear()
118100

119101
testHelperFunc()
120102
if len(tw.lines) > 0 {
121-
t.Errorf("WarnOnOnce printed out a warning a second time when it shouldn't have")
103+
t.Errorf("BugTracebackfOnce printed out a warning a second time when it shouldn't have")
122104
}
123105
},
124106
"testWarnOnceDoesntClobberOthers": func(t *testing.T) {
125-
WARN_ON_ONCE(true)
107+
BugTracebackfOnce("error")
126108
if len(tw.lines) == 0 {
127-
t.Errorf("First WarnOnOnce didn't print anything when it should have")
109+
t.Errorf("First BugTracebackfOnce didn't print anything when it should have")
128110
}
129111
tw.clear()
130112

131-
WARN_ON_ONCE(true)
113+
BugTracebackfOnce("error")
132114
if len(tw.lines) == 0 {
133-
t.Errorf("Second WarnOnOnce didn't print anything when it should have")
115+
t.Errorf("Second BugTracebackfOnce didn't print anything when it should have")
134116
}
135117
},
136118
}

0 commit comments

Comments
 (0)