[tarantool-patches] Re: [PATCH 1/3] Fix: prevent guard-breaker optimization

[Alex Khatskevich]

On 10.10.2018 17:26, Alexander Turenko wrote:
> On Wed, Aug 08, 2018 at 02:10:01PM +0300, AKhatskevich wrote:
>> In case of very aggressive optimizations the compiler can
>> optimize guard-breaker function away and the `unit/guard`
>> test would fail.
> I think it is good to mention the specific compiler options (and a
> certain compiler you are using to reproduce it) to give a user an idea
> when things are going wrong and so what is the fix does.
>
> Is it due to discarding the noinline attribute in case of LTO on gcc? So
> 'volatile' prevents inlining the function? I just guessing here, but I
> think the commit message should clarify such things if possible.
>
>> ---
>>   test/unit/guard.cc | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/unit/guard.cc b/test/unit/guard.cc
>> index 231b44c7d..2082dfd48 100644
>> --- a/test/unit/guard.cc
>> +++ b/test/unit/guard.cc
>> @@ -13,7 +13,7 @@ static int __attribute__((noinline))
>>   stack_break_f(char *ptr)
>>   {
>>   	char block[2048];
>> -	char sum = 0;
>> +	volatile char sum = 0;
> I think a reason of using 'volatile' keyword should be always properly
> commented in the code, because it always fix some unobvious compiler
> behaviour.
Discussed verbally.
Added a comment near volatile keyword.
>>   	memset(block, 0xff, 2048);
>>   	sum += block[block[4]];
>>   	ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;
>> -- 
>> 2.14.1
>>
>>

New diff:

commit d7acdc09aa471ea15b3821bab205d8fcdb9280c1
Author: AKhatskevich <avkhatskevich at tarantool.org>
Date:   Tue Aug 7 16:47:39 2018 +0300

     Fix: prevent guard-breaker optimization

     In case of very aggressive optimizations the compiler can
     optimize guard-breaker function away and the `unit/guard`
     test would fail.

diff --git a/test/unit/guard.cc b/test/unit/guard.cc
index 231b44c7d..3d42fee31 100644
--- a/test/unit/guard.cc
+++ b/test/unit/guard.cc
@@ -13,7 +13,11 @@ static int __attribute__((noinline))
  stack_break_f(char *ptr)
  {
      char block[2048];
-    char sum = 0;
+    /*
+     * Make sum volatile to prevent a compiler from
+     * optimizing away call to this function.
+     */
+    volatile char sum = 0;
      memset(block, 0xff, 2048);
      sum += block[block[4]];
      ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;