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

On March 17, 2018 7:56:40 AM PDT, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>I wrote:
>> Ouch. That test is in fact new as of 31 Dec, and what this seems to
>> prove is that plpgsql's handling of loop-variable overflow doesn't
>> work on fulmar.
>
>Some of the other icc-using critters haven't reported in since
>December, either :-(
>
>Looking at the code, we do this like so:
>
> /*
> * Increase/decrease loop value, unless it would overflow, in which
> * case exit the loop.
> */
> if (stmt->reverse)
> {
> if ((int32) (loop_value - step_value) > loop_value)
> break;
> loop_value -= step_value;
> }
> else
> {
> if ((int32) (loop_value + step_value) < loop_value)
> break;
> loop_value += step_value;
> }
>
>I imagine what's happening is that the compiler is assuming no overflow
>occurs (due to lacking any equivalent of -fwrapv), then deducing that
>the
>if-tests are no-ops and throwing them away.
>
>We could avoid the dependency on -fwrapv with something like
>
> if (stmt->reverse)
> {
> if (loop_value < (PG_INT32_MIN + step_value))
> break;
> loop_value -= step_value;
> }
> else
> {
> if (loop_value > (PG_INT32_MAX - step_value))
> break;
> loop_value += step_value;
> }
>
>which is safe because we enforce step_value > 0. It's kind of ugly
>because it hard-codes knowledge of what the limits are, but we may not
>have much choice.

On the current branch just using the new overflow safe functions in int.h should work. But unless we are OK leaving this broken in the back branches, or want to backport the functionality, that's probably not sufficient.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.