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 early return in lowLevelLocalizingSlice init #949

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

ronawho
Copy link
Contributor

@ronawho ronawho commented Oct 6, 2021

Previously, when the lowLevelLocalizingSlice took an empty region, we
just returned from the initializer and I was expecting the compiler to
insert the default field init. However, the compiler does not do that,
which leaves the fields uninitialized. This leaves them with random
values, which was causing issues where the record had a random address
and isOwned value, which caused us to try and free a random address in
deinit. Here just manually init the fields to work around this.

I opened chapel-lang/chapel#18524 upstream to request an error message

Previously, when the `lowLevelLocalizingSlice` took an empty region, we
just returned from the initializer and I was expecting the compiler to
insert the default field init. However, the compiler does not do that,
which leaves the fields uninitialized. This leaves them with random
values, which was causing issues where the record had a random address
and isOwned value, which caused us to try and free a random address in
deinit. Here just manually init the fields to work around this.

I opened chapel-lang/chapel 18524 upstream to request an error message
@ronawho
Copy link
Contributor Author

ronawho commented Oct 6, 2021

This was noticed while trying to upgrade to 1.25, which changed the stack layout in some way that led to UnitTestSipHash (which calls lowLevelLocalizingSlice with an empty region) to try to free a random address that was never allocated. This initializer was wrong in 1.24 too, just happened to get all 0 values so we didn't notice before.

@reuster986 reuster986 merged commit 7c18062 into Bears-R-Us:master Oct 6, 2021
@ronawho ronawho deleted the fix-llls-early-return branch October 6, 2021 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants