MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Sergey Glukhov Date:January 29 2010 1:01pm
Subject:bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3337)
Bug#45195
View as plain text  
#At file:///home/gluh/MySQL/mysql-5.1-bug-45195/ based on revid:staale.smedseng@stripped

 3337 Sergey Glukhov	2010-01-29
      Bug#45195 valgrind warnings about uninitialized values in store_record_in_cache()
      The problem becomes apparent only if HAVE_purify is undefined.
      It related to the part of code placed in open_table_from_share() fuction
      where we initialize record buffer only if HAVE_purify is enabled.
      So in case of HAVE_purify=OFF record buffer is not initialized
      on open table stage.
      Next we read key, find NULL value and update appropriate null bit
      but do not update record buffer. After that the record is stored
      in the join cache(store_record_in_cache). For CHAR fields we
      strip trailing spaces and in our case this procedure uses
      uninitialized record buffer.
      The fix is to skip stripping space procedure in case of null values
      for CHAR fields(partially based on 6.0 JOIN_CACHE implementation).
     @ mysql-test/r/join.result
        test case
     @ mysql-test/t/join.test
        test case
     @ sql/field.cc
        code updated according to new CACHE_FIELD struct
     @ sql/sql_select.cc
        code updated according to new CACHE_FIELD struct
     @ sql/sql_select.h
        CACHE_FIELD struct:
        added new fields: Field *field, uint type;
        removed fields: Field_blob *blob_field, bool strip;

    modified:
      mysql-test/r/join.result
      mysql-test/t/join.test
      sql/field.cc
      sql/sql_select.cc
      sql/sql_select.h
=== modified file 'mysql-test/r/join.result'
--- a/mysql-test/r/join.result	2009-10-30 08:03:18 +0000
+++ b/mysql-test/r/join.result	2010-01-29 13:00:52 +0000
@@ -1128,3 +1128,21 @@ EXECUTE stmt;
 DEALLOCATE PREPARE stmt;
 DROP VIEW v1;
 DROP TABLE t1, t2;
+FLUSH TABLES;
+CREATE TABLE t1(a CHAR(9),b INT,KEY(b),KEY(a)) ENGINE=MYISAM;
+CREATE TABLE t2(a CHAR(9),b INT,KEY(b),KEY(a)) ENGINE=MYISAM;
+INSERT INTO t1 VALUES ('1',null),(null,null);
+INSERT INTO t2 VALUES ('1',null),(null,null);
+CREATE TABLE mm1(a CHAR(9),b INT,KEY(b),KEY(a))
+ENGINE=MERGE  UNION=(t1,t2);
+SELECT t1.a FROM mm1,t1;
+a
+NULL
+1
+NULL
+1
+NULL
+1
+NULL
+1
+DROP TABLE t1, t2, mm1;

=== modified file 'mysql-test/t/join.test'
--- a/mysql-test/t/join.test	2009-10-30 08:03:18 +0000
+++ b/mysql-test/t/join.test	2010-01-29 13:00:52 +0000
@@ -804,3 +804,16 @@ DEALLOCATE PREPARE stmt;
 
 DROP VIEW v1;
 DROP TABLE t1, t2;
+
+#
+# Bug#45195 valgrind warnings about uninitialized values in store_record_in_cache()
+#
+FLUSH TABLES;
+CREATE TABLE t1(a CHAR(9),b INT,KEY(b),KEY(a)) ENGINE=MYISAM;
+CREATE TABLE t2(a CHAR(9),b INT,KEY(b),KEY(a)) ENGINE=MYISAM;
+INSERT INTO t1 VALUES ('1',null),(null,null);
+INSERT INTO t2 VALUES ('1',null),(null,null);
+CREATE TABLE mm1(a CHAR(9),b INT,KEY(b),KEY(a))
+ENGINE=MERGE  UNION=(t1,t2);
+SELECT t1.a FROM mm1,t1;
+DROP TABLE t1, t2, mm1;

=== modified file 'sql/field.cc'
--- a/sql/field.cc	2010-01-13 10:28:42 +0000
+++ b/sql/field.cc	2010-01-29 13:00:52 +0000
@@ -1705,11 +1705,10 @@ uint Field::fill_cache_field(CACHE_FIELD
   uint store_length;
   copy->str=ptr;
   copy->length=pack_length();
-  copy->blob_field=0;
+  copy->field= this;
   if (flags & BLOB_FLAG)
   {
-    copy->blob_field=(Field_blob*) this;
-    copy->strip=0;
+    copy->type= CACHE_BLOB;
     copy->length-= table->s->blob_ptr_size;
     return copy->length;
   }
@@ -1717,15 +1716,15 @@ uint Field::fill_cache_field(CACHE_FIELD
            (type() == MYSQL_TYPE_STRING && copy->length >= 4 &&
             copy->length < 256))
   {
-    copy->strip=1;				/* Remove end space */
+    copy->type= CACHE_STRIPPED;
     store_length= 2;
   }
   else
   {
-    copy->strip=0;
+    copy->type= 0;
     store_length= 0;
   }
-  return copy->length+ store_length;
+  return copy->length + store_length;
 }
 
 

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2009-12-23 15:11:22 +0000
+++ b/sql/sql_select.cc	2010-01-29 13:00:52 +0000
@@ -14108,7 +14108,7 @@ join_init_cache(THD *thd,JOIN_TAB *table
       {
 	used_fields--;
 	length+=field->fill_cache_field(copy);
-	if (copy->blob_field)
+	if (copy->type == CACHE_BLOB)
 	  (*blob_ptr++)=copy;
 	if (field->real_maybe_null())
 	  null_fields++;
@@ -14123,8 +14123,8 @@ join_init_cache(THD *thd,JOIN_TAB *table
     {						/* must copy null bits */
       copy->str= tables[i].table->null_flags;
       copy->length= tables[i].table->s->null_bytes;
-      copy->strip=0;
-      copy->blob_field=0;
+      copy->type=0;
+      copy->field=0;
       length+=copy->length;
       copy++;
       cache->fields++;
@@ -14134,8 +14134,8 @@ join_init_cache(THD *thd,JOIN_TAB *table
     {
       copy->str= (uchar*) &tables[i].table->null_row;
       copy->length=sizeof(tables[i].table->null_row);
-      copy->strip=0;
-      copy->blob_field=0;
+      copy->type=0;
+      copy->field=0;
       length+=copy->length;
       copy++;
       cache->fields++;
@@ -14160,9 +14160,10 @@ used_blob_length(CACHE_FIELD **ptr)
   uint length,blob_length;
   for (length=0 ; *ptr ; ptr++)
   {
-    (*ptr)->blob_length=blob_length=(*ptr)->blob_field->get_length();
+    Field_blob *field_blob= (Field_blob *) (*ptr)->field;
+    (*ptr)->blob_length=blob_length= field_blob->get_length();
     length+=blob_length;
-    (*ptr)->blob_field->get_ptr(&(*ptr)->str);
+    field_blob->get_ptr(&(*ptr)->str);
   }
   return length;
 }
@@ -14191,30 +14192,35 @@ store_record_in_cache(JOIN_CACHE *cache)
   cache->records++;
   for (copy=cache->field ; copy < end_field; copy++)
   {
-    if (copy->blob_field)
+    if (copy->type == CACHE_BLOB)
     {
+      Field_blob *blob_field= (Field_blob *) copy->field;
       if (last_record)
       {
-	copy->blob_field->get_image(pos, copy->length+sizeof(char*), 
-				    copy->blob_field->charset());
+	blob_field->get_image(pos, copy->length+sizeof(char*), 
+                              blob_field->charset());
 	pos+=copy->length+sizeof(char*);
       }
       else
       {
-	copy->blob_field->get_image(pos, copy->length, // blob length
-				    copy->blob_field->charset());
+	blob_field->get_image(pos, copy->length, // blob length
+                              blob_field->charset());
 	memcpy(pos+copy->length,copy->str,copy->blob_length);  // Blob data
 	pos+=copy->length+copy->blob_length;
       }
     }
     else
     {
-      if (copy->strip)
+      if (copy->type == CACHE_STRIPPED)
       {
 	uchar *str,*end;
-	for (str=copy->str,end= str+copy->length;
-	     end > str && end[-1] == ' ' ;
-	     end--) ;
+        Field *field= copy->field;
+        if (field && field->maybe_null() && field->is_null())
+          end= str= copy->str;
+        else
+          for (str=copy->str,end= str+copy->length;
+               end > str && end[-1] == ' ' ;
+               end--) ;
 	length=(uint) (end-str);
 	memcpy(pos+2, str, length);
         int2store(pos, length);
@@ -14263,23 +14269,24 @@ read_cached_record(JOIN_TAB *tab)
        copy < end_field;
        copy++)
   {
-    if (copy->blob_field)
+    if (copy->type == CACHE_BLOB)
     {
+      Field_blob *blob_field= (Field_blob *) copy->field;
       if (last_record)
       {
-	copy->blob_field->set_image(pos, copy->length+sizeof(char*),
-				    copy->blob_field->charset());
+	blob_field->set_image(pos, copy->length+sizeof(char*),
+                              blob_field->charset());
 	pos+=copy->length+sizeof(char*);
       }
       else
       {
-	copy->blob_field->set_ptr(pos, pos+copy->length);
-	pos+=copy->length+copy->blob_field->get_length();
+	blob_field->set_ptr(pos, pos+copy->length);
+	pos+=copy->length + blob_field->get_length();
       }
     }
     else
     {
-      if (copy->strip)
+      if (copy->type == CACHE_STRIPPED)
       {
         length= uint2korr(pos);
 	memcpy(copy->str, pos+2, length);

=== modified file 'sql/sql_select.h'
--- a/sql/sql_select.h	2010-01-15 14:09:20 +0000
+++ b/sql/sql_select.h	2010-01-29 13:00:52 +0000
@@ -95,6 +95,10 @@ typedef struct st_table_ref
 } TABLE_REF;
 
 
+
+#define CACHE_BLOB      1        /* blob field  */
+#define CACHE_STRIPPED  2        /* field stripped of trailing spaces */
+
 /**
   CACHE_FIELD and JOIN_CACHE is used on full join to cache records in outer
   table
@@ -103,8 +107,8 @@ typedef struct st_table_ref
 typedef struct st_cache_field {
   uchar *str;
   uint length, blob_length;
-  Field_blob *blob_field;
-  bool strip;
+  Field *field;
+  uint type;    /**< category of the of the copied field (CACHE_BLOB et al.) */
 } CACHE_FIELD;
 
 


Attachment: [text/bzr-bundle] bzr/sergey.glukhov@sun.com-20100129130052-x9uzvq8otj1dq60e.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (Sergey.Glukhov:3337)Bug#45195Sergey Glukhov29 Jan