Invalid PartialEq implementation for unions

[asomers]

union semun, in unix/bsd/apple/mod.rs, has three fields. On LP64, these fields will have unequal sizes. But its PartialEq and Hash implementations only check the smallest field. So the following code should fail (untested, because I don't have access to a Mac):

use libc::semun;

#[test]
fn semun_eq() {
    let x = semun{ val: 0x44556677 };
    let y = semun{ array: 0x0011223344556677 };
    assert!(x != y);
}

A similar situation holds on Linux for __c_anonymous_ptrace_syscall_info_data. That union has three members of different sizes, and its PartialEq implementation will return turn as long as any pair of fields compare equal. Effectively, that means it's only comparing the smallest field.

There are other unions in libc, but I haven't audited them all.

[RalfJung]

Note that an implementation that compares the larger, ptr-sized fields would be unsound, since semun { val: 0 } leaves those fields uninitialized -- so reading them (at integer or raw ptr type) is UB.

[asomers]

Crap, I never thought of that. Are you saying that it's impossible to correctly implement PartialEq for a union type then? In that case, would the only way to do it be to create a newtype that always zero-initializes the entire union before initializing any field?

[RalfJung]

Are you saying that it's impossible to correctly implement PartialEq for a union type then?

Depends on what you mean by 'correctly'... if you mean it should compare all the bytes, then yes.

Probably the type should just not implement PartialEq since the expected semantics cannot be provided. But it might be too late for that due to backwards compatibility constraints.

[notgull]

But it might be too late for that due to backwards compatibility constraints.

Is there some way to indicate that the PartialEq implementation is deprecated? I think that would be useful if we wanted to move in the direction of removing it.

[asomers]

I don't think so. AFAIK you still can't deprecate a trait impl rust-lang/rust#39935 .

[Ralith]

New instance of this unsoundness are still being introduced, e.g.:

libc/src/unix/haiku/native.rs

Lines 516 to 525 in a0f5b4b

	impl PartialEq for __c_anonymous_cpu_topology_info_data {
	fn eq(&self, other: &__c_anonymous_cpu_topology_info_data) -> bool {
	unsafe {
	self.root == other.root
	|| self.package == other.package
	|| self.core == other.core
	}
	}
	}
	impl Eq for __c_anonymous_cpu_topology_info_data {}

[tgross35]

To be discussed as part of #3880