Commit 9e619739 authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

cmd/compile: copy all fields during SubstAny

Consider these functions:

func f(a any) int
func g(a any) int

Prior to this change, since f and g have identical signatures,
they would share a single generated func type.

types.SubstAny makes a shallow type copy, even after instantiation,
f and g share a single generated Result type.
So if you instantiate f with any=T, call dowidth,
instantiate g with any=U, and call dowidth,
and if sizeof(T) != sizeof(U),
then the Offset of the result for f is now wrong.

I don't believe this happens at all right now, but it bit me hard when
experimenting with some other compiler changes.
And it's hard to debug. It results in rare stack corruption, causing
problems far from the actual source of the problem.

To fix this, change SubstAny to make deep copies of TSTRUCTs.

name        old alloc/op      new alloc/op      delta
Template         35.3MB ± 0%       35.4MB ± 0%  +0.23%  (p=0.008 n=5+5)
Unicode          29.1MB ± 0%       29.1MB ± 0%  +0.16%  (p=0.008 n=5+5)
GoTypes           122MB ± 0%        122MB ± 0%  +0.16%  (p=0.008 n=5+5)
Compiler          513MB ± 0%        514MB ± 0%  +0.19%  (p=0.008 n=5+5)
SSA              1.94GB ± 0%       1.94GB ± 0%  +0.01%  (p=0.008 n=5+5)
Flate            24.2MB ± 0%       24.2MB ± 0%  +0.08%  (p=0.008 n=5+5)
GoParser         28.5MB ± 0%       28.5MB ± 0%  +0.24%  (p=0.008 n=5+5)
Reflect          86.2MB ± 0%       86.3MB ± 0%  +0.09%  (p=0.008 n=5+5)
Tar              34.9MB ± 0%       34.9MB ± 0%  +0.13%  (p=0.008 n=5+5)
XML              47.0MB ± 0%       47.1MB ± 0%  +0.18%  (p=0.008 n=5+5)
[Geo mean]       80.9MB            81.0MB       +0.15%

name        old allocs/op     new allocs/op     delta
Template           348k ± 0%         349k ± 0%  +0.38%  (p=0.008 n=5+5)
Unicode            340k ± 0%         340k ± 0%  +0.21%  (p=0.008 n=5+5)
GoTypes           1.27M ± 0%        1.28M ± 0%  +0.27%  (p=0.008 n=5+5)
Compiler          4.90M ± 0%        4.92M ± 0%  +0.36%  (p=0.008 n=5+5)
SSA               15.3M ± 0%        15.3M ± 0%  +0.03%  (p=0.008 n=5+5)
Flate              232k ± 0%         233k ± 0%  +0.14%  (p=0.008 n=5+5)
GoParser           291k ± 0%         292k ± 0%  +0.42%  (p=0.008 n=5+5)
Reflect           1.05M ± 0%        1.05M ± 0%  +0.14%  (p=0.008 n=5+5)
Tar                343k ± 0%         344k ± 0%  +0.22%  (p=0.008 n=5+5)
XML                428k ± 0%         430k ± 0%  +0.36%  (p=0.008 n=5+5)
[Geo mean]         807k              809k       +0.25%

Change-Id: I62134db642206cded01920dc1d8a7da61f7ca0ac
Reviewed-on: https://go-review.googlesource.com/c/147038
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
parent 44dcb5cb
......@@ -665,23 +665,18 @@ func SubstAny(t *Type, types *[]*Type) *Type {
}
case TSTRUCT:
// Make a copy of all fields, including ones whose type does not change.
// This prevents aliasing across functions, which can lead to later
// fields getting their Offset incorrectly overwritten.
fields := t.FieldSlice()
var nfs []*Field
nfs := make([]*Field, len(fields))
for i, f := range fields {
nft := SubstAny(f.Type, types)
if nft == f.Type {
continue
}
if nfs == nil {
nfs = append([]*Field(nil), fields...)
}
nfs[i] = f.Copy()
nfs[i].Type = nft
}
if nfs != nil {
t = t.copy()
t.SetFields(nfs)
}
t = t.copy()
t.SetFields(nfs)
}
return t
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment