From c45ebef05edcb217be8f9bf1d7649763132727cc Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Mon, 17 Oct 2022 15:57:48 -0400 Subject: [PATCH] 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 Auto-Submit: Michael Pratt TryBot-Result: Gopher Robot Reviewed-by: Michael Knyszek --- src/runtime/debuglog.go | 15 +++++++++++++-- src/runtime/debuglog_test.go | 12 ++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/runtime/debuglog.go b/src/runtime/debuglog.go index 1fc7dd5555..b18774e6c0 100644 --- a/src/runtime/debuglog.go +++ b/src/runtime/debuglog.go @@ -304,7 +304,12 @@ func (l *dlogger) s(x string) *dlogger { l.w.uvarint(uint64(uintptr(unsafe.Pointer(strData)) - datap.etext)) } else { 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 { b = b[:debugLogStringLimit] } @@ -655,7 +660,13 @@ func (r *debugLogReader) printVal() bool { case debugLogConstString: len, ptr := int(r.uvarint()), uintptr(r.uvarint()) 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) case debugLogStringOverflow: diff --git a/src/runtime/debuglog_test.go b/src/runtime/debuglog_test.go index 10dc72cf51..18c54a81b9 100644 --- a/src/runtime/debuglog_test.go +++ b/src/runtime/debuglog_test.go @@ -24,6 +24,7 @@ package runtime_test import ( "fmt" + "internal/testenv" "regexp" "runtime" "strings" @@ -155,3 +156,14 @@ func TestDebugLogLongString(t *testing.T) { 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) + } +} -- 2.50.0