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

Commit7e7abed

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 parent24f64fb commit7e7abed

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-2018, PostgreSQL Global Development Group
@@ -26,6 +41,40 @@
2641

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

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

88137

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

297+
/* Assertion to help catch any bit functions that don't pad correctly */
298+
VARBIT_CORRECTLY_PADDED(s);
299+
266300
bitlen=VARBITLEN(s);
267301
len= (bitlen+3) /4;
268302
result= (char*)palloc(len+2);
@@ -303,8 +337,6 @@ bit_recv(PG_FUNCTION_ARGS)
303337
VarBit*result;
304338
intlen,
305339
bitlen;
306-
intipad;
307-
bits8mask;
308340

309341
bitlen=pq_getmsgint(buf,sizeof(int32));
310342
if (bitlen<0||bitlen>VARBITMAXLEN)
@@ -329,13 +361,8 @@ bit_recv(PG_FUNCTION_ARGS)
329361

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

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

340367
PG_RETURN_VARBIT_P(result);
341368
}
@@ -366,8 +393,6 @@ bit(PG_FUNCTION_ARGS)
366393
boolisExplicit=PG_GETARG_BOOL(2);
367394
VarBit*result;
368395
intrlen;
369-
intipad;
370-
bits8mask;
371396

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

403423
PG_RETURN_VARBIT_P(result);
404424
}
@@ -573,6 +593,9 @@ varbit_out(PG_FUNCTION_ARGS)
573593
k,
574594
len;
575595

596+
/* Assertion to help catch any bit functions that don't pad correctly */
597+
VARBIT_CORRECTLY_PADDED(s);
598+
576599
len=VARBITLEN(s);
577600
result= (char*)palloc(len+1);
578601
sp=VARBITS(s);
@@ -619,8 +642,6 @@ varbit_recv(PG_FUNCTION_ARGS)
619642
VarBit*result;
620643
intlen,
621644
bitlen;
622-
intipad;
623-
bits8mask;
624645

625646
bitlen=pq_getmsgint(buf,sizeof(int32));
626647
if (bitlen<0||bitlen>VARBITMAXLEN)
@@ -645,13 +666,8 @@ varbit_recv(PG_FUNCTION_ARGS)
645666

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

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

656672
PG_RETURN_VARBIT_P(result);
657673
}
@@ -719,8 +735,6 @@ varbit(PG_FUNCTION_ARGS)
719735
boolisExplicit=PG_GETARG_BOOL(2);
720736
VarBit*result;
721737
intrlen;
722-
intipad;
723-
bits8mask;
724738

725739
/* No work if typmod is invalid or supplied data matches it already */
726740
if (len <=0||len >=VARBITLEN(arg))
@@ -739,13 +753,8 @@ varbit(PG_FUNCTION_ARGS)
739753

740754
memcpy(VARBITS(result),VARBITS(arg),VARBITBYTES(result));
741755

742-
/* Make sure last byte is zero-padded if needed */
743-
ipad=VARBITPAD(result);
744-
if (ipad>0)
745-
{
746-
mask=BITMASK <<ipad;
747-
*(VARBITS(result)+VARBITBYTES(result)-1) &=mask;
748-
}
756+
/* Make sure last byte is correctly zero-padded */
757+
VARBIT_PAD(result);
749758

750759
PG_RETURN_VARBIT_P(result);
751760
}
@@ -1003,6 +1012,8 @@ bit_catenate(VarBit *arg1, VarBit *arg2)
10031012
}
10041013
}
10051014

1015+
/* The pad bits should be already zero at this point */
1016+
10061017
returnresult;
10071018
}
10081019

@@ -1036,14 +1047,12 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
10361047
intbitlen,
10371048
rbitlen,
10381049
len,
1039-
ipad=0,
10401050
ishift,
10411051
i;
10421052
inte,
10431053
s1,
10441054
e1;
1045-
bits8mask,
1046-
*r,
1055+
bits8*r,
10471056
*ps;
10481057

10491058
bitlen=VARBITLEN(arg);
@@ -1108,13 +1117,9 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
11081117
r++;
11091118
}
11101119
}
1111-
/* Do we need to pad at the end? */
1112-
ipad=VARBITPAD(result);
1113-
if (ipad>0)
1114-
{
1115-
mask=BITMASK <<ipad;
1116-
*(VARBITS(result)+len-1) &=mask;
1117-
}
1120+
1121+
/* Make sure last byte is correctly zero-padded */
1122+
VARBIT_PAD(result);
11181123
}
11191124

11201125
returnresult;
@@ -1236,7 +1241,7 @@ bit_and(PG_FUNCTION_ARGS)
12361241
for (i=0;i<VARBITBYTES(arg1);i++)
12371242
*r++=*p1++&*p2++;
12381243

1239-
/* Padding is not needed as & of 0pad is 0 */
1244+
/* Padding is not needed as & of 0pads is 0 */
12401245

12411246
PG_RETURN_VARBIT_P(result);
12421247
}
@@ -1258,7 +1263,6 @@ bit_or(PG_FUNCTION_ARGS)
12581263
bits8*p1,
12591264
*p2,
12601265
*r;
1261-
bits8mask;
12621266

12631267
bitlen1=VARBITLEN(arg1);
12641268
bitlen2=VARBITLEN(arg2);
@@ -1277,13 +1281,7 @@ bit_or(PG_FUNCTION_ARGS)
12771281
for (i=0;i<VARBITBYTES(arg1);i++)
12781282
*r++=*p1++ |*p2++;
12791283

1280-
/* Pad the result */
1281-
mask=BITMASK <<VARBITPAD(result);
1282-
if (mask)
1283-
{
1284-
r--;
1285-
*r &=mask;
1286-
}
1284+
/* Padding is not needed as | of 0 pads is 0 */
12871285

12881286
PG_RETURN_VARBIT_P(result);
12891287
}
@@ -1305,7 +1303,6 @@ bitxor(PG_FUNCTION_ARGS)
13051303
bits8*p1,
13061304
*p2,
13071305
*r;
1308-
bits8mask;
13091306

13101307
bitlen1=VARBITLEN(arg1);
13111308
bitlen2=VARBITLEN(arg2);
@@ -1325,13 +1322,7 @@ bitxor(PG_FUNCTION_ARGS)
13251322
for (i=0;i<VARBITBYTES(arg1);i++)
13261323
*r++=*p1++ ^*p2++;
13271324

1328-
/* Pad the result */
1329-
mask=BITMASK <<VARBITPAD(result);
1330-
if (mask)
1331-
{
1332-
r--;
1333-
*r &=mask;
1334-
}
1325+
/* Padding is not needed as ^ of 0 pads is 0 */
13351326

13361327
PG_RETURN_VARBIT_P(result);
13371328
}
@@ -1347,7 +1338,6 @@ bitnot(PG_FUNCTION_ARGS)
13471338
VarBit*result;
13481339
bits8*p,
13491340
*r;
1350-
bits8mask;
13511341

13521342
result= (VarBit*)palloc(VARSIZE(arg));
13531343
SET_VARSIZE(result,VARSIZE(arg));
@@ -1358,13 +1348,8 @@ bitnot(PG_FUNCTION_ARGS)
13581348
for (;p<VARBITEND(arg);p++)
13591349
*r++= ~*p;
13601350

1361-
/* Pad the result */
1362-
mask=BITMASK <<VARBITPAD(result);
1363-
if (mask)
1364-
{
1365-
r--;
1366-
*r &=mask;
1367-
}
1351+
/* Must zero-pad the result, because extra bits are surely 1's here */
1352+
VARBIT_PAD_LAST(result,r);
13681353

13691354
PG_RETURN_VARBIT_P(result);
13701355
}
@@ -1431,6 +1416,8 @@ bitshiftleft(PG_FUNCTION_ARGS)
14311416
*r=0;
14321417
}
14331418

1419+
/* The pad bits should be already zero at this point */
1420+
14341421
PG_RETURN_VARBIT_P(result);
14351422
}
14361423

@@ -1497,6 +1484,8 @@ bitshiftright(PG_FUNCTION_ARGS)
14971484
if ((++r)<VARBITEND(result))
14981485
*r= (*p << (BITS_PER_BYTE-ishift))&BITMASK;
14991486
}
1487+
/* We may have shifted 1's into the pad bits, so fix that */
1488+
VARBIT_PAD_LAST(result,r);
15001489
}
15011490

15021491
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