miri_to_const bug on big-endian targets

[RalfJung]

Description

I just noticed the miri_to_const function. This seems to duplicate a bunch of the logic in try_destructure_mir_constant_for_diagnostics, maybe it would make sense to fully migrate to that query instead?

In particular the current logic is partially wrong: it has support for ConstValue::Indirect arrays of floats. However, using from_le_bytes to convert interpreter floats to an f32 is going to go wrong on big-endian targets. You need to take into account the endianess of the current compiler session. Or ideally you just use all the functions that already exist in rustc to work with interpreter data, e.g. Allocation::read_scalar.

(Also, we are using "Miri" as the name for the tool these days, so the function is somewhat misnamed. This is really just converting a MIR constant into a Clippy constant.)

Version

No response

Additional Labels

No response

[RalfJung]

Ah, in field_of_struct you are using try_destructure_mir_constant_for_diagnostics, nice. So it's really just that float array logic that should be changed to either read_scalar or try_destructure_mir_constant_for_diagnostics.

[RalfJung]

Oh, also that code is completely ignoring the offset in ConstValue::ByRef, so it could actually end up loading the wrong data from memory.

[Jarcho]

The scalar floats are also read wrong when cross compiling between big and little endian targets, right?

[RalfJung]

Yes, when I said they are wrong on big-endian targets I meant they are wrong whenever the --target is big-endian. The host endianess does not matter.

(Having a big-endian host and a little-endian target should work with the current code, though.)

[RalfJung]

Note that there were some recent rustc changes here, so if code this is now changed on the clippy side it would probably lead to conflicts. We should await the next rustc-to-clippy sync.

[Jarcho]

I was referring to conversion a little above that.

ty::Float(FloatTy::F32) => Some(Constant::F32(f32::from_bits(
    int.try_into().expect("invalid f32 bit representation"),
))),
ty::Float(FloatTy::F64) => Some(Constant::F64(f64::from_bits(
    int.try_into().expect("invalid f64 bit representation"),
))),

[RalfJung]

That should be fine, ScalarInt is always using host endianess.