2072302 - Reject zero-sized element types in ThinVec gecko-ffi mode

Status: RESOLVED FIXED

(Core :: XPCOM, task)

Description

[Max Inden]

Spun off bug 2067654, at emilio's suggestion.

The Happy Eyeballs glue declared the C++ mozilla::net::NetAddr as an opaque handle:

#[repr(C)]
pub union NetAddr {
    _private: [u8; 0],
}

and then used it as a collection element type, ThinVec<NetAddr>, which cbindgen renames to nsTArray<NetAddr> on the C++ side. C++ passed an array of real 112-byte elements and Rust walked it with a stride of 0. The length and the base pointer were both right, so nothing crashed and no debug_assert! fired: an N-address DNS answer became N copies of the first address.

Proposal: make ThinVec<T> refuse a zero-sized T when gecko-ffi is on. data_raw() already notes that padding::<T>() holds "~static assertions against types that are incompatible with the current feature flags" and is called before every data pointer.

#[cfg(all(feature = "gecko-ffi", not(test)))]
const {
    assert!(
        mem::size_of::<T>() != 0,
        "ThinVec<T> cannot bridge to nsTArray<T> when T is zero-sized"
    )
};

Tried against the vendored 0.2.16 with a crate that only borrows and iterates a ThinVec<NetAddr> and never constructs one, which is the shape bug 2067654 had:

error[E0080]: evaluation panicked: ThinVec<T> cannot bridge to nsTArray<T> when T is zero-sized
note: the above error was encountered while instantiating `fn ThinVec::<NetAddr>::data_raw`

Two caveats:

• It is a post-monomorphization error, so cargo check stays quiet and only a real build reports it. That is enough here, since every glue crate gets built.
• thin-vec's own tests use ThinVec<()> (thin_vec![(); 5], ThinVec::<()>::with_capacity), so the assert needs the not(test) arm, the way EMPTY_HEADER is already cfg'd, or cargo test --features gecko-ffi breaks.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: GitHub Pull Request

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2072302 - Avoid using ThinVec with a zero-size type in speculationrules. r=#dom-core,avandolder

A single variant enum uses a ZST under the hood by default. Prevent it
to be able to forbid ZSTs in ThinVec altogether.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2072302 - Update thin-vec to forbid ZSTs when in FFI mode. r=mxinden

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/73f959f21589
https://hg.mozilla.org/integration/autoland/rev/b546fdb2c8a1
Avoid using ThinVec with a zero-size type in speculationrules. r=avandolder,dom-core-reviewers

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a24f385ace94
https://hg.mozilla.org/integration/autoland/rev/c2c74e6e07b0
Update thin-vec to forbid ZSTs when in FFI mode. r=mxinden,supply-chain-reviewers

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b546fdb2c8a1

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c2c74e6e07b0