Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5ac0d93

Browse files
committed
Fix failure to zero-pad the result of bitshiftright().
If the bitstring length is not a multiple of 8, we'd shift therightmost bits into the pad space, which must be zeroes --- bit_cmp,for one, depends on that. This'd lead to the result failing tocompare equal to what it should compare equal to, as reported inbug #16013 from Daryl Waycott.This is, if memory serves, not the first such bug in the bitstringfunctions. In hopes of making it the last one, do a bit more workthan minimally necessary to fix the bug:* Add assertion checks to bit_out() and varbit_out() to complain ifthey are given incorrectly-padded input. This will improve theodds that manual testing of any new patch finds problems.* Encapsulate the padding-related logic in macros to make iteasier to use.Also, remove unnecessary padding logic from bit_or() and bitxor().Somebody had already noted that we need not re-pad the result ofbit_and() since the inputs are required to be the same length,but failed to extrapolate that to the other two.Also, move a comment block that once was near the head of varbit.c(but people kept putting other stuff in front of it), to put it inthe header block.Note for the release notes: if anyone has inconsistent data as aresult of saving the output of bitshiftright() in a table, it'spossible to fix it with something likeUPDATE mytab SET bitcol = ~(~bitcol) WHERE bitcol != ~(~bitcol);This has been broken since day one, so back-patch to all supportedbranches.Discussion:https://postgr.es/m/16013-c2765b6996aacae9@postgresql.org
1 parent0a2f894 commit5ac0d93

File tree

4 files changed

+154
-89
lines changed

4 files changed

+154
-89
lines changed

‎src/backend/utils/adt/varbit.c‎

Lines changed: 77 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,21 @@
33
* varbit.c
44
* Functions for the SQL datatypes BIT() and BIT VARYING().
55
*
6+
* The data structure contains the following elements:
7+
* header -- length of the whole data structure (incl header)
8+
* in bytes (as with all varying length datatypes)
9+
* data section -- private data section for the bits data structures
10+
* bitlength -- length of the bit string in bits
11+
* bitdata -- bit string, most significant byte first
12+
*
13+
* The length of the bitdata vector should always be exactly as many
14+
* bytes as are needed for the given bitlength. If the bitlength is
15+
* not a multiple of 8, the extra low-order padding bits of the last
16+
* byte must be zeroes.
17+
*
18+
* attypmod is defined as the length of the bit string in bits, or for
19+
* varying bits the maximum length.
20+
*
621
* Code originally contributed by Adriaan Joubert.
722
*
823
* Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
@@ -27,6 +42,40 @@
2742

2843
#defineHEXDIG(z) ((z)<10 ? ((z)+'0') : ((z)-10+'A'))
2944

45+
/* Mask off any bits that should be zero in the last byte of a bitstring */
46+
#defineVARBIT_PAD(vb) \
47+
do { \
48+
int32pad_ = VARBITPAD(vb); \
49+
Assert(pad_ >= 0 && pad_ < BITS_PER_BYTE); \
50+
if (pad_ > 0) \
51+
*(VARBITS(vb) + VARBITBYTES(vb) - 1) &= BITMASK << pad_; \
52+
} while (0)
53+
54+
/*
55+
* Many functions work byte-by-byte, so they have a pointer handy to the
56+
* last-plus-one byte, which saves a cycle or two.
57+
*/
58+
#defineVARBIT_PAD_LAST(vb,ptr) \
59+
do { \
60+
int32pad_ = VARBITPAD(vb); \
61+
Assert(pad_ >= 0 && pad_ < BITS_PER_BYTE); \
62+
if (pad_ > 0) \
63+
*((ptr) - 1) &= BITMASK << pad_; \
64+
} while (0)
65+
66+
/* Assert proper padding of a bitstring */
67+
#ifdefUSE_ASSERT_CHECKING
68+
#defineVARBIT_CORRECTLY_PADDED(vb) \
69+
do { \
70+
int32pad_ = VARBITPAD(vb); \
71+
Assert(pad_ >= 0 && pad_ < BITS_PER_BYTE); \
72+
Assert(pad_ == 0 || \
73+
(*(VARBITS(vb) + VARBITBYTES(vb) - 1) & ~(BITMASK << pad_)) == 0); \
74+
} while (0)
75+
#else
76+
#defineVARBIT_CORRECTLY_PADDED(vb) ((void) 0)
77+
#endif
78+
3079
staticVarBit*bit_catenate(VarBit*arg1,VarBit*arg2);
3180
staticVarBit*bitsubstring(VarBit*arg,int32s,int32l,
3281
boollength_not_specified);
@@ -87,24 +136,6 @@ anybit_typmodout(int32 typmod)
87136
}
88137

89138

90-
/*----------
91-
*attypmod -- contains the length of the bit string in bits, or for
92-
* varying bits the maximum length.
93-
*
94-
*The data structure contains the following elements:
95-
* header -- length of the whole data structure (incl header)
96-
* in bytes. (as with all varying length datatypes)
97-
* data section -- private data section for the bits data structures
98-
*bitlength -- length of the bit string in bits
99-
*bitdata -- bit string, most significant byte first
100-
*
101-
*The length of the bitdata vector should always be exactly as many
102-
*bytes as are needed for the given bitlength. If the bitlength is
103-
*not a multiple of 8, the extra low-order padding bits of the last
104-
*byte must be zeroes.
105-
*----------
106-
*/
107-
108139
/*
109140
* bit_in -
110141
* converts a char string to the internal representation of a bitstring.
@@ -264,6 +295,9 @@ bit_out(PG_FUNCTION_ARGS)
264295
len,
265296
bitlen;
266297

298+
/* Assertion to help catch any bit functions that don't pad correctly */
299+
VARBIT_CORRECTLY_PADDED(s);
300+
267301
bitlen=VARBITLEN(s);
268302
len= (bitlen+3) /4;
269303
result= (char*)palloc(len+2);
@@ -304,8 +338,6 @@ bit_recv(PG_FUNCTION_ARGS)
304338
VarBit*result;
305339
intlen,
306340
bitlen;
307-
intipad;
308-
bits8mask;
309341

310342
bitlen=pq_getmsgint(buf,sizeof(int32));
311343
if (bitlen<0||bitlen>VARBITMAXLEN)
@@ -330,13 +362,8 @@ bit_recv(PG_FUNCTION_ARGS)
330362

331363
pq_copymsgbytes(buf, (char*)VARBITS(result),VARBITBYTES(result));
332364

333-
/* Make sure last byte is zero-padded if needed */
334-
ipad=VARBITPAD(result);
335-
if (ipad>0)
336-
{
337-
mask=BITMASK <<ipad;
338-
*(VARBITS(result)+VARBITBYTES(result)-1) &=mask;
339-
}
365+
/* Make sure last byte is correctly zero-padded */
366+
VARBIT_PAD(result);
340367

341368
PG_RETURN_VARBIT_P(result);
342369
}
@@ -367,8 +394,6 @@ bit(PG_FUNCTION_ARGS)
367394
boolisExplicit=PG_GETARG_BOOL(2);
368395
VarBit*result;
369396
intrlen;
370-
intipad;
371-
bits8mask;
372397

373398
/* No work if typmod is invalid or supplied data matches it already */
374399
if (len <=0||len>VARBITMAXLEN||len==VARBITLEN(arg))
@@ -394,12 +419,7 @@ bit(PG_FUNCTION_ARGS)
394419
* if source data was shorter than target length (we assume the last byte
395420
* of the source data was itself correctly zero-padded).
396421
*/
397-
ipad=VARBITPAD(result);
398-
if (ipad>0)
399-
{
400-
mask=BITMASK <<ipad;
401-
*(VARBITS(result)+VARBITBYTES(result)-1) &=mask;
402-
}
422+
VARBIT_PAD(result);
403423

404424
PG_RETURN_VARBIT_P(result);
405425
}
@@ -574,6 +594,9 @@ varbit_out(PG_FUNCTION_ARGS)
574594
k,
575595
len;
576596

597+
/* Assertion to help catch any bit functions that don't pad correctly */
598+
VARBIT_CORRECTLY_PADDED(s);
599+
577600
len=VARBITLEN(s);
578601
result= (char*)palloc(len+1);
579602
sp=VARBITS(s);
@@ -620,8 +643,6 @@ varbit_recv(PG_FUNCTION_ARGS)
620643
VarBit*result;
621644
intlen,
622645
bitlen;
623-
intipad;
624-
bits8mask;
625646

626647
bitlen=pq_getmsgint(buf,sizeof(int32));
627648
if (bitlen<0||bitlen>VARBITMAXLEN)
@@ -646,13 +667,8 @@ varbit_recv(PG_FUNCTION_ARGS)
646667

647668
pq_copymsgbytes(buf, (char*)VARBITS(result),VARBITBYTES(result));
648669

649-
/* Make sure last byte is zero-padded if needed */
650-
ipad=VARBITPAD(result);
651-
if (ipad>0)
652-
{
653-
mask=BITMASK <<ipad;
654-
*(VARBITS(result)+VARBITBYTES(result)-1) &=mask;
655-
}
670+
/* Make sure last byte is correctly zero-padded */
671+
VARBIT_PAD(result);
656672

657673
PG_RETURN_VARBIT_P(result);
658674
}
@@ -729,8 +745,6 @@ varbit(PG_FUNCTION_ARGS)
729745
boolisExplicit=PG_GETARG_BOOL(2);
730746
VarBit*result;
731747
intrlen;
732-
intipad;
733-
bits8mask;
734748

735749
/* No work if typmod is invalid or supplied data matches it already */
736750
if (len <=0||len >=VARBITLEN(arg))
@@ -749,13 +763,8 @@ varbit(PG_FUNCTION_ARGS)
749763

750764
memcpy(VARBITS(result),VARBITS(arg),VARBITBYTES(result));
751765

752-
/* Make sure last byte is zero-padded if needed */
753-
ipad=VARBITPAD(result);
754-
if (ipad>0)
755-
{
756-
mask=BITMASK <<ipad;
757-
*(VARBITS(result)+VARBITBYTES(result)-1) &=mask;
758-
}
766+
/* Make sure last byte is correctly zero-padded */
767+
VARBIT_PAD(result);
759768

760769
PG_RETURN_VARBIT_P(result);
761770
}
@@ -1013,6 +1022,8 @@ bit_catenate(VarBit *arg1, VarBit *arg2)
10131022
}
10141023
}
10151024

1025+
/* The pad bits should be already zero at this point */
1026+
10161027
returnresult;
10171028
}
10181029

@@ -1046,14 +1057,12 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
10461057
intbitlen,
10471058
rbitlen,
10481059
len,
1049-
ipad=0,
10501060
ishift,
10511061
i;
10521062
inte,
10531063
s1,
10541064
e1;
1055-
bits8mask,
1056-
*r,
1065+
bits8*r,
10571066
*ps;
10581067

10591068
bitlen=VARBITLEN(arg);
@@ -1118,13 +1127,9 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
11181127
r++;
11191128
}
11201129
}
1121-
/* Do we need to pad at the end? */
1122-
ipad=VARBITPAD(result);
1123-
if (ipad>0)
1124-
{
1125-
mask=BITMASK <<ipad;
1126-
*(VARBITS(result)+len-1) &=mask;
1127-
}
1130+
1131+
/* Make sure last byte is correctly zero-padded */
1132+
VARBIT_PAD(result);
11281133
}
11291134

11301135
returnresult;
@@ -1246,7 +1251,7 @@ bit_and(PG_FUNCTION_ARGS)
12461251
for (i=0;i<VARBITBYTES(arg1);i++)
12471252
*r++=*p1++&*p2++;
12481253

1249-
/* Padding is not needed as & of 0pad is 0 */
1254+
/* Padding is not needed as & of 0pads is 0 */
12501255

12511256
PG_RETURN_VARBIT_P(result);
12521257
}
@@ -1268,7 +1273,6 @@ bit_or(PG_FUNCTION_ARGS)
12681273
bits8*p1,
12691274
*p2,
12701275
*r;
1271-
bits8mask;
12721276

12731277
bitlen1=VARBITLEN(arg1);
12741278
bitlen2=VARBITLEN(arg2);
@@ -1287,13 +1291,7 @@ bit_or(PG_FUNCTION_ARGS)
12871291
for (i=0;i<VARBITBYTES(arg1);i++)
12881292
*r++=*p1++ |*p2++;
12891293

1290-
/* Pad the result */
1291-
mask=BITMASK <<VARBITPAD(result);
1292-
if (mask)
1293-
{
1294-
r--;
1295-
*r &=mask;
1296-
}
1294+
/* Padding is not needed as | of 0 pads is 0 */
12971295

12981296
PG_RETURN_VARBIT_P(result);
12991297
}
@@ -1315,7 +1313,6 @@ bitxor(PG_FUNCTION_ARGS)
13151313
bits8*p1,
13161314
*p2,
13171315
*r;
1318-
bits8mask;
13191316

13201317
bitlen1=VARBITLEN(arg1);
13211318
bitlen2=VARBITLEN(arg2);
@@ -1335,13 +1332,7 @@ bitxor(PG_FUNCTION_ARGS)
13351332
for (i=0;i<VARBITBYTES(arg1);i++)
13361333
*r++=*p1++ ^*p2++;
13371334

1338-
/* Pad the result */
1339-
mask=BITMASK <<VARBITPAD(result);
1340-
if (mask)
1341-
{
1342-
r--;
1343-
*r &=mask;
1344-
}
1335+
/* Padding is not needed as ^ of 0 pads is 0 */
13451336

13461337
PG_RETURN_VARBIT_P(result);
13471338
}
@@ -1357,7 +1348,6 @@ bitnot(PG_FUNCTION_ARGS)
13571348
VarBit*result;
13581349
bits8*p,
13591350
*r;
1360-
bits8mask;
13611351

13621352
result= (VarBit*)palloc(VARSIZE(arg));
13631353
SET_VARSIZE(result,VARSIZE(arg));
@@ -1368,13 +1358,8 @@ bitnot(PG_FUNCTION_ARGS)
13681358
for (;p<VARBITEND(arg);p++)
13691359
*r++= ~*p;
13701360

1371-
/* Pad the result */
1372-
mask=BITMASK <<VARBITPAD(result);
1373-
if (mask)
1374-
{
1375-
r--;
1376-
*r &=mask;
1377-
}
1361+
/* Must zero-pad the result, because extra bits are surely 1's here */
1362+
VARBIT_PAD_LAST(result,r);
13781363

13791364
PG_RETURN_VARBIT_P(result);
13801365
}
@@ -1441,6 +1426,8 @@ bitshiftleft(PG_FUNCTION_ARGS)
14411426
*r=0;
14421427
}
14431428

1429+
/* The pad bits should be already zero at this point */
1430+
14441431
PG_RETURN_VARBIT_P(result);
14451432
}
14461433

@@ -1507,6 +1494,8 @@ bitshiftright(PG_FUNCTION_ARGS)
15071494
if ((++r)<VARBITEND(result))
15081495
*r= (*p << (BITS_PER_BYTE-ishift))&BITMASK;
15091496
}
1497+
/* We may have shifted 1's into the pad bits, so fix that */
1498+
VARBIT_PAD_LAST(result,r);
15101499
}
15111500

15121501
PG_RETURN_VARBIT_P(result);

‎src/include/utils/varbit.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121

2222
/*
2323
* Modeled on struct varlena from postgres.h, but data type is bits8.
24+
*
25+
* Caution: if bit_len is not a multiple of BITS_PER_BYTE, the low-order
26+
* bits of the last byte of bit_dat[] are unused and MUST be zeroes.
27+
* (This allows bit_cmp() to not bother masking the last byte.)
28+
* Also, there should not be any excess bytes counted in the header length.
2429
*/
2530
typedefstruct
2631
{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp