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 <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
Michael Pratt 2022-10-17 15:57:48 -04:00 committed by Gopher Robot
parent 7cf06f070e
commit c45ebef05e
2 changed files with 25 additions and 2 deletions

View file

@ -304,7 +304,12 @@ func (l *dlogger) s(x string) *dlogger {
l.w.uvarint(uint64(uintptr(unsafe.Pointer(strData)) - datap.etext)) l.w.uvarint(uint64(uintptr(unsafe.Pointer(strData)) - datap.etext))
} else { } else {
l.w.byte(debugLogString) l.w.byte(debugLogString)
b := unsafe.Slice(strData, len(x)) // We can't use unsafe.Slice as it may panic, which isn't safe
// in this (potentially) nowritebarrier context.
var b []byte
bb := (*slice)(unsafe.Pointer(&b))
bb.array = unsafe.Pointer(strData)
bb.len, bb.cap = len(x), len(x)
if len(b) > debugLogStringLimit { if len(b) > debugLogStringLimit {
b = b[:debugLogStringLimit] b = b[:debugLogStringLimit]
} }
@ -655,7 +660,13 @@ func (r *debugLogReader) printVal() bool {
case debugLogConstString: case debugLogConstString:
len, ptr := int(r.uvarint()), uintptr(r.uvarint()) len, ptr := int(r.uvarint()), uintptr(r.uvarint())
ptr += firstmoduledata.etext ptr += firstmoduledata.etext
s := unsafe.String((*byte)(unsafe.Pointer(ptr)), len) // We can't use unsafe.String as it may panic, which isn't safe
// in this (potentially) nowritebarrier context.
str := stringStruct{
str: unsafe.Pointer(ptr),
len: len,
}
s := *(*string)(unsafe.Pointer(&str))
print(s) print(s)
case debugLogStringOverflow: case debugLogStringOverflow:

View file

@ -24,6 +24,7 @@ package runtime_test
import ( import (
"fmt" "fmt"
"internal/testenv"
"regexp" "regexp"
"runtime" "runtime"
"strings" "strings"
@ -155,3 +156,14 @@ func TestDebugLogLongString(t *testing.T) {
t.Fatalf("want %q, got %q", want, got) t.Fatalf("want %q, got %q", want, got)
} }
} }
// TestDebugLogBuild verifies that the runtime builds with -tags=debuglog.
func TestDebugLogBuild(t *testing.T) {
testenv.MustHaveGoBuild(t)
// It doesn't matter which program we build, anything will rebuild the
// runtime.
if _, err := buildTestProg(t, "testprog", "-tags=debuglog"); err != nil {
t.Fatal(err)
}
}