maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Michael Widenius <monty@xxxxxxxxxxxx> writes:

> ------------------------------------------------------------
> revno: 2992
> revision-id: monty@xxxxxxxxxxxx-20110517214756-6f7fv5uk68i4hb3b
> parent: sanja@xxxxxxxxxxxx-20110516132109-h2g9qzrfq3g9t6em
> committer: Michael Widenius <monty@xxxxxxxxxxxx>
> 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.