-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: dead pointers kept alive across loop iterations #18336
Comments
/cc @randall77 |
Experimenting further, the issue can be reproduced even without inlining:
Here Of course, in this case you could add Perhaps we need to emit VARKILL instructions for non-escaping Addrtaken variables when their innermost enclosing We can't emit them at just the innermost block, because that would break:
|
CL https://golang.org/cl/34552 mentions this issue. |
Finding just the right spot for a VARKILL here seems like @dr2chase's cup o' tea. |
This should now be fixed due to stack tracing (CLs https://go-review.googlesource.com/c/go/+/134155 and https://go-review.googlesource.com/c/go/+/134156) |
Change https://golang.org/cl/141117 mentions this issue: |
Prior to stack tracing, inlining could cause dead pointers to be kept alive in some loops. See #18336 and CL 31674. The adjustment removed by this change preserved the inlining status quo in the face of Node structure changes, to avoid creating new problems. Now that stack tracing provides precision, these hacks can be removed. Of course, our inlining code model is already hacky (#17566), but at least now there will be fewer epicyclical hacks. Newly inline-able functions in std cmd as a result of this change: hash/adler32/adler32.go:65:6: can inline (*digest).UnmarshalBinary hash/fnv/fnv.go:281:6: can inline (*sum32).UnmarshalBinary hash/fnv/fnv.go:292:6: can inline (*sum32a).UnmarshalBinary reflect/value.go:1298:6: can inline Value.OverflowComplex compress/bzip2/bit_reader.go:25:6: can inline newBitReader encoding/xml/xml.go:365:6: can inline (*Decoder).switchToReader vendor/golang_org/x/crypto/cryptobyte/builder.go:77:6: can inline (*Builder).AddUint16 crypto/x509/x509.go:1851:58: can inline buildExtensions.func2.1.1 crypto/x509/x509.go:1871:58: can inline buildExtensions.func2.3.1 crypto/x509/x509.go:1883:58: can inline buildExtensions.func2.4.1 cmd/vet/internal/cfg/builder.go:463:6: can inline (*builder).labeledBlock crypto/tls/handshake_messages.go:1450:6: can inline (*newSessionTicketMsg).marshal crypto/tls/handshake_server.go:769:6: can inline (*serverHandshakeState).clientHelloInfo crypto/tls/handshake_messages.go:1171:6: can inline (*nextProtoMsg).unmarshal cmd/link/internal/amd64/obj.go:40:6: can inline Init cmd/link/internal/ppc64/obj.go:40:6: can inline Init net/http/httputil/persist.go:54:6: can inline NewServerConn net/http/fcgi/child.go:83:6: can inline newResponse cmd/compile/internal/ssa/poset.go:245:6: can inline (*poset).newnode Change-Id: I19e8e383a6273849673d35189a9358870665f82f Reviewed-on: https://go-review.googlesource.com/c/141117 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ilya Tocar <ilya.tocar@intel.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A little while back we noticed that 01bf5cc caused an internal test to start failing due to allocating too much memory. Simplifying, the test is basically:
This works as intended when reflect.ValueOf is not inlined. But 01bf5cc happened to cause reflect.ValueOf to become eligilble for inlining, and suddenly m was being kept alive during the in.pop() calls.
I've produced two partially minimized repros:
One with //go:noinline that pases: https://play.golang.org/p/Bu1qLfD60c
One without that fails: https://play.golang.org/p/iTNJxtmiSN
The same problem exists in 1.7.3, so targeting for 1.9.
The text was updated successfully, but these errors were encountered: