Bug #59905 Plugin boolean variables don't behave well on unsigned-char machines
Submitted: 2 Feb 2011 22:29 Modified: 30 May 2013 1:58
Reporter: [ name withheld ] Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:5.1, 5.5.8, 5.6.3 OS:Any
Assigned to:
Tags: Contribution
Triage: Needs Triage: D3 (Medium)

[2 Feb 2011 22:29] [ name withheld ]
Description:
mysql 5.5.8 fails its regression tests when built on a machine that considers C's "char" type to be unsigned char, such as PPC.  The failures look like this:

sys_vars.rpl_semi_sync_master_enabled_basic [ fail ]
        Test ended at 2011-01-31 17:18:49
CURRENT_TEST: sys_vars.rpl_semi_sync_master_enabled_basic
--- /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2010-12-03 20:58:25.000000000 +0300
+++ /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.reject	2011-01-31 20:18:49.313402710 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_master_enabled;
 @@global.rpl_semi_sync_master_enabled
--1
+255
 select @@session.rpl_semi_sync_master_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_master_enabled';
mysqltest: Result content mismatch

sys_vars.rpl_semi_sync_slave_enabled_basic [ fail ]
        Test ended at 2011-01-31 17:18:50
CURRENT_TEST: sys_vars.rpl_semi_sync_slave_enabled_basic
--- /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2010-12-03 20:58:26.000000000 +0300
+++ /builddir/build/BUILD/mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.reject	2011-01-31 20:18:50.555904753 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+255
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';
mysqltest: Result content mismatch

Observed on Fedora rawhide and RHEL-6, but I don't believe the specific OS matters, only the C compiler's ideas about "char".

How to repeat:
1. Build on PPC, or another architecture where char is unsigned by default.  It'd likely also work to use a compiler switch that changes the default signedness of char, but I didn't try that.

2. Run the standard regression tests.

Suggested fix:
The minimum fix would be to paper over the problem by forcing bool variables to print signed everywhere, for example with this horrid kluge:

diff -Naur mysql-5.5.8.orig/sql/item_func.cc mysql-5.5.8/sql/item_func.cc
--- mysql-5.5.8.orig/sql/item_func.cc	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/sql/item_func.cc	2011-02-01 19:01:08.536298538 -0500
@@ -5442,7 +5442,7 @@
     case SHOW_LONGLONG: get_sys_var_safe (ulonglong);
     case SHOW_HA_ROWS:  get_sys_var_safe (ha_rows);
     case SHOW_BOOL:     get_sys_var_safe (bool);
-    case SHOW_MY_BOOL:  get_sys_var_safe (my_bool);
+    case SHOW_MY_BOOL:  get_sys_var_safe (signed char);
     case SHOW_DOUBLE:
       {
         double dval= val_real();

I don't much care for that though, because I think the behavior shown in the regression tests is wrong/insane anyway.  When you set a boolean variable to 1, it does not meet the principle of least astonishment for it to print out as -1.  I think the real bug is that somebody stuck a minus sign into check_func_bool() --- what the heck is the reasoning for that?  I'm planning to use the following patch instead:

diff -Naur mysql-5.5.8.orig/sql/sql_plugin.cc mysql-5.5.8/sql/sql_plugin.cc
--- mysql-5.5.8.orig/sql/sql_plugin.cc	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/sql/sql_plugin.cc	2011-02-01 20:34:10.218305349 -0500
@@ -2024,7 +2024,7 @@
       goto err;
     result= (int) tmp;
   }
-  *(my_bool *) save= -result;
+  *(my_bool *) save= result ? true : false;
   return 0;
 err:
   return 1;
diff -Naur mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result
--- mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2010-12-03 12:58:25.000000000 -0500
+++ mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_master_enabled_basic.result	2011-02-01 21:53:20.006302245 -0500
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_master_enabled;
 @@global.rpl_semi_sync_master_enabled
--1
+1
 select @@session.rpl_semi_sync_master_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_master_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_master_enabled';
diff -Naur mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result
--- mysql-5.5.8.orig/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2010-12-03 12:58:26.000000000 -0500
+++ mysql-5.5.8/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2011-02-01 21:53:59.689249491 -0500
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+1
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';

Note that these two regression tests are the only ones in the "standard" suite that show any change in results from this, but there might be some more elsewhere.
[11 Mar 2011 19:20] Sveta Smirnova
Thank you for the report.

Verified as described using x86_64 architecture and gcc option -funsigned-char
[11 Mar 2011 21:15] Davi Arnaut
Also affects 5.1.

The minus in sql_plugin.cc looks like a residual left over. At first, the backing type of boolean variables was int (which in itself was a bug), but later got converted to my_bool.
[19 Nov 2012 7:55] Clint Byrum
This also affects the ARM architecture, and can be seen causing spurious failures in the test suite on Ubuntu builds for armel and armhf:

https://launchpadlibrarian.net/110613104/buildlog_ubuntu-quantal-armhf.mysql-5.5_5.5.25a-0...

sys_vars.rpl_semi_sync_slave_enabled_basic [ fail ]
        Test ended at 2012-07-20 01:24:11

CURRENT_TEST: sys_vars.rpl_semi_sync_slave_enabled_basic
--- /build/buildd/mysql-5.5-5.5.25a/builddir/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.result	2012-07-20 03:26:15.000000000 +0300
+++ /build/buildd/mysql-5.5-5.5.25a/builddir/mysql-test/suite/sys_vars/r/rpl_semi_sync_slave_enabled_basic.reject	2012-07-20 04:24:11.000000000 +0300
@@ -45,7 +45,7 @@
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable and should be set with SET GLOBAL
 select @@global.rpl_semi_sync_slave_enabled;
 @@global.rpl_semi_sync_slave_enabled
--1
+255
 select @@session.rpl_semi_sync_slave_enabled;
 ERROR HY000: Variable 'rpl_semi_sync_slave_enabled' is a GLOBAL variable
 show global variables like 'rpl_semi_sync_slave_enabled';

Given that there are real, viable ARM servers on the market now, this seems rather important to get solved.

Tracking in Ubuntu here:

https://bugs.launchpad.net/mysql-server/+bug/1080582
[30 May 2013 1:58] Paul Dubois
Noted in 5.6.13, 5.7.2 changelogs.

Boolean plugin system variables did not behave well on machines where
char is unsigned; some code attempted to assign a negative value to
these.