Re: [Maria-developers] [Commits] Rev 2992: Removed some alias warnings in lp:maria/5.3

Michael Widenius writes:

------------------------------------------------------------ revno: 2992 revision-id: monty@askmonty.org-20110517214756-6f7fv5uk68i4hb3b parent: sanja@askmonty.org-20110516132109-h2g9qzrfq3g9t6em committer: Michael Widenius branch nick: maria-5.3 timestamp: Wed 2011-05-18 00:47:56 +0300 message: Removed some alias warnings Fixed alias bug when compiling with gcc 4.2.4 that caused subselect.test to fail

"Fix strict aliasing bug that caused subselect.test to fail" ("alias bug" can mean different things. And the problem is not GCC specific, see below)

=== modified file 'sql/item_subselect.cc' --- a/sql/item_subselect.cc 2011-05-16 12:07:04 +0000 +++ b/sql/item_subselect.cc 2011-05-17 21:47:56 +0000

@@ -3563,7 +3562,15 @@ subselect_single_select_engine::change_r } else result= res; - return select_lex->join->change_result(result); + + /* + We can't use 'result' below as gcc 4.2.4's alias optimization + assumes that result was not changed by thd->change_item_tree(). + I tried to find a solution to make gcc happy, but could not find anything + that would not require a lot of extra code that would be harder to manage + than the current code. + */ + return select_lex->join->change_result(res); }

I do not agree with this patch. Instead do something like this: @@ -3559,7 +3559,9 @@ subselect_single_select_engine::change_r nothing special about Item* pointer so it is safe conversion. We do not change the interface to be compatible with MySQL. */ - thd->change_item_tree((Item**) &result, (Item*)res); + Item *temp_item; + thd->change_item_tree(&temp_item, (Item*)res); + result= (select_result_interceptor *)temp_item; } else result= res; This makes the code correct (at least with respect to standard C++ strict aliasing), not just work-around the problem for one specific combination of compiler flags for one specific compiler version. The problem has nothing to do with any particular GCC version (except that this is where we happened to detect it), the problem is passing a select_result_interceptor** when an Item** is needed. This is in violation of the strict aliasing rule in standard C++. ---- However, I think we urgently need to start using -fno-strict-aliasing (and probably -fno-strict-overflow as well). Davi told me MySQL@Oracle is doing this as well. In effect there are two dialects of C/C++, even though they are rarely identified as such. - "System C/C++", which is a high-level assembler, where everything is bytes and pointers are just 8 (or 4) byte integers. - "Application C/C++", which is a type-safe language, and pointers are abstract objects that cannot be arbitrarily manipulated. Clearly, many (most?) developers are programming in "System C/C++", including you. That is fine, Postgress, the Linux Kernel, they use the same dialect. However, GCC by default uses "Application C/C++" (-fstrict-aliasing). We need to tell GCC which dialict of C/C++ we are using (-fno-strict-aliasing for "System C/C++"). If we could just agree on which dialect we use, and let GCC know our choice, things will be much better. - Kristian.