fmt: avoid storing input arguments on pp to help escape analysis
This is part of a series of CLs that aim to reduce how often
interface arguments escape for the print functions in fmt,
such as val here:
func f(val int) {
fmt.Sprintf("%d", val)
}
Prior to this change, arguments immediately escape in doPrintf
because they are stored on the *pp in printArg:
parameter a leaks to <heap> for (*pp).doPrintf with derefs=1:
flow: <heap> ← *a:
from a[argNum] (dot of pointer) at .\print.go:1077:18
from (*pp).printArg(p, a[argNum], rune(c)) (call parameter) at .\print.go:1077:16
parameter arg leaks to <heap> for (*pp).printArg with derefs=0:
flow: <heap> ← arg:
from p.arg = arg (assign) at .\print.go:682:8
The *pp is heap allocated, and the heap cannot point to stack
variables, so the arguments currently cannot be on the stack.
This change instead threads through the input arguments as individual
type any and reflect.Value parameters, rather than storing on the *pp.
After this change, input arguments still escape, but now for
other reasons.
Updates #8618
Updates #62653
Change-Id: I81984daeceb761ce4ce269b150b888950ce2c5d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/524938
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/src/fmt/print.go b/src/fmt/print.go
index 2340cee..067c93f 100644
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -120,12 +120,6 @@
type pp struct {
buf buffer
- // arg holds the current item, as an interface{}.
- arg any
-
- // value is used instead of arg for reflect values.
- value reflect.Value
-
// fmt is used to format basic items such as integers or strings.
fmt fmt
@@ -175,8 +169,6 @@
p.wrappedErrs = nil
}
- p.arg = nil
- p.value = reflect.Value{}
p.wrappedErrs = p.wrappedErrs[:0]
ppFree.Put(p)
}
@@ -379,20 +371,20 @@
p.buf.writeByte('?')
}
-func (p *pp) badVerb(verb rune) {
+func (p *pp) badVerb(arg any, value reflect.Value, verb rune) {
p.erroring = true
p.buf.writeString(percentBangString)
p.buf.writeRune(verb)
p.buf.writeByte('(')
switch {
- case p.arg != nil:
- p.buf.writeString(reflect.TypeOf(p.arg).String())
+ case arg != nil:
+ p.buf.writeString(reflect.TypeOf(arg).String())
p.buf.writeByte('=')
- p.printArg(p.arg, 'v')
- case p.value.IsValid():
- p.buf.writeString(p.value.Type().String())
+ p.printArg(arg, 'v')
+ case value.IsValid():
+ p.buf.writeString(value.Type().String())
p.buf.writeByte('=')
- p.printValue(p.value, 'v', 0)
+ p.printValue(value, 'v', 0)
default:
p.buf.writeString(nilAngleString)
}
@@ -400,12 +392,12 @@
p.erroring = false
}
-func (p *pp) fmtBool(v bool, verb rune) {
+func (p *pp) fmtBool(arg any, value reflect.Value, v bool, verb rune) {
switch verb {
case 't', 'v':
p.fmt.fmtBoolean(v)
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
@@ -419,7 +411,7 @@
}
// fmtInteger formats a signed or unsigned integer.
-func (p *pp) fmtInteger(v uint64, isSigned bool, verb rune) {
+func (p *pp) fmtInteger(arg any, value reflect.Value, v uint64, isSigned bool, verb rune) {
switch verb {
case 'v':
if p.fmt.sharpV && !isSigned {
@@ -444,13 +436,13 @@
case 'U':
p.fmt.fmtUnicode(v)
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
// fmtFloat formats a float. The default precision for each verb
// is specified as last argument in the call to fmt_float.
-func (p *pp) fmtFloat(v float64, size int, verb rune) {
+func (p *pp) fmtFloat(arg any, value reflect.Value, v float64, size int, verb rune) {
switch verb {
case 'v':
p.fmt.fmtFloat(v, size, 'g', -1)
@@ -461,32 +453,32 @@
case 'F':
p.fmt.fmtFloat(v, size, 'f', 6)
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
// fmtComplex formats a complex number v with
// r = real(v) and j = imag(v) as (r+ji) using
// fmtFloat for r and j formatting.
-func (p *pp) fmtComplex(v complex128, size int, verb rune) {
+func (p *pp) fmtComplex(arg any, value reflect.Value, v complex128, size int, verb rune) {
// Make sure any unsupported verbs are found before the
// calls to fmtFloat to not generate an incorrect error string.
switch verb {
case 'v', 'b', 'g', 'G', 'x', 'X', 'f', 'F', 'e', 'E':
oldPlus := p.fmt.plus
p.buf.writeByte('(')
- p.fmtFloat(real(v), size/2, verb)
+ p.fmtFloat(arg, value, real(v), size/2, verb)
// Imaginary part always has a sign.
p.fmt.plus = true
- p.fmtFloat(imag(v), size/2, verb)
+ p.fmtFloat(arg, value, imag(v), size/2, verb)
p.buf.writeString("i)")
p.fmt.plus = oldPlus
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
-func (p *pp) fmtString(v string, verb rune) {
+func (p *pp) fmtString(arg any, value reflect.Value, v string, verb rune) {
switch verb {
case 'v':
if p.fmt.sharpV {
@@ -503,7 +495,7 @@
case 'q':
p.fmt.fmtQ(v)
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
@@ -547,13 +539,13 @@
}
}
-func (p *pp) fmtPointer(value reflect.Value, verb rune) {
+func (p *pp) fmtPointer(arg any, value reflect.Value, verb rune) {
var u uintptr
switch value.Kind() {
case reflect.Chan, reflect.Func, reflect.Map, reflect.Pointer, reflect.Slice, reflect.UnsafePointer:
u = uintptr(value.UnsafePointer())
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
return
}
@@ -579,9 +571,9 @@
case 'p':
p.fmt0x64(uint64(u), !p.fmt.sharp)
case 'b', 'o', 'd', 'x', 'X':
- p.fmtInteger(uint64(u), unsigned, verb)
+ p.fmtInteger(arg, value, uint64(u), unsigned, verb)
default:
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
}
}
@@ -619,15 +611,15 @@
}
}
-func (p *pp) handleMethods(verb rune) (handled bool) {
+func (p *pp) handleMethods(arg any, value reflect.Value, verb rune) (handled bool) {
if p.erroring {
return
}
if verb == 'w' {
// It is invalid to use %w other than with Errorf or with a non-error arg.
- _, ok := p.arg.(error)
+ _, ok := arg.(error)
if !ok || !p.wrapErrs {
- p.badVerb(verb)
+ p.badVerb(arg, value, verb)
return true
}
// If the arg is a Formatter, pass 'v' as the verb to it.
@@ -635,18 +627,18 @@
}
// Is it a Formatter?
- if formatter, ok := p.arg.(Formatter); ok {
+ if formatter, ok := arg.(Formatter); ok {
handled = true
- defer p.catchPanic(p.arg, verb, "Format")
+ defer p.catchPanic(arg, verb, "Format")
formatter.Format(p, verb)
return
}
// If we're doing Go syntax and the argument knows how to supply it, take care of it now.
if p.fmt.sharpV {
- if stringer, ok := p.arg.(GoStringer); ok {
+ if stringer, ok := arg.(GoStringer); ok {
handled = true
- defer p.catchPanic(p.arg, verb, "GoString")
+ defer p.catchPanic(arg, verb, "GoString")
// Print the result of GoString unadorned.
p.fmt.fmtS(stringer.GoString())
return
@@ -661,17 +653,17 @@
// The duplication in the bodies is necessary:
// setting handled and deferring catchPanic
// must happen before calling the method.
- switch v := p.arg.(type) {
+ switch v := arg.(type) {
case error:
handled = true
- defer p.catchPanic(p.arg, verb, "Error")
- p.fmtString(v.Error(), verb)
+ defer p.catchPanic(arg, verb, "Error")
+ p.fmtString(arg, value, v.Error(), verb)
return
case Stringer:
handled = true
- defer p.catchPanic(p.arg, verb, "String")
- p.fmtString(v.String(), verb)
+ defer p.catchPanic(arg, verb, "String")
+ p.fmtString(arg, value, v.String(), verb)
return
}
}
@@ -680,15 +672,12 @@
}
func (p *pp) printArg(arg any, verb rune) {
- p.arg = arg
- p.value = reflect.Value{}
-
if arg == nil {
switch verb {
case 'T', 'v':
p.fmt.padString(nilAngleString)
default:
- p.badVerb(verb)
+ p.badVerb(arg, reflect.Value{}, verb)
}
return
}
@@ -700,61 +689,61 @@
p.fmt.fmtS(reflect.TypeOf(arg).String())
return
case 'p':
- p.fmtPointer(reflect.ValueOf(arg), 'p')
+ p.fmtPointer(arg, reflect.ValueOf(arg), 'p')
return
}
// Some types can be done without reflection.
switch f := arg.(type) {
case bool:
- p.fmtBool(f, verb)
+ p.fmtBool(arg, reflect.Value{}, f, verb)
case float32:
- p.fmtFloat(float64(f), 32, verb)
+ p.fmtFloat(arg, reflect.Value{}, float64(f), 32, verb)
case float64:
- p.fmtFloat(f, 64, verb)
+ p.fmtFloat(arg, reflect.Value{}, f, 64, verb)
case complex64:
- p.fmtComplex(complex128(f), 64, verb)
+ p.fmtComplex(arg, reflect.Value{}, complex128(f), 64, verb)
case complex128:
- p.fmtComplex(f, 128, verb)
+ p.fmtComplex(arg, reflect.Value{}, f, 128, verb)
case int:
- p.fmtInteger(uint64(f), signed, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), signed, verb)
case int8:
- p.fmtInteger(uint64(f), signed, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), signed, verb)
case int16:
- p.fmtInteger(uint64(f), signed, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), signed, verb)
case int32:
- p.fmtInteger(uint64(f), signed, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), signed, verb)
case int64:
- p.fmtInteger(uint64(f), signed, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), signed, verb)
case uint:
- p.fmtInteger(uint64(f), unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), unsigned, verb)
case uint8:
- p.fmtInteger(uint64(f), unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), unsigned, verb)
case uint16:
- p.fmtInteger(uint64(f), unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), unsigned, verb)
case uint32:
- p.fmtInteger(uint64(f), unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), unsigned, verb)
case uint64:
- p.fmtInteger(f, unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, f, unsigned, verb)
case uintptr:
- p.fmtInteger(uint64(f), unsigned, verb)
+ p.fmtInteger(arg, reflect.Value{}, uint64(f), unsigned, verb)
case string:
- p.fmtString(f, verb)
+ p.fmtString(arg, reflect.Value{}, f, verb)
case []byte:
p.fmtBytes(f, verb, "[]byte")
case reflect.Value:
// Handle extractable values with special methods
// since printValue does not handle them at depth 0.
if f.IsValid() && f.CanInterface() {
- p.arg = f.Interface()
- if p.handleMethods(verb) {
+ arg = f.Interface() // TODO(thepudds): Currently causes f to escape.
+ if p.handleMethods(arg, reflect.Value{}, verb) {
return
}
}
p.printValue(f, verb, 0)
default:
// If the type is not simple, it might have methods.
- if !p.handleMethods(verb) {
+ if !p.handleMethods(arg, reflect.Value{}, verb) {
// Need to use reflection, since the type had no
// interface methods that could be used for formatting.
p.printValue(reflect.ValueOf(f), verb, 0)
@@ -767,13 +756,11 @@
func (p *pp) printValue(value reflect.Value, verb rune, depth int) {
// Handle values with special methods if not already handled by printArg (depth == 0).
if depth > 0 && value.IsValid() && value.CanInterface() {
- p.arg = value.Interface()
- if p.handleMethods(verb) {
+ arg := value.Interface() // TODO(thepudds): Currently causes value to escape.
+ if p.handleMethods(arg, value, verb) {
return
}
}
- p.arg = nil
- p.value = value
switch f := value; value.Kind() {
case reflect.Invalid:
@@ -784,25 +771,25 @@
case 'v':
p.buf.writeString(nilAngleString)
default:
- p.badVerb(verb)
+ p.badVerb(nil, value, verb)
}
}
case reflect.Bool:
- p.fmtBool(f.Bool(), verb)
+ p.fmtBool(nil, value, f.Bool(), verb)
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
- p.fmtInteger(uint64(f.Int()), signed, verb)
+ p.fmtInteger(nil, value, uint64(f.Int()), signed, verb)
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
- p.fmtInteger(f.Uint(), unsigned, verb)
+ p.fmtInteger(nil, value, f.Uint(), unsigned, verb)
case reflect.Float32:
- p.fmtFloat(f.Float(), 32, verb)
+ p.fmtFloat(nil, value, f.Float(), 32, verb)
case reflect.Float64:
- p.fmtFloat(f.Float(), 64, verb)
+ p.fmtFloat(nil, value, f.Float(), 64, verb)
case reflect.Complex64:
- p.fmtComplex(f.Complex(), 64, verb)
+ p.fmtComplex(nil, value, f.Complex(), 64, verb)
case reflect.Complex128:
- p.fmtComplex(f.Complex(), 128, verb)
+ p.fmtComplex(nil, value, f.Complex(), 128, verb)
case reflect.String:
- p.fmtString(f.String(), verb)
+ p.fmtString(nil, value, f.String(), verb)
case reflect.Map:
if p.fmt.sharpV {
p.buf.writeString(f.Type().String())
@@ -925,7 +912,7 @@
}
fallthrough
case reflect.Chan, reflect.Func, reflect.UnsafePointer:
- p.fmtPointer(f, verb)
+ p.fmtPointer(nil, f, verb)
default:
p.unknownType(f)
}
