From 63358eaff6058e2fa0ade9b452a7ffd7ce3f4ba8 Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Fri, 26 Jun 2020 23:27:26 -0700 Subject: [PATCH] libpayload: cbgfx: Fix add_fractions() overflow reduction log2(1) is 0 and log2(0) is -1. If we have the int64_t 0xffffffff then log2(0xffffffff >> 31) = log2(0x1) = 0, so the current reduction code would not shift. That's a bad idea, though, since 0xffffffff when interpreted as an int32_t would become a negative number. We need to always shift one more than the current code does to get a safe reduction. This also means we can get rid of another compare/branch since -1 is the smallest result log2() can return, so the shift can no longer go negative now. Signed-off-by: Julius Werner Change-Id: Ib1eb6364c35c26924804261c02171139cdbd1034 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42845 Tested-by: build bot (Jenkins) Reviewed-by: Yu-Ping Wu Reviewed-by: Joel Kitching Reviewed-by: Paul Menzel --- payloads/libpayload/drivers/video/graphics.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/payloads/libpayload/drivers/video/graphics.c b/payloads/libpayload/drivers/video/graphics.c index 13eac28ea5..fa72c9b743 100644 --- a/payloads/libpayload/drivers/video/graphics.c +++ b/payloads/libpayload/drivers/video/graphics.c @@ -85,13 +85,9 @@ static void add_fractions(struct fraction *out, n = (int64_t)f1->n * f2->d + (int64_t)f2->n * f1->d; d = (int64_t)f1->d * f2->d; /* Simplest way to reduce the fraction until fitting in int32_t */ - shift = log2(MAX(ABS(n), ABS(d)) >> 31); - if (shift > 0) { - n >>= shift; - d >>= shift; - } - out->n = n; - out->d = d; + shift = log2(MAX(ABS(n), ABS(d)) >> 31) + 1; + out->n = n >> shift; + out->d = d >> shift; } static void add_scales(struct scale *out,