1
0
mirror of https://github.com/SerenityOS/serenity synced 2024-07-01 11:15:37 +00:00

LibJS+AK: Fix integer overflow UB on (any Int32 - -2147483648)

It wasn't safe to use addition_would_overflow(a, -b) to check if
subtraction (a - b) would overflow, since it doesn't cover this case.

I don't know why we didn't have subtraction_would_overflow(), so this
patch adds it. :^)
This commit is contained in:
Andreas Kling 2024-05-18 10:58:11 +02:00
parent 1de475b404
commit b2e6843055
3 changed files with 27 additions and 1 deletions

View File

@ -359,6 +359,22 @@ public:
#endif
}
template<typename U, typename V>
[[nodiscard]] static constexpr bool subtraction_would_overflow(U u, V v)
{
#if __has_builtin(__builtin_sub_overflow_p)
return __builtin_sub_overflow_p(u, v, (T)0);
#elif __has_builtin(__builtin_sub_overflow)
T result;
return __builtin_sub_overflow(u, v, &result);
#else
Checked checked;
checked = u;
checked -= v;
return checked.has_overflow();
#endif
}
template<typename U, typename V>
static constexpr T saturating_add(U a, V b)
{

View File

@ -948,7 +948,7 @@ ThrowCompletionOr<void> Sub::execute_impl(Bytecode::Interpreter& interpreter) co
if (lhs.is_number() && rhs.is_number()) {
if (lhs.is_int32() && rhs.is_int32()) {
if (!Checked<i32>::addition_would_overflow(lhs.as_i32(), -rhs.as_i32())) {
if (!Checked<i32>::subtraction_would_overflow(lhs.as_i32(), rhs.as_i32())) {
interpreter.set(m_dst, Value(lhs.as_i32() - rhs.as_i32()));
return {};
}

View File

@ -0,0 +1,10 @@
test("basic integer overflow correctness", () => {
expect(2147483647 + 1).toBe(2147483648);
expect(2147483648 - 1).toBe(2147483647);
expect(0 - 2147483647).toBe(-2147483647);
expect(0 - 2147483648).toBe(-2147483648);
expect(0 - -2147483647).toBe(2147483647);
expect(0 - -2147483648).toBe(2147483648);
expect(0 + -2147483647).toBe(-2147483647);
expect(0 + -2147483648).toBe(-2147483648);
});