On Thu, 18 Apr 2024, Jakub Jelinek wrote: > Hi! > > __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. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK. Thanks, Richard. > 2024-04-18 Jakub Jelinek <ja...@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. > > --- gcc/internal-fn.cc.jj 2024-03-23 08:22:50.490607002 +0100 > +++ gcc/internal-fn.cc 2024-04-17 13:44:21.673594413 +0200 > @@ -1631,7 +1631,11 @@ expand_mul_overflow (location_t loc, tre > rtx target = NULL_RTX; > signop sign; > enum insn_code icode; > + int save_flag_trapv = flag_trapv; > > + /* We don't want any __mulv?i3 etc. calls from the expansion of > + these internal functions, so disable -ftrapv temporarily. */ > + flag_trapv = 0; > done_label = gen_label_rtx (); > do_error = gen_label_rtx (); > > @@ -2479,6 +2483,7 @@ expand_mul_overflow (location_t loc, tre > else > expand_arith_overflow_result_store (lhs, target, mode, res); > } > + flag_trapv = save_flag_trapv; > } > > /* Expand UBSAN_CHECK_* internal function if it has vector operands. */ > @@ -2499,7 +2504,11 @@ expand_vector_ubsan_overflow (location_t > rtx resvr = NULL_RTX; > unsigned HOST_WIDE_INT const_cnt = 0; > bool use_loop_p = (!cnt.is_constant (&const_cnt) || const_cnt > 4); > + int save_flag_trapv = flag_trapv; > > + /* We don't want any __mulv?i3 etc. calls from the expansion of > + these internal functions, so disable -ftrapv temporarily. */ > + flag_trapv = 0; > if (lhs) > { > optab op; > @@ -2629,6 +2638,7 @@ expand_vector_ubsan_overflow (location_t > } > else if (resvr) > emit_move_insn (lhsr, resvr); > + flag_trapv = save_flag_trapv; > } > > /* Expand UBSAN_CHECK_ADD call STMT. */ > @@ -2707,7 +2717,11 @@ expand_arith_overflow (enum tree_code co > prec0 = MIN (prec0, pr); > pr = get_min_precision (arg1, uns1_p ? UNSIGNED : SIGNED); > prec1 = MIN (prec1, pr); > + int save_flag_trapv = flag_trapv; > > + /* We don't want any __mulv?i3 etc. calls from the expansion of > + these internal functions, so disable -ftrapv temporarily. */ > + flag_trapv = 0; > /* If uns0_p && uns1_p, precop is minimum needed precision > of unsigned type to hold the exact result, otherwise > precop is minimum needed precision of signed type to > @@ -2748,6 +2762,7 @@ expand_arith_overflow (enum tree_code co > ops.location = loc; > rtx tem = expand_expr_real_2 (&ops, NULL_RTX, mode, EXPAND_NORMAL); > expand_arith_overflow_result_store (lhs, target, mode, tem); > + flag_trapv = save_flag_trapv; > return; > } > > @@ -2771,6 +2786,7 @@ expand_arith_overflow (enum tree_code co > if (integer_zerop (arg0) && !unsr_p) > { > expand_neg_overflow (loc, lhs, arg1, false, NULL); > + flag_trapv = save_flag_trapv; > return; > } > /* FALLTHRU */ > @@ -2781,6 +2797,7 @@ expand_arith_overflow (enum tree_code co > case MULT_EXPR: > expand_mul_overflow (loc, lhs, arg0, arg1, unsr_p, > unsr_p, unsr_p, false, NULL); > + flag_trapv = save_flag_trapv; > return; > default: > gcc_unreachable (); > @@ -2826,6 +2843,7 @@ expand_arith_overflow (enum tree_code co > else > expand_mul_overflow (loc, lhs, arg0, arg1, unsr_p, > uns0_p, uns1_p, false, NULL); > + flag_trapv = save_flag_trapv; > return; > } > > --- gcc/testsuite/gcc.dg/pr114753.c.jj 2024-04-17 13:55:16.246482369 > +0200 > +++ gcc/testsuite/gcc.dg/pr114753.c 2024-04-17 13:54:14.035352376 +0200 > @@ -0,0 +1,14 @@ > +/* PR middle-end/114753 */ > +/* { dg-do run } */ > +/* { dg-options "-O2 -ftrapv" } */ > + > +int > +main () > +{ > + volatile long long i = __LONG_LONG_MAX__; > + volatile long long j = 2; > + long long k; > + if (!__builtin_mul_overflow (i, j, &k) || k != -2LL) > + __builtin_abort (); > + return 0; > +} > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)