(Re)move some GC pushes to be defensive about #35708 #38355
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Somebody sent me a (Julia 1.4) trace that turned out to have the
same root cause as #35708. I took a look around just to be sure
that there was no other instance of this pattern and while I
didn't see any, I did see a useless push/pull pair as well
as a GC_PUSH of an uninitialized struct. While neither are a
problem by themselves, they will prevent the GC analyzer from
giving an error if any of the function in between ever become
safepoints (since the GC analyzer doesn't track initilized-ness).
I think it as a rule of thumb, we should never push uninitialized
values into a GC frame. Doing so assumes that there are no safepoints
before the value is fully initialized, but if that is the case,
the GC_PUSH may also be delayed until after initialization and if
the assumption ever changes, at least the GC analyzer will catch it.