I came across the following code and was wondering if it was really sound (obviously this breaks tokio's type encapsulation, but this is for a debugging/tracepoint library, that part is OK here: USDT tracepoints only take integer types, that is part of the kernel ABI for this feature.):

    #[inline]
    fn id_to_u64(id: tokio::task::Id) -> u64 {
        unsafe {
            // SAFETY: Based on training and experience, I know that a
            // `tokio::task::Id` is represented as a single `NonZeroU64`.
            union TrustMeOnThis {
                id: tokio::task::Id,
                int: NonZeroU64,
            }
            const {
                assert!(size_of::<tokio::task::Id>() == size_of::<NonZeroU64>());
                assert!(align_of::<tokio::task::Id>() == align_of::<NonZeroU64>());
            };
            TrustMeOnThis { id }.int.get()
        }
    }

While the statement is currently true, what happens if Id changes to a u64 instead of a NonZeroU64? That would mean you could create a NonZeroU64 with an invalid bit pattern.

However, I think the code would be sound if it was changed to:

    #[inline]
    fn id_to_u64(id: tokio::task::Id) -> u64 {
        unsafe {
            // SAFETY: Based on training and experience, I know that a
            // `tokio::task::Id` is represented as a single `NonZeroU64`.
            union TrustMeOnThis {
                id: tokio::task::Id,
                int: u64,
            }
            const {
                assert!(size_of::<tokio::task::Id>() == size_of::<u64>());
                assert!(align_of::<tokio::task::Id>() == align_of::<u64>());
            };
            TrustMeOnThis { id }.int
        }
    }

My reasoning is that it should be perfectly fine since all bit patterns of a u64 is valid, so regardless of what the Id is, it should be sound.

6 posts - 3 participants

Read full topic