From fa490e8022e05e505f8f15481b848a2729337b16 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 14 Aug 2019 23:46:47 +0300 Subject: [PATCH] Don't copy uninitialized bytes when copying varstrings When using field_conv(), which is called in case of field1=field2 copy in fill_records(), full varstring's was copied, including unitialized bytes. This caused valgrind to compilain about usage of unitialized bytes when using Aria static length records. Fixed by not using memcpy when copying varstrings but instead just copy the real bytes. --- sql/field.cc | 10 ++++++++++ sql/field.h | 7 +------ sql/handler.h | 3 ++- storage/maria/ha_maria.cc | 2 +- storage/myisam/ha_myisam.cc | 5 +++-- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/sql/field.cc b/sql/field.cc index 746f418415c..16fb0060d91 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7554,6 +7554,16 @@ int Field_varstring::save_field_metadata(uchar *metadata_ptr) return 2; } + +bool Field_varstring::memcpy_field_possible(const Field *from) const +{ + return (Field_str::memcpy_field_possible(from) && + !compression_method() == !from->compression_method() && + length_bytes == ((Field_varstring*) from)->length_bytes && + !(table->file->ha_table_flags() & HA_RECORD_MUST_BE_CLEAN_ON_WRITE)); +} + + int Field_varstring::store(const char *from,size_t length,CHARSET_INFO *cs) { DBUG_ASSERT(marked_for_write_or_computed()); diff --git a/sql/field.h b/sql/field.h index 24a8a617657..af659e18db7 100644 --- a/sql/field.h +++ b/sql/field.h @@ -3781,12 +3781,7 @@ public: length_bytes : 0); } Copy_func *get_copy_func(const Field *from) const; - bool memcpy_field_possible(const Field *from) const - { - return Field_str::memcpy_field_possible(from) && - !compression_method() == !from->compression_method() && - length_bytes == ((Field_varstring*) from)->length_bytes; - } + bool memcpy_field_possible(const Field *from) const; void update_data_type_statistics(Data_type_statistics *st) const { st->m_variable_string_count++; diff --git a/sql/handler.h b/sql/handler.h index cbde570483f..63d0bf2215c 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -212,7 +212,8 @@ enum enum_alter_inplace_result { #define HA_HAS_NEW_CHECKSUM (1ULL << 38) #define HA_CAN_VIRTUAL_COLUMNS (1ULL << 39) #define HA_MRR_CANT_SORT (1ULL << 40) -#define HA_RECORD_MUST_BE_CLEAN_ON_WRITE (1ULL << 41) /* unused */ +/* All of VARCHAR is stored, including bytes after real varchar data */ +#define HA_RECORD_MUST_BE_CLEAN_ON_WRITE (1ULL << 41) /* This storage engine supports condition pushdown diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index d07d30330a0..6ea50f21675 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -1195,7 +1195,7 @@ int ha_maria::open(const char *name, int mode, uint test_if_locked) that all bytes in the row is properly reset. */ if (file->s->data_file_type == STATIC_RECORD && - (file->s->has_varchar_fields | file->s->has_null_fields)) + (file->s->has_varchar_fields || file->s->has_null_fields)) int_table_flags|= HA_RECORD_MUST_BE_CLEAN_ON_WRITE; for (i= 0; i < table->s->keys; i++) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index bc3fbf2dbd5..aaf0217aefb 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -886,8 +886,9 @@ int ha_myisam::open(const char *name, int mode, uint test_if_locked) the full row to ensure we don't get any errors from valgrind and that all bytes in the row is properly reset. */ - if ((file->s->options & HA_OPTION_PACK_RECORD) && - (file->s->has_varchar_fields | file->s->has_null_fields)) + if (!(file->s->options & + (HA_OPTION_PACK_RECORD | HA_OPTION_COMPRESS_RECORD)) && + (file->s->has_varchar_fields || file->s->has_null_fields)) int_table_flags|= HA_RECORD_MUST_BE_CLEAN_ON_WRITE; for (i= 0; i < table->s->keys; i++)