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 a bunch of UBs #7

Merged
merged 1 commit into from
Aug 3, 2024
Merged

Fix a bunch of UBs #7

merged 1 commit into from
Aug 3, 2024

Conversation

ChayimFriedman2
Copy link
Contributor

  • Creating a reference then using it to read more bytes than its type is UB in Stacked Borrows
  • Converting a shared reference to a reference to UnsafeCell is UB in Stacked Borrows, writing to it is UB in Tree Borrows too
  • Writing normally (*p = ..., not std::ptr::write(p, ...) to an uninitialized pointer is maybe UB because it drops the old data

Fix all of those. I didn't do a full audit and didn't check the orderings, but the tests pass Miri now.

@jonhoo jonhoo mentioned this pull request Aug 3, 2024
@jonhoo
Copy link
Owner

jonhoo commented Aug 3, 2024

Thank you! Would you mind merging from main (which I've renamed from master) to get CI working again (ref #8)?

 - Creating a reference then using it to read more bytes than its type is UB in Stacked Borrows
 - Converting a shared reference to a reference to `UnsafeCell` is UB in Stacked Borrows, writing to it is UB in Tree Borrows too
 - Writing normally (`*p = ...`, not `std::ptr::write(p, ...)` to an uninitialized pointer is maybe UB because it drops the old data

Fix all of those. I didn't do a full audit and didn't check the orderings, but the tests pass Miri now.
@ChayimFriedman2
Copy link
Contributor Author

Done.

Copy link

codecov bot commented Aug 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.8%. Comparing base (66fd152) to head (86f5faf).

Additional details and impacted files
Files Coverage Δ
src/lib.rs 87.8% <100.0%> (-0.1%) ⬇️

@jonhoo jonhoo merged commit 1e8bf4a into jonhoo:main Aug 3, 2024
19 checks passed
@jonhoo
Copy link
Owner

jonhoo commented Aug 3, 2024

Releasing in #9

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.

2 participants