From df389d0135d65ae7a05e2265fd54c7172dbb9e47 Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Wed, 25 Aug 2010 19:57:53 +0400 Subject: [PATCH] Bug#55077: Assertion failed: width > 0 && to != ((void *)0), file .\dtoa.c The assertion failure was correct because the 'width' argument of my_gcvt() has the signed integer type, whereas the unsigned value UINT_MAX32 was being passed by the caller (Field_double::val_str()) leading to a negative width in my_gcvt(). The following chain of problems was found by further analysis: 1. The display width for a floating point number is calculated in Field_double::val_str() as either field_length or the maximum possible length of string representation of a floating point number, whichever is greater. Since in the bug's test case field_length is UINT_MAX32, we get the same value as the display width. This does not make any sense because for numeric values field_length only matters for ZEROFILL columns, otherwise it does not make sense to allocate that much memory just to print a number. Field_float::val_str() has a similar problem. 2. Even if the above wasn't the case, we would still get a crash on a slightly different test case when trying to allocate UINT_MAX32 bytes with String::alloc() because the latter does not handle such large input values correctly due to alignment overflows. 3. Even when String::alloc() is fixed to return an error when an alignment overflow occurs, there is still a problem because almost no callers check its return value, and Field_double::val_str() is not an exception (same for Field_float::val_str()). 4. Even if all of the above wasn't the case, creating a Field_double object with UINT_MAX32 as its field_length does not make much sense either, since the .frm code limits it to MAX_FIELD_CHARLENGTH (255) bytes. Such a beast can only be created by create_tmp_field_from_item() from an Item with REAL_RESULT as its result_type() and UINT_MAX32 as its max_length. 5. For the bug's test case, the above condition (REAL_RESULT Item with max_length = UINT_MAX32) was a result of Item_func_if::fix_length_and_dec() "shortcutting" aggregation of argument types when one of the arguments was a constant NULL. In this case, the attributes of the aggregated type were simply copied from the other, non-NULL argument, but max_length was still calculated as per the general, non-shortcut case, by choosing the greatest of argument's max_length, which is obviously not correct. The patch addresses all of the above problems, even though fixing the assertion failure for the particular test case would require only a subset of the above problems to be solved. --- client/sql_string.cc | 10 ++++++++-- mysql-test/r/func_if.result | 10 ++++++++++ mysql-test/t/func_if.test | 12 ++++++++++++ sql/field.cc | 21 ++++++++++++++++----- sql/item_cmpfunc.cc | 27 +++++++++++++++------------ sql/sql_string.cc | 10 ++++++++-- 6 files changed, 69 insertions(+), 21 deletions(-) diff --git a/client/sql_string.cc b/client/sql_string.cc index 6b749409a64..dec6ac94eb2 100644 --- a/client/sql_string.cc +++ b/client/sql_string.cc @@ -31,9 +31,12 @@ ** String functions *****************************************************************************/ -bool String::real_alloc(uint32 arg_length) +bool String::real_alloc(uint32 length) { - arg_length=ALIGN_SIZE(arg_length+1); + uint32 arg_length= ALIGN_SIZE(length + 1); + DBUG_ASSERT(arg_length > length); + if (arg_length <= length) + return TRUE; /* Overflow */ str_length=0; if (Alloced_length < arg_length) { @@ -56,6 +59,9 @@ bool String::real_alloc(uint32 arg_length) bool String::realloc(uint32 alloc_length) { uint32 len=ALIGN_SIZE(alloc_length+1); + DBUG_ASSERT(len > alloc_length); + if (len <= alloc_length) + return TRUE; /* Overflow */ if (Alloced_length < len) { char *new_ptr; diff --git a/mysql-test/r/func_if.result b/mysql-test/r/func_if.result index 955a784f04c..c70589a27b7 100644 --- a/mysql-test/r/func_if.result +++ b/mysql-test/r/func_if.result @@ -186,3 +186,13 @@ MAX(IFNULL(CAST(c AS UNSIGNED), 0)) 12345678901234567890 DROP TABLE t1; End of 5.0 tests +# +# Bug#55077: Assertion failed: width > 0 && to != ((void *)0), file .\dtoa.c +# +CREATE TABLE t1 (a LONGBLOB, b DOUBLE); +INSERT INTO t1 VALUES (NULL, 0), (NULL, 1); +SELECT IF(b, (SELECT a FROM t1 LIMIT 1), b) c FROM t1 GROUP BY c; +c +NULL +0 +DROP TABLE t1; diff --git a/mysql-test/t/func_if.test b/mysql-test/t/func_if.test index 4efea8e195e..91f70bb98d7 100644 --- a/mysql-test/t/func_if.test +++ b/mysql-test/t/func_if.test @@ -165,3 +165,15 @@ DROP TABLE t1; --echo End of 5.0 tests + + +--echo # +--echo # Bug#55077: Assertion failed: width > 0 && to != ((void *)0), file .\dtoa.c +--echo # + +CREATE TABLE t1 (a LONGBLOB, b DOUBLE); +INSERT INTO t1 VALUES (NULL, 0), (NULL, 1); + +SELECT IF(b, (SELECT a FROM t1 LIMIT 1), b) c FROM t1 GROUP BY c; + +DROP TABLE t1; diff --git a/sql/field.cc b/sql/field.cc index fc55426b177..0e55b624633 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -4189,6 +4189,7 @@ String *Field_float::val_str(String *val_buffer, String *val_ptr __attribute__((unused))) { ASSERT_COLUMN_MARKED_FOR_READ; + DBUG_ASSERT(field_length <= MAX_FIELD_CHARLENGTH); float nr; #ifdef WORDS_BIGENDIAN if (table->s->db_low_byte_first) @@ -4199,8 +4200,13 @@ String *Field_float::val_str(String *val_buffer, #endif memcpy(&nr, ptr, sizeof(nr)); - uint to_length=max(field_length,70); - val_buffer->alloc(to_length); + uint to_length= 70; + if (val_buffer->alloc(to_length)) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + return val_buffer; + } + char *to=(char*) val_buffer->ptr(); size_t len; @@ -4209,7 +4215,7 @@ String *Field_float::val_str(String *val_buffer, else { /* - We are safe here because the buffer length is >= 70, and + We are safe here because the buffer length is 70, and fabs(float) < 10^39, dec < NOT_FIXED_DEC. So the resulting string will be not longer than 69 chars + terminating '\0'. */ @@ -4506,6 +4512,7 @@ String *Field_double::val_str(String *val_buffer, String *val_ptr __attribute__((unused))) { ASSERT_COLUMN_MARKED_FOR_READ; + DBUG_ASSERT(field_length <= MAX_FIELD_CHARLENGTH); double nr; #ifdef WORDS_BIGENDIAN if (table->s->db_low_byte_first) @@ -4515,9 +4522,13 @@ String *Field_double::val_str(String *val_buffer, else #endif doubleget(nr,ptr); + uint to_length= DOUBLE_TO_STRING_CONVERSION_BUFFER_SIZE; + if (val_buffer->alloc(to_length)) + { + my_error(ER_OUT_OF_RESOURCES, MYF(0)); + return val_buffer; + } - uint to_length=max(field_length, DOUBLE_TO_STRING_CONVERSION_BUFFER_SIZE); - val_buffer->alloc(to_length); char *to=(char*) val_buffer->ptr(); size_t len; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 641d3726aca..8c0f22b0947 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2560,27 +2560,30 @@ Item_func_if::fix_length_and_dec() cached_result_type= arg2_type; collation.set(args[2]->collation.collation); cached_field_type= args[2]->field_type(); + max_length= args[2]->max_length; + return; } - else if (null2) + + if (null2) { cached_result_type= arg1_type; collation.set(args[1]->collation.collation); cached_field_type= args[1]->field_type(); + max_length= args[1]->max_length; + return; + } + + agg_result_type(&cached_result_type, args + 1, 2); + if (cached_result_type == STRING_RESULT) + { + if (agg_arg_charsets_for_string_result(collation, args + 1, 2)) + return; } else { - agg_result_type(&cached_result_type, args+1, 2); - if (cached_result_type == STRING_RESULT) - { - if (agg_arg_charsets_for_string_result(collation, args + 1, 2)) - return; - } - else - { - collation.set_numeric(); // Number - } - cached_field_type= agg_field_type(args + 1, 2); + collation.set_numeric(); // Number } + cached_field_type= agg_field_type(args + 1, 2); uint32 char_length; if ((cached_result_type == DECIMAL_RESULT ) diff --git a/sql/sql_string.cc b/sql/sql_string.cc index 762eebba031..4b7dab243d2 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -31,9 +31,12 @@ ** String functions *****************************************************************************/ -bool String::real_alloc(uint32 arg_length) +bool String::real_alloc(uint32 length) { - arg_length=ALIGN_SIZE(arg_length+1); + uint32 arg_length= ALIGN_SIZE(length + 1); + DBUG_ASSERT(arg_length > length); + if (arg_length <= length) + return TRUE; /* Overflow */ str_length=0; if (Alloced_length < arg_length) { @@ -56,6 +59,9 @@ bool String::real_alloc(uint32 arg_length) bool String::realloc(uint32 alloc_length) { uint32 len=ALIGN_SIZE(alloc_length+1); + DBUG_ASSERT(len > alloc_length); + if (len <= alloc_length) + return TRUE; /* Overflow */ if (Alloced_length < len) { char *new_ptr;