fix: const eval of *_with_overflow intrinsics for signed integers

[MintSoup]

The consteval overflow flag calculation for the signed with_overflow functions is not correct.
Example:

const FLAG: bool = 0i8.overflowing_sub(1).1;

Hover over FLAG shows true whereas it should clearly be false.

The blast radius of this bug is much larger than this contrived example, as this also breaks struct layout calculation for signed NonZero types. This in turn breaks layout calculation for structs from many popular crates in the ecosystem (notably chrono naive types) and their transitive dependents.

Fixes #22871.

AI disclosure: AI has been used to help troubleshoot the issue and write the fix.

[ChayimFriedman2]

The existing check is good enough (and simpler). You don't need to duplicate it, and certainly not change it. The if should only return (ans, u128overflow, if is_signed { 0xFF } then { 0 }) and then you change the check for 0 in out_of_range to check for 0 or 0xFF.

View changes since the review

[MintSoup]

Did you mean something like this?

let is_signed = matches!(lhs.ty.kind(), TyKind::Int(_));
let op_size = self.size_of_sized(lhs.ty, locals, "operand of add_with_overflow")?;
let lhs = u128::from_le_bytes(pad16(lhs.get(self)?, is_signed));
let rhs = u128::from_le_bytes(pad16(rhs.get(self)?, is_signed));
let (ans, is_overflow) = match name {
    "add_with_overflow" => lhs.overflowing_add(rhs),
    "sub_with_overflow" => lhs.overflowing_sub(rhs),
    "mul_with_overflow" => lhs.overflowing_mul(rhs),
    _ => unreachable!(),
};
let extra = if is_signed { 0xFF } else { 0 };
let out_of_range =
    ans.to_le_bytes()[op_size..].iter().any(|&it| it != 0 && it != extra);
let is_overflow = vec![u8::from(is_overflow || out_of_range)];

I'm afraid this won't work for sub_with_overflow(0i8, 1)

[ChayimFriedman2]

No, rather:

let (ans, is_overflow) = if is_signed {
    let (lhs, rhs) = (lhs as i128, rhs as i128);
    let ans = match name {
        "add_with_overflow" => lhs.overflowing_add(rhs),
        "sub_with_overflow" => lhs.overflowing_sub(rhs),
        "mul_with_overflow" => lhs.overflowing_mul(rhs),
        _ => unreachable!(),
    } as u128;
    (ans, 0xFF)
} else {
    let ans = match name {
        "add_with_overflow" => lhs.overflowing_add(rhs),
        "sub_with_overflow" => lhs.overflowing_sub(rhs),
        "mul_with_overflow" => lhs.overflowing_mul(rhs),
        _ => unreachable!(),
    };
    (ans, 0)
};

[MintSoup]

That doesn't compile, I believe you meant this?

let (ans, is_overflow, bg) = if is_signed {
    let (lhs, rhs) = (lhs as i128, rhs as i128);
    let (ans, overflow) = match name {
        "add_with_overflow" => lhs.overflowing_add(rhs),
        "sub_with_overflow" => lhs.overflowing_sub(rhs),
        "mul_with_overflow" => lhs.overflowing_mul(rhs),
        _ => unreachable!(),
    };
    (ans as u128, overflow, 0xFF)
} else {
    let (ans, overflow) = match name {
        "add_with_overflow" => lhs.overflowing_add(rhs),
        "sub_with_overflow" => lhs.overflowing_sub(rhs),
        "mul_with_overflow" => lhs.overflowing_mul(rhs),
        _ => unreachable!(),
    };
    (ans, overflow, 0)
};

let out_of_range =
    ans.to_le_bytes()[op_size..].iter().any(|&it| it != bg && it != 0);

Still fails, for e.g. -128 - 1, reports as no overflow. I believe the background byte needs to depend on the actual value, not just the signedness.