Remove decimal alignment quirk from the type loader (#38603)
authorJan Kotas <jkotas@microsoft.com>
Tue, 30 Jun 2020 16:45:01 +0000 (09:45 -0700)
committerGitHub <noreply@github.com>
Tue, 30 Jun 2020 16:45:01 +0000 (09:45 -0700)
System.Decimal fields did not match Win32 DECIMAL type for historic reasons. It required type loader to have a quirk to artificially inflate System.Decimal alignment to match the alignment of Win32 DECIMAL type to make interop work well.

This change is fixing the System.Decimal fields to match Win32 DECIMAL type and removing the quirk from the type loader since it does not belong there.

The downsides are:
- Slightly lower code quality on 32-bit platforms. 32-bit platforms are not our high performance targets anymore.
- Explicit implementation of ISerializable is required on System.Decimal for binary serialization compatibility.

Fixes #38390

src/coreclr/src/vm/classnames.h
src/coreclr/src/vm/methodtablebuilder.cpp
src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs
src/libraries/System.Private.CoreLib/src/System/Decimal.cs
src/libraries/System.Runtime/ref/System.Runtime.cs

index 9ae2e5c..d6f48e8 100644 (file)
 #define g_ObjectModelAsmName "System.ObjectModel"
 #define g_ColorClassName "System.Drawing.Color"
 #define g_ColorTranslatorClassName "System.Drawing.ColorTranslator"
-#define g_SystemUriClassName "System.Uri"
-#define g_INotifyCollectionChangedName "System.Collections.Specialized.INotifyCollectionChanged"
-#define g_NotifyCollectionChangedEventHandlerName "System.Collections.Specialized.NotifyCollectionChangedEventHandler"
-#define g_NotifyCollectionChangedEventArgsName "System.Collections.Specialized.NotifyCollectionChangedEventArgs"
-#define g_NotifyCollectionChangedEventArgsMarshalerName "System.Runtime.InteropServices.WindowsRuntime.NotifyCollectionChangedEventArgsMarshaler"
-#define g_INotifyPropertyChangedName "System.ComponentModel.INotifyPropertyChanged"
-#define g_PropertyChangedEventHandlerName "System.ComponentModel.PropertyChangedEventHandler"
-#define g_PropertyChangedEventArgsName "System.ComponentModel.PropertyChangedEventArgs"
-#define g_ICommandName "System.Windows.Input.ICommand"
 #define g_ComObjectName "__ComObject"
 #endif // FEATURE_COMINTEROP
 
-
 #define g_DateClassName "System.DateTime"
 #define g_DateTimeOffsetClassName "System.DateTimeOffset"
 #define g_DecimalClassName "System.Decimal"
-#define g_DecimalName "Decimal"
 
 #define g_Vector64ClassName "System.Runtime.Intrinsics.Vector64`1"
 #define g_Vector64Name "Vector64`1"
 #define g_StringBufferName "StringBuilder"
 #define g_StringClassName "System.String"
 #define g_StringName "String"
-#define g_SharedStaticsClassName "System.SharedStatics"
 
 #define g_ThreadClassName "System.Threading.Thread"
 #define g_TypeClassName   "System.Type"
index c1284a6..0ca6e14 100644 (file)
@@ -9724,23 +9724,6 @@ void MethodTableBuilder::CheckForSystemTypes()
             pMT->SetInternalCorElementType (ELEMENT_TYPE_I);
         }
 #endif
-#if defined(ALIGN_ACCESS) || defined(FEATURE_64BIT_ALIGNMENT)
-        else if (strcmp(name, g_DecimalName) == 0)
-        {
-            // This is required because native layout of System.Decimal causes it to be aligned
-            // differently to the layout of the native DECIMAL structure, which will cause
-            // data misalignent exceptions if Decimal is embedded in another type.
-
-            EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo();
-            pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = sizeof(ULONGLONG);
-
-#ifdef FEATURE_64BIT_ALIGNMENT
-            // Also need to mark the type so it will be allocated on a 64-bit boundary for
-            // platforms that won't do this naturally.
-            SetAlign8Candidate();
-#endif
-        }
-#endif // ALIGN_ACCESS || FEATURE_64BIT_ALIGNMENT
     }
     else
     {
index 2cf32df..8f14561 100644 (file)
@@ -14,19 +14,15 @@ namespace System
     public partial struct Decimal
     {
         // Low level accessors used by a DecCalc and formatting
-        internal uint High => (uint)hi;
-        internal uint Low => (uint)lo;
-        internal uint Mid => (uint)mid;
+        internal uint High => _hi32;
+        internal uint Low => (uint)_lo64;
+        internal uint Mid => (uint)(_lo64 >> 32);
 
-        internal bool IsNegative => flags < 0;
+        internal bool IsNegative => _flags < 0;
 
-        internal int Scale => (byte)(flags >> ScaleShift);
+        internal int Scale => (byte)(_flags >> ScaleShift);
 
-#if BIGENDIAN
-        private ulong Low64 => ((ulong)Mid << 32) | Low;
-#else
-        private ulong Low64 => Unsafe.As<int, ulong>(ref Unsafe.AsRef(in lo));
-#endif
+        private ulong Low64 => _lo64;
 
         private static ref DecCalc AsMutable(ref decimal d) => ref Unsafe.As<decimal, DecCalc>(ref d);
 
@@ -50,16 +46,23 @@ namespace System
             private uint uflags;
             [FieldOffset(4)]
             private uint uhi;
+#if BIGENDIAN
+            [FieldOffset(8)]
+            private uint umid;
+            [FieldOffset(12)]
+            private uint ulo;
+#else
             [FieldOffset(8)]
             private uint ulo;
             [FieldOffset(12)]
             private uint umid;
+#endif
 
             /// <summary>
-            /// The low and mid fields combined in little-endian order
+            /// The low and mid fields combined
             /// </summary>
             [FieldOffset(8)]
-            private ulong ulomidLE;
+            private ulong ulomid;
 
             private uint High
             {
@@ -85,13 +88,8 @@ namespace System
 
             private ulong Low64
             {
-#if BIGENDIAN
-                get { return ((ulong)umid << 32) | ulo; }
-                set { umid = (uint)(value >> 32); ulo = (uint)value; }
-#else
-                get => ulomidLE;
-                set => ulomidLE = value;
-#endif
+                get => ulomid;
+                set => ulomid = value;
             }
 
             private const uint SignMask = 0x80000000;
@@ -163,7 +161,7 @@ namespace System
                 MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref s, 1)).Fill(0xCD);
             }
 
-            #region Decimal Math Helpers
+#region Decimal Math Helpers
 
             private static unsafe uint GetExponent(float f)
             {
@@ -1249,16 +1247,16 @@ ThrowOverflow:
             /// </summary>
             internal static int VarDecCmp(in decimal d1, in decimal d2)
             {
-                if ((d2.Low | d2.Mid | d2.High) == 0)
+                if ((d2.Low64 | d2.High) == 0)
                 {
-                    if ((d1.Low | d1.Mid | d1.High) == 0)
+                    if ((d1.Low64 | d1.High) == 0)
                         return 0;
-                    return (d1.flags >> 31) | 1;
+                    return (d1._flags >> 31) | 1;
                 }
-                if ((d1.Low | d1.Mid | d1.High) == 0)
-                    return -((d2.flags >> 31) | 1);
+                if ((d1.Low64 | d1.High) == 0)
+                    return -((d2._flags >> 31) | 1);
 
-                int sign = (d1.flags >> 31) - (d2.flags >> 31);
+                int sign = (d1._flags >> 31) - (d2._flags >> 31);
                 if (sign != 0)
                     return sign;
                 return VarDecCmpSub(in d1, in d2);
@@ -1266,9 +1264,9 @@ ThrowOverflow:
 
             private static int VarDecCmpSub(in decimal d1, in decimal d2)
             {
-                int flags = d2.flags;
+                int flags = d2._flags;
                 int sign = (flags >> 31) | 1;
-                int scale = flags - d1.flags;
+                int scale = flags - d1._flags;
 
                 ulong low64 = d1.Low64;
                 uint high = d1.High;
@@ -1899,10 +1897,10 @@ ReturnZero:
 
             internal static int GetHashCode(in decimal d)
             {
-                if ((d.Low | d.Mid | d.High) == 0)
+                if ((d.Low64 | d.High) == 0)
                     return 0;
 
-                uint flags = (uint)d.flags;
+                uint flags = (uint)d._flags;
                 if ((flags & ScaleMask) == 0 || (d.Low & 1) != 0)
                     return (int)(flags ^ d.High ^ d.Mid ^ d.Low);
 
index cc32ce5..ec9cc19 100644 (file)
@@ -58,7 +58,7 @@ namespace System
     [Serializable]
     [System.Runtime.Versioning.NonVersionable] // This only applies to field layout
     [System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
-    public readonly partial struct Decimal : IFormattable, IComparable, IConvertible, IComparable<decimal>, IEquatable<decimal>, IDeserializationCallback, ISpanFormattable
+    public readonly partial struct Decimal : IFormattable, IComparable, IConvertible, IComparable<decimal>, IEquatable<decimal>, ISpanFormattable, ISerializable, IDeserializationCallback
     {
         // Sign mask for the flags field. A value of zero in this bit indicates a
         // positive Decimal value, and a value of one in this bit indicates a
@@ -102,13 +102,11 @@ namespace System
         // and finally bit 31 indicates the sign of the Decimal value, 0 meaning
         // positive and 1 meaning negative.
         //
-        // NOTE: Do not change the order in which these fields are declared. The
-        // native methods in this class rely on this particular order.
-        // Do not rename (binary serialization).
-        private readonly int flags;
-        private readonly int hi;
-        private readonly int lo;
-        private readonly int mid;
+        // NOTE: Do not change the order and types of these fields. The layout has to
+        // match Win32 DECIMAL type.
+        private readonly int _flags;
+        private readonly uint _hi32;
+        private readonly ulong _lo64;
 
         // Constructs a Decimal from an integer value.
         //
@@ -116,16 +114,15 @@ namespace System
         {
             if (value >= 0)
             {
-                flags = 0;
+                _flags = 0;
             }
             else
             {
-                flags = SignMask;
+                _flags = SignMask;
                 value = -value;
             }
-            lo = value;
-            mid = 0;
-            hi = 0;
+            _lo64 = (uint)value;
+            _hi32 = 0;
         }
 
         // Constructs a Decimal from an unsigned integer value.
@@ -133,10 +130,9 @@ namespace System
         [CLSCompliant(false)]
         public Decimal(uint value)
         {
-            flags = 0;
-            lo = (int)value;
-            mid = 0;
-            hi = 0;
+            _flags = 0;
+            _lo64 = value;
+            _hi32 = 0;
         }
 
         // Constructs a Decimal from a long value.
@@ -145,16 +141,15 @@ namespace System
         {
             if (value >= 0)
             {
-                flags = 0;
+                _flags = 0;
             }
             else
             {
-                flags = SignMask;
+                _flags = SignMask;
                 value = -value;
             }
-            lo = (int)value;
-            mid = (int)(value >> 32);
-            hi = 0;
+            _lo64 = (ulong)value;
+            _hi32 = 0;
         }
 
         // Constructs a Decimal from an unsigned long value.
@@ -162,10 +157,9 @@ namespace System
         [CLSCompliant(false)]
         public Decimal(ulong value)
         {
-            flags = 0;
-            lo = (int)value;
-            mid = (int)(value >> 32);
-            hi = 0;
+            _flags = 0;
+            _lo64 = value;
+            _hi32 = 0;
         }
 
         // Constructs a Decimal from a float value.
@@ -182,6 +176,28 @@ namespace System
             DecCalc.VarDecFromR8(value, out AsMutable(ref this));
         }
 
+        private Decimal(SerializationInfo info, StreamingContext context)
+        {
+            if (info == null)
+                throw new ArgumentNullException(nameof(info));
+
+            _flags = info.GetInt32("flags");
+            _hi32 = (uint)info.GetInt32("hi");
+            _lo64 = (uint)info.GetInt32("lo") + ((ulong)info.GetInt32("mid") << 32);
+        }
+
+        void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
+        {
+            if (info == null)
+                throw new ArgumentNullException(nameof(info));
+
+            // Serialize both the old and the new format
+            info.AddValue("flags", _flags);
+            info.AddValue("hi", (int)High);
+            info.AddValue("lo", (int)Low);
+            info.AddValue("mid", (int)Mid);
+        }
+
         //
         // Decimal <==> Currency conversion.
         //
@@ -261,10 +277,9 @@ namespace System
                 int f = bits[3];
                 if (IsValid(f))
                 {
-                    lo = bits[0];
-                    mid = bits[1];
-                    hi = bits[2];
-                    flags = f;
+                    _lo64 = (uint)bits[0] + ((ulong)(uint)bits[1] << 32);
+                    _hi32 = (uint)bits[2];
+                    _flags = f;
                     return;
                 }
             }
@@ -277,19 +292,18 @@ namespace System
         {
             if (scale > 28)
                 throw new ArgumentOutOfRangeException(nameof(scale), SR.ArgumentOutOfRange_DecimalScale);
-            this.lo = lo;
-            this.mid = mid;
-            this.hi = hi;
-            flags = ((int)scale) << 16;
+            _lo64 = (uint)lo + ((ulong)(uint)mid << 32);
+            _hi32 = (uint)hi;
+            _flags = ((int)scale) << 16;
             if (isNegative)
-                flags |= SignMask;
+                _flags |= SignMask;
         }
 
         void IDeserializationCallback.OnDeserialization(object? sender)
         {
             // OnDeserialization is called after each instance of this class is deserialized.
             // This callback method performs decimal validation after being deserialized.
-            if (!IsValid(flags))
+            if (!IsValid(_flags))
                 throw new SerializationException(SR.Overflow_Decimal);
         }
 
@@ -298,10 +312,9 @@ namespace System
         {
             if (IsValid(flags))
             {
-                this.lo = lo;
-                this.mid = mid;
-                this.hi = hi;
-                this.flags = flags;
+                _lo64 = (uint)lo + ((ulong)(uint)mid << 32);
+                _hi32 = (uint)hi;
+                _flags = flags;
                 return;
             }
             throw new ArgumentException(SR.Arg_DecBitCtor);
@@ -310,7 +323,7 @@ namespace System
         private Decimal(in decimal d, int flags)
         {
             this = d;
-            this.flags = flags;
+            _flags = flags;
         }
 
         // Returns the absolute value of the given Decimal. If d is
@@ -319,7 +332,7 @@ namespace System
         //
         internal static decimal Abs(in decimal d)
         {
-            return new decimal(in d, d.flags & ~SignMask);
+            return new decimal(in d, d._flags & ~SignMask);
         }
 
         // Adds two Decimal values.
@@ -334,7 +347,7 @@ namespace System
         // towards positive infinity.
         public static decimal Ceiling(decimal d)
         {
-            int flags = d.flags;
+            int flags = d._flags;
             if ((flags & ScaleMask) != 0)
                 DecCalc.InternalRound(ref AsMutable(ref d), (byte)(flags >> ScaleShift), MidpointRounding.ToPositiveInfinity);
             return d;
@@ -406,7 +419,7 @@ namespace System
         //
         public static decimal Floor(decimal d)
         {
-            int flags = d.flags;
+            int flags = d._flags;
             if ((flags & ScaleMask) != 0)
                 DecCalc.InternalRound(ref AsMutable(ref d), (byte)(flags >> ScaleShift), MidpointRounding.ToNegativeInfinity);
             return d;
@@ -528,7 +541,7 @@ namespace System
         //
         public static int[] GetBits(decimal d)
         {
-            return new int[] { d.lo, d.mid, d.hi, d.flags };
+            return new int[] { (int)d.Low, (int)d.Mid, (int)d.High, d._flags };
         }
 
         /// <summary>
@@ -545,10 +558,10 @@ namespace System
                 ThrowHelper.ThrowArgumentException_DestinationTooShort();
             }
 
-            destination[0] = d.lo;
-            destination[1] = d.mid;
-            destination[2] = d.hi;
-            destination[3] = d.flags;
+            destination[0] = (int)d.Low;
+            destination[1] = (int)d.Mid;
+            destination[2] = (int)d.High;
+            destination[3] = d._flags;
             return 4;
         }
 
@@ -567,10 +580,10 @@ namespace System
                 return false;
             }
 
-            destination[0] = d.lo;
-            destination[1] = d.mid;
-            destination[2] = d.hi;
-            destination[3] = d.flags;
+            destination[0] = (int)d.Low;
+            destination[1] = (int)d.Mid;
+            destination[2] = (int)d.High;
+            destination[3] = d._flags;
             valuesWritten = 4;
             return true;
         }
@@ -578,25 +591,13 @@ namespace System
         internal static void GetBytes(in decimal d, byte[] buffer)
         {
             Debug.Assert(buffer != null && buffer.Length >= 16, "[GetBytes]buffer != null && buffer.Length >= 16");
-            buffer[0] = (byte)d.lo;
-            buffer[1] = (byte)(d.lo >> 8);
-            buffer[2] = (byte)(d.lo >> 16);
-            buffer[3] = (byte)(d.lo >> 24);
-
-            buffer[4] = (byte)d.mid;
-            buffer[5] = (byte)(d.mid >> 8);
-            buffer[6] = (byte)(d.mid >> 16);
-            buffer[7] = (byte)(d.mid >> 24);
 
-            buffer[8] = (byte)d.hi;
-            buffer[9] = (byte)(d.hi >> 8);
-            buffer[10] = (byte)(d.hi >> 16);
-            buffer[11] = (byte)(d.hi >> 24);
+            Span<byte> span = buffer;
 
-            buffer[12] = (byte)d.flags;
-            buffer[13] = (byte)(d.flags >> 8);
-            buffer[14] = (byte)(d.flags >> 16);
-            buffer[15] = (byte)(d.flags >> 24);
+            BinaryPrimitives.WriteInt32LittleEndian(span, (int)d.Low);
+            BinaryPrimitives.WriteInt32LittleEndian(span.Slice(4), (int)d.Mid);
+            BinaryPrimitives.WriteInt32LittleEndian(span.Slice(8), (int)d.High);
+            BinaryPrimitives.WriteInt32LittleEndian(span.Slice(12), d._flags);
         }
 
         internal static decimal ToDecimal(ReadOnlySpan<byte> span)
@@ -642,7 +643,7 @@ namespace System
         //
         public static decimal Negate(decimal d)
         {
-            return new decimal(in d, d.flags ^ SignMask);
+            return new decimal(in d, d._flags ^ SignMask);
         }
 
         // Rounds a Decimal value to a given number of decimal places. The value
@@ -671,7 +672,7 @@ namespace System
             return d;
         }
 
-        internal static int Sign(in decimal d) => (d.lo | d.mid | d.hi) == 0 ? 0 : (d.flags >> 31) | 1;
+        internal static int Sign(in decimal d) => (d.Low64 | d.High) == 0 ? 0 : (d._flags >> 31) | 1;
 
         // Subtracts two Decimal values.
         //
@@ -757,9 +758,9 @@ namespace System
         public static int ToInt32(decimal d)
         {
             Truncate(ref d);
-            if ((d.hi | d.mid) == 0)
+            if ((d.High | d.Mid) == 0)
             {
-                int i = d.lo;
+                int i = (int)d.Low;
                 if (!d.IsNegative)
                 {
                     if (i >= 0) return i;
@@ -780,7 +781,7 @@ namespace System
         public static long ToInt64(decimal d)
         {
             Truncate(ref d);
-            if (d.hi == 0)
+            if (d.High == 0)
             {
                 long l = (long)d.Low64;
                 if (!d.IsNegative)
@@ -825,7 +826,7 @@ namespace System
         public static uint ToUInt32(decimal d)
         {
             Truncate(ref d);
-            if ((d.hi | d.mid) == 0)
+            if ((d.High| d.Mid) == 0)
             {
                 uint i = d.Low;
                 if (!d.IsNegative || i == 0)
@@ -842,7 +843,7 @@ namespace System
         public static ulong ToUInt64(decimal d)
         {
             Truncate(ref d);
-            if (d.hi == 0)
+            if (d.High == 0)
             {
                 ulong l = d.Low64;
                 if (!d.IsNegative || l == 0)
@@ -872,7 +873,7 @@ namespace System
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static void Truncate(ref decimal d)
         {
-            int flags = d.flags;
+            int flags = d._flags;
             if ((flags & ScaleMask) != 0)
                 DecCalc.InternalRound(ref AsMutable(ref d), (byte)(flags >> ScaleShift), MidpointRounding.ToZero);
         }
@@ -943,7 +944,7 @@ namespace System
 
         public static decimal operator +(decimal d) => d;
 
-        public static decimal operator -(decimal d) => new decimal(in d, d.flags ^ SignMask);
+        public static decimal operator -(decimal d) => new decimal(in d, d._flags ^ SignMask);
 
         public static decimal operator ++(decimal d) => Add(d, One);
 
index f8d831d..667e8fc 100644 (file)
@@ -1538,7 +1538,7 @@ namespace System
         public override string ToString() { throw null; }
         public string ToString(System.IFormatProvider? provider) { throw null; }
     }
-    public readonly partial struct Decimal : System.IComparable, System.IComparable<decimal>, System.IConvertible, System.IEquatable<decimal>, System.IFormattable, System.Runtime.Serialization.IDeserializationCallback
+    public readonly partial struct Decimal : System.IComparable, System.IComparable<decimal>, System.IConvertible, System.IEquatable<decimal>, System.IFormattable, System.Runtime.Serialization.IDeserializationCallback, System.Runtime.Serialization.ISerializable
     {
         private readonly int _dummyPrimitive;
         [System.Runtime.CompilerServices.DecimalConstantAttribute((byte)0, (byte)0, (uint)4294967295, (uint)4294967295, (uint)4294967295)]
@@ -1555,8 +1555,8 @@ namespace System
         public Decimal(int value) { throw null; }
         public Decimal(int lo, int mid, int hi, bool isNegative, byte scale) { throw null; }
         public Decimal(int[] bits) { throw null; }
-        public Decimal(System.ReadOnlySpan<int> bits) { throw null; }
         public Decimal(long value) { throw null; }
+        public Decimal(System.ReadOnlySpan<int> bits) { throw null; }
         public Decimal(float value) { throw null; }
         [System.CLSCompliantAttribute(false)]
         public Decimal(uint value) { throw null; }
@@ -1651,6 +1651,7 @@ namespace System
         uint System.IConvertible.ToUInt32(System.IFormatProvider? provider) { throw null; }
         ulong System.IConvertible.ToUInt64(System.IFormatProvider? provider) { throw null; }
         void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object? sender) { }
+        void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
         public static byte ToByte(System.Decimal value) { throw null; }
         public static double ToDouble(System.Decimal d) { throw null; }
         public static short ToInt16(System.Decimal value) { throw null; }