std::from_chars may abort when used with -m32 -ftrapv on some values. Without -m32, or without -ftrapv, or using clang, the code works correctly. To reproduce: $ cat a.cpp #include <charconv> #include <cstdint> #include <string_view> int main() { int64_t result{}; std::string_view str{"-9223372036854775808"}; (void)std::from_chars(str.begin(), str.end(), result); return result != -9223372036854775807 - 1; } $ g++ -m32 -ftrapv -std=c++17 ./a.cpp && ./a.out Aborted (core dumped)
The abort happens here: if (__builtin_mul_overflow(__val, __sign, &__tmp)) With __val = 9223372036854775808LL __sign = -1LL The libgcc2.c:__mulvdi3 function reaches the abort() on line 375
(In reply to Jonathan Wakely from comment #1) > With __val = 9223372036854775808LL __sign = -1LL Oops, that should be __val = 9223372036854775808ULL and __sign = -1 i.e. int main() { unsigned long long val = 9223372036854775808ULL; int sign = -1; long long res; return __builtin_mul_overflow(val, sign, &res); } This shouldn't trap.
Confirmed. .MUL_OVERFLOW expansion likely ends up calling expand_binop which decides for itself whether the op traps based on the result type.
#0 expand_binop (mode=E_DImode, binoptab=smulv_optab, op0=0x7ffff71b53f0, op1=0x7ffff71b85a0, target=0x0, unsignedp=0, methods=OPTAB_LIB_WIDEN) at /space/rguenther/src/gcc/gcc/optabs.cc:1497 #1 0x000000000116edf5 in expand_mult (mode=E_DImode, op0=0x7ffff71b53f0, op1=0x7ffff71b85a0, target=0x0, unsignedp=0, no_libcall=false) at /space/rguenther/src/gcc/gcc/expmed.cc:3610 #2 0x00000000011a38e0 in expand_expr_real_2 (ops=0x7fffffffcef0, target=0x0, tmode=E_DImode, modifier=EXPAND_NORMAL) at /space/rguenther/src/gcc/gcc/expr.cc:10275 #3 0x0000000001359608 in expand_mul_overflow (loc=2147483653, lhs=<ssa_name 0x7ffff7016ee8 6>, arg0=<nop_expr 0x7ffff7181aa0>, arg1=<nop_expr 0x7ffff7181a80>, unsr_p=false, uns0_p=false, uns1_p=true, is_ubsan=false, datap=0x0) at /space/rguenther/src/gcc/gcc/internal-fn.cc:2359 #4 0x000000000135b7f7 in expand_arith_overflow (code=MULT_EXPR, stmt=<gimple_call 0x7ffff7019f30>) at /space/rguenther/src/gcc/gcc/internal-fn.cc:2827 #5 0x000000000135b9c3 in expand_MUL_OVERFLOW (stmt=0x7ffff7019f30) at /space/rguenther/src/gcc/gcc/internal-fn.cc:2876 so a hack would be to reset flag_trapv around 2354 For unsigned multiplication when high parts are both non-zero 2355 this overflows always. */ 2356 ops.code = MULT_EXPR; 2357 ops.op0 = make_tree (type, op0); 2358 ops.op1 = make_tree (type, op1); 2359 tem = expand_expr_real_2 (&ops, NULL_RTX, mode, EXPAND_NORMAL); in expand_mul_overflow.
I wouldn't call it a hack, I'd say it is the right fix. Though, we have tons of those in internal-fn.cc ops.code = MULT_EXPR; ops.code = MULT_EXPR; ops.code = MULT_EXPR; ops.code = MULT_EXPR; ops.code = MULT_EXPR; ops.code = WIDEN_MULT_EXPR; ops.code = MULT_HIGHPART_EXPR; ops.code = MULT_EXPR; ops.code = WIDEN_MULT_EXPR; ops.code = MULT_EXPR; ops.code = MULT_EXPR; ops.code = PLUS_EXPR; ops.code = TRUNC_DIV_EXPR; ops.code = TRUNC_MOD_EXPR; plus some which reuse earlier set ops.code plus some which use just some variable as ops.code. Sure, I think WIDEN_MULT_EXPR/MULT_HIGHPART_EXPR and the div/mod neither probably consider flag_trapv, but MULT_EXPR/PLUS_EXPR does (though, perhaps the PLUS_EXPR is for vectors only). So, the question is where to put that. To cover everything, bet best would be to put it into expand_mul_overflow and expand_arith_overflow and expand_vector_ubsan_overflow.
Created attachment 57972 [details] gcc14-pr114753.patch Untested fix.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:6c152c9db3b5b9d43e12846fb7a44977c0b65fc2 commit r14-10012-g6c152c9db3b5b9d43e12846fb7a44977c0b65fc2 Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Apr 18 09:45:14 2024 +0200 internal-fn: Temporarily disable flag_trapv during .{ADD,SUB,MUL}_OVERFLOW etc. expansion [PR114753] __builtin_{add,sub,mul}_overflow{,_p} builtins are well defined for all inputs even for -ftrapv, and the -fsanitize=signed-integer-overflow ifns shouldn't abort in libgcc but emit the desired ubsan diagnostics or abort depending on -fsanitize* setting regardless of -ftrapv. The expansion of these internal functions uses expand_expr* in various places (e.g. MULT_EXPR at least in 2 spots), so temporarily disabling flag_trapv in all those spots would be hard. The following patch disables it around the bodies of 3 functions which can do the expand_expr calls. If it was in the C++ FE, I'd use some RAII sentinel, but I don't think we have one in the middle-end. 2024-04-18 Jakub Jelinek <jakub@redhat.com> PR middle-end/114753 * internal-fn.cc (expand_mul_overflow): Save flag_trapv and temporarily clear it for the duration of the function, then restore previous value. (expand_vector_ubsan_overflow): Likewise. (expand_arith_overflow): Likewise. * gcc.dg/pr114753.c: New test.
Fixed.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:33bf8e5385099c2963f278bff38e4f917eddf1d8 commit r14-10041-g33bf8e5385099c2963f278bff38e4f917eddf1d8 Author: Jakub Jelinek <jakub@redhat.com> Date: Fri Apr 19 18:15:39 2024 +0200 internal-fn: Fix up expand_arith_overflow [PR114753] During backporting I've noticed I've missed one return spot for the restoration of the original flag_trapv flag value. 2024-04-19 Jakub Jelinek <jakub@redhat.com> PR middle-end/114753 * internal-fn.cc (expand_arith_overflow): Add one missing restore of flag_trapv before return.
The releases/gcc-13 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:10f689596ad1633f4f5de1852c8f82a993fe948e commit r13-8635-g10f689596ad1633f4f5de1852c8f82a993fe948e Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Apr 18 09:45:14 2024 +0200 internal-fn: Temporarily disable flag_trapv during .{ADD,SUB,MUL}_OVERFLOW etc. expansion [PR114753] __builtin_{add,sub,mul}_overflow{,_p} builtins are well defined for all inputs even for -ftrapv, and the -fsanitize=signed-integer-overflow ifns shouldn't abort in libgcc but emit the desired ubsan diagnostics or abort depending on -fsanitize* setting regardless of -ftrapv. The expansion of these internal functions uses expand_expr* in various places (e.g. MULT_EXPR at least in 2 spots), so temporarily disabling flag_trapv in all those spots would be hard. The following patch disables it around the bodies of 3 functions which can do the expand_expr calls. If it was in the C++ FE, I'd use some RAII sentinel, but I don't think we have one in the middle-end. 2024-04-18 Jakub Jelinek <jakub@redhat.com> PR middle-end/114753 * internal-fn.cc (expand_mul_overflow): Save flag_trapv and temporarily clear it for the duration of the function, then restore previous value. (expand_vector_ubsan_overflow): Likewise. (expand_arith_overflow): Likewise. * gcc.dg/pr114753.c: New test. (cherry picked from commit 6c152c9db3b5b9d43e12846fb7a44977c0b65fc2)
Fixed for 13.3 too.
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:b3ef00f8b8d577d7b62cea36c13cf087a3b13d0c commit r12-10525-gb3ef00f8b8d577d7b62cea36c13cf087a3b13d0c Author: Jakub Jelinek <jakub@redhat.com> Date: Thu Apr 18 09:45:14 2024 +0200 internal-fn: Temporarily disable flag_trapv during .{ADD,SUB,MUL}_OVERFLOW etc. expansion [PR114753] __builtin_{add,sub,mul}_overflow{,_p} builtins are well defined for all inputs even for -ftrapv, and the -fsanitize=signed-integer-overflow ifns shouldn't abort in libgcc but emit the desired ubsan diagnostics or abort depending on -fsanitize* setting regardless of -ftrapv. The expansion of these internal functions uses expand_expr* in various places (e.g. MULT_EXPR at least in 2 spots), so temporarily disabling flag_trapv in all those spots would be hard. The following patch disables it around the bodies of 3 functions which can do the expand_expr calls. If it was in the C++ FE, I'd use some RAII sentinel, but I don't think we have one in the middle-end. 2024-04-18 Jakub Jelinek <jakub@redhat.com> PR middle-end/114753 * internal-fn.cc (expand_mul_overflow): Save flag_trapv and temporarily clear it for the duration of the function, then restore previous value. (expand_vector_ubsan_overflow): Likewise. (expand_arith_overflow): Likewise. * gcc.dg/pr114753.c: New test. (cherry picked from commit 6c152c9db3b5b9d43e12846fb7a44977c0b65fc2)
Should be fixed for 12.4+ too.