List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:July 5 2010 8:51pm
Subject:bzr commit into mysql-5.1-bugteam branch (davi:3459) Bug#42733
View as plain text  
# At a local mysql-5.1-bugteam repository of davi

 3459 Davi Arnaut	2010-07-05
      Bug#42733: Type-punning warnings when compiling MySQL --
                 strict aliasing violations.
      
      Another rather noisy violation of strict aliasing rules
      is the spatial code which makes use of stack-based memory
      (of type Geometry_buffer) to provide placement for Geometry
      objects. Although a placement new is allowed to dynamically
      change the type of a object, the object returned by the
      new placement was being ignored and the original stack-based
      object was being casted to the new type, thus violating strict
      aliasing rules.
      
      The solution is to reorganize the code so that the object
      returned by the new placement is used instead of casting the
      original object. Also, to ensure that the stack-based object
      is properly aligned with respect to the objects it provides
      placement for, a set of compiler-dependent macros and types
      are introduced so that the alignment of objects can be inquired
      and specified.
     @ include/Makefile.am
        Add new header.
     @ include/my_compiler.h
        Add header to house compiler-dependent features.
        
        Use compiler/version to detect available features at build time
        instead of configure time in order to avoid common pitfalls in
        autoconf-based checks (such as attributes being ignored by certain
        compilers).
        
        Add macros to inquire and set alignment.
        
        Add my_aligned_storage which is similar to the C++0x aligned_storage
        type, which provides a POD-type suitable for use as uninitialized
        storage for any object.
        
        Use partial template specialization due to historical weakness
        of using attributes with template arguments.
     @ include/my_global.h
        Remove now-unnecessary macros.
     @ sql/spatial.cc
        Make object creation functions return the object whose type
        was dynamically changed by the new placement.
        
        Move static method from the header in order to avoid having
        to access a forward declaration.
     @ sql/spatial.h
        Object creation callbacks now take a array of chars as the
        storage area.
        
        Move create_by_typeid to a source file as to not access the
        forward declaration of Geometry_buffer.
        
        Ensure that Geometry_buffer is properly aligned.
     @ sql/sql_show.cc
        Use newly added aligned storage helper.

    added:
      include/my_compiler.h
    modified:
      include/Makefile.am
      include/my_global.h
      sql/spatial.cc
      sql/spatial.h
      sql/sql_show.cc
=== modified file 'include/Makefile.am'
--- a/include/Makefile.am	2009-03-20 11:18:29 +0000
+++ b/include/Makefile.am	2010-07-05 20:51:07 +0000
@@ -36,7 +36,7 @@ noinst_HEADERS =	config-win.h config-net
 			my_nosys.h my_alarm.h queues.h rijndael.h sha1.h \
 			my_aes.h my_tree.h my_trie.h hash.h thr_alarm.h \
 			thr_lock.h t_ctype.h violite.h my_md5.h base64.h \
-			my_handler.h my_time.h \
+			my_handler.h my_time.h my_compiler.h \
 			my_vle.h my_user.h my_atomic.h atomic/nolock.h \
 			atomic/rwlock.h atomic/x86-gcc.h atomic/x86-msvc.h \
 			atomic/gcc_builtins.h my_libwrap.h my_stacktrace.h

=== added file 'include/my_compiler.h'
--- a/include/my_compiler.h	1970-01-01 00:00:00 +0000
+++ b/include/my_compiler.h	2010-07-05 20:51:07 +0000
@@ -0,0 +1,127 @@
+#ifndef MY_COMPILER_INCLUDED
+#define MY_COMPILER_INCLUDED
+
+/* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
+
+/**
+  Header for compiler-dependent features.
+
+  Intended to contain a set of reusable wrappers for preprocessor
+  macros, attributes, pragmas, and any other features that are
+  specific to a target compiler.
+*/
+
+#include <my_global.h>                          /* stddef.h offsetof */
+
+/**
+  Compiler-dependent internal convenience macros.
+*/
+
+/* GNU C/C++ */
+#if defined __GNUC__
+/* Any after 2.95... */
+# define MY_ALIGN_EXT
+
+/* Microsoft Visual C++ */
+#elif defined _MSC_VER
+# define MY_ALIGNOF(type)   __alignof(type)
+# define MY_ALIGNED(n)      __declspec(align(n))
+
+/* Oracle Solaris Studio */
+#elif defined(__SUNPRO_C) || defined(__SUNPRO_CC)
+# if __SUNPRO_CC >= 0x590
+#   define MY_ALIGN_EXT
+# endif
+
+/* IBM XL C/C++ */
+#elif defined __xlC__
+# if __xlC__ >= 0x0600
+#   define MY_ALIGN_EXT
+# endif
+
+/* HP aCC */
+#elif defined(__HP_aCC) || defined(__HP_cc)
+# if (__HP_aCC >= 60000) || (__HP_cc >= 60000)
+#   define MY_ALIGN_EXT
+# endif
+#endif
+
+#ifdef MY_ALIGN_EXT
+/** Specifies the minimum alignment of a type. */
+# define MY_ALIGNOF(type)   __alignof__(type)
+/** Determine the alignment requirement of a type. */
+# define MY_ALIGNED(n)      __attribute__((__aligned__((n))))
+#endif
+
+/**
+  Generic compiler-dependent features.
+*/
+#ifndef MY_ALIGNOF
+# ifdef __cplusplus
+    /* Invalid for non-POD types, but most compilers give the right answer. */
+    template<class type> struct my_alignof_helper { char m1; type m2; };
+#   define MY_ALIGNOF(type)   offsetof(my_alignof_helper<type>, m2)
+# else
+#   define MY_ALIGNOF(type)   offsetof(struct { char m1; type m2; }, m2)
+# endif
+#endif
+
+/**
+  C++ Type Traits
+*/
+
+#ifdef __cplusplus
+
+/**
+  Opaque storage with a particular alignment.
+*/
+# if defined(MY_ALIGNED)
+/* Partial specialization used due to MSVC++. */
+template<size_t alignment> struct my_alignment_imp;
+template<> struct MY_ALIGNED(1) my_alignment_imp<1> {};
+template<> struct MY_ALIGNED(2) my_alignment_imp<2> {};
+template<> struct MY_ALIGNED(4) my_alignment_imp<4> {};
+template<> struct MY_ALIGNED(8) my_alignment_imp<8> {};
+template<> struct MY_ALIGNED(16) my_alignment_imp<16> {};
+/* ... expand as necessary. */
+# else
+template<size_t alignment>
+struct my_alignment_imp { double m1; };
+# endif
+
+/**
+  A POD type with a given size and alignment.
+
+  @remark If the compiler does not support a alignment attribute
+          (MY_ALIGN macro), the default alignment of a double is
+          used instead.
+
+  @tparam size        The minimum size.
+  @tparam alignment   The desired alignment: 1, 2, 4, 8 or 16.
+*/
+template <size_t size, size_t alignment>
+struct my_aligned_storage
+{
+  union
+  {
+    char data[size];
+    my_alignment_imp<alignment> align;
+  };
+};
+
+#endif /* __cplusplus */
+
+#endif /* MY_COMPILER_INCLUDED */

=== modified file 'include/my_global.h'
--- a/include/my_global.h	2010-07-02 18:30:47 +0000
+++ b/include/my_global.h	2010-07-05 20:51:07 +0000
@@ -941,9 +941,6 @@ typedef long long	my_ptrdiff_t;
 #define ADD_TO_PTR(ptr,size,type) (type) ((uchar*) (ptr)+size)
 #define PTR_BYTE_DIFF(A,B) (my_ptrdiff_t) ((uchar*) (A) - (uchar*) (B))
 
-#define MY_DIV_UP(A, B) (((A) + (B) - 1) / (B))
-#define MY_ALIGNED_BYTE_ARRAY(N, S, T) T N[MY_DIV_UP(S, sizeof(T))]
-
 /*
   Custom version of standard offsetof() macro which can be used to get
   offsets of members in class for non-POD types (according to the current

=== modified file 'sql/spatial.cc'
--- a/sql/spatial.cc	2009-08-28 16:21:54 +0000
+++ b/sql/spatial.cc	2010-07-05 20:51:07 +0000
@@ -53,7 +53,7 @@ static Geometry::Class_info **ci_collect
                                 Geometry::ci_collection+Geometry::wkb_last + 1;
 
 Geometry::Class_info::Class_info(const char *name, int type_id,
-					 void(*create_func)(void *)):
+                                 create_geom_t create_func):
   m_type_id(type_id), m_create_func(create_func)
 {
   m_name.str= (char *) name;
@@ -62,39 +62,39 @@ Geometry::Class_info::Class_info(const c
   ci_collection[type_id]= this;
 }
 
-static void create_point(void *buffer)
+static Geometry *create_point(char *buffer)
 {
-  new(buffer) Gis_point;
+  return new (buffer) Gis_point;
 }
 
-static void create_linestring(void *buffer)
+static Geometry *create_linestring(char *buffer)
 {
-  new(buffer) Gis_line_string;
+  return new (buffer) Gis_line_string;
 }
 
-static void create_polygon(void *buffer)
+static Geometry *create_polygon(char *buffer)
 {
-  new(buffer) Gis_polygon;
+  return new (buffer) Gis_polygon;
 }
 
-static void create_multipoint(void *buffer)
+static Geometry *create_multipoint(char *buffer)
 {
-  new(buffer) Gis_multi_point;
+  return new (buffer) Gis_multi_point;
 }
 
-static void create_multipolygon(void *buffer)
+static Geometry *create_multipolygon(char *buffer)
 {
-  new(buffer) Gis_multi_polygon;
+  return new (buffer) Gis_multi_polygon;
 }
 
-static void create_multilinestring(void *buffer)
+static Geometry *create_multilinestring(char *buffer)
 {
-  new(buffer) Gis_multi_line_string;
+  return new (buffer) Gis_multi_line_string;
 }
 
-static void create_geometrycollection(void *buffer)
+static Geometry *create_geometrycollection(char *buffer)
 {
-  new(buffer) Gis_geometry_collection;
+  return new (buffer) Gis_geometry_collection;
 }
 
 
@@ -145,6 +145,15 @@ Geometry::Class_info *Geometry::find_cla
 }
 
 
+Geometry *Geometry::create_by_typeid(Geometry_buffer *buffer, int type_id)
+{
+  Class_info *ci;
+  if (!(ci= find_class(type_id)))
+    return NULL;
+  return (*ci->m_create_func)(buffer->data);
+}
+
+
 Geometry *Geometry::construct(Geometry_buffer *buffer,
                               const char *data, uint32 data_len)
 {
@@ -170,6 +179,7 @@ Geometry *Geometry::create_from_wkt(Geom
 {
   LEX_STRING name;
   Class_info *ci;
+  Geometry *result;
 
   if (trs->get_next_word(&name))
   {
@@ -179,9 +189,7 @@ Geometry *Geometry::create_from_wkt(Geom
   if (!(ci= find_class(name.str, name.length)) ||
       wkt->reserve(1 + 4, 512))
     return NULL;
-  (*ci->m_create_func)((void *)buffer);
-  Geometry *result= (Geometry *)buffer;
-  
+  result= (*ci->m_create_func)(buffer->data);
   wkt->q_append((char) wkb_ndr);
   wkt->q_append((uint32) result->get_class_info()->m_type_id);
   if (trs->check_next_symbol('(') ||

=== modified file 'sql/spatial.h'
--- a/sql/spatial.h	2009-06-17 14:56:44 +0000
+++ b/sql/spatial.h	2010-07-05 20:51:07 +0000
@@ -16,6 +16,8 @@
 #ifndef _spatial_h
 #define _spatial_h
 
+#include <my_compiler.h>
+
 #ifdef HAVE_SPATIAL
 
 const uint SRID_SIZE= 4;
@@ -227,13 +229,15 @@ public:
     wkb_ndr= 1     /* Little Endian */
   };                                    
 
+  typedef Geometry *(*create_geom_t)(char *);
+
   class Class_info
   {
   public:
     LEX_STRING m_name;
     int m_type_id;
-    void (*m_create_func)(void *);
-    Class_info(const char *name, int type_id, void(*create_func)(void *));
+    create_geom_t m_create_func;
+    Class_info(const char *name, int type_id, create_geom_t create_func);
   };
 
   virtual const Class_info *get_class_info() const=0;
@@ -263,15 +267,7 @@ public:
   virtual int geometry_n(uint32 num, String *result) const { return -1; }
 
 public:
-  static Geometry *create_by_typeid(Geometry_buffer *buffer, int type_id)
-  {
-    Class_info *ci;
-    if (!(ci= find_class((int) type_id)))
-      return NULL;
-    (*ci->m_create_func)((void *)buffer);
-    return my_reinterpret_cast(Geometry *)(buffer);
-  }
-
+  static Geometry *create_by_typeid(Geometry_buffer *buffer, int type_id);
   static Geometry *construct(Geometry_buffer *buffer,
                              const char *data, uint32 data_len);
   static Geometry *create_from_wkt(Geometry_buffer *buffer,
@@ -528,11 +524,8 @@ public:
   const Class_info *get_class_info() const;
 };
 
-const int geometry_buffer_size= sizeof(Gis_point);
-struct Geometry_buffer
-{
-  void *arr[(geometry_buffer_size - 1)/sizeof(void *) + 1];
-};
+struct Geometry_buffer :
+  my_aligned_storage<sizeof(Gis_point), MY_ALIGNOF(Gis_point)> {};
 
 #endif /*HAVE_SPATAIAL*/
 #endif

=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc	2010-06-25 08:01:47 +0000
+++ b/sql/sql_show.cc	2010-07-05 20:51:07 +0000
@@ -2202,8 +2202,8 @@ static bool show_status_array(THD *thd,
                               bool ucase_names,
                               COND *cond)
 {
-  MY_ALIGNED_BYTE_ARRAY(buff_data, SHOW_VAR_FUNC_BUFF_SIZE, long);
-  char * const buff= (char *) &buff_data;
+  my_aligned_storage<SHOW_VAR_FUNC_BUFF_SIZE, MY_ALIGNOF(long)> buffer;
+  char * const buff= buffer.data;
   char *prefix_end;
   /* the variable name should not be longer than 64 characters */
   char name_buffer[64];


Attachment: [text/bzr-bundle] bzr/davi.arnaut@sun.com-20100705205107-3h214r8jix2mtdss.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (davi:3459) Bug#42733Davi Arnaut5 Jul
Re: bzr commit into mysql-5.1-bugteam branch (davi:3459) Bug#42733Alexey Botchkov13 Jul