From 3eabde409114b9198630a3ec3c5e07b429f3fc23 Mon Sep 17 00:00:00 2001
From: "mats@romeo.(none)" <>
Date: Mon, 20 Nov 2006 21:27:39 +0100
Subject: [PATCH 1/2] BUG#24403 (valgrind complained on uninited st_table for
InnoDB and RBR): Fix to correct behaviour of find_and_fetch_row() for tables
that have primary keys stored in storage engines that support the fast method
to fetch rows given a primary key. The method uses position() to retrieve the
key for a given record and rnd_pos() to position the internal "cursor" at the
row. Rnd_pos() returns the found record in table->record[0], so the record
has to be moved to table->record[1] for further processing after calling
find_and_fetch_row().
---
sql/log_event.cc | 88 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 69 insertions(+), 19 deletions(-)
diff --git a/sql/log_event.cc b/sql/log_event.cc
index aa692609d0e..09ea7742399 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -140,6 +140,21 @@ static void pretty_print_str(IO_CACHE* cache, char* str, int len)
}
#endif /* MYSQL_CLIENT */
+#ifdef HAVE_purify
+static void
+valgrind_check_mem(void *ptr, size_t len)
+{
+ static volatile uchar dummy;
+ for (volatile uchar *p= (uchar*) ptr ; p != (uchar*) ptr + len ; ++p)
+ {
+ int const c = *p;
+ if (c < 128)
+ dummy= c + 1;
+ else
+ dummy = c - 1;
+ }
+}
+#endif
#if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT)
@@ -5530,6 +5545,8 @@ unpack_row(RELAY_LOG_INFO *rli,
if (bitmap_is_set(cols, field_ptr - begin_ptr))
{
+ DBUG_ASSERT(table->record[0] <= f->ptr);
+ DBUG_ASSERT(f->ptr < table->record[0] + table->s->reclength + (f->pack_length_in_rec() == 0));
f->move_field_offset(offset);
ptr= f->unpack(f->ptr, ptr);
f->move_field_offset(-offset);
@@ -5564,10 +5581,6 @@ unpack_row(RELAY_LOG_INFO *rli,
{
uint32 const mask= NOT_NULL_FLAG | NO_DEFAULT_VALUE_FLAG;
- DBUG_PRINT("debug", ("flags = 0x%x, mask = 0x%x, flags & mask = 0x%x",
- (*field_ptr)->flags, mask,
- (*field_ptr)->flags & mask));
-
if (event_type == WRITE_ROWS_EVENT &&
((*field_ptr)->flags & mask) == mask)
{
@@ -5690,7 +5703,6 @@ int Rows_log_event::exec_event(st_relay_log_info *rli)
We also invalidate the query cache for all the tables, since
they will now be changed.
*/
-
TABLE_LIST *ptr;
for (ptr= rli->tables_to_lock ; ptr ; ptr= ptr->next_global)
{
@@ -5747,7 +5759,7 @@ int Rows_log_event::exec_event(st_relay_log_info *rli)
if ((error= do_prepare_row(thd, rli, table, row_start, &row_end)))
break; // We should perform the after-row operation even in
// the case of error
-
+
DBUG_ASSERT(row_end != NULL); // cannot happen
DBUG_ASSERT(row_end <= (const char*)m_rows_end);
@@ -6423,8 +6435,8 @@ int Write_rows_log_event::do_after_row_operations(TABLE *table, int error)
int Write_rows_log_event::do_prepare_row(THD *thd, RELAY_LOG_INFO *rli,
TABLE *table,
- char const *row_start,
- char const **row_end)
+ char const *const row_start,
+ char const **const row_end)
{
DBUG_ASSERT(table != NULL);
DBUG_ASSERT(row_start && row_end);
@@ -6495,7 +6507,7 @@ copy_extra_record_fields(TABLE *table,
my_ptrdiff_t master_fields)
{
DBUG_PRINT("info", ("Copying to %p "
- "from field %d at offset %u "
+ "from field %ld at offset %u "
"to field %d at offset %u",
table->record[0],
master_fields, master_reclength,
@@ -6733,8 +6745,26 @@ static bool record_compare(TABLE *table)
/*
Find the row given by 'key', if the table has keys, or else use a table scan
- to find (and fetch) the row. If the engine allows random access of the
- records, a combination of position() and rnd_pos() will be used.
+ to find (and fetch) the row.
+
+ If the engine allows random access of the records, a combination of
+ position() and rnd_pos() will be used.
+
+ @param table Pointer to table to search
+ @param key Pointer to key to use for search, if table has key
+
+ @pre table->record[0] shall contain the row to locate
+ and key shall contain a key to use for searching, if
+ the engine has a key.
+
+ @post If the return value is zero, table->record[1]
+ will contain the fetched row and the internal "cursor" will refer to
+ the row. If the return value is non-zero,
+ table->record[1] is undefined. In either case,
+ table->record[0] is undefined.
+
+ @return Zero if the row was successfully fetched into
+ table->record[1], error code otherwise.
*/
static int find_and_fetch_row(TABLE *table, byte *key)
@@ -6756,7 +6786,17 @@ static int find_and_fetch_row(TABLE *table, byte *key)
the "cursor" (i.e., record[0] in this case) at the correct row.
*/
table->file->position(table->record[0]);
- DBUG_RETURN(table->file->rnd_pos(table->record[0], table->file->ref));
+ int error= table->file->rnd_pos(table->record[0], table->file->ref);
+ /*
+ rnd_pos() returns the record in table->record[0], so we have to
+ move it to table->record[1].
+ */
+ bmove_align(table->record[1], table->record[0], table->s->reclength);
+#ifdef HAVE_purify
+ if (error == 0)
+ valgrind_check_mem(table->record[1], table->s->reclength);
+#endif
+ DBUG_RETURN(error);
}
DBUG_ASSERT(table->record[1]);
@@ -6770,7 +6810,7 @@ static int find_and_fetch_row(TABLE *table, byte *key)
int error;
/* We have a key: search the table using the index */
if (!table->file->inited && (error= table->file->ha_index_init(0, FALSE)))
- return error;
+ DBUG_RETURN(error);
/*
We need to set the null bytes to ensure that the filler bit are
@@ -6807,6 +6847,9 @@ static int find_and_fetch_row(TABLE *table, byte *key)
if (table->key_info->flags & HA_NOSAME)
{
table->file->ha_index_end();
+#ifdef HAVE_purify
+ valgrind_check_mem(table->record[1], table->s->reclength);
+#endif
DBUG_RETURN(0);
}
@@ -6883,9 +6926,16 @@ static int find_and_fetch_row(TABLE *table, byte *key)
table->file->ha_rnd_end();
DBUG_ASSERT(error == HA_ERR_END_OF_FILE || error == 0);
+#ifdef HAVE_purify
+ if (error == 0)
+ valgrind_check_mem(table->record[1], table->s->reclength);
+#endif
DBUG_RETURN(error);
}
+#ifdef HAVE_purify
+ valgrind_check_mem(table->record[1], table->s->reclength);
+#endif
DBUG_RETURN(0);
}
#endif
@@ -6974,8 +7024,8 @@ int Delete_rows_log_event::do_after_row_operations(TABLE *table, int error)
int Delete_rows_log_event::do_prepare_row(THD *thd, RELAY_LOG_INFO *rli,
TABLE *table,
- char const *row_start,
- char const **row_end)
+ char const *const row_start,
+ char const **const row_end)
{
int error;
DBUG_ASSERT(row_start && row_end);
@@ -7111,8 +7161,8 @@ int Update_rows_log_event::do_after_row_operations(TABLE *table, int error)
int Update_rows_log_event::do_prepare_row(THD *thd, RELAY_LOG_INFO *rli,
TABLE *table,
- char const *row_start,
- char const **row_end)
+ char const *const row_start,
+ char const **const row_end)
{
int error;
DBUG_ASSERT(row_start && row_end);
@@ -7127,11 +7177,11 @@ int Update_rows_log_event::do_prepare_row(THD *thd, RELAY_LOG_INFO *rli,
table, m_width, table->record[0],
row_start, &m_cols, row_end, &m_master_reclength,
table->read_set, UPDATE_ROWS_EVENT);
- row_start = *row_end;
+ char const *next_start = *row_end;
/* m_after_image is the after image for the update */
error= unpack_row(rli,
table, m_width, m_after_image,
- row_start, &m_cols, row_end, &m_master_reclength,
+ next_start, &m_cols, row_end, &m_master_reclength,
table->write_set, UPDATE_ROWS_EVENT);
DBUG_DUMP("record[0]", (const char *)table->record[0], table->s->reclength);
From 600a6a676cfb066a7e96b465e28de715ca7c925a Mon Sep 17 00:00:00 2001
From: "mats@romeo.(none)" <>
Date: Tue, 21 Nov 2006 13:57:52 +0100
Subject: [PATCH 2/2] BUG#24403 (valgrind complaint on uninited st_table for
innodb + rbr): Removing DBUG_DUMP printouts for valgrind builds since they
trigger warnings. Removing valgrind memory checks completely. Removing
bzero() of record when opening table that was added earlier.
---
sql/log_event.cc | 73 +++++++++++++++++++++++++++++++++++-------------
sql/sql_class.cc | 8 +++++-
sql/table.cc | 4 +--
3 files changed, 61 insertions(+), 24 deletions(-)
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 09ea7742399..24bee9eb678 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -5417,7 +5417,13 @@ int Rows_log_event::do_add_row_data(byte *const row_data,
DBUG_ENTER("Rows_log_event::do_add_row_data");
DBUG_PRINT("enter", ("row_data: 0x%lx length: %lu", (ulong) row_data,
length));
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
DBUG_DUMP("row_data", (const char*)row_data, min(length, 32));
+#endif
DBUG_ASSERT(m_rows_buf <= m_rows_cur);
DBUG_ASSERT(!m_rows_buf || m_rows_end && m_rows_buf < m_rows_end);
@@ -5504,7 +5510,9 @@ unpack_row(RELAY_LOG_INFO *rli,
char const **row_end, ulong *master_reclength,
MY_BITMAP* const rw_set, Log_event_type const event_type)
{
+ DBUG_ENTER("unpack_row");
DBUG_ASSERT(record && row);
+ DBUG_PRINT("enter", ("row=0x%lx; record=0x%lx", row, record));
my_ptrdiff_t const offset= record - (byte*) table->record[0];
my_size_t master_null_bytes= table->s->null_bytes;
@@ -5548,6 +5556,8 @@ unpack_row(RELAY_LOG_INFO *rli,
DBUG_ASSERT(table->record[0] <= f->ptr);
DBUG_ASSERT(f->ptr < table->record[0] + table->s->reclength + (f->pack_length_in_rec() == 0));
f->move_field_offset(offset);
+
+ DBUG_PRINT("info", ("unpacking column '%s' to 0x%lx", f->field_name, f->ptr));
ptr= f->unpack(f->ptr, ptr);
f->move_field_offset(-offset);
/* Field...::unpack() cannot return 0 */
@@ -5595,7 +5605,7 @@ unpack_row(RELAY_LOG_INFO *rli,
(*field_ptr)->set_default();
}
- return error;
+ DBUG_RETURN(error);
}
int Rows_log_event::exec_event(st_relay_log_info *rli)
@@ -6060,7 +6070,13 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len,
DBUG_PRINT("info",("event_len=%ld, common_header_len=%d, post_header_len=%d",
event_len, common_header_len, post_header_len));
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
DBUG_DUMP("event buffer", buf, event_len);
+#endif
/* Read the post-header */
const char *post_start= buf + common_header_len;
@@ -6784,6 +6800,17 @@ static int find_and_fetch_row(TABLE *table, byte *key)
row reference using the position() member function (it will be
stored in table->file->ref) and the use rnd_pos() to position
the "cursor" (i.e., record[0] in this case) at the correct row.
+
+ TODO: Add a check that the correct record has been fetched by
+ comparing with the original record. Take into account that the
+ record on the master and slave can be of different
+ length. Something along these lines should work:
+
+ ADD>>> store_record(table,record[1]);
+ int error= table->file->rnd_pos(table->record[0], table->file->ref);
+ ADD>>> DBUG_ASSERT(memcmp(table->record[1], table->record[0],
+ table->s->reclength) == 0);
+
*/
table->file->position(table->record[0]);
int error= table->file->rnd_pos(table->record[0], table->file->ref);
@@ -6792,15 +6819,9 @@ static int find_and_fetch_row(TABLE *table, byte *key)
move it to table->record[1].
*/
bmove_align(table->record[1], table->record[0], table->s->reclength);
-#ifdef HAVE_purify
- if (error == 0)
- valgrind_check_mem(table->record[1], table->s->reclength);
-#endif
DBUG_RETURN(error);
}
- DBUG_ASSERT(table->record[1]);
-
/* We need to retrieve all fields */
/* TODO: Move this out from this function to main loop */
table->use_all_columns();
@@ -6812,6 +6833,15 @@ static int find_and_fetch_row(TABLE *table, byte *key)
if (!table->file->inited && (error= table->file->ha_index_init(0, FALSE)))
DBUG_RETURN(error);
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
+ DBUG_DUMP("table->record[0]", table->record[0], table->s->reclength);
+ DBUG_DUMP("table->record[1]", table->record[1], table->s->reclength);
+#endif
+
/*
We need to set the null bytes to ensure that the filler bit are
all set when returning. There are storage engines that just set
@@ -6830,6 +6860,14 @@ static int find_and_fetch_row(TABLE *table, byte *key)
DBUG_RETURN(error);
}
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
+ DBUG_DUMP("table->record[0]", table->record[0], table->s->reclength);
+ DBUG_DUMP("table->record[1]", table->record[1], table->s->reclength);
+#endif
/*
Below is a minor "optimization". If the key (i.e., key number
0) has the HA_NOSAME flag set, we know that we have found the
@@ -6847,9 +6885,6 @@ static int find_and_fetch_row(TABLE *table, byte *key)
if (table->key_info->flags & HA_NOSAME)
{
table->file->ha_index_end();
-#ifdef HAVE_purify
- valgrind_check_mem(table->record[1], table->s->reclength);
-#endif
DBUG_RETURN(0);
}
@@ -6926,16 +6961,9 @@ static int find_and_fetch_row(TABLE *table, byte *key)
table->file->ha_rnd_end();
DBUG_ASSERT(error == HA_ERR_END_OF_FILE || error == 0);
-#ifdef HAVE_purify
- if (error == 0)
- valgrind_check_mem(table->record[1], table->s->reclength);
-#endif
DBUG_RETURN(error);
}
-#ifdef HAVE_purify
- valgrind_check_mem(table->record[1], table->s->reclength);
-#endif
DBUG_RETURN(0);
}
#endif
@@ -7184,9 +7212,14 @@ int Update_rows_log_event::do_prepare_row(THD *thd, RELAY_LOG_INFO *rli,
next_start, &m_cols, row_end, &m_master_reclength,
table->write_set, UPDATE_ROWS_EVENT);
- DBUG_DUMP("record[0]", (const char *)table->record[0], table->s->reclength);
- DBUG_DUMP("m_after_image", (const char *)m_after_image, table->s->reclength);
-
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
+ DBUG_DUMP("record[0]", (const char *)table->record[0], m_master_reclength);
+ DBUG_DUMP("m_after_image", (const char *)m_after_image, m_master_reclength);
+#endif
/*
If we will access rows using the random access method, m_key will
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index b041a733bb9..20acb28bc77 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -2727,11 +2727,17 @@ int THD::binlog_update_row(TABLE* table, bool is_trans,
before_record);
my_size_t const after_size= pack_row(table, cols, after_row,
after_record);
-
+
+ /*
+ Don't print debug messages when running valgrind since they can
+ trigger false warnings.
+ */
+#ifndef HAVE_purify
DBUG_DUMP("before_record", (const char *)before_record, table->s->reclength);
DBUG_DUMP("after_record", (const char *)after_record, table->s->reclength);
DBUG_DUMP("before_row", (const char *)before_row, before_size);
DBUG_DUMP("after_row", (const char *)after_row, after_size);
+#endif
Rows_log_event* const ev=
binlog_prepare_pending_rows_event(table, server_id, cols, colcnt,
diff --git a/sql/table.cc b/sql/table.cc
index c74f72e2539..7f80b95c954 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -1377,9 +1377,7 @@ int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias,
if (!(record= (byte*) alloc_root(&outparam->mem_root,
share->rec_buff_length * records)))
goto err; /* purecov: inspected */
-#ifdef HAVE_purify
- bzero(record, share->rec_buff_length * records);
-#endif
+
if (records == 0)
{
/* We are probably in hard repair, and the buffers should not be used */