cmd/compile, runtime: track argument stack slot liveness

Currently, for stack traces (e.g. at panic or when runtime.Stack
is called), we print argument values from the stack. With register
ABI, we may never store the argument to stack therefore the
argument value on stack may be meaningless. This causes confusion.

This CL makes the compiler keep trace of which argument stack
slots are meaningful. If it is meaningful, it will be printed in
stack traces as before. If it may not be meaningful, it will be
printed as the stack value with a question mark ("?"). In general,
the value could be meaningful on some code paths but not others
depending on the execution, and the compiler couldn't know
statically, so we still print the stack value, instead of not
printing it at all. Also note that if the argument variable is
updated in the function body the printed value may be stale (like
before register ABI) but still considered meaningful.

Arguments passed on stack are always meaningful therefore always
printed without a question mark. Results are never printed, as
before.

(Due to a bug in the compiler we sometimes don't spill args into
their dedicated spill slots (as we should), causing it having
fewer meaningful values than it should be.)

This increases binary sizes a bit:
            old       new
hello      1129760   1142080  +1.09%
cmd/go    13932320  14088016  +1.12%
cmd/link   6267696   6329168  +0.98%

Fixes #45728.

Change-Id: I308a0402e5c5ab94ca0953f8bd85a56acd28f58c
Reviewed-on: https://go-review.googlesource.com/c/go/+/352057
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

4 files changed, 146 insertions, 4 deletions

diff --git a/src/runtime/funcdata.h b/src/runtime/funcdata.h
index 15f1b5c9a1..a454dcaa69 100644
--- a/src/runtime/funcdata.h
+++ b/src/runtime/funcdata.h
@@ -11,6 +11,7 @@
 #define PCDATA_UnsafePoint 0
 #define PCDATA_StackMapIndex 1
 #define PCDATA_InlTreeIndex 2
+#define PCDATA_ArgLiveIndex 3
 
 #define FUNCDATA_ArgsPointerMaps 0 /* garbage collector blocks */
 #define FUNCDATA_LocalsPointerMaps 1
@@ -18,6 +19,7 @@
 #define FUNCDATA_InlTree 3
 #define FUNCDATA_OpenCodedDeferInfo 4 /* info for func with open-coded defers */
 #define FUNCDATA_ArgInfo 5
+#define FUNCDATA_ArgLiveInfo 6
 
 // Pseudo-assembly statements.
 
diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go
index ced39026c9..41161d6f90 100644
--- a/src/runtime/symtab.go
+++ b/src/runtime/symtab.go
@@ -301,6 +301,7 @@ const (
 	_PCDATA_UnsafePoint   = 0
 	_PCDATA_StackMapIndex = 1
 	_PCDATA_InlTreeIndex  = 2
+	_PCDATA_ArgLiveIndex  = 3
 
 	_FUNCDATA_ArgsPointerMaps    = 0
 	_FUNCDATA_LocalsPointerMaps  = 1
@@ -308,6 +309,7 @@ const (
 	_FUNCDATA_InlTree            = 3
 	_FUNCDATA_OpenCodedDeferInfo = 4
 	_FUNCDATA_ArgInfo            = 5
+	_FUNCDATA_ArgLiveInfo        = 6
 
 	_ArgsSizeUnknown = -0x80000000
 )
diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 7e1b14ccf2..5de1abce9a 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -427,7 +427,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 				}
 				print(name, "(")
 				argp := unsafe.Pointer(frame.argp)
-				printArgs(f, argp)
+				printArgs(f, argp, tracepc)
 				print(")\n")
 				print("\t", file, ":", line)
 				if frame.pc > f.entry() {
@@ -540,7 +540,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 }
 
 // printArgs prints function arguments in traceback.
-func printArgs(f funcInfo, argp unsafe.Pointer) {
+func printArgs(f funcInfo, argp unsafe.Pointer, pc uintptr) {
 	// The "instruction" of argument printing is encoded in _FUNCDATA_ArgInfo.
 	// See cmd/compile/internal/ssagen.emitArgInfo for the description of the
 	// encoding.
@@ -564,7 +564,25 @@ func printArgs(f funcInfo, argp unsafe.Pointer) {
 		return
 	}
 
-	print1 := func(off, sz uint8) {
+	liveInfo := funcdata(f, _FUNCDATA_ArgLiveInfo)
+	liveIdx := pcdatavalue(f, _PCDATA_ArgLiveIndex, pc, nil)
+	startOffset := uint8(0xff) // smallest offset that needs liveness info (slots with a lower offset is always live)
+	if liveInfo != nil {
+		startOffset = *(*uint8)(liveInfo)
+	}
+
+	isLive := func(off, slotIdx uint8) bool {
+		if liveInfo == nil || liveIdx <= 0 {
+			return true // no liveness info, always live
+		}
+		if off < startOffset {
+			return true
+		}
+		bits := *(*uint8)(add(liveInfo, uintptr(liveIdx)+uintptr(slotIdx/8)))
+		return bits&(1<<(slotIdx%8)) != 0
+	}
+
+	print1 := func(off, sz, slotIdx uint8) {
 		x := readUnaligned64(add(argp, uintptr(off)))
 		// mask out irrelevant bits
 		if sz < 8 {
@@ -576,6 +594,9 @@ func printArgs(f funcInfo, argp unsafe.Pointer) {
 			}
 		}
 		print(hex(x))
+		if !isLive(off, slotIdx) {
+			print("?")
+		}
 	}
 
 	start := true
@@ -585,6 +606,7 @@ func printArgs(f funcInfo, argp unsafe.Pointer) {
 		}
 	}
 	pi := 0
+	slotIdx := uint8(0) // register arg spill slot index
 printloop:
 	for {
 		o := p[pi]
@@ -609,7 +631,10 @@ printloop:
 			printcomma()
 			sz := p[pi]
 			pi++
-			print1(o, sz)
+			print1(o, sz, slotIdx)
+			if o >= startOffset {
+				slotIdx++
+			}
 		}
 		start = false
 	}
diff --git a/src/runtime/traceback_test.go b/src/runtime/traceback_test.go
index 83b86a7e90..de9580ae53 100644
--- a/src/runtime/traceback_test.go
+++ b/src/runtime/traceback_test.go
@@ -6,6 +6,7 @@ package runtime_test
 
 import (
 	"bytes"
+	"internal/goexperiment"
 	"runtime"
 	"testing"
 )
@@ -13,6 +14,13 @@ import (
 var testTracebackArgsBuf [1000]byte
 
 func TestTracebackArgs(t *testing.T) {
+	abiSel := func(x, y string) string { // select expected output based on ABI
+		if goexperiment.RegabiArgs {
+			return x
+		}
+		return y
+	}
+
 	tests := []struct {
 		fn     func() int
 		expect string
@@ -105,6 +113,52 @@ func TestTracebackArgs(t *testing.T) {
 			func() int { return testTracebackArgs8d(testArgsType8d{1, 2, 3, 4, 5, 6, 7, 8, [3]int{9, 10, 11}, 12}) },
 			"testTracebackArgs8d({0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, {0x9, 0xa, ...}, ...})",
 		},
+
+		// Register argument liveness.
+		// 1, 3 are used and live, 2, 4 are dead (in register ABI).
+		// Address-taken (7) and stack ({5, 6}) args are always live.
+		{
+			func() int {
+				poisonStack() // poison arg area to make output deterministic
+				return testTracebackArgs9(1, 2, 3, 4, [2]int{5, 6}, 7)
+			},
+			abiSel(
+				"testTracebackArgs9(0x1, 0xffffffff?, 0x3, 0xff?, {0x5, 0x6}, 0x7)",
+				"testTracebackArgs9(0x1, 0x2, 0x3, 0x4, {0x5, 0x6}, 0x7)"),
+		},
+		// No live.
+		// (Note: this assume at least 5 int registers if register ABI is used.)
+		{
+			func() int {
+				poisonStack() // poison arg area to make output deterministic
+				return testTracebackArgs10(1, 2, 3, 4, 5)
+			},
+			abiSel(
+				"testTracebackArgs10(0xffffffff?, 0xffffffff?, 0xffffffff?, 0xffffffff?, 0xffffffff?)",
+				"testTracebackArgs10(0x1, 0x2, 0x3, 0x4, 0x5)"),
+		},
+		// Conditional spills.
+		// Spill in conditional, not executed.
+		{
+			func() int {
+				poisonStack() // poison arg area to make output deterministic
+				return testTracebackArgs11a(1, 2, 3)
+			},
+			abiSel(
+				"testTracebackArgs11a(0xffffffff?, 0xffffffff?, 0xffffffff?)",
+				"testTracebackArgs11a(0x1, 0x2, 0x3)"),
+		},
+		// 2 spills in conditional, not executed; 3 spills in conditional, executed, but not statically known.
+		// So print 0x3?.
+		{
+			func() int {
+				poisonStack() // poison arg area to make output deterministic
+				return testTracebackArgs11b(1, 2, 3, 4)
+			},
+			abiSel(
+				"testTracebackArgs11b(0xffffffff?, 0xffffffff?, 0x3?, 0x4)",
+				"testTracebackArgs11b(0x1, 0x2, 0x3, 0x4)"),
+		},
 	}
 	for _, test := range tests {
 		n := test.fn()
@@ -290,3 +344,62 @@ func testTracebackArgs8d(a testArgsType8d) int {
 	}
 	return n
 }
+
+//go:noinline
+func testTracebackArgs9(a int64, b int32, c int16, d int8, x [2]int, y int) int {
+	if a < 0 {
+		println(&y) // take address, make y live, even if no longer used at traceback
+	}
+	n := runtime.Stack(testTracebackArgsBuf[:], false)
+	if a < 0 {
+		// use half of in-reg args to keep them alive, the other half are dead
+		return int(a) + int(c)
+	}
+	return n
+}
+
+//go:noinline
+func testTracebackArgs10(a, b, c, d, e int32) int {
+	// no use of any args
+	return runtime.Stack(testTracebackArgsBuf[:], false)
+}
+
+// norace to avoid race instrumentation changing spill locations.
+//
+//go:norace
+//go:noinline
+func testTracebackArgs11a(a, b, c int32) int {
+	if a < 0 {
+		println(a, b, c) // spill in a conditional, may not execute
+	}
+	if b < 0 {
+		return int(a + b + c)
+	}
+	return runtime.Stack(testTracebackArgsBuf[:], false)
+}
+
+// norace to avoid race instrumentation changing spill locations.
+//
+//go:norace
+//go:noinline
+func testTracebackArgs11b(a, b, c, d int32) int {
+	var x int32
+	if a < 0 {
+		print() // spill b in a conditional
+		x = b
+	} else {
+		print() // spill c in a conditional
+		x = c
+	}
+	if d < 0 { // d is always needed
+		return int(x + d)
+	}
+	return runtime.Stack(testTracebackArgsBuf[:], false)
+}
+
+// Poison the arg area with deterministic values.
+//
+//go:noinline
+func poisonStack() [20]int {
+	return [20]int{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1}
+}