Bug#52168 decimal casting catastrophes: crashes and valgrind errors on simple casts

The problem is that if a NULL is stored in an Item_cache_decimal object,
the associated my_decimal object is not initialized.  However, it is still
accessed when val_int() is called. The fix is to check for null_value
within val_int(), and return without accessing the my_decimal object when
the cached value is NULL.

Bug#52122 reports the same issue for val_real(), and this patch also includes
fixes for val_real() and val_str() and corresponding test cases from that
bug report.  

Also, NULL is returned from val_decimal() when value is null. This will
avoid that callers access an uninitialized my_decimal object.

Made similar changes to all other Item_cache classes.  Now all val_*
methods should return a well defined value when actual value is NULL.

mysql-test/r/type_decimal.result

@@ -966,3 +966,31 @@ max(case 1 when 1 then c else null end)
 300.00
 drop table t1;
 End of 5.0 tests
+CREATE TABLE t1 (a INTEGER);
+INSERT INTO t1 VALUES (NULL);
+CREATE TABLE t2 (b INTEGER);
+INSERT INTO t2 VALUES (NULL), (NULL);
+SELECT b FROM t1 JOIN t2 WHERE CONVERT(a, DECIMAL)|CONVERT(b, DECIMAL);
+b
+DROP TABLE t1, t2;
+CREATE TABLE t1 (col0 INTEGER, col1 REAL);
+CREATE TABLE t2 (col0 INTEGER);
+INSERT INTO t1 VALUES (0, 0.0), (NULL, NULL);
+INSERT INTO t2 VALUES (1);
+SELECT 1 FROM t1 
+JOIN
+( 
+SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+GROUP BY t2.col0
+) AS subq  
+WHERE t1.col1 + CAST(subq.col0 AS DECIMAL);
+1
+SELECT 1 FROM t1 
+JOIN
+( 
+SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+GROUP BY t2.col0
+) AS subq  
+WHERE CONCAT(t1.col1, CAST(subq.col0 AS DECIMAL));
+1
+DROP TABLE t1, t2;

mysql-test/t/type_decimal.test

@@ -542,3 +542,44 @@ select max(case 1 when 1 then c else null end) from t1 group by c;
 drop table t1;
 
 --echo End of 5.0 tests
+
+#
+# Bug#52168 decimal casting catastrophes: 
+#           crashes and valgrind errors on simple casts
+#
+
+# Uninitialized read when calling Item_cache_decimal::val_int()
+CREATE TABLE t1 (a INTEGER);
+INSERT INTO t1 VALUES (NULL);
+CREATE TABLE t2 (b INTEGER);
+INSERT INTO t2 VALUES (NULL), (NULL);
+SELECT b FROM t1 JOIN t2 WHERE CONVERT(a, DECIMAL)|CONVERT(b, DECIMAL);
+DROP TABLE t1, t2;
+
+#
+# Bug#52122 crash when converting derived table column to decimal
+#  
+CREATE TABLE t1 (col0 INTEGER, col1 REAL);
+CREATE TABLE t2 (col0 INTEGER);
+INSERT INTO t1 VALUES (0, 0.0), (NULL, NULL);
+INSERT INTO t2 VALUES (1);
+
+# Uninitialized read when calling Item_cache_decimal::val_real()
+SELECT 1 FROM t1 
+JOIN
+( 
+  SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+  GROUP BY t2.col0
+) AS subq  
+WHERE t1.col1 + CAST(subq.col0 AS DECIMAL);
+
+# Uninitialized read when calling Item_cache_decimal::val_str()
+SELECT 1 FROM t1 
+JOIN
+( 
+  SELECT t2.col0 FROM t2 RIGHT JOIN t1 USING(col0) 
+  GROUP BY t2.col0
+) AS subq  
+WHERE CONCAT(t1.col1, CAST(subq.col0 AS DECIMAL));
+
+DROP TABLE t1, t2;

sql/item.cc

@@ -7431,7 +7431,7 @@ void Item_cache_int::store(Item *item, longlong val_arg)
 String *Item_cache_int::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   str->set(value, default_charset());
   return str;
@@ -7441,7 +7441,7 @@ String *Item_cache_int::val_str(String *str)
 my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   int2my_decimal(E_DEC_FATAL_ERROR, value, unsigned_flag, decimal_val);
   return decimal_val;
@@ -7450,7 +7450,7 @@ my_decimal *Item_cache_int::val_decimal(my_decimal *decimal_val)
 double Item_cache_int::val_real()
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0.0;
   return (double) value;
 }
@@ -7458,7 +7458,7 @@ double Item_cache_int::val_real()
 longlong Item_cache_int::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   return value;
 }
@@ -7514,7 +7514,7 @@ String *Item_cache_datetime::val_str(String *str)
 my_decimal *Item_cache_datetime::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value_int())
+  if (!has_value())
     return NULL;
   int2my_decimal(E_DEC_FATAL_ERROR, int_value, unsigned_flag, decimal_val);
   return decimal_val;
@@ -7550,7 +7550,7 @@ bool Item_cache_real::cache_value()
 double Item_cache_real::val_real()
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0.0;
   return value;
 }
@@ -7558,7 +7558,7 @@ double Item_cache_real::val_real()
 longlong Item_cache_real::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   return (longlong) rint(value);
 }
@@ -7567,7 +7567,7 @@ longlong Item_cache_real::val_int()
 String* Item_cache_real::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   str->set_real(value, decimals, default_charset());
   return str;
@@ -7577,7 +7577,7 @@ String* Item_cache_real::val_str(String *str)
 my_decimal *Item_cache_real::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   double2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
   return decimal_val;
@@ -7599,7 +7599,7 @@ double Item_cache_decimal::val_real()
 {
   DBUG_ASSERT(fixed);
   double res;
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0.0;
   my_decimal2double(E_DEC_FATAL_ERROR, &decimal_value, &res);
   return res;
@@ -7609,7 +7609,7 @@ longlong Item_cache_decimal::val_int()
 {
   DBUG_ASSERT(fixed);
   longlong res;
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   my_decimal2int(E_DEC_FATAL_ERROR, &decimal_value, unsigned_flag, &res);
   return res;
@@ -7618,7 +7618,7 @@ longlong Item_cache_decimal::val_int()
 String* Item_cache_decimal::val_str(String *str)
 {
   DBUG_ASSERT(fixed);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   my_decimal_round(E_DEC_FATAL_ERROR, &decimal_value, decimals, FALSE,
                    &decimal_value);
@@ -7629,7 +7629,7 @@ String* Item_cache_decimal::val_str(String *str)
 my_decimal *Item_cache_decimal::val_decimal(my_decimal *val)
 {
   DBUG_ASSERT(fixed);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   return &decimal_value;
 }
@@ -7665,7 +7665,7 @@ double Item_cache_str::val_real()
   DBUG_ASSERT(fixed == 1);
   int err_not_used;
   char *end_not_used;
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0.0;
   if (value)
     return my_strntod(value->charset(), (char*) value->ptr(),
@@ -7678,7 +7678,7 @@ longlong Item_cache_str::val_int()
 {
   DBUG_ASSERT(fixed == 1);
   int err;
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   if (value)
     return my_strntoll(value->charset(), value->ptr(),
@@ -7691,7 +7691,7 @@ longlong Item_cache_str::val_int()
 String* Item_cache_str::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   return value;
 }
@@ -7700,7 +7700,7 @@ String* Item_cache_str::val_str(String *str)
 my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return NULL;
   if (value)
     string2my_decimal(E_DEC_FATAL_ERROR, value, decimal_val);
@@ -7712,7 +7712,7 @@ my_decimal *Item_cache_str::val_decimal(my_decimal *decimal_val)
 
 int Item_cache_str::save_in_field(Field *field, bool no_conversions)
 {
-  if (!value_cached && !cache_value())
+  if (!has_value())
     return 0;
   int res= Item_cache::save_in_field(field, no_conversions);
   return (is_varbinary && field->type() == MYSQL_TYPE_STRING &&

sql/item.h

@@ -3191,6 +3191,15 @@ class Item_cache: public Item_basic_constant
   {
     return this == item;
   }
+  /**
+     Check if saved item has a non-NULL value.
+     Will cache value of saved item if not already done. 
+     @return TRUE if cached value is non-NULL.
+   */
+  bool has_value()
+  {
+    return (value_cached || cache_value()) && !null_value;
+  }
   virtual void store(Item *item);
   virtual bool cache_value()= 0;
   bool basic_const_item() const