[6.1] [rbi] Fix demangling of sending results when demangling impl-function-type #78666
Conversation
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
…}() propagates result options. I discovered this while trying to figure out why we were dropping a sending bit when computing a reabstraction thunk in the tests for the following commit. This turned out to be the reason why. (cherry picked from commit bd37998)
The issue here is that the demangler (since we have a postfix mangling) parses parameters/results/etc and then uses earlier postfix type arguments to attach the relevant types to the parameters/results/etc. Since the flag for a sending result was placed in between the parameters and results, we get an off by one error. Rather than fix that specific issue by introducing an offset for the off by one error, I used the fact that the impl-function part of the mangling is not ABI and can be modified to move the bit used to signify a sending result to before the parameters so the whole problem is avoided. I also while I was doing this looked through the sending result mangling for any further issues and fixed them as I found them. rdar://141962865 (cherry picked from commit cb2d756)
(cherry picked from commit 9960079)
|
@swift-ci test |
rjmccall
reviewed
Jan 16, 2025
… old mangler. (cherry picked from commit 0ea22f8)
|
@swift-ci test |
gottesmm
changed the title
gottesmm
changed the title
|
@rjmccall can I get a review again here? |
rjmccall
approved these changes
Jan 30, 2025
DougGregor
approved these changes
Jan 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change fixes the demangling of sending results in the non-ABI impl-function-type part of the mangling that is used during reabstraction thunk generation.
Swift's mangling is a postfix mangling. We treat impl-function-type's mangling character 'I' as an operator that takes the types for parameters, results, yields, error result as parameters to the operator. The implementation of the demangler assumed that as it parsed nodes that all nodes that it demangled from impl-function-type (e.x.: flags on the function, information about params/results) would always have nodes that needed to be paired with the typed arguments to the operator were always at the end in the same order as the arguments. The demangler would after parsing the actual impl-function-type walk backwards along the child node list and add the typed argument information to the child node.
With that information in hand, the bug that occurred here is that the flag for sending results was originally placed in between the parameter and result information in impl-function-type. This would cause the nodes that were supposed to become children of the result nodes to instead be placed on the sending result flag node. The fix was to take advantage of this not being ABI to just move the bit to before the param part of the mangling so this can no longer happen.
While doing this, I also discovered a few other places where we were missing support for sending results in the old demangler/remangler so I added some quick support there as well just in case.
Scope: Affects demangle paths where a reabstraction thunk is used to reabstract in some way a function with a sending result. We produce a correct mangling, but the actual demangling will produce a bogus demangling tree, so anything that relies on being able to demangle the reabstraction thunk will cause issues. Luckily reabstraction thunks are not exported symbols so nothing can rely in the wild on linking against such a demangled symbol.
Risk: Low
Testing: Added new demangling tests and also added the specific swift example that exposed the code
Issue: rdar://141962865
Reviewer: @rjmccall
Original PRs:
#78612
#78660
#78889