Hello Sergei, Please review a patch for MDEV-13972.
The patch is for 5.5, as we agreed in slack. Thanks!
commit 003c19c7677e1ce5b18b4081e21b7864700f8250 Author: Alexander Barkov <[email protected]> Date: Thu Oct 5 21:07:19 2017 +0400 MDEV-13972 crash in Item_func_sec_to_time::get_date Item_func_sec_to_time::get_date() called a value method two times: - once val_int() or val_decimal() inside Item::get_seconds() - then val_str() to generate a warning, in case an out-of-range value This approach was wrong for two reasons: 1. val_str() in some cases can return NULL even if val_int()/val_decimal() previously returned a not-NULL value. This caused the crash reported in the bug: the warning generation code in Item_func_sec_to_time::get_date() did not expect val_str() to return NULL. 2. Calling val_str() to generate warnings fires the function side effect again. For example, consider SEC_TO_TIME(f1()), where f1() is a stored function doing some INSERTs as a side effect. If f1() returns an out-of-range value, the call for val_str() to generate the warning forces the INSERTs to happen again, which is wrong. The side effect must happen only once per call. Fixing the code not to do a dedicated call for val_str() for warnings. To preserve the old warnings recorded in *.result files, four methods where added: - get_seconds_from_string() - get_seconds_from_int() - get_seconds_from_decimal() - get_seconds_from_real() A new structure Secondf_st was added: - to avoid having too many arguments in get_seconds_from_xxx() - to share common code, e.g. between double_to_datetime_with_warn() and get_seconds_from_real() - to share the code easier in the future - to have the code in item.cc and item_timefunc.cc smaller As a good side effect, a redundant warning in func_time.result disappeared diff --git a/mysql-test/r/func_time.result b/mysql-test/r/func_time.result index 68b1e0f..b46e7f3 100644 --- a/mysql-test/r/func_time.result +++ b/mysql-test/r/func_time.result @@ -1042,7 +1042,6 @@ SEC_TO_TIME(CAST(-1 AS UNSIGNED)) 838:59:59 Warnings: Note 1105 Cast to unsigned converted negative integer to it's positive complement -Note 1105 Cast to unsigned converted negative integer to it's positive complement Warning 1292 Truncated incorrect time value: '18446744073709551615' SET NAMES latin1; SET character_set_results = NULL; @@ -2626,3 +2625,126 @@ DROP TABLE t1; SELECT 1 MOD ADDTIME( '13:58:57', '00:00:01' ) + 2; 1 MOD ADDTIME( '13:58:57', '00:00:01' ) + 2 3 +# +# MDEV-13972 crash in Item_func_sec_to_time::get_date +# +DO TO_DAYS(SEC_TO_TIME(TIME(CEILING(UUID())))) /*sporadic warnings*/; +DO TO_DAYS(SEC_TO_TIME(MAKEDATE('',RAND(~(''))))); +Warnings: +Warning 1292 Truncated incorrect INTEGER value: '' +Warning 1292 Truncated incorrect INTEGER value: '' +Warning 1292 Truncated incorrect time value: '20000101' +SELECT TO_DAYS(SEC_TO_TIME(MAKEDATE(0,RAND(~0)))); +TO_DAYS(SEC_TO_TIME(MAKEDATE(0,RAND(~0)))) +0 +Warnings: +Warning 1292 Truncated incorrect time value: '20000101' +SELECT SEC_TO_TIME(MAKEDATE(0,RAND(~0))); +SEC_TO_TIME(MAKEDATE(0,RAND(~0))) +838:59:59 +Warnings: +Warning 1292 Truncated incorrect time value: '20000101' +SELECT SEC_TO_TIME(3020400); +SEC_TO_TIME(3020400) +838:59:59 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(3020400e0); +SEC_TO_TIME(3020400e0) +838:59:59.999999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(3020400.0); +SEC_TO_TIME(3020400.0) +838:59:59.9 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.0' +SELECT SEC_TO_TIME(3020400.00); +SEC_TO_TIME(3020400.00) +838:59:59.99 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.00' +SELECT SEC_TO_TIME(3020400.000); +SEC_TO_TIME(3020400.000) +838:59:59.999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.000' +SELECT SEC_TO_TIME(3020400.0000); +SEC_TO_TIME(3020400.0000) +838:59:59.9999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.0000' +SELECT SEC_TO_TIME(3020400.00000); +SEC_TO_TIME(3020400.00000) +838:59:59.99999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.00000' +SELECT SEC_TO_TIME(3020400.000000); +SEC_TO_TIME(3020400.000000) +838:59:59.999999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.000000' +SELECT SEC_TO_TIME(3020400.0000000); +SEC_TO_TIME(3020400.0000000) +838:59:59.999999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400.0000000' +SELECT SEC_TO_TIME('3020400'); +SEC_TO_TIME('3020400') +838:59:59.999999 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(TIME(3020400)); +SEC_TO_TIME(TIME(3020400)) +838:59:59 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(TIME('302:04:00')); +SEC_TO_TIME(TIME('302:04:00')) +838:59:59 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(TIMESTAMP('0000-00-03 02:04:00')); +SEC_TO_TIME(TIMESTAMP('0000-00-03 02:04:00')) +838:59:59 +Warnings: +Warning 1292 Truncated incorrect time value: '3020400' +SELECT SEC_TO_TIME(3020399); +SEC_TO_TIME(3020399) +838:59:59 +SELECT SEC_TO_TIME(3020399e0); +SEC_TO_TIME(3020399e0) +838:59:59 +SELECT SEC_TO_TIME(3020399.0); +SEC_TO_TIME(3020399.0) +838:59:59.0 +SELECT SEC_TO_TIME(3020399.00); +SEC_TO_TIME(3020399.00) +838:59:59.00 +SELECT SEC_TO_TIME(3020399.000); +SEC_TO_TIME(3020399.000) +838:59:59.000 +SELECT SEC_TO_TIME(3020399.0000); +SEC_TO_TIME(3020399.0000) +838:59:59.0000 +SELECT SEC_TO_TIME(3020399.00000); +SEC_TO_TIME(3020399.00000) +838:59:59.00000 +SELECT SEC_TO_TIME(3020399.000000); +SEC_TO_TIME(3020399.000000) +838:59:59.000000 +SELECT SEC_TO_TIME(3020399.0000000); +SEC_TO_TIME(3020399.0000000) +838:59:59.000000 +SELECT SEC_TO_TIME('3020399'); +SEC_TO_TIME('3020399') +838:59:59 +SELECT SEC_TO_TIME(TIME(3020359)); +SEC_TO_TIME(TIME(3020359)) +838:59:19 +SELECT SEC_TO_TIME(TIME('302:03:59')); +SEC_TO_TIME(TIME('302:03:59')) +838:59:19 +SELECT SEC_TO_TIME(TIMESTAMP('0000-00-03 02:03:59')); +SEC_TO_TIME(TIMESTAMP('0000-00-03 02:03:59')) +838:59:19 diff --git a/mysql-test/t/func_time.test b/mysql-test/t/func_time.test index 92e1c38..dc6b747 100644 --- a/mysql-test/t/func_time.test +++ b/mysql-test/t/func_time.test @@ -1602,3 +1602,44 @@ DROP TABLE t1; --echo # MDEV-10524 Assertion `arg1_int >= 0' failed in Item_func_additive_op::result_precision() --echo # SELECT 1 MOD ADDTIME( '13:58:57', '00:00:01' ) + 2; + +--echo # +--echo # MDEV-13972 crash in Item_func_sec_to_time::get_date +--echo # + +--disable_warnings +DO TO_DAYS(SEC_TO_TIME(TIME(CEILING(UUID())))) /*sporadic warnings*/; +--enable_warnings +DO TO_DAYS(SEC_TO_TIME(MAKEDATE('',RAND(~(''))))); +SELECT TO_DAYS(SEC_TO_TIME(MAKEDATE(0,RAND(~0)))); +SELECT SEC_TO_TIME(MAKEDATE(0,RAND(~0))); +# +# TIME_MAX_VALUE_SECONDS = 838*3600+59*60+59 = 3020400 +# +SELECT SEC_TO_TIME(3020400); +SELECT SEC_TO_TIME(3020400e0); +SELECT SEC_TO_TIME(3020400.0); +SELECT SEC_TO_TIME(3020400.00); +SELECT SEC_TO_TIME(3020400.000); +SELECT SEC_TO_TIME(3020400.0000); +SELECT SEC_TO_TIME(3020400.00000); +SELECT SEC_TO_TIME(3020400.000000); +SELECT SEC_TO_TIME(3020400.0000000); +SELECT SEC_TO_TIME('3020400'); +SELECT SEC_TO_TIME(TIME(3020400)); +SELECT SEC_TO_TIME(TIME('302:04:00')); +SELECT SEC_TO_TIME(TIMESTAMP('0000-00-03 02:04:00')); + +SELECT SEC_TO_TIME(3020399); +SELECT SEC_TO_TIME(3020399e0); +SELECT SEC_TO_TIME(3020399.0); +SELECT SEC_TO_TIME(3020399.00); +SELECT SEC_TO_TIME(3020399.000); +SELECT SEC_TO_TIME(3020399.0000); +SELECT SEC_TO_TIME(3020399.00000); +SELECT SEC_TO_TIME(3020399.000000); +SELECT SEC_TO_TIME(3020399.0000000); +SELECT SEC_TO_TIME('3020399'); +SELECT SEC_TO_TIME(TIME(3020359)); +SELECT SEC_TO_TIME(TIME('302:03:59')); +SELECT SEC_TO_TIME(TIMESTAMP('0000-00-03 02:03:59')); diff --git a/sql/item.cc b/sql/item.cc index fdfbba3..73a1acd 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1407,22 +1407,82 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate) return null_value|= !(fuzzydate & TIME_FUZZY_DATES); } -bool Item::get_seconds(ulonglong *sec, ulong *sec_part) + +bool Item::get_seconds(Secondf_st *sec, const char *type, ulonglong maxsec) { - if (decimals == 0) - { // optimize for an important special case - longlong val= val_int(); - bool neg= val < 0 && !unsigned_flag; - *sec= neg ? -val : val; - *sec_part= 0; - return neg; + switch (cmp_type()) { + case STRING_RESULT: + return get_seconds_from_string(sec, type, maxsec); + case TIME_RESULT: + case DECIMAL_RESULT: + if (decimals == 0) // optimize for an important special case + return get_seconds_from_int(sec, type, maxsec); + return get_seconds_from_decimal(sec, type, maxsec); + case INT_RESULT: + return get_seconds_from_int(sec, type, maxsec); + case REAL_RESULT: + return get_seconds_from_real(sec, type, maxsec); + case ROW_RESULT: + case IMPOSSIBLE_RESULT: + break; } + DBUG_ASSERT(0); + sec->init(); + return 0; +} + + +bool Item::get_seconds_from_int(Secondf_st *sec, + const char *type, ulonglong maxsec) +{ + longlong val= val_int(); + sec->set(val, unsigned_flag); + sec->check_range_with_warn(type, maxsec, ErrConvInteger(val, unsigned_flag)); + return null_value; +} + + +bool Item::get_seconds_from_real(Secondf_st *sec, + const char *type, ulonglong maxsec) +{ + double val= val_real(); + sec->set(val); + sec->check_range_with_warn(type, maxsec, ErrConvDouble(val)); + return null_value; +} + + +bool Item::get_seconds_from_decimal(Secondf_st *sec, + const char *type, ulonglong maxsec) +{ my_decimal tmp, *dec= val_decimal(&tmp); if (!dec) - return 0; - return my_decimal2seconds(dec, sec, sec_part); + { + sec->init(); + return true; + } + sec->set(dec); + sec->check_range_with_warn(type, maxsec, ErrConvDecimal(dec)); + return false; } + +bool Item::get_seconds_from_string(Secondf_st *sec, + const char *type, ulonglong maxsec) +{ + char buff[64]; + String tmp(buff, sizeof(buff), &my_charset_bin), *res= val_str(&tmp); + if (!res) + { + sec->init(); + return true; + } + sec->set(res); + sec->check_range_with_warn(type, maxsec, ErrConvString(res)); + return false; +} + + CHARSET_INFO *Item::default_charset() { return current_thd->variables.collation_connection; diff --git a/sql/item.h b/sql/item.h index 9db5c7e..86883bd 100644 --- a/sql/item.h +++ b/sql/item.h @@ -28,6 +28,7 @@ #include "unireg.h" // REQUIRED: for other includes #include "thr_malloc.h" /* sql_calloc */ #include "field.h" /* Derivation */ +#include "sql_time.h" /* Secondf_st */ C_MODE_START #include <ma_dyncol.h> @@ -1098,7 +1099,18 @@ class Item { virtual bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); bool get_time(MYSQL_TIME *ltime) { return get_date(ltime, TIME_TIME_ONLY); } - bool get_seconds(ulonglong *sec, ulong *sec_part); + /* + Get a signed SECOND+MICROSECOND value: + @param sec - the value will be written here + @param type - the data type name for warnings + @param maxsec - if the SECOND value is greater than maxsec, issue a warning + @retval - false on success, true on NULL + */ + bool get_seconds(Secondf_st *sec, const char *type, ulonglong maxsec); + bool get_seconds_from_int(Secondf_st *sec, const char *type, ulonglong maxsec); + bool get_seconds_from_string(Secondf_st *sec, const char *type, ulonglong maxsec); + bool get_seconds_from_decimal(Secondf_st *sec, const char *type, ulonglong maxsec); + bool get_seconds_from_real(Secondf_st *sec, const char *type, ulonglong maxsec); virtual bool get_date_result(MYSQL_TIME *ltime, ulonglong fuzzydate) { return get_date(ltime,fuzzydate); } /* diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc index 0ed1506..72c3e95 100644 --- a/sql/item_timefunc.cc +++ b/sql/item_timefunc.cc @@ -1700,42 +1700,33 @@ bool Item_func_sysdate_local::get_date(MYSQL_TIME *res, bool Item_func_sec_to_time::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) { DBUG_ASSERT(fixed == 1); - bool sign; - ulonglong sec; - ulong sec_part; + Secondf_st sec; bzero((char *)ltime, sizeof(*ltime)); ltime->time_type= MYSQL_TIMESTAMP_TIME; - sign= args[0]->get_seconds(&sec, &sec_part); - - if ((null_value= args[0]->null_value)) - return 1; + if ((null_value= (args[0]->get_seconds(&sec, + "time", TIME_MAX_VALUE_SECONDS)))) + return true; - ltime->neg= sign; - if (sec > TIME_MAX_VALUE_SECONDS) + ltime->neg= sec.m_neg; + if (sec.m_second > TIME_MAX_VALUE_SECONDS) goto overflow; - DBUG_ASSERT(sec_part <= TIME_MAX_SECOND_PART); + DBUG_ASSERT(sec.m_usecond <= TIME_MAX_SECOND_PART); - ltime->hour= (uint) (sec/3600); - ltime->minute= (uint) (sec % 3600) /60; - ltime->second= (uint) sec % 60; - ltime->second_part= sec_part; + ltime->hour= (uint) (sec.m_second / 3600); + ltime->minute= (uint) (sec.m_second % 3600) /60; + ltime->second= (uint) sec.m_second % 60; + ltime->second_part= sec.m_usecond; return 0; overflow: /* use check_time_range() to set ltime to the max value depending on dec */ int unused; - char buf[100]; - String tmp(buf, sizeof(buf), &my_charset_bin), *err= args[0]->val_str(&tmp); - ltime->hour= TIME_MAX_HOUR+1; check_time_range(ltime, decimals, &unused); - make_truncated_value_warning(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, - err->ptr(), err->length(), - MYSQL_TIMESTAMP_TIME, NullS); return 0; } @@ -1929,21 +1920,18 @@ void Item_func_from_unixtime::fix_length_and_dec() bool Item_func_from_unixtime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date __attribute__((unused))) { - bool sign; - ulonglong sec; - ulong sec_part; + Secondf_st sec; bzero((char *)ltime, sizeof(*ltime)); ltime->time_type= MYSQL_TIMESTAMP_TIME; - sign= args[0]->get_seconds(&sec, &sec_part); - - if (args[0]->null_value || sign || sec > TIMESTAMP_MAX_VALUE) + if (args[0]->get_seconds(&sec, "", ULONGLONG_MAX) || + sec.m_neg || sec.m_second > TIMESTAMP_MAX_VALUE) return (null_value= 1); - tz->gmt_sec_to_TIME(ltime, (my_time_t)sec); + tz->gmt_sec_to_TIME(ltime, (my_time_t) sec.m_second); - ltime->second_part= sec_part; + ltime->second_part= sec.m_usecond; return (null_value= 0); } @@ -2735,12 +2723,11 @@ bool Item_func_maketime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) bool overflow= 0; longlong hour= args[0]->val_int(); longlong minute= args[1]->val_int(); - ulonglong second; - ulong microsecond; - bool neg= args[2]->get_seconds(&second, µsecond); + Secondf_st sec; - if (args[0]->null_value || args[1]->null_value || args[2]->null_value || - minute < 0 || minute > 59 || neg || second > 59) + if (args[0]->null_value || args[1]->null_value || + args[2]->get_seconds(&sec, "", ULONGLONG_MAX) || + minute < 0 || minute > 59 || sec.m_neg || sec.m_second > 59) return (null_value= 1); bzero(ltime, sizeof(*ltime)); @@ -2761,8 +2748,8 @@ bool Item_func_maketime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) { ltime->hour= (uint) ((hour < 0 ? -hour : hour)); ltime->minute= (uint) minute; - ltime->second= (uint) second; - ltime->second_part= microsecond; + ltime->second= (uint) sec.m_second; + ltime->second_part= sec.m_usecond; } else { @@ -2771,7 +2758,8 @@ bool Item_func_maketime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) ltime->second= TIME_MAX_SECOND; char buf[28]; char *ptr= longlong10_to_str(hour, buf, args[0]->unsigned_flag ? 10 : -10); - int len = (int)(ptr - buf) + sprintf(ptr, ":%02u:%02u", (uint)minute, (uint)second); + int len= (int)(ptr - buf) + sprintf(ptr, ":%02u:%02u", + (uint) minute, (uint) sec.m_second); make_truncated_value_warning(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, buf, len, MYSQL_TIMESTAMP_TIME, NullS); diff --git a/sql/sql_time.cc b/sql/sql_time.cc index d912a7b..fc3c644 100644 --- a/sql/sql_time.cc +++ b/sql/sql_time.cc @@ -27,6 +27,44 @@ #define MAX_DAY_NUMBER 3652424L + +void Secondf_st::set(double value) +{ + if ((m_neg= value < 0)) + value= -value; + if (value > LONGLONG_MAX) + value= static_cast<double>(LONGLONG_MAX); + m_second= static_cast<ulonglong>(floor(value)); + m_usecond= static_cast<ulong>((value - floor(value))*TIME_SECOND_PART_FACTOR); +} + + +void Secondf_st::set(const my_decimal *dec) +{ + m_neg= my_decimal2seconds(dec, &m_second, &m_usecond); +} + + +void Secondf_st::set(const String *str) +{ + my_decimal dec; + (void) str2my_decimal(E_DEC_FATAL_ERROR, + str->ptr(), str->length(), str->charset(), &dec); + set(&dec); +} + + +void Secondf_st::check_range_with_warn(const char *type, + ulonglong maxsec, + const ErrConv &err) +{ + if (m_second > maxsec) + push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, + ER_TRUNCATED_WRONG_VALUE, ER(ER_TRUNCATED_WRONG_VALUE), + type, err.ptr()); +} + + /* Some functions to calculate dates */ /* @@ -399,18 +437,10 @@ bool double_to_datetime_with_warn(double value, MYSQL_TIME *ltime, ulonglong fuzzydate, const char *field_name) { const ErrConvDouble str(value); - bool neg= value < 0; - - if (neg) - value= -value; - - if (value > LONGLONG_MAX) - value= static_cast<double>(LONGLONG_MAX); - - longlong nr= static_cast<ulonglong>(floor(value)); - uint sec_part= static_cast<ulong>((value - floor(value))*TIME_SECOND_PART_FACTOR); - return number_to_time_with_warn(neg, nr, sec_part, ltime, fuzzydate, &str, - field_name); + Secondf_st sec2; + sec2.set(value); + return number_to_time_with_warn(sec2.m_neg, sec2.m_second, sec2.m_usecond, + ltime, fuzzydate, &str, field_name); } diff --git a/sql/sql_time.h b/sql/sql_time.h index 9becdcd..4a6307d 100644 --- a/sql/sql_time.h +++ b/sql/sql_time.h @@ -32,6 +32,33 @@ typedef struct st_known_date_time_format KNOWN_DATE_TIME_FORMAT; #define WEEK_YEAR 2 #define WEEK_FIRST_WEEKDAY 4 + +struct Secondf_st +{ +public: + ulonglong m_second; + ulong m_usecond; + bool m_neg; + void init() + { + m_neg= false; + m_second= 0; + m_usecond= 0; + } + void set(double nr); + void set(longlong val, bool unsigned_flag) + { + m_neg= val < 0 && !unsigned_flag; + m_second= m_neg ? -val : val; + m_usecond= 0; + } + void set(const my_decimal *dec); + void set(const String *str); + void check_range_with_warn(const char *type, ulonglong maxsec, + const ErrConv &err); +}; + + ulong convert_period_to_month(ulong period); ulong convert_month_to_period(ulong month); bool time_to_datetime(MYSQL_TIME *ltime);
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
