Reliance on undefined behaviour in << operator - Mailing list pgsql-hackers

From Craig Ringer
Subject Reliance on undefined behaviour in << operator
Date
Msg-id CAMsr+YE+0KJuOJfbB2nLVfU+14R50Yi90e_8DewLV9jX+ro1zg@mail.gmail.com
Whole thread Raw
Responses Re: Reliance on undefined behaviour in << operator  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Re: Reliance on undefined behaviour in << operator  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Hi all

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.



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.

Consideration of << with a negative right-operand, and of
out-of-bounds >>, is probably also needed.

Thoughts?

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: T_PrivGrantee is left in NodeTag
Next
From: Amit Langote
Date:
Subject: Obsolete cross-references to set_append_rel_pathlist in comments