MIRI failure in array::ffi::tests::test_struct and other ffi tests

[alamb]

Describe the bug
The test array::ffi::tests::test_struct signals a MIRI failure

To Reproduce

  export MIRIFLAGS="-Zmiri-disable-isolation"
  cargo +nightly miri setup
  MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test -p arrow -- --ignored ffi

The tests fails with variations of the following

error: Undefined Behavior: trying to reborrow for SharedReadOnly at alloc14433597, but parent tag <39379552> does not have an appropriate item in the borrow stack
   --> arrow/src/ffi.rs:141:22
    |
141 |         for child in private_data.children.iter() {
    |                      ^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for SharedReadOnly at alloc14433597, but parent tag <39379552> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `ffi::release_schema` at arrow/src/ffi.rs:141:22
note: inside `<ffi::FFI_ArrowSchema as std::ops::Drop>::drop` at arrow/src/ffi.rs:238:39
   --> arrow/src/ffi.rs:238:39
    |
238 |             Some(release) => unsafe { release(self) },
    |                                       ^^^^^^^^^^^^^
    = note: inside `std::ptr::drop_in_place::<ffi::FFI_ArrowSchema> - shim(Some(ffi::FFI_ArrowSchema))` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:192:1
    = note: inside `std::sync::Arc::<ffi::FFI_ArrowSchema>::drop_slow` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1055:18
    = note: inside `<std::sync::Arc<ffi::FFI_ArrowSchema> as std::ops::Drop>::drop` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1597:13
    = note: inside `std::ptr::drop_in_place::<std::sync::Arc<ffi::FFI_ArrowSchema>> - shim(Some(std::sync::Arc<ffi::FFI_ArrowSchema>))` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:192:1
    = note: inside `std::ptr::drop_in_place::<ffi::ArrowArray> - shim(Some(ffi::ArrowArray))` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:192:1
note: inside `array::ffi::<impl std::convert::TryFrom<ffi::ArrowArray> for array::data::ArrayData>::try_from` at arrow/src/array/ffi.rs:35:5
   --> arrow/src/array/ffi.rs:35:5
    |
35  |     }
    |     ^
note: inside `array::ffi::tests::test_round_trip` at arrow/src/array/ffi.rs:71:23
   --> arrow/src/array/ffi.rs:71:23
    |
71  |         let result = &ArrayData::try_from(d1)?;
    |                       ^^^^^^^^^^^^^^^^^^^^^^^
note: inside `array::ffi::tests::test_struct` at arrow/src/array/ffi.rs:128:9
   --> arrow/src/array/ffi.rs:128:9
    |
128 |         test_round_trip(data)
    |         ^^^^^^^^^^^^^^^^^^^^^
note: inside closure at arrow/src/array/ffi.rs:99:5
   --> arrow/src/array/ffi.rs:99:5
    |
99  | /     fn test_struct() -> Result<()> {
100 | |         let inner = StructArray::from(vec![
101 | |             (
102 | |                 Field::new("a1", DataType::Boolean, false),
...   |
128 | |         test_round_trip(data)
129 | |     }
    | |_____^
    = note: inside `<[closure@arrow/src/array/ffi.rs:99:5: 129:6] as std::ops::FnOnce<()>>::call_once - shim` at /usr/share/rust/.rustup/toolchains/nightly-2021-07-04-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5

And

test datatypes::ffi::tests::test_field ... error: Undefined Behavior: trying to reborrow for SharedReadOnly at alloc708431, but parent tag <1852137> does not have an appropriate item in the borrow stack
   --> arrow/src/ffi.rs:141:22
    |
141 |         for child in private_data.children.iter() {
    |                      ^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for SharedReadOnly at alloc708431, but parent tag <1852137> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `ffi::release_schema` at arrow/src/ffi.rs:141:22
note: inside `<ffi::FFI_ArrowSchema as std::ops::Drop>::drop` at arrow/src/ffi.rs:238:39
   --> arrow/src/ffi.rs:238:39
    |
238 |             Some(release) => unsafe { release(self) },
    |                                       ^^^^^^^^^^^^^
    = note: inside `std::ptr::drop_in_place::<ffi::FFI_ArrowSchema> - shim(Some(ffi::FFI_ArrowSchema))` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:192:1
note: inside `datatypes::ffi::tests::round_trip_field` at arrow/src/datatypes/ffi.rs:298:5
   --> arrow/src/datatypes/ffi.rs:298:5
    |
298 |     }
    |     ^
note: inside `datatypes::ffi::tests::test_field` at arrow/src/datatypes/ffi.rs:331:9
   --> arrow/src/datatypes/ffi.rs:331:9
    |
331 |         round_trip_field(Field::new("test", dtype, true))?;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside closure at arrow/src/datatypes/ffi.rs:329:5
   --> arrow/src/datatypes/ffi.rs:329:5
    |
329 | /     fn test_field() -> Result<()> {
330 | |         let dtype = DataType::Struct(vec![Field::new("a", DataType::Utf8, true)]);
331 | |         round_trip_field(Field::new("test", dtype, true))?;
332 | |         Ok(())
333 | |     }
    | |_____^
    = note: inside `<[closure@arrow/src/datatypes/ffi.rs:329:5: 333:6] as std::ops::FnOnce<()>>::call_once - shim` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `test::__rust_begin_short_backtrace::<fn()>` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:577:5
    = note: inside closure at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:568:30
    = note: inside `<[closure@test::run_test::{closure#2}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1572:9
    = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:347:9
    = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /Users/alamb/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:401:40

and

Expected behavior
The test should pass without any issues being flagged by MIRI

Additional context
Test disabled in #578

Example of failing test: https://github.com/apache/arrow-rs/pull/578/checks?check_run_id=3118470856

[alamb]

The failing test was added in https://github.com/apache/arrow-rs/pull/287/files from @roee88 . @jorgecarleitao and I signed off on it.

[roee88]

Hi @alamb,

Without jumping into conclusions.. there is also an option that the test itself is not the issue because it actually successfully passed the miri check at the time in #323 (I tested with miri quite a lot when I worked on the ffi parts). There have been changes to the ffi code since then and it's possible that one of them re-introduced the error that was fixed in #323.

[alamb]

Thanks @roee88 -- that is a good point. Whoever works on this ticket can perhaps look for other changes to FFI that may have introduced the MIRI issue as a way to isolate what was going on