-
Notifications
You must be signed in to change notification settings - Fork 866
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
Get MIRI running against parquet crate #614
Comments
As noted by @houqp -- I originally filed this in the wrong repo by mistake: apache/datafusion#773 |
🤔 though I tried to get a clean MIRI run against the parquet2 crate (arrow_dev) alamb@MacBook-Pro:~/Software/parquet2$ MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test -- --skip io And I got some errors; I am probably running it incorrectly 😢
|
Not really familiar with that code, but there is a FIXME comment about the alignment after pointer casts here: arrow-rs/parquet/src/util/bit_util.rs Line 556 in 1d6c374
If the alignment can't be guaranteed to be valid for u32, one possible solution could be to replace all |
Small update, I tried running MIRI: arrow-rs$ MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test -p parquet Got error:
Seems like MIRI doesn't support |
Miri now supports tempfile on latest nightly. Running command: MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test -p parquet -- -Z unstable-options --report-time
Certain tests took an extremely long time to execute:
And finally failed at test
|
THanks for trying @Jefffrey -- I agree I don't know what that error means, |
I started looking into this again and think we can replace |
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
The MIRI tool helps crates ensure they have no undefined behavior https://github.com/rust-lang/miri
To improve the stability / security of the parquet crate, it would be cool to run MIRI against that too.
Describe the solution you'd like
Ideally, we would run same kind of MIRI checks on the parquet crate as we do now on arrow: https://github.com/apache/arrow-rs/blob/master/.github/workflows/miri.yaml#L60
This means a command something like
cargo +nightly miri test -p parquet
Sadly, when I run this locally the very first test fails with an alignment issue
Describe alternatives you've considered
The parquet2 crate in https://github.com/jorgecarleitao/parquet2 from @jorgecarleitao apparently already has parquet running under MIR this already running, so depending on if that gets integrated into the main arrow-rs repo, that might change how we go about getting a clean run
Additional context
Thanks to help from @roee88 , @jhorstmann and others who I probably missed, we now have MIRI checks validated for the arrow crate on each PR: https://github.com/apache/arrow-rs/runs/3154551440
Which runs a command such as:
cargo miri test -p arrow -- --skip csv --skip ipc --skip json
The text was updated successfully, but these errors were encountered: