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

[EPIC] Fix safety issues in unsafe code #507

Closed
1 task done
andygrove opened this issue Jun 3, 2024 · 1 comment · Fixed by #546
Closed
1 task done

[EPIC] Fix safety issues in unsafe code #507

andygrove opened this issue Jun 3, 2024 · 1 comment · Fixed by #546
Labels
enhancement New feature or request
Milestone

Comments

@andygrove
Copy link
Member

andygrove commented Jun 3, 2024

What is the problem the feature request solves?

TL;DR when we upgrade to Rust 1.78 we discover that we have unsafe code that is really not safe and relies on undefined behavior.

The Rust standard library has a number of assertions for the preconditions of unsafe functions, but historically they have only been enabled in #[cfg(debug_assertions)] builds of the standard library to avoid affecting release performance. However, since the standard library is usually compiled and distributed in release mode, most Rust developers weren't ever executing these checks at all.

Starting with Rust 1.78, the condition for these assertions is delayed until code generation, so they will be checked depending on the user's own setting for debug assertions -- enabled by default in debug and test builds. This change helps users catch undefined behavior in their code, though the details of how much is checked are generally not stable.

Read more:

https://blog.rust-lang.org/2024/05/02/Rust-1.78.0.html#asserting-unsafe-preconditions

Here is one example of an assertion that we see when running with Rust 1.78

unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`

Issues:

Describe the potential solution

No response

Additional context

No response

@andygrove andygrove added the enhancement New feature or request label Jun 3, 2024
@andygrove
Copy link
Member Author

These are the failing tests:

test execution::datafusion::spark_hash::tests::test_str ... thread caused non-unwinding panic. aborting.
test execution::sort::tests::test_rdxsort ... thread caused non-unwinding panic. aborting.
test parquet::util::hash_util::tests::test_crc32 ... thread caused non-unwinding panic. aborting.
test parquet::util::hash_util::tests::test_murmur2_64a ... thread caused non-unwinding panic. aborting.

@andygrove andygrove changed the title chore: Make Comet compatible with Rust 1.78 Make Comet compatible with Rust 1.78 Jun 10, 2024
@andygrove andygrove changed the title Make Comet compatible with Rust 1.78 Stop relying on undefined behavior in unsafe code Jun 10, 2024
@andygrove andygrove added this to the 0.1.0 milestone Jun 10, 2024
@andygrove andygrove modified the milestones: 0.1.0, 0.2.0 Jun 11, 2024
@andygrove andygrove changed the title Stop relying on undefined behavior in unsafe code [EPIC] Fix safety issues in unsafe code Jun 11, 2024
@andygrove andygrove modified the milestones: 0.2.0, 0.1.0 Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant