Add ALIGNAS and ALIGNOF macros to ensure proper alignment of StaticMemorySingletonTraits

base/memory/singleton.h

Index: base/memory/singleton.h
diff --git a/base/memory/singleton.h b/base/memory/singleton.h
index d76070019435323869c9b2d13e6baa2f982e271e..bd54b6a14e5a5697d7dfdea2f4bd803904d50ef3 100644
--- a/base/memory/singleton.h
+++ b/base/memory/singleton.h
@@ -23,6 +23,7 @@
 #include "base/at_exit.h"
 #include "base/atomicops.h"
 #include "base/base_export.h"
+#include "base/memory/aligned_memory.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/thread_restrictions.h"
 
@@ -106,18 +107,14 @@ struct StaticMemorySingletonTraits {
   // WARNING: User has to deal with get() in the singleton class
   // this is traits for returning NULL.
   static Type* New() {
+    // Only constructs once and returns pointer; otherwise returns NULL.
     if (base::subtle::NoBarrier_AtomicExchange(&dead_, 1))

[Sigurður Ásgeirsson, 2012/02/22 15:31:32]

I couldn't find a discussion of this anywhere in our headers, but I assume that
NoBarrier means that writes on this core prior to the atomic op are allowed to
be reordered past here?
In any case the implementation looks correct to me, the acquire/release dance in
the singleton proper should be sufficient for static buffers.

[Jeffrey Yasskin (google), 2012/02/22 18:38:42]

Both writes and reads can be reordered in either direction. On x86, only the
compiler will actually do such reordering, but on ARM and PPC (dunno about MIPS)
the processor will do it too.
I think this AtomicExchange is enough for the guarantee
StaticMemorySingletonTraits wants to provide, and that the ordering in Singleton
is enough for its guarantees.

       return NULL;
-    Type* ptr = reinterpret_cast<Type*>(buffer_);
 
-    // We are protected by a memory barrier.
-    new(ptr) Type();
-    return ptr;
+    return new(buffer_.data()) Type;

[Jeffrey Yasskin (google), 2012/02/22 18:38:42]

Since you removed the ()s after "Type", this won't zero-initialize types if they
don't have constructors. You may not actually have any singleton
constructor-less types, but I'd leave the ()s in for safety.

[jbates, 2012/02/22 19:37:15]

Done.

   }
 
   static void Delete(Type* p) {
-    base::subtle::NoBarrier_Store(&dead_, 1);

[jbates, 2012/02/21 23:39:04]

dead_ is already set to 1 at the beginning of New().

-    base::subtle::MemoryBarrier();
     if (p != NULL)
       p->Type::~Type();
   }
@@ -131,16 +128,13 @@ struct StaticMemorySingletonTraits {
   }
 
  private:
-  static const size_t kBufferSize = (sizeof(Type) +
-                                     sizeof(intptr_t) - 1) / sizeof(intptr_t);
-  static intptr_t buffer_[kBufferSize];
-
+  static base::AlignedMemory<Type> buffer_;
   // Signal the object was already deleted, so it is not revived.
   static base::subtle::Atomic32 dead_;
 };
 
-template <typename Type> intptr_t
-    StaticMemorySingletonTraits<Type>::buffer_[kBufferSize];
+template <typename Type> base::AlignedMemory<Type>
+    StaticMemorySingletonTraits<Type>::buffer_;
 template <typename Type> base::subtle::Atomic32
     StaticMemorySingletonTraits<Type>::dead_ = 0;