Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42845 )
Change subject: libpayload: cbgfx: Fix add_fractions() overflow reduction ......................................................................
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 jwerner@chromium.org Change-Id: Ib1eb6364c35c26924804261c02171139cdbd1034 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42845 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Yu-Ping Wu yupingso@google.com Reviewed-by: Joel Kitching kitching@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M payloads/libpayload/drivers/video/graphics.c 1 file changed, 3 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Joel Kitching: Looks good to me, approved Yu-Ping Wu: Looks good to me, but someone else must approve
diff --git a/payloads/libpayload/drivers/video/graphics.c b/payloads/libpayload/drivers/video/graphics.c index 13eac28..fa72c9b 100644 --- a/payloads/libpayload/drivers/video/graphics.c +++ b/payloads/libpayload/drivers/video/graphics.c @@ -85,13 +85,9 @@ 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,