Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 170854b

Browse files
authored
Cleanup array related FCalls (#22097)
* Cleanup Array FCalls * Disable outdated CoreFX tests dotnet/corefx#34700
1 parent 614966d commit 170854b

File tree

22 files changed

+170
-348
lines changed

22 files changed

+170
-348
lines changed

src/System.Private.CoreLib/Resources/Strings.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3655,4 +3655,13 @@
36553655
<data name="InvalidOp_InvalidNewEnumVariant" xml:space="preserve">
36563656
<value>The returned enumerator does not implement IEnumVARIANT.</value>
36573657
</data>
3658+
<data name="Argument_NotIsomorphic" xml:space="preserve">
3659+
<value>Object contains non-primitive or non-blittable data.</value>
3660+
</data>
3661+
<data name="Argument_StructArrayTooLarge" xml:space="preserve">
3662+
<value>Array size exceeds addressing limitations.</value>
3663+
</data>
3664+
<data name="IndexOutOfRange_ArrayWithOffset" xml:space="preserve">
3665+
<value>ArrayWithOffset: offset exceeds array size.</value>
3666+
</data>
36583667
</root>

src/System.Private.CoreLib/shared/System/Runtime/InteropServices/ArrayWithOffset.cs

Lines changed: 33 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,50 @@
44

55
using System.Runtime.CompilerServices;
66

7+
#if BIT64
8+
using nuint = System.UInt64;
9+
#else
10+
using nuint = System.UInt32;
11+
#endif
12+
713
namespace System.Runtime.InteropServices
814
{
915
public struct ArrayWithOffset
1016
{
17+
private object m_array;
18+
private int m_offset;
19+
private int m_count;
20+
1121
// From MAX_SIZE_FOR_INTEROP in mlinfo.h
1222
private const int MaxSizeForInterop = 0x7ffffff0;
1323

1424
public ArrayWithOffset(object array, int offset)
1525
{
26+
int totalSize = 0;
27+
if (array != null)
28+
{
29+
if (!(array is Array arrayObj) || (arrayObj.Rank != 1) || !Marshal.IsPinnable(arrayObj))
30+
{
31+
throw new ArgumentException(SR.Argument_NotIsomorphic);
32+
}
33+
34+
nuint nativeTotalSize = (nuint)arrayObj.LongLength * (nuint)arrayObj.GetElementSize();
35+
if (nativeTotalSize > MaxSizeForInterop)
36+
{
37+
throw new ArgumentException(SR.Argument_StructArrayTooLarge);
38+
}
39+
40+
totalSize = (int)nativeTotalSize;
41+
}
42+
43+
if ((uint)offset > (uint)totalSize)
44+
{
45+
throw new IndexOutOfRangeException(SR.IndexOutOfRange_ArrayWithOffset);
46+
}
47+
1648
m_array = array;
1749
m_offset = offset;
18-
m_count = 0;
19-
m_count = CalculateCount();
50+
m_count = totalSize - offset;
2051
}
2152

2253
public object GetArray() => m_array;
@@ -44,58 +75,5 @@ public bool Equals(ArrayWithOffset obj)
4475
{
4576
return !(a == b);
4677
}
47-
48-
#if CORECLR // TODO: Cleanup
49-
[MethodImpl(MethodImplOptions.InternalCall)]
50-
private extern int CalculateCount();
51-
#else
52-
private int CalculateCount()
53-
{
54-
if (m_array == null)
55-
{
56-
if (m_offset != 0)
57-
{
58-
throw new IndexOutOfRangeException(SR.IndexOutOfRange_ArrayWithOffset);
59-
}
60-
61-
return 0;
62-
}
63-
else
64-
{
65-
Array arrayObj = m_array as Array;
66-
if (arrayObj == null)
67-
{
68-
throw new ArgumentException(SR.Argument_NotIsomorphic);
69-
}
70-
71-
if (arrayObj.Rank != 1)
72-
{
73-
throw new ArgumentException(SR.Argument_NotIsomorphic);
74-
}
75-
76-
if (!arrayObj.IsBlittable())
77-
{
78-
throw new ArgumentException(SR.Argument_NotIsomorphic);
79-
}
80-
81-
int totalSize = checked(arrayObj.Length * arrayObj.GetElementSize());
82-
if (totalSize > MaxSizeForInterop)
83-
{
84-
throw new ArgumentException(SR.Argument_StructArrayTooLarge);
85-
}
86-
87-
if (m_offset > totalSize)
88-
{
89-
throw new IndexOutOfRangeException(SR.IndexOutOfRange_ArrayWithOffset);
90-
}
91-
92-
return totalSize - m_offset;
93-
}
94-
}
95-
#endif // !CORECLR
96-
97-
private object m_array;
98-
private int m_offset;
99-
private int m_count;
10078
}
10179
}

src/System.Private.CoreLib/src/System/Array.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,10 @@ public extern int Rank
555555
public extern int GetLowerBound(int dimension);
556556

557557
[MethodImplAttribute(MethodImplOptions.InternalCall)]
558-
internal extern int GetDataPtrOffsetInternal();
558+
internal extern ref byte GetRawArrayData();
559+
560+
[MethodImplAttribute(MethodImplOptions.InternalCall)]
561+
internal extern int GetElementSize();
559562

560563
// Number of elements in the Array.
561564
int ICollection.Count

src/System.Private.CoreLib/src/System/Buffer.cs

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,6 @@ public static int ByteLength(Array array)
6060
return _ByteLength(array);
6161
}
6262

63-
// Gets a particular byte out of the array. The array must be an
64-
// array of primitives.
65-
//
66-
// This essentially does the following:
67-
// return ((byte*)array) + index.
68-
//
69-
[MethodImplAttribute(MethodImplOptions.InternalCall)]
70-
private static extern byte _GetByte(Array array, int index);
71-
7263
public static byte GetByte(Array array, int index)
7364
{
7465
// Is the array present?
@@ -80,21 +71,12 @@ public static byte GetByte(Array array, int index)
8071
throw new ArgumentException(SR.Arg_MustBePrimArray, nameof(array));
8172

8273
// Is the index in valid range of the array?
83-
if (index < 0 || index >= _ByteLength(array))
74+
if ((uint)index >= (uint)_ByteLength(array))
8475
throw new ArgumentOutOfRangeException(nameof(index));
8576

86-
return _GetByte(array, index);
77+
return Unsafe.Add<byte>(ref array.GetRawArrayData(), index);
8778
}
8879

89-
// Sets a particular byte in an the array. The array must be an
90-
// array of primitives.
91-
//
92-
// This essentially does the following:
93-
// *(((byte*)array) + index) = value.
94-
//
95-
[MethodImplAttribute(MethodImplOptions.InternalCall)]
96-
private static extern void _SetByte(Array array, int index, byte value);
97-
9880
public static void SetByte(Array array, int index, byte value)
9981
{
10082
// Is the array present?
@@ -106,11 +88,10 @@ public static void SetByte(Array array, int index, byte value)
10688
throw new ArgumentException(SR.Arg_MustBePrimArray, nameof(array));
10789

10890
// Is the index in valid range of the array?
109-
if (index < 0 || index >= _ByteLength(array))
91+
if ((uint)index >= (uint)_ByteLength(array))
11092
throw new ArgumentOutOfRangeException(nameof(index));
11193

112-
// Make the FCall to do the work
113-
_SetByte(array, index, value);
94+
Unsafe.Add<byte>(ref array.GetRawArrayData(), index) = value;
11495
}
11596

11697
// This is currently used by System.IO.UnmanagedMemoryStream

src/System.Private.CoreLib/src/System/RtType.cs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2614,11 +2614,7 @@ public override FieldInfo[] GetFields(BindingFlags bindingAttr)
26142614
public override Type[] GetInterfaces()
26152615
{
26162616
RuntimeType[] candidates = Cache.GetInterfaceList(MemberListType.All, null);
2617-
Type[] interfaces = new Type[candidates.Length];
2618-
for (int i = 0; i < candidates.Length; i++)
2619-
JitHelpers.UnsafeSetArrayElement(interfaces, i, candidates[i]);
2620-
2621-
return interfaces;
2617+
return new ReadOnlySpan<Type>(candidates).ToArray();
26222618
}
26232619

26242620
public override Type[] GetNestedTypes(BindingFlags bindingAttr)
@@ -3430,11 +3426,7 @@ public override string[] GetEnumNames()
34303426
string[] ret = Enum.InternalGetNames(this);
34313427

34323428
// Make a copy since we can't hand out the same array since users can modify them
3433-
string[] retVal = new string[ret.Length];
3434-
3435-
Array.Copy(ret, 0, retVal, 0, ret.Length);
3436-
3437-
return retVal;
3429+
return new ReadOnlySpan<string>(ret).ToArray();
34383430
}
34393431

34403432
public override Array GetEnumValues()

src/System.Private.CoreLib/src/System/Runtime/CompilerServices/jithelpers.cs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,6 @@ internal static int EnumCompareTo<T>(T x, T y) where T : struct, Enum
9999
return x.CompareTo(y);
100100
}
101101

102-
// Set the given element in the array without any type or range checks
103-
[MethodImplAttribute(MethodImplOptions.InternalCall)]
104-
internal static extern void UnsafeSetArrayElement(object[] target, int index, object element);
105-
106102
internal static ref byte GetRawData(this object obj) =>
107103
ref Unsafe.As<RawData>(obj).Data;
108104

src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,23 @@
44

55
using System.Collections.Generic;
66
using System.Reflection;
7-
using System.Reflection.Emit;
87
using System.Security;
98
using System.Text;
109
using System.Runtime.CompilerServices;
1110
using System.Runtime.ConstrainedExecution;
12-
using Win32Native = Microsoft.Win32.Win32Native;
11+
using Microsoft.Win32;
1312
using System.Diagnostics;
1413
using System.Runtime.InteropServices.ComTypes;
1514
using System.StubHelpers;
1615

16+
using Internal.Runtime.CompilerServices;
17+
18+
#if BIT64
19+
using nuint = System.UInt64;
20+
#else
21+
using nuint = System.UInt32;
22+
#endif // BIT64
23+
1724
namespace System.Runtime.InteropServices
1825
{
1926
/// <summary>
@@ -238,15 +245,25 @@ public static IntPtr OffsetOf(Type t, string fieldName)
238245

239246
/// <summary>
240247
/// IMPORTANT NOTICE: This method does not do any verification on the array.
241-
/// It must be used with EXTREME CAUTION since passing in an array that is
242-
/// not pinned or in the fixed heap can cause unexpected results.
248+
/// It must be used with EXTREME CAUTION since passing in invalid index or
249+
/// an array that is not pinned can cause unexpected results.
243250
/// </summary>
244-
[MethodImpl(MethodImplOptions.InternalCall)]
245-
public static extern IntPtr UnsafeAddrOfPinnedArrayElement(Array arr, int index);
251+
public static unsafe IntPtr UnsafeAddrOfPinnedArrayElement(Array arr, int index)
252+
{
253+
if (arr == null)
254+
throw new ArgumentNullException(nameof(arr));
246255

247-
public static IntPtr UnsafeAddrOfPinnedArrayElement<T>(T[] arr, int index)
256+
void* pRawData = Unsafe.AsPointer(ref arr.GetRawArrayData());
257+
return (IntPtr)((byte*)pRawData + (uint)index * (nuint)arr.GetElementSize());
258+
}
259+
260+
public static unsafe IntPtr UnsafeAddrOfPinnedArrayElement<T>(T[] arr, int index)
248261
{
249-
return UnsafeAddrOfPinnedArrayElement((Array)arr, index);
262+
if (arr == null)
263+
throw new ArgumentNullException(nameof(arr));
264+
265+
void* pRawData = Unsafe.AsPointer(ref arr.GetRawSzArrayData());
266+
return (IntPtr)((byte*)pRawData + (uint)index * (nuint)Unsafe.SizeOf<T>());
250267
}
251268

252269
public static void Copy(int[] source, int startIndex, IntPtr destination, int length)
@@ -289,8 +306,17 @@ public static void Copy(IntPtr[] source, int startIndex, IntPtr destination, int
289306
CopyToNative(source, startIndex, destination, length);
290307
}
291308

292-
[MethodImpl(MethodImplOptions.InternalCall)]
293-
private static extern void CopyToNative(object source, int startIndex, IntPtr destination, int length);
309+
private static unsafe void CopyToNative<T>(T[] source, int startIndex, IntPtr destination, int length)
310+
{
311+
if (source == null)
312+
throw new ArgumentNullException(nameof(source));
313+
if (destination == IntPtr.Zero)
314+
throw new ArgumentNullException(nameof(destination));
315+
316+
// The rest of the argument validation is done by CopyTo
317+
318+
new Span<T>(source, startIndex, length).CopyTo(new Span<T>((void*)destination, length));
319+
}
294320

295321
public static void Copy(IntPtr source, int[] destination, int startIndex, int length)
296322
{
@@ -332,9 +358,22 @@ public static void Copy(IntPtr source, IntPtr[] destination, int startIndex, int
332358
CopyToManaged(source, destination, startIndex, length);
333359
}
334360

335-
[MethodImpl(MethodImplOptions.InternalCall)]
336-
private static extern void CopyToManaged(IntPtr source, object destination, int startIndex, int length);
337-
361+
private static unsafe void CopyToManaged<T>(IntPtr source, T[] destination, int startIndex, int length)
362+
{
363+
if (source == IntPtr.Zero)
364+
throw new ArgumentNullException(nameof(source));
365+
if (destination == null)
366+
throw new ArgumentNullException(nameof(destination));
367+
if (startIndex < 0)
368+
throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndex);
369+
if (length < 0)
370+
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NeedNonNegNum);
371+
372+
// The rest of the argument validation is done by CopyTo
373+
374+
new Span<T>((void*)source, length).CopyTo(new Span<T>(destination, startIndex, length));
375+
}
376+
338377
public static byte ReadByte(object ptr, int ofs)
339378
{
340379
return ReadValueSlow(ptr, ofs, (IntPtr nativeHome, int offset) => ReadByte(nativeHome, offset));
@@ -837,6 +876,9 @@ public static object PtrToStructure(IntPtr ptr, Type structureType)
837876

838877
public static void DestroyStructure<T>(IntPtr ptr) => DestroyStructure(ptr, typeof(T));
839878

879+
[MethodImpl(MethodImplOptions.InternalCall)]
880+
internal static extern bool IsPinnable(object obj);
881+
840882
#if FEATURE_COMINTEROP
841883
/// <summary>
842884
/// Returns the HInstance for this module. Returns -1 if the module doesn't have

src/classlibnative/bcltype/arraynative.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,21 +133,29 @@ FCIMPL1(INT64, ArrayNative::GetLongLengthNoRank, ArrayBase* array)
133133
FCIMPLEND
134134

135135

136-
FCIMPL1(INT32, ArrayNative::GetDataPtrOffsetInternal, ArrayBase* array)
136+
FCIMPL1(void*, ArrayNative::GetRawArrayData, ArrayBase* array)
137137
{
138138
FCALL_CONTRACT;
139139

140140
VALIDATEOBJECT(array);
141141

142-
if (array == NULL)
143-
FCThrow(kNullReferenceException);
142+
_ASSERTE(array != NULL);
144143

145-
return ArrayBase::GetDataPtrOffset(array->GetMethodTable());
144+
return array->GetDataPtr();
146145
}
147146
FCIMPLEND
148147

148+
FCIMPL1(INT32, ArrayNative::GetElementSize, ArrayBase* array)
149+
{
150+
FCALL_CONTRACT;
151+
152+
VALIDATEOBJECT(array);
149153

154+
_ASSERTE(array != NULL);
150155

156+
return (INT32)array->GetComponentSize();
157+
}
158+
FCIMPLEND
151159

152160

153161

src/classlibnative/bcltype/arraynative.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ class ArrayNative
2626
{
2727
public:
2828
static FCDECL1(INT32, GetRank, ArrayBase* pArray);
29-
static FCDECL1(INT32, GetDataPtrOffsetInternal, ArrayBase* array);
29+
static FCDECL1(void*, GetRawArrayData, ArrayBase* array);
30+
static FCDECL1(INT32, GetElementSize, ArrayBase* array);
3031
static FCDECL2(INT32, GetLowerBound, ArrayBase* pArray, unsigned int dimension);
3132
static FCDECL2(INT32, GetUpperBound, ArrayBase* pArray, unsigned int dimension);
3233
static FCDECL1(INT32, GetLengthNoRank, ArrayBase* pArray);

src/dlls/mscorrc/mscorrc.rc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,6 @@ BEGIN
821821
CORSECATTR_E_BAD_ACTION "Security custom attribute has invalid SecurityAction."
822822

823823
IDS_EE_COPY_OUTOFRANGE "Requested range extends past the end of the array."
824-
IDS_EE_ARRAYWITHOFFSETOVERFLOW "ArrayWithOffset: offset exceeds array size."
825824
IDS_EE_NOCUSTOMMARSHALER "A call to GetInstance() for custom marshaler '%1' returned null, which is not allowed."
826825
IDS_EE_SIZECONTROLOUTOFRANGE "Array size control parameter index is out of range."
827826
IDS_EE_SIZECONTROLBADTYPE "Array size control parameter type not supported."

0 commit comments

Comments
 (0)