Re: Reliance on undefined behaviour in << operator

On Wed, Sep 16, 2015 at 3:16 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> Our implementation of << is a direct wrapper around the C operator. It
> does not check the right-hand side's value.
>
> Datum
> int8shl(PG_FUNCTION_ARGS)
> {
> int64 arg1 = PG_GETARG_INT64(0);
> int32 arg2 = PG_GETARG_INT32(1);
>
> PG_RETURN_INT64(arg1 << arg2);
> }
>
> This means that an operation like:
>
> 1::bigint << 65
>
> directly relies on the compiler and platforms' handling of the
> undefined shift. On x64 intel gcc linux it does a rotation but that's
> not AFAIK guaranteed by anything, and we should probably not be
> relying on this or exposing it at the user level.

I agree.

> Pg returns:
>
> test=> SELECT BIGINT '1' << 66;
> ?column?
> ----------
> 4
> (1 row)
>
> A test program:
>
> #include "stdio.h"
> int main(int argc, char * argv[])
> {
> printf("Result is %ld", 1l << 66);
> return 0;
> }
>
> returns zero when the compiler constant-folds, but when done at runtime:
>
> #include <stdio.h>
> #include <stdlib.h>
> int main(int argc, char * argv[])
> {
> const char * num = "66";
> printf("Result is %ld", 1l << atoi(num));
> return 0;
> }
>
>
> IMO we should specify the behaviour in this case. Then issue a WARNING
> that gets promoted to an ERROR in a few versions.

I disagree. Such warnings are prone to be really annoying, e.g. by
generating massive log spam. I'd say that we should either make a
large shift return 0 - which seems like the intuitively right behavior
to me: if you shift in N zeros where N is greater than the the word
size, then you should end up with all zeroes - or throw an error right
away. I'd vote for the former.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company