bit2swaz commented on issue #10679:
URL: https://github.com/apache/arrow-rs/issues/10679#issuecomment-5334574651
> some sort of reproducer (ad hoc) that shows this is actually a bug
import a schema from pyarrow but keep it as an `FFI_ArrowSchema` instead of
converting to a `Schema`, then call `with_metadata`:
```rust
let mut c_schema = FFI_ArrowSchema::empty();
py_schema.call_method1("_export_to_c", (&raw mut c_schema as usize,))?;
// c_schema.private_data now belongs to pyarrow, not a SchemaPrivateData
let c_schema = c_schema.with_metadata([("k", "v")])?; // safe api, no unsafe
```
`with_metadata` runs the [cast at
ffi.rs#L246](https://github.com/apache/arrow-rs/blob/cb20cc4bfd0fd29bf31d4c4c5c783174d9fcbff6/arrow-schema/src/ffi.rs#L246),
treating pyarrow's `private_data` as a `SchemaPrivateData` and writing thru
it. thats UB. `_export_to_c` is stable pyarrow api. arrow-rs converts an
imported schema to a native `Schema` right away so this only bites a downstream
user who keeps the raw `FFI_ArrowSchema` around
one snag on B tho: the `release == release_schema` check doesnt hold up.
`std::ptr::fn_addr_eq` can return false for the same function, and it makes 6
ffi tests fail under miri (which CI runs) so it can wrongly reject a schema
arrow-rs built
B needs a different detector then, like a magic tag as the first field of
`SchemaPrivateData`. do you want me to sort that out, or just make the methods
`unsafe` (A)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
