Re: strange failure in plpgsql_control tests (on fulmar, ICC 14.0.3)

On 03/17/2018 07:20 PM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> Not sure, but the backbranches seem to be working fine, and the commit
>> that triggers the issue is from December 31. Maybe the issue was there
>> but we were lucky not to trip on it before.
> 
> Yeah, we were simply not testing that overflow-detection code before.
> Undoubtedly it would fail in the back branches too if we tested it.
> 
>> Anyway, I can confirm that the fix suggested by Tom does the trick
>> (well, at least on Fulmar, which is running icc 14.0.3). I've also
>> disassembled exec_stmt_fori both with and without the patch - reading
>> assembly in not my strength, but if you're interested it's attached. The
>> interesting part seems to be the last ~50 lines or so.
> 
> Hm, did you get the "master" and "patched" versions backwards?  The
> allegedly-patched version does the !reverse case like this:
> 
>    0x00007f71219457ae <+2200>:	mov    -0x108(%rbp),%eax
>    0x00007f71219457b4 <+2206>:	test   %eax,%eax
>    0x00007f71219457b6 <+2208>:	jl     0x7f71219457cf <exec_stmt_fori+2233>
>    0x00007f71219457b8 <+2210>:	mov    -0x108(%rbp),%eax
>    0x00007f71219457be <+2216>:	add    -0x110(%rbp),%eax
>    0x00007f71219457c4 <+2222>:	mov    %eax,-0x110(%rbp)
>    0x00007f71219457ca <+2228>:	jmpq   0x7f7121945573 <exec_stmt_fori+1629>
> 
> so that it's apparently optimized
> 
>             if ((int32) (loop_value + step_value) < loop_value)
>                 break;
> 
> into
> 
>             if (step_value < 0)
>                 break;
> 
> which of course never exits the loop.  That's slightly different
> (and stupider) than what I'd been hypothesizing, but it's a valid
> transformation if you ignore the possibility of integer overflow.
> 

Yeah, it seems I've mixed up the files by accident. Sorry.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services