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

Stop assuming pointer type for generic arguments #383

Closed
wants to merge 2 commits into from

Conversation

MaxDesiatov
Copy link
Collaborator

Seems to fully resolve #367.

You were right @foscomputerservices, this was a strict aliasing issue. 😅 This forum post made it crystal clear to me:

Binding a local variable's memory to a different type poisons that local variable

@MaxDesiatov MaxDesiatov added the bug Something isn't working label Feb 21, 2021
@MaxDesiatov MaxDesiatov marked this pull request as ready for review February 21, 2021 21:21
@MaxDesiatov MaxDesiatov requested a review from a team February 21, 2021 21:22
Copy link
Member

@carson-katri carson-katri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@MaxDesiatov
Copy link
Collaborator Author

MaxDesiatov commented Feb 23, 2021

This doesn't make the issue completely go away. I'm still seeing it in slightly larger apps, and I still can't reproduce it with a test renderer to add a unit test. I suspect this only shuffles some memory around and makes the set of conditions for reproduction shift. Working on a slightly different approach for this right now...

@MaxDesiatov MaxDesiatov marked this pull request as draft February 23, 2021 21:24
@MaxDesiatov
Copy link
Collaborator Author

Even though we've found a different way to solve the original issue, I'd like to keep this open (albeit in the draft state) until we have some benchmarking automation to compare if there are any performance or code size benefits by any chance.

@MaxDesiatov MaxDesiatov closed this Aug 9, 2022
@MaxDesiatov MaxDesiatov deleted the strict-aliasing-fix branch August 9, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Infinite loops w/ 100% CPU usage caused by stack overflows
2 participants