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>

diff --git a/src/runtime/debuglog.go b/src/runtime/debuglog.go
index 1fc7dd5555334d557e576bf0a5b1c3df78887556..b18774e6c0a591a0f6f052916cec242fd8f61db8 100644 (file)
--- 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 10dc72cf51b11fd9895a8a3a5f80e2e411f57e8a..18c54a81b93642a144fbca91ceeffff17570a690 100644 (file)
--- 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)
+       }
+}