Skip to content
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

Fix invalid return address 0 in GDScript #71098

Closed
wants to merge 1 commit into from

Conversation

jordi-star
Copy link
Contributor

@jordi-star jordi-star commented Jan 9, 2023

Fixes #70936, created by 0c15844.
In the past, all method returns would have a temporary return address assigned to it, even if its return type is null. Now, the return address can be empty (Address:NIL, which has an address of 0), but the bytecode still attempts to send the returned value to this address. Address:NIL is the default state of the Address class when using an empty constructor, so sending the returned value here probably messes some stuff up for things that use Address() to represent an empty address (Such as the unary operator).

I'm not too knowledgable on the inner workings of the VM/Compiler, so this may not be the correct fix. The bug is gone and all tests pass, so I assume this was the problem...
If this is not the correct fix, please let me know 👍

@jordi-star jordi-star requested a review from a team as a code owner January 9, 2023 06:05
@jordi-star
Copy link
Contributor Author

cc @vnen

@vnen
Copy link
Member

vnen commented Jan 9, 2023

This really only work by accident because the MethodBind call and the ptrcall instructions have the same arguments. It that ceases to be the case in the future this will introduce problems. I also think there are other cases where this is a problem, not only with ptrcall.

In any case, I have a fix for this bug ready: #71107

@akien-mga
Copy link
Member

Superseded by #71107.

@jordi-star
Copy link
Contributor Author

jordi-star commented Jan 9, 2023

Awesome, thanks for letting me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PropertyTweener's methods' return value are not discarded and effect the unary operator at next line.
3 participants