[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

Freddy, Ye via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 4 09:20:03 PDT 2021


FreddyYe marked an inline comment as done.
FreddyYe added a comment.

Addressed. THX review!



================
Comment at: clang/test/Sema/builtins-overflow.c:45
+    unsigned _ExtInt(128) result;
+    _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support special combination operands (signed, signed, unsigned*) of more than 64 bits}}
+  }
----------------
aaron.ballman wrote:
> Yeah, this diagnostic really doesn't tell me what's going wrong with the code or how to fix it. Do we basically want to prevent using larger-than-64-bit argument types with mixed signs? Or are there other problematic circumstances?
Yes, let me try to refine. I can explain more what happened to such input combination.
According to gcc's the definition on this builtin: https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
'These built-in functions promote the first two operands into infinite precision signed type and perform multiply on those promoted operands. The result is then cast to the type the third pointer argument points to and stored there.' 

Since signing integer has a smaller range than unsigned integer. And now the API in compiler-rt (`__muloti4`) to checking 128 integer's multiplying is implemented in signed version. So the overflow max absolute value it can check is 2^127. When the result input is larger equal than 128 bits, `__muloti4` has no usage. We should prevent this situation for now. Or the backend will crush as the example shows.

I found the input operand doesn't need both of them larger than 64 bits, but just the sum of their larger 128. I'll refine in my patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107420/new/

https://reviews.llvm.org/D107420



More information about the cfe-commits mailing list