BUG #5237: strange int->bit and bit->int conversions
The following bug has been logged online:
Bug reference: 5237
Logged by: Roman Kononov
Email address: kononov@ftml.net
PostgreSQL version: 8.4.1
Operating system: GNU/Linux x86_64
Description: strange int->bit and bit->int conversions
Details:
test=# select (11::int4<<23 | 11::int4)::bit(32);
00000101100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(33);
000001011100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(39);
000001010000101100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(40);
0000000000000101100000000000000000001011
The ::bit(33) and ::bit(39) conversions seem wrong.
test-std=# select 1::int4::bit(32)::int4;
1
test-std=# select 1::int4::bit(33)::int4;
ERROR: integer out of range
Why is it out of range?
The bitfromint8() and bitfromint4() are hosed. They produce wrong
results when the BIT size is more than 64 and 32 respectively, and the
BIT size is not multiple of 8, and the most significant byte of the
integer value is not 0x00 or 0xff.
For example:
test=# select (11::int4<<23 | 11::int4)::bit(32);
00000101100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(33);
000001011100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(39);
000001010000101100000000000000000001011
test=# select (11::int4<<23 | 11::int4)::bit(40);
0000000000000101100000000000000000001011
The ::bit(33) and ::bit(39) conversions are wrong.
The patch re-implements the conversion functions.
Attachments:
int-to-bit.patchtext/x-patch; name=int-to-bit.patchDownload+32-58
Roman Kononov <kononov@ftml.net> writes:
The bitfromint8() and bitfromint4() are hosed. They produce wrong
results when the BIT size is more than 64 and 32 respectively, and the
BIT size is not multiple of 8, and the most significant byte of the
integer value is not 0x00 or 0xff.
Hm, you're right, it's definitely busted ...
The patch re-implements the conversion functions.
... but I don't much care for this patch. It's unreadable, uncommented,
and doesn't even try to follow Postgres coding conventions.
It looks to me like the actual bug is that the "first fractional byte"
case ought to shift by destbitsleft - 8 not srcbitsleft - 8. A
secondary problem (which your patch fails to fix) is that if the
compiler chooses to implement signed >> as zero-fill rather than
sign-bit-fill (which it is allowed to do per C99) then we'll get
wrong, or at least not the desired, results. So I arrive at
the attached patch.
Unfortunately we've just missed the window for 8.4.2 etc, but I'll
get this committed for the next updates.
regards, tom lane
Woops, forgot to copy the list.
Show quoted text
On Sat, Dec 12, 2009 at 2:15 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sat, Dec 12, 2009 at 2:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Roman Kononov <kononov@ftml.net> writes:
The bitfromint8() and bitfromint4() are hosed. They produce wrong
results when the BIT size is more than 64 and 32 respectively, and the
BIT size is not multiple of 8, and the most significant byte of the
integer value is not 0x00 or 0xff.Hm, you're right, it's definitely busted ...
The patch re-implements the conversion functions.
... but I don't much care for this patch. It's unreadable, uncommented,
and doesn't even try to follow Postgres coding conventions.It looks to me like the actual bug is that the "first fractional byte"
case ought to shift by destbitsleft - 8 not srcbitsleft - 8. A
secondary problem (which your patch fails to fix) is that if the
compiler chooses to implement signed >> as zero-fill rather than
sign-bit-fill (which it is allowed to do per C99) then we'll get
wrong, or at least not the desired, results. So I arrive at
the attached patch.I'm not sure this fixes it, although I haven't tested. When we take
the /* store first fractional byte */ branch, destbitsleft is between
1 and 7 bits greater than srcbitsleft. We then subtract 8 from
destbitsleft, and the comment on the next line now asserts that the
two are equal. That doesn't seem right.Also, I thought about the sign extension problem, but aren't we
chopping those bits off anyway on the next line?...Robert
Import Notes
Reply to msg id not found: 603c8f070912121115g6ad01b5bk26b91d4826f232af@mail.gmail.com
Robert Haas <robertmhaas@gmail.com> writes:
I'm not sure this fixes it, although I haven't tested. �When we take
the /* store first fractional byte */ branch, destbitsleft is between
1 and 7 bits greater than srcbitsleft. �We then subtract 8 from
destbitsleft, and the comment on the next line now asserts that the
two are equal. �That doesn't seem right.
The comment's a bit bogus. What it should say, probably, is that the
*correct* value of srcbitsleft is now equal to destbitsleft so we aren't
bothering to track it anymore. If we were being fully anal we'd have
reduced srcbitsleft by some number less than 8 inside the if-branch.
Also, I thought about the sign extension problem, but aren't we
chopping those bits off anyway on the next line?
Nope, we'd have already stored the wrong bits into the output byte.
regards, tom lane