Skip to content

Commit c45ebef

Browse files
prattmicgopherbot
authored andcommitted
runtime: avoid unsafe.{Slice,String} in debuglog
CL 428157 and CL 428759 switched debuglog to using unsafe.String and unsafe.Slice, which broke the build with -tags=debuglog because this is a no write barrier context, but runtime.unsafeString and unsafeSlice can panic, which includes write barriers. We could add a panicCheck1 path to these functions to reallow write barriers, but it is a big mess to pass around the caller PC, particularly since the compiler generates calls. It is much simpler to just avoid unsafe.String and Slice. Also add a basic test to build the runtime with -tags=debuglog to help avoid future regressions. For #54854. Change-Id: I702418b986fbf189664e9aa4f40bc7de4d9e7781 Reviewed-on: https://go-review.googlesource.com/c/go/+/443380 Run-TryBot: Michael Pratt <[email protected]> Auto-Submit: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent 7cf06f0 commit c45ebef

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

src/runtime/debuglog.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,12 @@ func (l *dlogger) s(x string) *dlogger {
304304
l.w.uvarint(uint64(uintptr(unsafe.Pointer(strData)) - datap.etext))
305305
} else {
306306
l.w.byte(debugLogString)
307-
b := unsafe.Slice(strData, len(x))
307+
// We can't use unsafe.Slice as it may panic, which isn't safe
308+
// in this (potentially) nowritebarrier context.
309+
var b []byte
310+
bb := (*slice)(unsafe.Pointer(&b))
311+
bb.array = unsafe.Pointer(strData)
312+
bb.len, bb.cap = len(x), len(x)
308313
if len(b) > debugLogStringLimit {
309314
b = b[:debugLogStringLimit]
310315
}
@@ -655,7 +660,13 @@ func (r *debugLogReader) printVal() bool {
655660
case debugLogConstString:
656661
len, ptr := int(r.uvarint()), uintptr(r.uvarint())
657662
ptr += firstmoduledata.etext
658-
s := unsafe.String((*byte)(unsafe.Pointer(ptr)), len)
663+
// We can't use unsafe.String as it may panic, which isn't safe
664+
// in this (potentially) nowritebarrier context.
665+
str := stringStruct{
666+
str: unsafe.Pointer(ptr),
667+
len: len,
668+
}
669+
s := *(*string)(unsafe.Pointer(&str))
659670
print(s)
660671

661672
case debugLogStringOverflow:

src/runtime/debuglog_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ package runtime_test
2424

2525
import (
2626
"fmt"
27+
"internal/testenv"
2728
"regexp"
2829
"runtime"
2930
"strings"
@@ -155,3 +156,14 @@ func TestDebugLogLongString(t *testing.T) {
155156
t.Fatalf("want %q, got %q", want, got)
156157
}
157158
}
159+
160+
// TestDebugLogBuild verifies that the runtime builds with -tags=debuglog.
161+
func TestDebugLogBuild(t *testing.T) {
162+
testenv.MustHaveGoBuild(t)
163+
164+
// It doesn't matter which program we build, anything will rebuild the
165+
// runtime.
166+
if _, err := buildTestProg(t, "testprog", "-tags=debuglog"); err != nil {
167+
t.Fatal(err)
168+
}
169+
}

0 commit comments

Comments
 (0)