3871 Tor Didriksen 2012-04-12
Bug#13871079 RQG_MYISAM_DML_ALTER_VALGRIND FAILS ON VALGRIND PN PB2
The class Copy_field contains a String tmp,
which may allocate memory on the heap.
That means that all instances of Copy_field
must be properly destroyed. Alas they are not.
Solution: don't use Copy_field::tmp for copying
from_field => tmp => to_field
in do_field_string()
@ sql/field.cc
In Field_set::val_str
return empty string (of appropriate character set) for an empty set.
@ sql/field.h
New private member in Field_enum: empty_set_string.
@ sql/field_conv.cc
In do_field_string, use an auto variable for copying
from_field => tmp => to_field
rather than copy->tmp.
@ sql/sql_class.h
Verifies that these objects are never destroyed....
@ unittest/gunit/alignment-t.cc
Add copyright notice.
@ unittest/gunit/fake_table.h
More initializations, to avoid valgrind warnings.
@ unittest/gunit/field-t.cc
New unit test.
@ unittest/gunit/field_timestamp-t.cc
Remove obsolete note about min/max macros.
modified:
sql/field.cc
sql/field.h
sql/field_conv.cc
sql/sql_class.h
unittest/gunit/alignment-t.cc
unittest/gunit/fake_table.h
unittest/gunit/field-t.cc
unittest/gunit/field_timestamp-t.cc
3870 Sujatha Sivakumar 2012-04-12 [merge]
upmerge from mysql-5.5 -> mysql-trunk.
modified:
mysql-test/extra/rpl_tests/rpl_row_basic.test
mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result
mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result
mysql-test/suite/rpl/r/rpl_row_basic_allow_batching.result
sql/log_event.cc
=== modified file 'sql/field.cc'
--- a/sql/field.cc 2012-03-30 15:38:01 +0000
+++ b/sql/field.cc 2012-04-12 14:29:14 +0000
@@ -8506,7 +8506,19 @@ String *Field_set::val_str(String *val_b
ulonglong tmp=(ulonglong) Field_enum::val_int();
uint bitnr=0;
- val_buffer->set("", 0, field_charset);
+ if (tmp == 0)
+ {
+ /*
+ Some callers expect *val_buffer to contain the result,
+ so we assign to it, rather than doing 'return &empty_set_string.
+ */
+ *val_buffer= empty_set_string;
+ return val_buffer;
+ }
+
+ val_buffer->set_charset(field_charset);
+ val_buffer->length(0);
+
while (tmp && bitnr < (uint) typelib->count)
{
if (tmp & 1)
=== modified file 'sql/field.h'
--- a/sql/field.h 2012-03-06 14:29:42 +0000
+++ b/sql/field.h 2012-04-12 14:29:14 +0000
@@ -3250,7 +3250,8 @@ public:
:Field_enum(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
unireg_check_arg, field_name_arg,
packlength_arg,
- typelib_arg,charset_arg)
+ typelib_arg,charset_arg),
+ empty_set_string("", 0, charset_arg)
{
flags= (flags & ~ENUM_FLAG) | SET_FLAG;
}
@@ -3270,6 +3271,8 @@ public:
DBUG_ASSERT(real_type() == MYSQL_TYPE_SET);
return new Field_set(*this);
}
+private:
+ const String empty_set_string;
};
=== modified file 'sql/field_conv.cc'
--- a/sql/field_conv.cc 2012-03-06 14:29:42 +0000
+++ b/sql/field_conv.cc 2012-04-12 14:29:14 +0000
@@ -291,10 +291,11 @@ static void do_save_blob(Copy_field *cop
static void do_field_string(Copy_field *copy)
{
char buff[MAX_FIELD_WIDTH];
- copy->tmp.set_quick(buff,sizeof(buff),copy->tmp.charset());
- copy->from_field->val_str(©->tmp);
- copy->to_field->store(copy->tmp.c_ptr_quick(),copy->tmp.length(),
- copy->tmp.charset());
+ String res(buff, sizeof(buff), copy->from_field->charset());
+ res.length(0U);
+
+ copy->from_field->val_str(&res);
+ copy->to_field->store(res.c_ptr_quick(), res.length(), res.charset());
}
@@ -568,7 +569,7 @@ void Copy_field::set(uchar *to,Field *fr
/*
To do:
- If 'save\ is set to true and the 'from' is a blob field, do_copy is set to
+ If 'save' is set to true and the 'from' is a blob field, do_copy is set to
do_save_blob rather than do_conv_blob. The only differences between them
appears to be:
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2012-03-21 21:12:45 +0000
+++ b/sql/sql_class.h 2012-04-12 14:29:14 +0000
@@ -4530,8 +4530,13 @@ public:
table(NULL), tab_ref(NULL), in_equality(NULL),
join_cond(NULL), copy_field(NULL)
{}
- ~Semijoin_mat_exec() {}
-
+private:
+ // Nobody deletes me apparently ...
+ ~Semijoin_mat_exec()
+ {
+ delete [] copy_field;
+ }
+public:
const uint table_count; // Number of tables in the sj-nest
const bool is_scan; // TRUE if executing as a scan, FALSE if lookup
bool materialized; // TRUE <=> materialization has been performed
=== modified file 'unittest/gunit/alignment-t.cc'
--- a/unittest/gunit/alignment-t.cc 2012-01-11 09:33:52 +0000
+++ b/unittest/gunit/alignment-t.cc 2012-04-12 14:29:14 +0000
@@ -1,3 +1,18 @@
+/* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; version 2 of the License.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02111-1307 USA */
+
// First include (the generated) my_config.h, to get correct platform defines.
#include "my_config.h"
#include <gtest/gtest.h>
=== modified file 'unittest/gunit/fake_table.h'
--- a/unittest/gunit/fake_table.h 2012-02-27 09:09:31 +0000
+++ b/unittest/gunit/fake_table.h 2012-04-12 14:29:14 +0000
@@ -30,6 +30,7 @@ public:
// fix if you plan to test with >32 columns.
column_bitmap_size= sizeof(int);
tmp_table= NO_TMP_TABLE;
+ db_low_byte_first= true;
}
};
@@ -61,6 +62,7 @@ class Fake_TABLE: public TABLE
field[i]->field_index= i;
}
const_table= true;
+ maybe_null= 0;
}
public:
=== modified file 'unittest/gunit/field-t.cc'
--- a/unittest/gunit/field-t.cc 2011-12-20 12:04:31 +0000
+++ b/unittest/gunit/field-t.cc 2012-04-12 14:29:14 +0000
@@ -1,4 +1,4 @@
-/* Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -18,6 +18,7 @@
#include <gtest/gtest.h>
#include "test_utils.h"
+#include "fake_table.h"
#include "field.h"
#include "sql_time.h"
@@ -30,29 +31,17 @@ using my_testing::Mock_error_handler;
class FieldTest : public ::testing::Test
{
protected:
- static void SetUpTestCase()
- {
- Server_initializer::SetUpTestCase();
- }
-
- static void TearDownTestCase()
- {
- Server_initializer::TearDownTestCase();
- }
-
- virtual void SetUp()
- {
- initializer.SetUp();
- }
+ static void SetUpTestCase() { Server_initializer::SetUpTestCase(); }
+ static void TearDownTestCase() { Server_initializer::TearDownTestCase(); }
- virtual void TearDown()
- {
- initializer.TearDown();
- }
+ virtual void SetUp() { initializer.SetUp(); }
+ virtual void TearDown() { initializer.TearDown(); }
THD *thd() { return initializer.thd(); }
Server_initializer initializer;
+
+ Field_set *create_field_set(TYPELIB *tl);
};
@@ -361,4 +350,61 @@ TEST_F(FieldTest, FieldTime)
}
+const char *type_names3[]= { "one", "two", "three", NULL };
+unsigned int type_lengths3[]= { 3U, 3U, 5U, 0U };
+TYPELIB tl3= { 3, "tl3", type_names3, type_lengths3 };
+
+const char *type_names4[]= { "one", "two", "three", "four", NULL };
+unsigned int type_lengths4[]= { 3U, 3U, 5U, 4U, 0U };
+TYPELIB tl4= { 4, "tl4", type_names4, type_lengths4 };
+
+
+Field_set *FieldTest::create_field_set(TYPELIB *tl)
+{
+ Field_set *f= new (thd()->mem_root)
+ Field_set(NULL, // ptr_arg
+ 42, // len_arg
+ NULL, // null_ptr_arg
+ '\0', // null_bit_arg
+ Field::NONE, // unireg_check_arg
+ "f1", // field_name_arg
+ 1, // packlength_arg
+ tl, // typelib_arg
+ &my_charset_latin1); // charset_arg
+ f->table= new Fake_TABLE(f);
+ return f;
+}
+
+
+// Bug#13871079 RQG_MYISAM_DML_ALTER_VALGRIND FAILS ON VALGRIND PN PB2
+TEST_F(FieldTest, CopyFieldSet)
+{
+ int err;
+ char fields[]= "one,two";
+ my_ulonglong typeset= find_typeset(fields, &tl3, &err);
+ EXPECT_EQ(0, err);
+
+ // Using two different TYPELIBs will set cf->do_copy to do_field_string().
+ Field_set *f_to= create_field_set(&tl3);
+ bitmap_set_all(f_to->table->write_set);
+ uchar to_fieldval= 0;
+ f_to->ptr= &to_fieldval;
+
+ Field_set *f_from= create_field_set(&tl4);
+ bitmap_set_all(f_from->table->write_set);
+ uchar from_fieldval= static_cast<uchar>(typeset);
+ f_from->ptr= &from_fieldval;
+
+ Copy_field *cf= new (thd()->mem_root) Copy_field;
+ cf->set(f_to, f_from, false);
+ cf->do_copy(cf);
+
+ // Copy_field DTOR is not invoked in all contexts, so we may leak memory.
+ EXPECT_FALSE(cf->tmp.is_alloced());
+
+ delete f_to->table;
+ delete f_from->table;
+}
+
+
}
=== modified file 'unittest/gunit/field_timestamp-t.cc'
--- a/unittest/gunit/field_timestamp-t.cc 2012-01-31 15:16:16 +0000
+++ b/unittest/gunit/field_timestamp-t.cc 2012-04-12 14:29:14 +0000
@@ -1,4 +1,4 @@
-/* Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -13,8 +13,7 @@
along with this program; if not, write to the Free Software Foundation,
51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA */
-// First include (the generated) my_config.h, to get correct platform defines,
-// then gtest.h (before any other MySQL headers), to avoid min() macros etc ...
+// First include (the generated) my_config.h, to get correct platform defines.
#include "my_config.h"
#include <gtest/gtest.h>
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (tor.didriksen:3870 to 3871) Bug#13871079 | Tor Didriksen | 12 Apr |
