This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Do not fast tail call if caller has multislot structs (#25885)#25910
Merged
jashook merged 1 commit intodotnet:release/3.0from Jul 28, 2019
Merged
Do not fast tail call if caller has multislot structs (#25885)#25910jashook merged 1 commit intodotnet:release/3.0from
jashook merged 1 commit intodotnet:release/3.0from
Conversation
* Do not fast tail call if caller has multislot structs * apply patch * Address offline feedback
sergiy-k
approved these changes
Jul 28, 2019
|
@jashook, please feel to merge this change if you and @AndyAyersMS are happy with the fix. Silent bad codegen is bad. :) |
Author
|
Will merge before the branch closes. I assume, @AndyAyersMS wont comment as it is Sunday. However, we have discussed the change that went into master (#25885). This change will force slightly less fast tail call decisions at the expense of generating correct fast tail calls.. I am happy with the change and will merge soon. Thank you for looking over the weekend @sergiy-k |
|
Thanks! |
AndyAyersMS
approved these changes
Jul 28, 2019
Author
|
Thank you @AndyAyersMS for the review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Description
Silent bad codegen when compiling a method with a multislot struct argument in the caller, who has a fast tail call which re-uses the caller's stack outgoing arg space.
Customer impact
There are no currently known customers. There have been related issues in the past, but not issues filed with >16 byte structs. That being said, there is a precedent of finding these sort of problems in release once released :(. See #24858 for a very similar issue.
Regression
This is not a regression. It goes back to coreclr 1.0.
Risk
Any method in Tier 1 which is a fast tail call candidate, which also has a >16 byte struct on unix is a possible candidate for incorrect codegen. The result would be random, incorrect values passed to the callee.
Original issue:
https://github.com/dotnet/coreclr/issues/25884
/cc @dotnet/jit-contrib
/cc @sergiy-k