Peter Eisentraut <pete...@gmx.net> writes: > With clang -fsanitize=undefined (clang-3.4), I get the following test failure > in ecpg > (it's the only one in the entire tree):
Hm. I don't know why you can't reproduce that in the backend, because when stepping through DecodeDateTime() on the input select '19990108foobar'::timestamptz; I definitely see it shifting 1 left 31 places: Breakpoint 2, DecodeDateTime (field=<value optimized out>, ftype=<value optimized out>, nf=2, dtype=0x7ffff6fec168, tm=0x7ffff6fec170, fsec=0x7ffff6fec1b4, tzp=0x7ffff6fec16c) at datetime.c:1193 1193 type = DecodeTimezoneAbbrev(i, field[i], &val, &valtz); (gdb) n 1194 if (type == UNKNOWN_FIELD) (gdb) p type $1 = 31 (gdb) s 1195 type = DecodeSpecial(i, field[i], &val); (gdb) s DecodeSpecial (field=1, lowtoken=0x7ffff6febf89 "foobar", val=0x7ffff6febf28) at datetime.c:3018 3018 { (gdb) fin Run till exit from #0 DecodeSpecial (field=1, lowtoken=0x7ffff6febf89 "foobar", val=0x7ffff6febf28) at datetime.c:3031 DecodeDateTime (field=<value optimized out>, ftype=<value optimized out>, nf=2, dtype=0x7ffff6fec168, tm=0x7ffff6fec170, fsec=0x7ffff6fec1b4, tzp=0x7ffff6fec16c) at datetime.c:1196 1196 if (type == IGNORE_DTF) Value returned is $2 = 31 (gdb) s 1199 tmask = DTK_M(type); (gdb) p type $3 = 31 (gdb) s 1200 switch (type) (gdb) p tmask $4 = -2147483648 > This patch fixes it: > -#define DTK_M(t) (0x01 << (t)) > +#define DTK_M(t) ((t) == UNKNOWN_FIELD ? 0 : 0x01 << (t)) Don't like that even a little bit. The intent of the code is perfectly clear, cf this comment in datetime.h: * Field types for time decoding. * * Can't have more of these than there are bits in an unsigned int * since these are turned into bit masks during parsing and decoding. So I think the correct fix is -#define DTK_M(t) (0x01 << (t)) +#define DTK_M(t) (0x01U << (t)) It looks to me like it doesn't actually matter at the moment, because anyplace where we apply DTK_M to a value that could be UNKNOWN_FIELD, we'll immediately after that either return an error or replace the tmask value with something else. So the lack of portability of this construction hasn't mattered. But we should fix it in a way that won't create time bombs for future code changes, and producing a zero mask from a valid field type code would be a time bomb. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers