Bug #47167 "set global innodb_file_format_check" cannot set value by User-Defined Variable
Submitted: 7 Sep 12:07 Modified: 11 Nov 13:26
Reporter: Yasufumi Kinoshita
Status: Patch approved
Category:Server: InnoDB Plugin Severity:S2 (Serious)
Version:5.1.37 + 1.0.4 OS:Any
Assigned to: Jimmy Yang Target Version:5.1+
Triage: Triaged: D2 (Serious)

[7 Sep 12:07] Yasufumi Kinoshita
Description:
"set global innodb_file_format_check" command seems to not able to set value using
User-Defined Variable.

How to repeat:
mysql> set @old_innodb_file_format_check=@@innodb_file_format_check;
Query OK, 0 rows affected (0.00 sec)

mysql> select @old_innodb_file_format_check;
+-------------------------------+
| @old_innodb_file_format_check |
+-------------------------------+
| Antelope                      |
+-------------------------------+
1 row in set (0.00 sec)

mysql> set global innodb_file_format_check = Barracuda;
Query OK, 0 rows affected (0.00 sec)

mysql> select @@innodb_file_format_check;
+----------------------------+
| @@innodb_file_format_check |
+----------------------------+
| Barracuda                  |
+----------------------------+
1 row in set (0.00 sec)

mysql> set global innodb_file_format_check = @old_innodb_file_format_check;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+--------------------------------------+
| Level   | Code | Message                              |
+---------+------+--------------------------------------+
| Warning | 1210 | Ignoring SET innodb_file_format=XG |
+---------+------+--------------------------------------+
1 row in set (0.00 sec)

mysql> select @@innodb_file_format_check;
+----------------------------+
| @@innodb_file_format_check |
+----------------------------+
| Barracuda                  |
+----------------------------+
1 row in set (0.00 sec)
[7 Sep 14:23] Sveta Smirnova
Thank you for the report.

Verified as described.

Seems to be some memory corruption:

set @old_innodb_file_format_check=@@innodb_file_format_check;
select @old_innodb_file_format_check;
@old_innodb_file_format_check
Antelope
set global innodb_file_format_check = Barracuda;
select @@innodb_file_format_check;
@@innodb_file_format_check
Barracuda
set global innodb_file_format_check = @old_innodb_file_format_check;
Warnings:
Warning 1210    Ignoring SET innodb_file_format=@?(H
select @@innodb_file_format_check;
@@innodb_file_format_check
Barracuda
[15 Sep 3:56] Jimmy Yang
In summary, this bug is caused by using a local string buffer (buff[] in
innodb_file_format_check_validate) across functions by saving its pointer in
var->save_result in sys_var_pluginvar::check() and re-using it in 
sys_var_pluginvar::update(). And this results in wrong return value
innodb_file_format_check_update() due to the stale string buffer and wrong information in
the buffer, causing the update fail.

In this case, we are setting a variable value "Antelope" from 
'@old_innodb_file_format_check' to variable 'innodb_file_format_check':

mysql> set global innodb_file_format_check = @old_innodb_file_format_check;

In innodb_file_format_check_validate(), the variable name is put in a local name buffer.
And it is saved for future use (which is wrong):

innodb_file_format_check_validate()
{
        const char*     file_format_input;
        char            buff[STRING_BUFFER_USUAL_SIZE]; <===

        file_format_input = value->val_str(value, buff, &len);
        ....
        *static_cast<const char**>(save) = file_format_input; <== Saved

        .....
}

(gdb) print file_format_input
$2 = 0x48d08300 "Antelope"
(gdb) print &buff
$3 = (char (*)[80]) 0x48d08300
(gdb) print (char *)buff
$4 = 0x48d08300 "Antelope"

(gdb) print save
$5 = (void *) 0x1af14e48

This "save" is the "&var->save_result" in the caller function
sys_var_pluginvar::check():

(gdb)  print (char *)var->save_result->locale_value
$14 = 0x48d08300 "Antelope"

function stack:
innodb_file_format_check_validate()
sys_var_pluginvar::check()
set_var::check()
sql_set_variables()
mysql_execute_command()

Subsequently, this "save" value (var->save_result) is used for the update call:
 plugin_var->update(thd, plugin_var, tgt, &var->save_result);

function stack:
innodb_file_format_check_update()
sys_var_pluginvar::update()
set_var::update()
sql_set_variables()
mysql_execute_command()

The update call innodb_file_format_check_update(), and the value to be updated is passed
from update() call with that saved stale name buffer (var->save_result->locale_value):

innodb_file_format_check_update()
{
   const char*     format_name_in;
   ...
   format_name_in = *static_cast<const char*const*>(save);
   format_id = innobase_file_format_name_lookup(format_name_in);
   ...
}

(gdb)  print format_name_in
$17 = 0x48d08300 "Antelope" <=== Notice this is a pointer to local variable buff[]
defined innodb_file_format_check_validate(), which no longer allocated

In the innobase_file_format_name_lookup(), this name buffer conntent becomes stale after
strtoul() (it can be messed up any time, but it happens here in this case).

(gdb) print format_name
$19 = 0x48d08300 ""

And as a result innobase_file_format_name_lookup() can never locate the name's
corresponding id and returns an id > DICT_TF_FORMAT_MAX and triggers the error:

innodb_file_format_check_update()
{
        ...
        format_id = innobase_file_format_name_lookup(format_name_in);
        if (format_id > DICT_TF_FORMAT_MAX) {
                /* DEFAULT is "on", which is invalid at runtime. */
                push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, <===
                                    ER_WRONG_ARGUMENTS,
                                    "Ignoring SET innodb_file_format=%s",
                                    format_name_in);
                return;
        }

}

This warning is shown when "show warnings" called:

mysql> set global innodb_file_format_check = @old_innodb_file_format_check;
Query OK, 0 rows affected, 1 warning (33 min 43.91 sec)

mysql> show warnings;
+---------+------+--------------------------------------+
| Level   | Code | Message                              |
+---------+------+--------------------------------------+
| Warning | 1210 | Ignoring SET innodb_file_format=P??H |
+---------+------+--------------------------------------+
1 row in set (0.00 sec)

To fix this, we might need to pre-allocated the value buffer in functions higher in stack
such as sql_set_variables(), or find another alternative to fetch the correct value
information in update().

thanks
Jimmy
[19 Sep 2:03] Jimmy Yang
Although we know the cause for this problem, the fix could be a bit complicated. As it
could involve code in the mysql layer. 

The observation is that the assignment of constant value does not have any problem. This
is due to the different behavior how the memory is allocated in the val_str() calls.

In fact, the value->val_str() call behaves differently with constant value assignment and
variable value assignment as functions map to functions in their own class:

Item_string::val_str   <== constant value assignment
Item_func_get_user_var::val_str  item_func.cc:4557 <== Variable value assignment

In the constant value assignment, the value->val_str() did not use the buffer passed in,
instead, it allocates a memory to hold it in item_val_str() call:

static const char *item_val_str()
{

  String str(buffer, *length, system_charset_info), *res;
  if (!(res= ((st_item_value_holder*)value)->item->val_str(&str)))
    return NULL;
  *length= res->length();
  if (res->c_ptr_quick() == buffer)
    return buffer;

  /*
    Lets be nice and create a temporary string since the
    buffer was too small
  */
  return current_thd->strmake(res->c_ptr_quick(), res->length()); <===
}

we could also mimic the behavior by setting the len to 0 when calling  value->val_str()
in innodb_file_format_name_validate(), this will also allow the memory to be allocated.
This will fix the stale problem. However, we need to consult with mysql team to see
whether this change will lead of some form of memory leak during the string operation by
allocating new memory everytime.

Thanks
Jimmy