anza-xyz / anza-xyz/wincode

`#[wincode(assert_zero_copy)` asymmetry

Open
#383 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
224
Forks
46
Avg merge
7d 15h
Merged PRs (30d)
4

Description

When deriving implementations for `SchemaWrite` together with the `#[wincode(assert_zero_copy)]` directive, the implementation only considers the `SchemaRead` implementation. This implicitly adds the trait bound for `SchemaRead` for that type. Moreover, it consumes the `TYPE_META` property from the `SchemaRead` implementation - which means the `SchemaWrite::TYPE_META` property is ignored, see PoC.

[Code](https://github.com/anza-xyz/wincode/blob/ce424cbcd1c3cca34ad16b8d462feb444fcbf172/wincode-derive/src/assert_zero_copy.rs#L55-L82)
```rust
const _assert_schema_read_impl: fn() = || {
// implicitly assumes we have a `SchemaRead` implementation
fn assert_schema_read_impl<'de, T: #crate_name::SchemaRead<'de, #config_path>>() {}
assert_schema_read_impl::<#ident>()
};
// ...
// Assert the struct has no padding bytes.
const _assert_no_padding: () = {
// we only look at SchemaRead implementation
if let #crate_name::TypeMeta::Static { size, .. } = <#ident as #crate_name::SchemaRead<'_, #config_path>>::TYPE_META {
if size != core::mem::size_of::<#ident>() {
panic!("wincode(assert_zero_copy) was applied to a type with padding");
}
} else {
panic!("wincode(assert_zero_copy) was applied to a type with `TypeMeta::Dynamic`");
}
};
```

### Fix

Generalize `assert_zero_copy` to assume the correct trait depending on where it is called from.

### PoC

```rust
/// A field type with deliberately asymmetric metadata: `SchemaRead::TYPE_META` claims a
/// padding-free `Static` zero-copy layout, while `SchemaWrite::TYPE_META` is `Dynamic`.
struct AsymMeta(u8);

// SAFETY: asserted only to exercise the derive's zero-copy assertions; `AsymMeta` is a
// single byte with no invalid bit patterns.
unsafe impl config::ZeroCopy for AsymMeta {}

unsafe impl SchemaWrite for AsymMeta {
type Src = Self;

// The write side does NOT advertise a static zero-copy layout.
const TYPE_META: TypeMeta = TypeMeta::Dynamic;

fn size_of(_src: &Self::Src) -> WriteResult {
Ok(1)
}
fn write(writer: impl Writer, src: &Self::Src) -> WriteResult<()> {
>::write(writer, &src.0)
}
}

unsafe impl<'de, C: Config> SchemaRead<'de, C> for AsymMeta {
type Dst = Self;

// The read side advertises a padding-free static zero-copy layout.
const TYPE_META: TypeMeta = TypeMeta::Static {
size: 1,
zero_copy: true,
};

fn read(reader: impl Reader<'de>, dst: &mut MaybeUninit) -> ReadResult<()> {
dst.write(AsymMeta(>::get(reader)?));
Ok(())
}
}

/// Derives `SchemaWrite` with `#[wincode(assert_zero_copy)]`, yet its
/// `SchemaWrite::TYPE_META` is `Dynamic` (inherited from the `AsymMeta` field).
///
/// It compiles anyway because `assert_zero_copy`'s padding check
/// (`wincode-derive/src/assert_zero_copy.rs:74-82`) inspects
/// `::TYPE_META` (which is `Static`), not the `SchemaWrite`
/// metadata produced by the derive the assertion is attached to.
#[repr(transparent)]
#[derive(SchemaWrite, SchemaRead)]
#[wincode(internal, assert_zero_copy)]
struct WrapperWriteDynamic(AsymMeta);

/// Showcases that the `SchemaWrite` derive's `assert_zero_copy` validates the wrong
/// trait's metadata. `WrapperWriteDynamic` passed the zero-copy assertion at compile
/// time (otherwise this test would not build), yet its `SchemaWrite::TYPE_META` — the
/// metadata governing the write path the assertion is attached to — is `Dynamic`.
#[test]
fn test_assert_zero_copy_ignores_schema_write_type_meta() {
// The write metadata the assertion should have validated is `Dynamic`...
assert!(matches!(
>::TYPE_META,
TypeMeta::Dynamic
));
// ...but the assertion only looked at the read metadata, which is `Static`.
assert!(matches!(
>::TYPE_META,
TypeMeta::Static { .. }
));
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.