-
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: funccompile is reentrant #33485
Labels
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Comments
Change https://golang.org/cl/254617 mentions this issue: |
Change https://golang.org/cl/254839 mentions this issue: |
Change https://golang.org/cl/254838 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Sep 14, 2020
funchdr and funcbody currently assume that either (1) Curfn == nil && dclcontext == PEXTERN, or (2) Curfn != nil && dclcontext == PAUTO. This is a reasonable assumption during parsing. However, these functions end up getting used in other contexts, and not all callers are so disciplined about Curfn/dclcontext handling. This CL changes them to save/restore arbitrary Curfn/dclcontext pairs instead. This is necessary for the followup CL, which pushes fninit earlier. Otherwise, Curfn/dclcontext fall out of sync, and funchdr panics. Passes toolstash-check. Updates #33485. Change-Id: I19b1be23db1bad6475345ae5c81bbdc66291a3a7 Reviewed-on: https://go-review.googlesource.com/c/go/+/254838 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Trust: Matthew Dempsky <mdempsky@google.com>
gopherbot
pushed a commit
that referenced
this issue
Sep 14, 2020
This allows the global initializers function to go through normal mid-end optimizations (e.g., inlining, escape analysis) like any other function. Updates #33485. Change-Id: I9bcfe98b8628d1aca09b4c238d8d3b74c69010a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/254839 Reviewed-by: Keith Randall <khr@golang.org> Trust: Matthew Dempsky <mdempsky@google.com>
Change https://golang.org/cl/254938 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Sep 15, 2020
Updates #33485. Change-Id: I3330860cdff1e9797466a7630bcdb7792c465b06 Reviewed-on: https://go-review.googlesource.com/c/go/+/254938 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
There's an awkward reentrancy issue with funccompile: funccompile calls compile, which calls dtypesym, which calls geneq/genhash/genwrapper, which call back into funccompile.
I don't think this is causing any problems today, but I was trying to work on #17728, and it led to some confusing issues due to SSA cache corruption.
It would probably be better if generated ASTs were pumped through the same compile workqueue that normal ASTs are compiled.
The text was updated successfully, but these errors were encountered: