Add ALIGNAS and ALIGNOF macros to ensure proper alignment of StaticMemorySingletonTraits

base/memory/singleton.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // PLEASE READ: Do you really need a singleton?
 //
 // Singletons make it hard to determine the lifetime of an object, which can
 // lead to buggy code and spurious crashes.
 //
 // Instead of adding another singleton into the mix, try to identify either:
@@ skipping 90 matching lines @@
 // possibility of loading briefly into someone else's process space, and
 // so leaking is not an option, as that would sabotage the state of your host
 // process once you've unloaded.
 template <typename Type>
 struct StaticMemorySingletonTraits {
   // WARNING: User has to deal with get() in the singleton class
   // this is traits for returning NULL.
   static Type* New() {
     if (base::subtle::NoBarrier_AtomicExchange(&dead_, 1))
       return NULL;
-    Type* ptr = reinterpret_cast<Type*>(buffer_);
+
+    // Calculate aligned address for object pointer. This rounds up to the
+    // nearest address whose alignment is ALIGNOF(Type).
+    intptr_t aligned_addr =
+        (reinterpret_cast<intptr_t>(buffer_) + ALIGNOF(Type) - 1) &
+        ~(ALIGNOF(Type) - 1);
 
     // We are protected by a memory barrier.

[Jeffrey Yasskin (google), 2012/02/15 22:27:19]

Unrelated to the current CL, but, really? I see a pretty explicit "NoBarrier" up
there, which can bite you on non-x86 platforms.

[jbates, 2012/02/15 22:51:33]

Hmm. I just assumed this code was already thread safe, but looking at it again,
it appears that the atomic code here doesn't protect from a race between New()
and Delete(). Perhaps New() and Delete() are always called sequentially on the
same thread. Is that the case?

-    new(ptr) Type();
-    return ptr;
+    return new(reinterpret_cast<void*>(aligned_addr)) Type();
   }
 
   static void Delete(Type* p) {
     base::subtle::NoBarrier_Store(&dead_, 1);
     base::subtle::MemoryBarrier();
     if (p != NULL)
       p->Type::~Type();
   }
 
   static const bool kRegisterAtExit = true;
   static const bool kAllowedToAccessOnNonjoinableThread = true;
 
   // Exposed for unittesting.
   static void Resurrect() {
     base::subtle::NoBarrier_Store(&dead_, 0);
   }
 
  private:
-  static const size_t kBufferSize = (sizeof(Type) +
-                                     sizeof(intptr_t) - 1) / sizeof(intptr_t);
-  static intptr_t buffer_[kBufferSize];
+  static const size_t kBufferSize = sizeof(Type) + ALIGNOF(Type) - 1;

[Sigurður Ásgeirsson, 2012/01/13 14:32:18]

This is pretty wasteful, as many existing instances will effectively double in
size. This will increase the binary size, increase cache misses etc.
If there isn't a good way to make the the alignment declaration stick to the
buffer declaration, I'd suggest using a new traits class, and using that for
instances that need non-default alignment. Instead of penalizing everyone, this
will only affect those who need non-default alignment.

[jbates, 2012/01/13 17:29:18]

How so? ALIGNOF will typically be 4 or 8 for most singletons.
Sadly ALIGNED(ALIGNOF(x)) only works in GCC. Really lame that MSVC complains
about that.
If we end up going that route, I think we should bump the alignment of this
class to 8 or 16. As is, if someone has uint64 types in their static singleton,
it may not be guaranteed to be aligned in the 32-bit build (it would depend on
the linker's default alignment for global data types, which is where we may be
getting lucky). A worse case would be vector types, which require 16 byte
alignment for the fast instructions.

[Jeffrey Yasskin (google), 2012/02/15 22:27:19]

Does MSVC accept ALIGNED(size_t-template-argument)? Does it allow passing
ALIGNOF(x) as a template argument? If so, you may be able to sneak it in that
way.

[jbates, 2012/02/15 22:51:33]

Yeah, hopefully that will work. I'll give it a try soon.

+  static uint8 buffer_[kBufferSize];
 
   // Signal the object was already deleted, so it is not revived.
   static base::subtle::Atomic32 dead_;
 };
 
-template <typename Type> intptr_t
+template <typename Type> uint8
     StaticMemorySingletonTraits<Type>::buffer_[kBufferSize];
 template <typename Type> base::subtle::Atomic32
     StaticMemorySingletonTraits<Type>::dead_ = 0;
 
 // The Singleton<Type, Traits, DifferentiatingType> class manages a single
 // instance of Type which will be created on first use and will be destroyed at
 // normal process exit). The Trait::Delete function will not be called on
 // abnormal process exit.
 //
 // DifferentiatingType is used as a key to differentiate two different
@@ skipping 129 matching lines @@
     instance_ = 0;
   }
   static base::subtle::AtomicWord instance_;
 };
 
 template <typename Type, typename Traits, typename DifferentiatingType>
 base::subtle::AtomicWord Singleton<Type, Traits, DifferentiatingType>::
     instance_ = 0;
 
 #endif  // BASE_MEMORY_SINGLETON_H_