Re: [PATCH][RFC] Fix UBSAN in postreload-gcse.c (PR rtl-optimization/87868).

2018-11-07 Thread Martin Liška
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).

2018-11-06 Thread Jeff Law
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).

2018-11-06 Thread Martin Liška
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;