On Wed, 2024-04-10 at 12:16 +0300, Alexander Monakov wrote: > > On Wed, 10 Apr 2024, stefan@xxxxxxxxx wrote: > > > Hi all, > > > > I just stumbled over an issue, which is present in almost all gcc versions. > > I worked around using inline assembly… > > Maybe gcc behaves correct and I am wrong? Here is the code: > > > > https://godbolt.org/z/cW8jcdh56 > > > > typedef unsigned long long int u64; > > typedef unsigned int u32; > > typedef unsigned short u16; > > > > u64 foo(u16 a, u16 b) { > > u32 x = a * b; > > u64 r = x; > > return r; > > } > > > > And on gcc 13.2 x86.64 you get > > > > foo: > > movzx esi, si > > movzx edi, di > > imul edi, esi > > movsx rax, edi > > ret > > > > > > There is a sign extension! The optimizer step discards the information > > > > x_6 = (u32) _3; > > > > and uses _3 directly instead, which is signed. > > > > Am I wrong or is it gcc? > > GCC is not wrong. When your code computes x: > > u32 x = a * b; > > 'a' and 'b' are first promoted to int according to C language rules, and > the multiplication happens in the signed int type, with UB on overflow. > The compiler deduces the range of signed int temporary holding the result > of the multiplication is [0, 0x7fffffff], which allows to propagate it > to the assignment of 'r' (which in the end produces a sign extension, > as you observed, so the propagation did not turn out to be useful). > > u16 * u16 is a famous footgun for sure. I'd suggest 'x = 1u * a * b' > as a fix for the code. Also note that -fsanitize=undefined detects the issue properly: $ cat t.c typedef unsigned long long int u64; typedef unsigned int u32; typedef unsigned short u16; __attribute__((noipa)) u64 foo(u16 a, u16 b) { u32 x = a * b; u64 r = x; return r; } int main() { __builtin_printf("%llx\n", foo(65535, 65535)); } $ cc t.c -O2 -fsanitize=undefined $ ./a.out t.c:7:15: runtime error: signed integer overflow: 65535 * 65535 cannot be represented in type 'int' fffe0001 -- Xi Ruoyao <xry111@xxxxxxxxxxx> School of Aerospace Science and Technology, Xidian University
