Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33986 )
Change subject: util/cbfstool: Use 64 bit integers in multiplications ......................................................................
Patch Set 1:
Patch Set 1:
(1 comment)
I'm somewhat confused how multiplying two u16 into an s32 could cause overflow or sign issues? This seems unnecessary. (Also, if you do want to do this, note that size_t is dependent on the host architecture which is not guaranteed to be x86_64. You should use an explicit-width type like Elf64_Off or uint64_t.)
It is possible for the two u16 to overflow an s32, which will lead to a negative number (which is already UB), and then converting to a u64 will sign-extend it to an extremely large integer. Eg.
int32_t x = -1; uint64_t y = x; // x is sign-extended to UINT64_MAX
For the size_t issue I suppose 64 bits itself doesn't matter as long as it is at least a u32, which can always hold a u16 * u16. I think this should be true on all platforms we support. I'll clarify this in the commit message.