Add EMPTY_BASES_DECL (#26980)
authorSteve MacLean <Steve.MacLean@microsoft.com>
Wed, 5 Feb 2020 18:04:40 +0000 (13:04 -0500)
committerGitHub <noreply@github.com>
Wed, 5 Feb 2020 18:04:40 +0000 (13:04 -0500)
* Add EMPTY_BASES_DECL

Add EMPTY_BASES_DECL to use MSVC __declspec(empty_bases).
This declaration tells MSVC to collapse empty base classes to
consume no space, as opposed to its default layout algoritihm
which requires base classes to consume at least one byte (in
multiple inheritance situations).

Mark a few classes which can contain empy base classes and may cause
layout differences between Windows and Clang

* Fix MapSHash deserialization

src/coreclr/src/inc/palclr.h
src/coreclr/src/inc/sarray.h
src/coreclr/src/inc/sbuffer.h
src/coreclr/src/inc/shash.h
src/coreclr/src/inc/sstring.h
src/coreclr/src/md/datasource/targettypes.cpp
src/coreclr/src/md/inc/VerifyLayouts.inc
src/coreclr/src/pal/inc/pal.h

index 25cd9e8..1fcd951 100644 (file)
 
 #define ANALYZER_NORETURN
 
+#ifdef _MSC_VER
+#define EMPTY_BASES_DECL __declspec(empty_bases)
+#else
+#define EMPTY_BASES_DECL
+#endif // !_MSC_VER
+
 //
 // CPP_ASSERT() can be used within a class definition, to perform a
 // compile-time assertion involving private names within the class.
index e27a8bb..c70d1a9 100644 (file)
@@ -123,8 +123,8 @@ class SArray
 
  public:
 
-    class Iterator : public CheckedIteratorBase<SArray<ELEMENT, BITWISE_COPY> >,
-                     public Indexer<ELEMENT, Iterator>
+    class EMPTY_BASES_DECL Iterator : public CheckedIteratorBase<SArray<ELEMENT, BITWISE_COPY> >,
+                                      public Indexer<ELEMENT, Iterator>
     {
         friend class SArray;
         friend class Indexer<ELEMENT, Iterator>;
@@ -193,7 +193,7 @@ class SArray
 // ================================================================================
 
 template <typename ELEMENT, COUNT_T SIZE, BOOL BITWISE_COPY = TRUE>
-class InlineSArray : public SArray<ELEMENT, BITWISE_COPY>
+class EMPTY_BASES_DECL InlineSArray : public SArray<ELEMENT, BITWISE_COPY>
 {
  private:
 #ifdef _MSC_VER
@@ -216,7 +216,7 @@ class InlineSArray : public SArray<ELEMENT, BITWISE_COPY>
 // ================================================================================
 
 template <typename ELEMENT, BOOL BITWISE_COPY = TRUE>
-class StackSArray : public InlineSArray<ELEMENT, STACK_ALLOC/sizeof(ELEMENT), BITWISE_COPY>
+class EMPTY_BASES_DECL StackSArray : public InlineSArray<ELEMENT, STACK_ALLOC/sizeof(ELEMENT), BITWISE_COPY>
 {
 };
 
index 9c30625..4ab7d75 100644 (file)
@@ -415,7 +415,7 @@ protected:
 
     friend class CheckedIteratorBase<SBuffer>;
 
-    class Index : public CheckedIteratorBase<SBuffer>
+    class EMPTY_BASES_DECL Index : public CheckedIteratorBase<SBuffer>
     {
         friend class SBuffer;
 
@@ -441,7 +441,7 @@ protected:
 
   public:
 
-    class CIterator : public Index, public Indexer<const BYTE, CIterator>
+    class EMPTY_BASES_DECL CIterator : public Index, public Indexer<const BYTE, CIterator>
     {
         friend class SBuffer;
 
@@ -456,7 +456,7 @@ protected:
         }
     };
 
-    class Iterator : public Index, public Indexer<BYTE, Iterator>
+    class EMPTY_BASES_DECL Iterator : public Index, public Indexer<BYTE, Iterator>
     {
         friend class SBuffer;
 
@@ -524,7 +524,7 @@ protected:
 #define BUFFER_ALIGNMENT 4
 
 template <COUNT_T size>
-class InlineSBuffer : public SBuffer
+class EMPTY_BASES_DECL InlineSBuffer : public SBuffer
 {
  private:
 #ifdef _MSC_VER
index 6452140..7ddc3e4 100644 (file)
@@ -138,8 +138,8 @@ class DefaultSHashTraits
 // Hash table class definition
 
 template <typename TRAITS>
-class SHash : public TRAITS
-            , private noncopyable
+class EMPTY_BASES_DECL SHash : public TRAITS
+                             , private noncopyable
 {
     friend class VerifyLayoutsMD;  // verifies class layout doesn't accidentally change
 
@@ -307,13 +307,8 @@ class SHash : public TRAITS
 
   public:
 
-    class Index
-#ifdef _DEBUG
-        // CheckedIteratorBase is a no-op in RET builds. having it as an empty base-class
-        // causes differences in the sizeof(SHash::Iterator) in DAC vs. non-DAC builds.
-        // avoid the issue by not specifying it as a base class in RET builds
+    class EMPTY_BASES_DECL Index
         : public CheckedIteratorBase< SHash<TRAITS> >
-#endif
     {
         friend class SHash;
         friend class Iterator;
@@ -382,7 +377,7 @@ class SHash : public TRAITS
         }
     };
 
-    class Iterator : public Index, public Enumerator<const element_t, Iterator>
+    class EMPTY_BASES_DECL Iterator : public Index, public Enumerator<const element_t, Iterator>
     {
         friend class SHash;
 
@@ -399,7 +394,7 @@ class SHash : public TRAITS
     // is artificially bumped to m_tableSize when the end of iteration is reached.
     // This allows a canonical End iterator to be used.
 
-    class KeyIndex : public Index
+    class EMPTY_BASES_DECL KeyIndex : public Index
     {
         friend class SHash;
         friend class KeyIterator;
@@ -464,7 +459,7 @@ class SHash : public TRAITS
         }
     };
 
-    class KeyIterator : public KeyIndex, public Enumerator<const element_t, KeyIterator>
+    class EMPTY_BASES_DECL KeyIterator : public KeyIndex, public Enumerator<const element_t, KeyIterator>
     {
         friend class SHash;
 
@@ -593,7 +588,7 @@ class SHash : public TRAITS
 
 // disables support for DAC marshaling. Useful for defining right-side only SHashes
 template <typename PARENT>
-class NonDacAwareSHashTraits : public PARENT
+class EMPTY_BASES_DECL NonDacAwareSHashTraits : public PARENT
 {
 public:
     typedef typename PARENT::element_t element_t;
@@ -603,7 +598,7 @@ public:
 // disables support for removing elements - produces slightly faster implementation
 
 template <typename PARENT>
-class NoRemoveSHashTraits : public PARENT
+class EMPTY_BASES_DECL NoRemoveSHashTraits : public PARENT
 {
 public:
     // explicitly declare local typedefs for these traits types, otherwise
@@ -620,7 +615,7 @@ public:
 // It relies on methods GetKey and Hash defined on ELEMENT
 
 template <typename ELEMENT, typename KEY>
-class PtrSHashTraits : public DefaultSHashTraits<ELEMENT *>
+class EMPTY_BASES_DECL PtrSHashTraits : public DefaultSHashTraits<ELEMENT *>
 {
   public:
 
@@ -650,7 +645,7 @@ class PtrSHashTraits : public DefaultSHashTraits<ELEMENT *>
 };
 
 template <typename ELEMENT, typename KEY>
-class PtrSHash : public SHash< PtrSHashTraits<ELEMENT, KEY> >
+class EMPTY_BASES_DECL PtrSHash : public SHash< PtrSHashTraits<ELEMENT, KEY> >
 {
 };
 
@@ -669,7 +664,7 @@ public:
 // a class that automatically deletes data referenced by the pointers (so effectively it takes ownership of the data)
 // since I was too lazy to implement Remove() APIs properly, removing entries is disallowed
 template <typename ELEMENT, typename KEY>
-class PtrSHashWithCleanup : public SHash< NoRemoveSHashTraits< PtrSHashWithCleanupTraits<ELEMENT, KEY> > >
+class EMPTY_BASES_DECL PtrSHashWithCleanup : public SHash< NoRemoveSHashTraits< PtrSHashWithCleanupTraits<ELEMENT, KEY> > >
 {
 };
 
@@ -772,7 +767,7 @@ public:
 // pointer hash tables.
 
 template <typename ElementT, typename CharT, typename ComparerT = CaseSensitiveStringCompareHash<CharT> >
-class StringSHashTraits : public PtrSHashTraits<ElementT, CharT const *>
+class EMPTY_BASES_DECL StringSHashTraits : public PtrSHashTraits<ElementT, CharT const *>
 {
 public:
     // explicitly declare local typedefs for these traits types, otherwise
@@ -816,7 +811,7 @@ struct StringHashElement
 };
 
 template <typename COMINTERFACE, typename CharT, typename ComparerT = CaseSensitiveStringCompareHash<CharT> >
-class StringHashWithCleanupTraits : public StringSHashTraits<StringHashElement<COMINTERFACE, CharT>, CharT, ComparerT>
+class EMPTY_BASES_DECL StringHashWithCleanupTraits : public StringSHashTraits<StringHashElement<COMINTERFACE, CharT>, CharT, ComparerT>
 {
 public:
     void OnDestructPerEntryCleanupAction(StringHashElement<COMINTERFACE, CharT> * e)
@@ -835,22 +830,22 @@ public:
 };
 
 template <typename COMINTERFACE, typename CharT, typename ComparerT = CaseSensitiveStringCompareHash<CharT> >
-class StringSHashWithCleanup : public SHash< StringHashWithCleanupTraits<COMINTERFACE, CharT, ComparerT> >
+class EMPTY_BASES_DECL StringSHashWithCleanup : public SHash< StringHashWithCleanupTraits<COMINTERFACE, CharT, ComparerT> >
 {
 };
 
 template <typename ELEMENT>
-class StringSHash : public SHash< StringSHashTraits<ELEMENT, CHAR> >
+class EMPTY_BASES_DECL StringSHash : public SHash< StringSHashTraits<ELEMENT, CHAR> >
 {
 };
 
 template <typename ELEMENT>
-class WStringSHash : public SHash< StringSHashTraits<ELEMENT, WCHAR> >
+class EMPTY_BASES_DECL WStringSHash : public SHash< StringSHashTraits<ELEMENT, WCHAR> >
 {
 };
 
 template <typename ELEMENT>
-class SStringSHashTraits : public PtrSHashTraits<ELEMENT, SString>
+class EMPTY_BASES_DECL SStringSHashTraits : public PtrSHashTraits<ELEMENT, SString>
 {
   public:
     typedef PtrSHashTraits<ELEMENT, SString> PARENT;
@@ -873,12 +868,12 @@ class SStringSHashTraits : public PtrSHashTraits<ELEMENT, SString>
 };
 
 template <typename ELEMENT>
-class SStringSHash : public SHash< SStringSHashTraits<ELEMENT> >
+class EMPTY_BASES_DECL SStringSHash : public SHash< SStringSHashTraits<ELEMENT> >
 {
 };
 
 template <typename ELEMENT>
-class SetSHashTraits : public DefaultSHashTraits<ELEMENT>
+class EMPTY_BASES_DECL SetSHashTraits : public DefaultSHashTraits<ELEMENT>
 {
 public:
     // explicitly declare local typedefs for these traits types, otherwise
@@ -906,7 +901,7 @@ public:
 };
 
 template <typename ELEMENT, typename TRAITS = NoRemoveSHashTraits< SetSHashTraits <ELEMENT> > >
-class SetSHash : public SHash< TRAITS >
+class EMPTY_BASES_DECL SetSHash : public SHash< TRAITS >
 {
     typedef SHash<TRAITS> PARENT;
 
@@ -918,7 +913,7 @@ public:
 };
 
 template <typename ELEMENT>
-class PtrSetSHashTraits : public SetSHashTraits<ELEMENT>
+class EMPTY_BASES_DECL PtrSetSHashTraits : public SetSHashTraits<ELEMENT>
 {
   public:
 
@@ -937,7 +932,7 @@ class PtrSetSHashTraits : public SetSHashTraits<ELEMENT>
 };
 
 template <typename PARENT_TRAITS>
-class DeleteElementsOnDestructSHashTraits : public PARENT_TRAITS
+class EMPTY_BASES_DECL DeleteElementsOnDestructSHashTraits : public PARENT_TRAITS
 {
 public:
     static inline void OnDestructPerEntryCleanupAction(typename PARENT_TRAITS::element_t e)
@@ -978,7 +973,7 @@ public:
 };
 
 template <typename KEY, typename VALUE>
-class MapSHashTraits : public DefaultSHashTraits< KeyValuePair<KEY,VALUE> >
+class EMPTY_BASES_DECL MapSHashTraits : public DefaultSHashTraits< KeyValuePair<KEY,VALUE> >
 {
 public:
     // explicitly declare local typedefs for these traits types, otherwise
@@ -1011,7 +1006,7 @@ public:
 };
 
 template <typename KEY, typename VALUE, typename TRAITS = NoRemoveSHashTraits< MapSHashTraits <KEY, VALUE> > >
-class MapSHash : public SHash< TRAITS >
+class EMPTY_BASES_DECL MapSHash : public SHash< TRAITS >
 {
     typedef SHash< TRAITS > PARENT;
 
@@ -1049,7 +1044,7 @@ public:
 };
 
 template <typename KEY, typename VALUE>
-class MapSHashWithRemove : public SHash< MapSHashTraits <KEY, VALUE> >
+class EMPTY_BASES_DECL MapSHashWithRemove : public SHash< MapSHashTraits <KEY, VALUE> >
 {
     typedef SHash< MapSHashTraits <KEY, VALUE> > PARENT;
 
index 0413813..a045917 100644 (file)
@@ -70,7 +70,7 @@ typedef const UTF8 *LPCUTF8;
 
 
 typedef DPTR(class SString) PTR_SString;
-class SString : private SBuffer
+class EMPTY_BASES_DECL SString : private SBuffer
 {
     friend struct _DacGlobals;
 
@@ -335,7 +335,7 @@ private:
 
  protected:
 
-    class UIndex : public SBuffer::Index
+    class EMPTY_BASES_DECL UIndex : public SBuffer::Index
     {
         friend class SString;
         friend class Indexer<WCHAR, UIterator>;
@@ -354,7 +354,7 @@ private:
 
  public:
 
-    class UIterator : public UIndex, public Indexer<WCHAR, UIterator>
+    class EMPTY_BASES_DECL UIterator : public UIndex, public Indexer<WCHAR, UIterator>
     {
         friend class SString;
 
@@ -389,7 +389,7 @@ private:
 
  protected:
 
-    class Index : public SBuffer::Index
+    class EMPTY_BASES_DECL Index : public SBuffer::Index
     {
         friend class SString;
 
@@ -421,7 +421,7 @@ private:
 
  public:
 
-    class CIterator : public Index, public Indexer<const WCHAR, CIterator>
+    class EMPTY_BASES_DECL CIterator : public Index, public Indexer<const WCHAR, CIterator>
     {
         friend class SString;
 
@@ -461,7 +461,7 @@ private:
         WCHAR operator[](int index) const { return Index::operator[](index); }
     };
 
-    class Iterator : public Index, public Indexer<WCHAR, Iterator>
+    class EMPTY_BASES_DECL Iterator : public Index, public Indexer<WCHAR, Iterator>
     {
         friend class SString;
 
@@ -805,7 +805,7 @@ private:
 // ===========================================================================
 
 template <COUNT_T MEMSIZE>
-class InlineSString : public SString
+class EMPTY_BASES_DECL InlineSString : public SString
 {
 private:
     BYTE m_inline[SBUFFER_PADDED_SIZE(MEMSIZE)];
@@ -978,7 +978,7 @@ typedef InlineSString<2 * 260> LongPathString;
 // ScratchBuffer classes are used by the GetXXX() routines to allocate scratch space in.
 // ================================================================================
 
-class SString::AbstractScratchBuffer : private SString
+class EMPTY_BASES_DECL SString::AbstractScratchBuffer : private SString
 {
   protected:
     // Do not use this class directly - use
@@ -987,7 +987,7 @@ class SString::AbstractScratchBuffer : private SString
 };
 
 template <COUNT_T MEMSIZE>
-class ScratchBuffer : public SString::AbstractScratchBuffer
+class EMPTY_BASES_DECL ScratchBuffer : public SString::AbstractScratchBuffer
 {
   private:
     BYTE m_inline[MEMSIZE];
index 32623b6..7ce94fd 100644 (file)
@@ -134,12 +134,6 @@ m_tableMax(0)
 HRESULT Target_MapSHash::ReadFrom(DataTargetReader & reader)
 {
     HRESULT hr = S_OK;
-    // Only the Windows MSVC compiler does this; not clang on Linux
-#ifdef _MSC_VER
-    IfFailRet(reader.Skip8()); // this byte gets used by the base class even though it has no members
-                               // I'm guessing this is so the 2nd base class (noncopyable) doesn't start at the same
-                               // location, but I can't be sure. Whatever the cause, the layout skips a byte.
-#endif // _MSC_VER
     IfFailRet(reader.ReadPointer(&m_table));
     IfFailRet(reader.Read32(&m_tableSize));
     IfFailRet(reader.Read32(&m_tableCount));
index 2fed0fb..36a2146 100644 (file)
@@ -283,7 +283,7 @@ BEGIN_TYPE(StgPoolReadOnly, sizeof(StgPoolSeg) + sizeof(void*))
 ALIGN_FIELD(StgPoolReadOnly, m_HotHeap, sizeof(MetaData::HotHeap), sizeof(void*))
 END_TYPE(StgPoolReadOnly, sizeof(void*))
 
-BEGIN_TYPE_ESCAPED(IGNORE_COMMAS(MapSHash<ULONG, ULONG>), MapSHash__ULONG__ULONG, sizeof(void*))
+BEGIN_TYPE_ESCAPED(IGNORE_COMMAS(MapSHash<ULONG, ULONG>), MapSHash__ULONG__ULONG, 0)
 FIELD_ESCAPED(IGNORE_COMMAS(MapSHash<ULONG, ULONG>), MapSHash__ULONG__ULONG, m_table, sizeof(void*))
 FIELD_ESCAPED(IGNORE_COMMAS(MapSHash<ULONG, ULONG>), MapSHash__ULONG__ULONG, m_tableSize, 4)
 FIELD_ESCAPED(IGNORE_COMMAS(MapSHash<ULONG, ULONG>), MapSHash__ULONG__ULONG, m_tableCount, 4)
index 86cbc41..7546046 100644 (file)
@@ -156,6 +156,9 @@ typedef PVOID NATIVE_LIBRARY_HANDLE;
 #define ANALYZER_NORETURN
 #endif
 
+#define EMPTY_BASES_DECL
+
+
 #if !defined(_MSC_VER) || defined(SOURCE_FORMATTING)
 #define __assume(x) (void)0
 #define __annotation(x)