Respect alignment of by-value storage.

include/boost/python/converter/obj_mgr_arg_from_python.hpp

@@ -81,9 +81,9 @@ inline object_manager_ref_arg_from_python<Ref>::object_manager_ref_arg_from_pyth
 # if defined(__EDG_VERSION__) && __EDG_VERSION__ <= 243
     // needed for warning suppression
     python::detail::borrowed_reference x_ = python::detail::borrowed_reference(x);
-    python::detail::construct_referent<Ref>(&m_result.bytes, x_);
+    python::detail::construct_referent<Ref>(m_result.bytes, x_);
 # else 
-    python::detail::construct_referent<Ref>(&m_result.bytes, (python::detail::borrowed_reference)x);
+    python::detail::construct_referent<Ref>(m_result.bytes, (python::detail::borrowed_reference)x);
 # endif 
 }
 

include/boost/python/converter/rvalue_from_python_data.hpp

@@ -9,6 +9,7 @@
 # include <boost/python/detail/referent_storage.hpp>
 # include <boost/python/detail/destroy.hpp>
 # include <boost/python/detail/type_traits.hpp>
+# include <boost/align/align.hpp>
 # include <boost/static_assert.hpp>
 # include <cstddef>
 
@@ -132,7 +133,13 @@ template <class T>
 inline rvalue_from_python_data<T>::~rvalue_from_python_data()
 {
     if (this->stage1.convertible == this->storage.bytes)
-        python::detail::destroy_referent<ref_type>(this->storage.bytes);
+    {
+        size_t allocated = sizeof(this->storage);
+        void *ptr = this->storage.bytes;
+        void *aligned_storage =
+            ::boost::alignment::align(boost::python::detail::alignment_of<T>::value, 0, ptr, allocated);
+        python::detail::destroy_referent<ref_type>(aligned_storage);
+    }
 }
 
 }}} // namespace boost::python::converter

include/boost/python/converter/shared_ptr_from_python.hpp

@@ -49,13 +49,17 @@ struct shared_ptr_from_python
       new (storage) SP<T>();
     else
     {
-      SP<void> hold_convertible_ref_count(
-	 (void*)0, shared_ptr_deleter(handle<>(borrowed(source))) );
-      // use aliasing constructor
-      new (storage) SP<T>(hold_convertible_ref_count,
-			  static_cast<T*>(data->convertible));
+      void *const storage = ((converter::rvalue_from_python_storage<SP<T> >*)data)->storage.bytes;
+      // Deal with the "None" case.
+      if (data->convertible == source)
+        new (storage) SP<T>();
+      else
+      {
+        SP<void> hold_convertible_ref_count((void*)0, shared_ptr_deleter(handle<>(borrowed(source))) );
+        // use aliasing constructor
+        new (storage) SP<T>(hold_convertible_ref_count, static_cast<T*>(data->convertible));
+      }
     }
-
     data->convertible = storage;
   }
 };

include/boost/python/detail/referent_storage.hpp

@@ -5,39 +5,21 @@
 #ifndef REFERENT_STORAGE_DWA200278_HPP
 # define REFERENT_STORAGE_DWA200278_HPP
 # include <boost/mpl/if.hpp>
+# include <boost/type_traits/aligned_storage.hpp>
 # include <cstddef>
 
 namespace boost { namespace python { namespace detail {
 
-struct alignment_dummy;
-typedef void (*function_ptr)();
-typedef int (alignment_dummy::*member_ptr);
-typedef int (alignment_dummy::*member_function_ptr)();
-
-# define BOOST_PYTHON_ALIGNER(T, n)                     \
-        typename mpl::if_c<                             \
-           sizeof(T) <= size, T, char>::type t##n
-
-// Storage for size bytes, aligned to all fundamental types no larger than size
-template <std::size_t size>
-union aligned_storage
+template <std::size_t size, std::size_t alignment = std::size_t(-1)>
+struct aligned_storage
 {
-    BOOST_PYTHON_ALIGNER(char, 0);
-    BOOST_PYTHON_ALIGNER(short, 1);
-    BOOST_PYTHON_ALIGNER(int, 2);
-    BOOST_PYTHON_ALIGNER(long, 3);
-    BOOST_PYTHON_ALIGNER(float, 4);
-    BOOST_PYTHON_ALIGNER(double, 5);
-    BOOST_PYTHON_ALIGNER(long double, 6);
-    BOOST_PYTHON_ALIGNER(void*, 7);
-    BOOST_PYTHON_ALIGNER(function_ptr, 8);
-    BOOST_PYTHON_ALIGNER(member_ptr, 9);
-    BOOST_PYTHON_ALIGNER(member_function_ptr, 10);
+  union type
+  {
+    typename ::boost::aligned_storage<size, alignment>::type data;
     char bytes[size];
+  };
 };
-
-# undef BOOST_PYTHON_ALIGNER
-
+      
   // Compute the size of T's referent. We wouldn't need this at all,
   // but sizeof() is broken in CodeWarriors <= 8.0
   template <class T> struct referent_size;
@@ -50,15 +32,12 @@ union aligned_storage
           std::size_t, value = sizeof(T));
   };
 
-
 // A metafunction returning a POD type which can store U, where T ==
 // U&. If T is not a reference type, returns a POD which can store T.
 template <class T>
 struct referent_storage
 {
-    typedef aligned_storage<
-        ::boost::python::detail::referent_size<T>::value
-    > type;
+    typedef typename aligned_storage<referent_size<T>::value, alignment_of<T>::value>::type type;
 };
 
 }}} // namespace boost::python::detail

include/boost/python/instance_holder.hpp

@@ -38,7 +38,7 @@ struct BOOST_PYTHON_DECL instance_holder : private noncopyable
     // Allocate storage for an object of the given size at the given
     // offset in the Python instance<> object if bytes are available
     // there. Otherwise allocate size bytes of heap memory.
-    static void* allocate(PyObject*, std::size_t offset, std::size_t size);
+    static void* allocate(PyObject*, std::size_t offset, std::size_t size, std::size_t alignment = 1);
 
     // Deallocate storage from the heap if it was not carved out of
     // the given Python object by allocate(), above.

include/boost/python/object/instance.hpp

@@ -6,7 +6,7 @@
 # define INSTANCE_DWA200295_HPP
 
 # include <boost/python/detail/prefix.hpp>
-
+# include <boost/python/detail/type_traits.hpp>
 # include <cstddef>
 
 namespace boost { namespace python
@@ -28,7 +28,7 @@ struct instance
     typedef typename boost::python::detail::type_with_alignment<
         boost::python::detail::alignment_of<Data>::value
     >::type align_t;
-          
+
     union
     {
         align_t align;
@@ -41,9 +41,10 @@ struct additional_instance_size
 {
     typedef instance<Data> instance_data;
     typedef instance<char> instance_char;
-    BOOST_STATIC_CONSTANT(
-        std::size_t, value = sizeof(instance_data)
-                           - BOOST_PYTHON_OFFSETOF(instance_char,storage));
+    BOOST_STATIC_CONSTANT(std::size_t,
+                          value = sizeof(instance_data) -
+                             BOOST_PYTHON_OFFSETOF(instance_char,storage) +
+                             boost::python::detail::alignment_of<Data>::value);
 };
 
 }}} // namespace boost::python::object

include/boost/python/object/make_holder.hpp

@@ -89,8 +89,9 @@ struct make_holder<N>
             BOOST_PP_ENUM_TRAILING_BINARY_PARAMS_Z(1, N, t, a))
         {
             typedef instance<Holder> instance_t;
-            
-            void* memory = Holder::allocate(p, offsetof(instance_t, storage), sizeof(Holder));
+
+            void* memory = Holder::allocate(p, offsetof(instance_t, storage), sizeof(Holder),
+                                            boost::python::detail::alignment_of<Holder>::value);
             try {
                 (new (memory) Holder(
                     p BOOST_PP_REPEAT_1ST(N, BOOST_PYTHON_DO_FORWARD_ARG, nil)))->install(p);

include/boost/python/object/make_instance.hpp

@@ -43,11 +43,14 @@ struct make_instance_impl
             
             // construct the new C++ object and install the pointer
             // in the Python object.
-            Derived::construct(&instance->storage, (PyObject*)instance, x)->install(raw_result);
+            Holder *holder =Derived::construct(instance->storage.bytes, (PyObject*)instance, x);
+            holder->install(raw_result);
               
             // Note the position of the internally-stored Holder,
             // for the sake of destruction
-            Py_SET_SIZE(instance, offsetof(instance_t, storage));
+            const size_t offset = reinterpret_cast<size_t>(holder) -
+              reinterpret_cast<size_t>(instance->storage.bytes) + offsetof(instance_t, storage);
+            Py_SET_SIZE(instance, offset);
 
             // Release ownership of the python object
             protect.cancel();
@@ -69,7 +72,10 @@ struct make_instance
     
     static inline Holder* construct(void* storage, PyObject* instance, reference_wrapper<T const> x)
     {
-        return new (storage) Holder(instance, x);
+        size_t allocated = objects::additional_instance_size<Holder>::value;
+        void* aligned_storage = ::boost::alignment::align(boost::python::detail::alignment_of<Holder>::value,
+                                                          sizeof(Holder), storage, allocated);
+        return new (aligned_storage) Holder(instance, x);
     }
 };
   

src/object/class.cpp

@@ -5,6 +5,7 @@
 
 #include <boost/python/detail/prefix.hpp>
 #include <boost/mpl/lambda.hpp> // #including this first is an intel6 workaround
+#include <boost/cstdint.hpp>
 
 #include <boost/python/object/class.hpp>
 #include <boost/python/object/instance.hpp>
@@ -721,28 +722,47 @@ namespace objects
 } // namespace objects
 
 
-void* instance_holder::allocate(PyObject* self_, std::size_t holder_offset, std::size_t holder_size)
+typedef unsigned int alignment_marker_t;
+
+void* instance_holder::allocate(PyObject* self_, std::size_t holder_offset, std::size_t holder_size, std::size_t alignment)
 {
     assert(PyType_IsSubtype(Py_TYPE(Py_TYPE(self_)), &class_metatype_object));
     objects::instance<>* self = (objects::instance<>*)self_;
     
-    int total_size_needed = holder_offset + holder_size;
+    int total_size_needed = holder_offset + holder_size + alignment - 1;
     
     if (-Py_SIZE(self) >= total_size_needed)
     {
         // holder_offset should at least point into the variable-sized part
         assert(holder_offset >= offsetof(objects::instance<>,storage));
 
+        size_t allocated = holder_size + alignment;
+        void* storage = (char*)self + holder_offset;
+        void* aligned_storage = ::boost::alignment::align(alignment, holder_size, storage, allocated);
+
         // Record the fact that the storage is occupied, noting where it starts
-        Py_SET_SIZE(self, holder_offset);
-        return (char*)self + holder_offset;
+        const size_t offset = reinterpret_cast<uintptr_t>(aligned_storage) - reinterpret_cast<uintptr_t>(storage) + holder_offset;
+        Py_SET_SIZE(self, offset);
+        return (char*)self + offset;
     }
     else
     {
-        void* const result = PyMem_Malloc(holder_size);
-        if (result == 0)
+        const size_t base_allocation = sizeof(alignment_marker_t) + holder_size + alignment - 1;
+        void* const base_storage = PyMem_Malloc(base_allocation);
+        if (base_storage == 0)
             throw std::bad_alloc();
-        return result;
+
+        const uintptr_t x = reinterpret_cast<uintptr_t>(base_storage) + sizeof(alignment_marker_t);
+        //this has problems for x -> max(void *)
+        //const size_t padding = alignment - ((x + sizeof(alignment_marker_t)) % alignment);
+        //only works for alignments with alignments of powers of 2, but no edge conditions
+        const uintptr_t padding = alignment == 1 ? 0 : ( alignment - (x & (alignment - 1)) );
+        const size_t aligned_offset = sizeof(alignment_marker_t) + padding;
+        void* const aligned_storage = (char *)base_storage + aligned_offset;
+        BOOST_ASSERT((char *) aligned_storage + holder_size <= (char *)base_storage + base_allocation);
+        alignment_marker_t* const marker_storage = reinterpret_cast<alignment_marker_t *>((char *)aligned_storage - sizeof(alignment_marker_t));
+        *marker_storage = static_cast<alignment_marker_t>(padding);
+        return aligned_storage;
     }
 }
 
@@ -752,7 +772,9 @@ void instance_holder::deallocate(PyObject* self_, void* storage) throw()
     objects::instance<>* self = (objects::instance<>*)self_;
     if (storage != (char*)self + Py_SIZE(self))
     {
-        PyMem_Free(storage);
+        alignment_marker_t* marker_storage = reinterpret_cast<alignment_marker_t *>((char *)storage - sizeof(alignment_marker_t));
+        void *malloced_storage = (char *) storage - sizeof(alignment_marker_t) - (*marker_storage);
+        PyMem_Free(malloced_storage);
     }
 }
 

test/Jamfile

@@ -132,6 +132,7 @@ bpl-test crossmod_exception
 
 [ bpl-test object ]
 [ bpl-test class ]
+[ bpl-test aligned_class ]
 [ bpl-test list ]
 [ bpl-test long ]
 [ bpl-test dict ]

test/aligned_class.cpp

@@ -0,0 +1,33 @@
+// Distributed under the Boost Software License, Version 1.0. (See
+// accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt)
+#include <boost/python/module.hpp>
+#include <boost/python/def.hpp>
+#include <boost/python/object.hpp>
+#include <boost/python/class.hpp>
+#include <boost/assert.hpp>
+#include <boost/cstdint.hpp>
+
+using namespace boost::python;
+
+struct BOOST_ALIGNMENT(32) X
+{
+    int x;
+    BOOST_ALIGNMENT(32) float f;
+    X(int n, float _f) : x(n), f(_f)
+    {
+        BOOST_ASSERT((reinterpret_cast<uintptr_t>(&f) % 32) == 0);
+    }
+};
+
+int x_function(X& x) { return x.x;}
+float f_function(X& x) { return x.f;}
+
+BOOST_PYTHON_MODULE(aligned_class_ext)
+{
+    class_<X>("X", init<int, float>());
+    def("x_function", x_function);
+    def("f_function", f_function);
+}
+
+#include "module_tail.cpp"

test/aligned_class.py

@@ -0,0 +1,44 @@
+# Distributed under the Boost
+# Software License, Version 1.0. (See accompanying
+# file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+'''
+>>> from aligned_class_ext import *
+
+Ensure sanity:
+
+    >>> x = X(42, 16)
+    >>> x_function(x)
+    42
+    >>> f_function(x)
+    16.0
+
+Demonstrate extraction in the presence of metaclass changes:
+
+    >>> class MetaX(X.__class__):
+    ...     def __new__(cls, *args):
+    ...         return super(MetaX, cls).__new__(cls, *args)
+    >>> class XPlusMetatype(X):
+    ...     __metaclass__ = MetaX
+    >>> x = XPlusMetatype(42, 16)
+    >>> x_function(x)
+    42
+    >>> f_function(x)
+    16.0
+
+
+'''
+
+def run(args = None):
+    import sys
+    import doctest
+
+    if args is not None:
+        sys.argv = args
+    return doctest.testmod(sys.modules.get(__name__))
+
+if __name__ == '__main__':
+    print("running...")
+    import sys
+    status = run()[0]
+    if (status == 0): print("Done.")
+    sys.exit(status)

test/fabscript

@@ -79,6 +79,8 @@ for t in [('injected',),
           ('callbacks',),
           ('defaults',),
           ('object',),
+          ('class',),
+          ('aligned_class',),
           ('list',),
           ('long',),
           ('dict',),