Skip to content

Commit

Permalink
MDEV-11349 (2/2) Fix some bogus-looking Valgrind warnings
Browse files Browse the repository at this point in the history
buf_block_init(): Initialize buf_page_t::flush_type.
For some reason, Valgrind 3.12.0 would seem to flag some
bits in adjacent bitfields as uninitialized, even though only
the two bits of flush_type were left uninitialized. Initialize
the field to get rid of many warnings.

buf_page_init_low(): Initialize buf_page_t::old.
For some reason, Valgrind 3.12.0 would seem to flag all 32
bits uninitialized when buf_page_init_for_read() invokes
buf_LRU_add_block(bpage, TRUE). This would trigger bogus warnings
for buf_page_t::freed_page_clock being uninitialized.
(The V-bits would later claim that only "old" is initialized
in the 32-bit word.) Perhaps recent compilers
(GCC 6.2.1 and clang 4.0.0) generate more optimized x86_64 code
for bitfield operations, confusing Valgrind?

mach_write_to_1(), mach_write_to_2(), mach_write_to_3():
Rewrite the assertions that ensure that the most significant
bits are zero. Apparently, clang 4.0.0 would optimize expressions
of the form ((n | 0xFF) <= 0x100) to (n <= 0x100). The redundant
0xFF was added in the first place in order to suppress a
Valgrind warning. (Valgrind would warn about comparing uninitialized
values even in the case when the uninitialized bits do not affect
the result of the comparison.)
  • Loading branch information
dr-m committed Nov 25, 2016
1 parent 8da33e3 commit a68d135
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 6 deletions.
2 changes: 2 additions & 0 deletions storage/innobase/buf/buf0buf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,7 @@ buf_block_init(
block->frame = frame;

block->page.buf_pool_index = buf_pool_index(buf_pool);
block->page.flush_type = BUF_FLUSH_LRU;
block->page.state = BUF_BLOCK_NOT_USED;
block->page.buf_fix_count = 0;
block->page.io_fix = BUF_IO_NONE;
Expand Down Expand Up @@ -3784,6 +3785,7 @@ buf_page_init_low(
bpage->flush_type = BUF_FLUSH_LRU;
bpage->io_fix = BUF_IO_NONE;
bpage->buf_fix_count = 0;
bpage->old = 0;
bpage->freed_page_clock = 0;
bpage->access_time = 0;
bpage->newest_modification = 0;
Expand Down
6 changes: 3 additions & 3 deletions storage/innobase/include/mach0data.ic
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ mach_write_to_1(
ulint n) /*!< in: ulint integer to be stored, >= 0, < 256 */
{
ut_ad(b);
ut_ad((n | 0xFFUL) <= 0xFFUL);
ut_ad((n & ~0xFFUL) == 0);

b[0] = (byte) n;
}
Expand Down Expand Up @@ -68,7 +68,7 @@ mach_write_to_2(
ulint n) /*!< in: ulint integer to be stored */
{
ut_ad(b);
ut_ad((n | 0xFFFFUL) <= 0xFFFFUL);
ut_ad((n & ~0xFFFFUL) == 0);

b[0] = (byte)(n >> 8);
b[1] = (byte)(n);
Expand Down Expand Up @@ -131,7 +131,7 @@ mach_write_to_3(
ulint n) /*!< in: ulint integer to be stored */
{
ut_ad(b);
ut_ad((n | 0xFFFFFFUL) <= 0xFFFFFFUL);
ut_ad((n & ~0xFFFFFFUL) == 0);

b[0] = (byte)(n >> 16);
b[1] = (byte)(n >> 8);
Expand Down
1 change: 1 addition & 0 deletions storage/xtradb/buf/buf0buf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,7 @@ buf_block_init(
block->frame = frame;

block->page.buf_pool_index = buf_pool_index(buf_pool);
block->page.flush_type = BUF_FLUSH_LRU;
block->page.state = BUF_BLOCK_NOT_USED;
block->page.buf_fix_count = 0;
block->page.io_fix = BUF_IO_NONE;
Expand Down
6 changes: 3 additions & 3 deletions storage/xtradb/include/mach0data.ic
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ mach_write_to_1(
ulint n) /*!< in: ulint integer to be stored, >= 0, < 256 */
{
ut_ad(b);
ut_ad((n | 0xFFUL) <= 0xFFUL);
ut_ad((n & ~0xFFUL) == 0);

b[0] = (byte) n;
}
Expand Down Expand Up @@ -67,7 +67,7 @@ mach_write_to_2(
ulint n) /*!< in: ulint integer to be stored */
{
ut_ad(b);
ut_ad((n | 0xFFFFUL) <= 0xFFFFUL);
ut_ad((n & ~0xFFFFUL) == 0);

b[0] = (byte)(n >> 8);
b[1] = (byte)(n);
Expand Down Expand Up @@ -115,7 +115,7 @@ mach_write_to_3(
ulint n) /*!< in: ulint integer to be stored */
{
ut_ad(b);
ut_ad((n | 0xFFFFFFUL) <= 0xFFFFFFUL);
ut_ad((n & ~0xFFFFFFUL) == 0);

b[0] = (byte)(n >> 16);
b[1] = (byte)(n >> 8);
Expand Down

0 comments on commit a68d135

Please sign in to comment.