Re: [PATCH][RFC] Fix UBSAN in postreload-gcse.c (PR rtl-optimization/87868).
On 11/6/18 7:55 PM, Jeff Law wrote: > On 11/6/18 7:05 AM, Martin Liška wrote: >> Hi. >> >> The patch is adding a check overflow in eliminate_partially_redundant_load. >> Question is whether the usage of conditional compilation of >> __builtin_mul_overflow >> is fine? >> >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> 2018-11-06 Martin Liska >> >> PR rtl-optimization/87868 >> * postreload-gcse.c (eliminate_partially_redundant_load): Set >> threshold to max_count if we would overflow. >> * profile-count.h: Make max_count a public constant. > OK. Though I do worry about how many of these things we'll have to > sprinkle over the sources over time. I suspect there's all kinds of > overflows just waiting to happen, some are obviously more important than > others. Sure! Btw. I've been preparing patch that will limit some of --param values as they tent to overlap if a user selects a big-enough value. Martin > > jeff >
Re: [PATCH][RFC] Fix UBSAN in postreload-gcse.c (PR rtl-optimization/87868).
On 11/6/18 7:05 AM, Martin Liška wrote: > Hi. > > The patch is adding a check overflow in eliminate_partially_redundant_load. > Question is whether the usage of conditional compilation of > __builtin_mul_overflow > is fine? > > Thanks, > Martin > > gcc/ChangeLog: > > 2018-11-06 Martin Liska > > PR rtl-optimization/87868 > * postreload-gcse.c (eliminate_partially_redundant_load): Set > threshold to max_count if we would overflow. > * profile-count.h: Make max_count a public constant. OK. Though I do worry about how many of these things we'll have to sprinkle over the sources over time. I suspect there's all kinds of overflows just waiting to happen, some are obviously more important than others. jeff
[PATCH][RFC] Fix UBSAN in postreload-gcse.c (PR rtl-optimization/87868).
Hi. The patch is adding a check overflow in eliminate_partially_redundant_load. Question is whether the usage of conditional compilation of __builtin_mul_overflow is fine? Thanks, Martin gcc/ChangeLog: 2018-11-06 Martin Liska PR rtl-optimization/87868 * postreload-gcse.c (eliminate_partially_redundant_load): Set threshold to max_count if we would overflow. * profile-count.h: Make max_count a public constant. --- gcc/postreload-gcse.c | 14 -- gcc/profile-count.h | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/gcc/postreload-gcse.c b/gcc/postreload-gcse.c index b56993183d0..399970c368a 100644 --- a/gcc/postreload-gcse.c +++ b/gcc/postreload-gcse.c @@ -1170,8 +1170,18 @@ eliminate_partially_redundant_load (basic_block bb, rtx_insn *insn, if (ok_count.to_gcov_type () < GCSE_AFTER_RELOAD_PARTIAL_FRACTION * not_ok_count.to_gcov_type ()) goto cleanup; - if (ok_count.to_gcov_type () - < GCSE_AFTER_RELOAD_CRITICAL_FRACTION * critical_count.to_gcov_type ()) + + gcov_type threshold; +#if (GCC_VERSION >= 5000) + if (__builtin_mul_overflow (GCSE_AFTER_RELOAD_CRITICAL_FRACTION, + critical_count.to_gcov_type (), &threshold)) +threshold = profile_count::max_count; +#else + threshold += GCSE_AFTER_RELOAD_CRITICAL_FRACTION * critical_count.to_gcov_type (); +#endif + + if (ok_count.to_gcov_type () < threshold) goto cleanup; /* Generate moves to the loaded register from where diff --git a/gcc/profile-count.h b/gcc/profile-count.h index f4d0c340a0a..5d3bcc75f6d 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -641,8 +641,8 @@ public: type to hold various extra stages. */ static const int n_bits = 61; -private: static const uint64_t max_count = ((uint64_t) 1 << n_bits) - 2; +private: static const uint64_t uninitialized_count = ((uint64_t) 1 << n_bits) - 1; uint64_t m_val : n_bits;