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 2340ceed8f294488d73d28e1ffd26a3fa67a2eb1..067c93f27dad7a36e92e50e9662f7d382cf0ab1f 100644 (file)
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -120,12 +120,6 @@ func (b *buffer) writeRune(r rune) {
 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 @@ func (p *pp) free() {
                p.wrappedErrs = nil
        }
 
-       p.arg = nil
-       p.value = reflect.Value{}
        p.wrappedErrs = p.wrappedErrs[:0]
        ppFree.Put(p)
 }
@@ -379,20 +371,20 @@ func (p *pp) unknownType(v reflect.Value) {
        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 @@ func (p *pp) badVerb(verb rune) {
        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 @@ func (p *pp) fmt0x64(v uint64, leading0x bool) {
 }
 
 // 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 @@ func (p *pp) fmtInteger(v uint64, isSigned bool, verb rune) {
        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 @@ func (p *pp) fmtFloat(v float64, size int, verb rune) {
        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 @@ func (p *pp) fmtString(v string, verb rune) {
        case 'q':
                p.fmt.fmtQ(v)
        default:
-               p.badVerb(verb)
+               p.badVerb(arg, value, verb)
        }
 }
 
@@ -547,13 +539,13 @@ func (p *pp) fmtBytes(v []byte, verb rune, typeString string) {
        }
 }
 
-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 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune) {
        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) catchPanic(arg any, verb rune, method string) {
        }
 }
 
-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 @@ func (p *pp) handleMethods(verb rune) (handled bool) {
        }
 
        // 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 @@ func (p *pp) handleMethods(verb rune) (handled bool) {
                        // 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) handleMethods(verb rune) (handled bool) {
 }
 
 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 @@ func (p *pp) printArg(arg any, verb rune) {
                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) printArg(arg any, verb rune) {
 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 @@ func (p *pp) printValue(value reflect.Value, verb rune, depth int) {
                        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 @@ func (p *pp) printValue(value reflect.Value, verb rune, depth int) {
                }
                fallthrough
        case reflect.Chan, reflect.Func, reflect.UnsafePointer:
-               p.fmtPointer(f, verb)
+               p.fmtPointer(nil, f, verb)
        default:
                p.unknownType(f)
        }