From db5277dee1e0a390f8dac3b6c811fa9289f54099 Mon Sep 17 00:00:00 2001 From: Steve MacLean Date: Wed, 5 Feb 2020 13:04:40 -0500 Subject: [PATCH] Add EMPTY_BASES_DECL (#26980) * 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 | 6 +++ src/coreclr/src/inc/sarray.h | 8 ++-- src/coreclr/src/inc/sbuffer.h | 8 ++-- src/coreclr/src/inc/shash.h | 55 ++++++++++++--------------- src/coreclr/src/inc/sstring.h | 18 ++++----- src/coreclr/src/md/datasource/targettypes.cpp | 6 --- src/coreclr/src/md/inc/VerifyLayouts.inc | 2 +- src/coreclr/src/pal/inc/pal.h | 3 ++ 8 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/coreclr/src/inc/palclr.h b/src/coreclr/src/inc/palclr.h index 25cd9e8..1fcd951 100644 --- a/src/coreclr/src/inc/palclr.h +++ b/src/coreclr/src/inc/palclr.h @@ -55,6 +55,12 @@ #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. diff --git a/src/coreclr/src/inc/sarray.h b/src/coreclr/src/inc/sarray.h index e27a8bb..c70d1a9 100644 --- a/src/coreclr/src/inc/sarray.h +++ b/src/coreclr/src/inc/sarray.h @@ -123,8 +123,8 @@ class SArray public: - class Iterator : public CheckedIteratorBase >, - public Indexer + class EMPTY_BASES_DECL Iterator : public CheckedIteratorBase >, + public Indexer { friend class SArray; friend class Indexer; @@ -193,7 +193,7 @@ class SArray // ================================================================================ template -class InlineSArray : public SArray +class EMPTY_BASES_DECL InlineSArray : public SArray { private: #ifdef _MSC_VER @@ -216,7 +216,7 @@ class InlineSArray : public SArray // ================================================================================ template -class StackSArray : public InlineSArray +class EMPTY_BASES_DECL StackSArray : public InlineSArray { }; diff --git a/src/coreclr/src/inc/sbuffer.h b/src/coreclr/src/inc/sbuffer.h index 9c30625..4ab7d75 100644 --- a/src/coreclr/src/inc/sbuffer.h +++ b/src/coreclr/src/inc/sbuffer.h @@ -415,7 +415,7 @@ protected: friend class CheckedIteratorBase; - class Index : public CheckedIteratorBase + class EMPTY_BASES_DECL Index : public CheckedIteratorBase { friend class SBuffer; @@ -441,7 +441,7 @@ protected: public: - class CIterator : public Index, public Indexer + class EMPTY_BASES_DECL CIterator : public Index, public Indexer { friend class SBuffer; @@ -456,7 +456,7 @@ protected: } }; - class Iterator : public Index, public Indexer + class EMPTY_BASES_DECL Iterator : public Index, public Indexer { friend class SBuffer; @@ -524,7 +524,7 @@ protected: #define BUFFER_ALIGNMENT 4 template -class InlineSBuffer : public SBuffer +class EMPTY_BASES_DECL InlineSBuffer : public SBuffer { private: #ifdef _MSC_VER diff --git a/src/coreclr/src/inc/shash.h b/src/coreclr/src/inc/shash.h index 6452140..7ddc3e4 100644 --- a/src/coreclr/src/inc/shash.h +++ b/src/coreclr/src/inc/shash.h @@ -138,8 +138,8 @@ class DefaultSHashTraits // Hash table class definition template -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 > -#endif { friend class SHash; friend class Iterator; @@ -382,7 +377,7 @@ class SHash : public TRAITS } }; - class Iterator : public Index, public Enumerator + class EMPTY_BASES_DECL Iterator : public Index, public Enumerator { 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 + class EMPTY_BASES_DECL KeyIterator : public KeyIndex, public Enumerator { friend class SHash; @@ -593,7 +588,7 @@ class SHash : public TRAITS // disables support for DAC marshaling. Useful for defining right-side only SHashes template -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 -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 -class PtrSHashTraits : public DefaultSHashTraits +class EMPTY_BASES_DECL PtrSHashTraits : public DefaultSHashTraits { public: @@ -650,7 +645,7 @@ class PtrSHashTraits : public DefaultSHashTraits }; template -class PtrSHash : public SHash< PtrSHashTraits > +class EMPTY_BASES_DECL PtrSHash : public SHash< PtrSHashTraits > { }; @@ -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 -class PtrSHashWithCleanup : public SHash< NoRemoveSHashTraits< PtrSHashWithCleanupTraits > > +class EMPTY_BASES_DECL PtrSHashWithCleanup : public SHash< NoRemoveSHashTraits< PtrSHashWithCleanupTraits > > { }; @@ -772,7 +767,7 @@ public: // pointer hash tables. template > -class StringSHashTraits : public PtrSHashTraits +class EMPTY_BASES_DECL StringSHashTraits : public PtrSHashTraits { public: // explicitly declare local typedefs for these traits types, otherwise @@ -816,7 +811,7 @@ struct StringHashElement }; template > -class StringHashWithCleanupTraits : public StringSHashTraits, CharT, ComparerT> +class EMPTY_BASES_DECL StringHashWithCleanupTraits : public StringSHashTraits, CharT, ComparerT> { public: void OnDestructPerEntryCleanupAction(StringHashElement * e) @@ -835,22 +830,22 @@ public: }; template > -class StringSHashWithCleanup : public SHash< StringHashWithCleanupTraits > +class EMPTY_BASES_DECL StringSHashWithCleanup : public SHash< StringHashWithCleanupTraits > { }; template -class StringSHash : public SHash< StringSHashTraits > +class EMPTY_BASES_DECL StringSHash : public SHash< StringSHashTraits > { }; template -class WStringSHash : public SHash< StringSHashTraits > +class EMPTY_BASES_DECL WStringSHash : public SHash< StringSHashTraits > { }; template -class SStringSHashTraits : public PtrSHashTraits +class EMPTY_BASES_DECL SStringSHashTraits : public PtrSHashTraits { public: typedef PtrSHashTraits PARENT; @@ -873,12 +868,12 @@ class SStringSHashTraits : public PtrSHashTraits }; template -class SStringSHash : public SHash< SStringSHashTraits > +class EMPTY_BASES_DECL SStringSHash : public SHash< SStringSHashTraits > { }; template -class SetSHashTraits : public DefaultSHashTraits +class EMPTY_BASES_DECL SetSHashTraits : public DefaultSHashTraits { public: // explicitly declare local typedefs for these traits types, otherwise @@ -906,7 +901,7 @@ public: }; template > > -class SetSHash : public SHash< TRAITS > +class EMPTY_BASES_DECL SetSHash : public SHash< TRAITS > { typedef SHash PARENT; @@ -918,7 +913,7 @@ public: }; template -class PtrSetSHashTraits : public SetSHashTraits +class EMPTY_BASES_DECL PtrSetSHashTraits : public SetSHashTraits { public: @@ -937,7 +932,7 @@ class PtrSetSHashTraits : public SetSHashTraits }; template -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 -class MapSHashTraits : public DefaultSHashTraits< KeyValuePair > +class EMPTY_BASES_DECL MapSHashTraits : public DefaultSHashTraits< KeyValuePair > { public: // explicitly declare local typedefs for these traits types, otherwise @@ -1011,7 +1006,7 @@ public: }; template > > -class MapSHash : public SHash< TRAITS > +class EMPTY_BASES_DECL MapSHash : public SHash< TRAITS > { typedef SHash< TRAITS > PARENT; @@ -1049,7 +1044,7 @@ public: }; template -class MapSHashWithRemove : public SHash< MapSHashTraits > +class EMPTY_BASES_DECL MapSHashWithRemove : public SHash< MapSHashTraits > { typedef SHash< MapSHashTraits > PARENT; diff --git a/src/coreclr/src/inc/sstring.h b/src/coreclr/src/inc/sstring.h index 0413813..a045917 100644 --- a/src/coreclr/src/inc/sstring.h +++ b/src/coreclr/src/inc/sstring.h @@ -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; @@ -354,7 +354,7 @@ private: public: - class UIterator : public UIndex, public Indexer + class EMPTY_BASES_DECL UIterator : public UIndex, public Indexer { 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 + class EMPTY_BASES_DECL CIterator : public Index, public Indexer { friend class SString; @@ -461,7 +461,7 @@ private: WCHAR operator[](int index) const { return Index::operator[](index); } }; - class Iterator : public Index, public Indexer + class EMPTY_BASES_DECL Iterator : public Index, public Indexer { friend class SString; @@ -805,7 +805,7 @@ private: // =========================================================================== template -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 -class ScratchBuffer : public SString::AbstractScratchBuffer +class EMPTY_BASES_DECL ScratchBuffer : public SString::AbstractScratchBuffer { private: BYTE m_inline[MEMSIZE]; diff --git a/src/coreclr/src/md/datasource/targettypes.cpp b/src/coreclr/src/md/datasource/targettypes.cpp index 32623b6..7ce94fd 100644 --- a/src/coreclr/src/md/datasource/targettypes.cpp +++ b/src/coreclr/src/md/datasource/targettypes.cpp @@ -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)); diff --git a/src/coreclr/src/md/inc/VerifyLayouts.inc b/src/coreclr/src/md/inc/VerifyLayouts.inc index 2fed0fb..36a2146 100644 --- a/src/coreclr/src/md/inc/VerifyLayouts.inc +++ b/src/coreclr/src/md/inc/VerifyLayouts.inc @@ -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), MapSHash__ULONG__ULONG, sizeof(void*)) +BEGIN_TYPE_ESCAPED(IGNORE_COMMAS(MapSHash), MapSHash__ULONG__ULONG, 0) FIELD_ESCAPED(IGNORE_COMMAS(MapSHash), MapSHash__ULONG__ULONG, m_table, sizeof(void*)) FIELD_ESCAPED(IGNORE_COMMAS(MapSHash), MapSHash__ULONG__ULONG, m_tableSize, 4) FIELD_ESCAPED(IGNORE_COMMAS(MapSHash), MapSHash__ULONG__ULONG, m_tableCount, 4) diff --git a/src/coreclr/src/pal/inc/pal.h b/src/coreclr/src/pal/inc/pal.h index 86cbc41..7546046 100644 --- a/src/coreclr/src/pal/inc/pal.h +++ b/src/coreclr/src/pal/inc/pal.h @@ -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) -- 2.7.4