From 4a7a2049a1a985b02fe5983e7e2a8d480270a72b Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Wed, 2 Jan 2019 03:21:11 +0100 Subject: [PATCH] strlen to managed code and vectorize (#21729) --- src/System.Private.CoreLib/shared/System/String.cs | 25 +++++++++++--- .../src/Microsoft/Win32/Win32Native.cs | 3 -- src/System.Private.CoreLib/src/System/RtType.cs | 6 ++-- .../src/System/Runtime/InteropServices/Marshal.cs | 38 ++++++++++++++++++---- .../src/System/StubHelpers.cs | 26 ++++++++++----- src/vm/ecalllist.h | 1 - src/vm/ilmarshalers.cpp | 12 +++---- src/vm/metasig.h | 1 + src/vm/mscorlib.h | 2 +- src/vm/stubhelpers.cpp | 13 -------- src/vm/stubhelpers.h | 1 - 11 files changed, 81 insertions(+), 47 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/String.cs b/src/System.Private.CoreLib/shared/System/String.cs index ea8b5e3..0958e86 100644 --- a/src/System.Private.CoreLib/shared/System/String.cs +++ b/src/System.Private.CoreLib/shared/System/String.cs @@ -163,11 +163,7 @@ namespace System if (pb == null) return Empty; - int numBytes = new ReadOnlySpan((byte*)value, int.MaxValue).IndexOf(0); - - // Check for overflow - if (numBytes < 0) - throw new ArgumentException(SR.Arg_MustBeNullTerminatedString); + int numBytes = strlen((byte*)value); return CreateStringForSByteConstructor(pb, numBytes); } @@ -655,6 +651,25 @@ namespace System return count; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static unsafe int strlen(byte* ptr) + { + // IndexOf processes memory in aligned chunks, and thus it won't crash even if it accesses memory beyond the null terminator. + int length = SpanHelpers.IndexOf(ref *ptr, (byte)'\0', int.MaxValue); + if (length < 0) + { + ThrowMustBeNullTerminatedString(); + } + + return length; + } + + private static void ThrowMustBeNullTerminatedString() + { + throw new ArgumentException(SR.Arg_MustBeNullTerminatedString); + } + // // IConvertible implementation // diff --git a/src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs b/src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs index dd808b3..e3bc76d 100644 --- a/src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs +++ b/src/System.Private.CoreLib/src/Microsoft/Win32/Win32Native.cs @@ -185,9 +185,6 @@ namespace Microsoft.Win32 [DllImport(Interop.Libraries.Kernel32, SetLastError = true)] internal static extern unsafe bool VirtualFree(void* address, UIntPtr numBytes, int pageFreeMode); - [DllImport(Interop.Libraries.Kernel32, CharSet = CharSet.Ansi, ExactSpelling = true, EntryPoint = "lstrlenA")] - internal static extern int lstrlenA(IntPtr ptr); - [DllImport(Interop.Libraries.Kernel32, CharSet = CharSet.Unicode, ExactSpelling = true, EntryPoint = "lstrlenW")] internal static extern int lstrlenW(IntPtr ptr); diff --git a/src/System.Private.CoreLib/src/System/RtType.cs b/src/System.Private.CoreLib/src/System/RtType.cs index 259c8a6..cf7d8a8 100644 --- a/src/System.Private.CoreLib/src/System/RtType.cs +++ b/src/System.Private.CoreLib/src/System/RtType.cs @@ -4819,15 +4819,17 @@ namespace System internal MdUtf8String(void* pStringHeap) { - m_pStringHeap = (byte*)pStringHeap; + byte* pStringBytes = (byte*)pStringHeap; if (pStringHeap != null) { - m_StringHeapByteLength = StubHelpers.StubHelpers.strlen((sbyte*)pStringHeap); + m_StringHeapByteLength = string.strlen(pStringBytes); } else { m_StringHeapByteLength = 0; } + + m_pStringHeap = pStringBytes; } internal unsafe MdUtf8String(byte* pUtf8String, int cUtf8String) diff --git a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs index 8af2cd2..09aa503 100644 --- a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs +++ b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs @@ -85,7 +85,7 @@ namespace System.Runtime.InteropServices return null; } - int nb = Win32Native.lstrlenA(ptr); + int nb = string.strlen((byte*)ptr); if (nb == 0) { return string.Empty; @@ -155,7 +155,7 @@ namespace System.Runtime.InteropServices return null; } - int nbBytes = StubHelpers.StubHelpers.strlen((sbyte*)ptr.ToPointer()); + int nbBytes = string.strlen((byte*)ptr); return PtrToStringUTF8(ptr, nbBytes); } @@ -1710,25 +1710,41 @@ namespace System.Runtime.InteropServices public static void ZeroFreeBSTR(IntPtr s) { + if (s == IntPtr.Zero) + { + return; + } RuntimeImports.RhZeroMemory(s, (UIntPtr)(Win32Native.SysStringLen(s) * 2)); FreeBSTR(s); } - public static void ZeroFreeCoTaskMemAnsi(IntPtr s) + public unsafe static void ZeroFreeCoTaskMemAnsi(IntPtr s) { - RuntimeImports.RhZeroMemory(s, (UIntPtr)(Win32Native.lstrlenA(s))); + if (s == IntPtr.Zero) + { + return; + } + RuntimeImports.RhZeroMemory(s, (UIntPtr)string.strlen((byte*)s)); FreeCoTaskMem(s); } public static void ZeroFreeCoTaskMemUnicode(IntPtr s) { + if (s == IntPtr.Zero) + { + return; + } RuntimeImports.RhZeroMemory(s, (UIntPtr)(Win32Native.lstrlenW(s) * 2)); FreeCoTaskMem(s); } public static unsafe void ZeroFreeCoTaskMemUTF8(IntPtr s) { - RuntimeImports.RhZeroMemory(s, (UIntPtr)System.StubHelpers.StubHelpers.strlen((sbyte*)s)); + if (s == IntPtr.Zero) + { + return; + } + RuntimeImports.RhZeroMemory(s, (UIntPtr)string.strlen((byte*)s)); FreeCoTaskMem(s); } @@ -1752,14 +1768,22 @@ namespace System.Runtime.InteropServices return s.MarshalToString(globalAlloc: true, unicode: true); ; } - public static void ZeroFreeGlobalAllocAnsi(IntPtr s) + public unsafe static void ZeroFreeGlobalAllocAnsi(IntPtr s) { - RuntimeImports.RhZeroMemory(s, (UIntPtr)(Win32Native.lstrlenA(s))); + if (s == IntPtr.Zero) + { + return; + } + RuntimeImports.RhZeroMemory(s, (UIntPtr)string.strlen((byte*)s)); FreeHGlobal(s); } public static void ZeroFreeGlobalAllocUnicode(IntPtr s) { + if (s == IntPtr.Zero) + { + return; + } RuntimeImports.RhZeroMemory(s, (UIntPtr)(Win32Native.lstrlenW(s) * 2)); FreeHGlobal(s); } diff --git a/src/System.Private.CoreLib/src/System/StubHelpers.cs b/src/System.Private.CoreLib/src/System/StubHelpers.cs index aef67a4..82b2886 100644 --- a/src/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/System.Private.CoreLib/src/System/StubHelpers.cs @@ -162,8 +162,10 @@ namespace System.StubHelpers { if (IntPtr.Zero == cstr) return null; - int nbBytes = StubHelpers.strlen((sbyte*)cstr); - return string.CreateStringFromEncoding((byte*)cstr, nbBytes, Encoding.UTF8); + + byte* pBytes = (byte*)cstr; + int nbBytes = string.strlen(pBytes); + return string.CreateStringFromEncoding(pBytes, nbBytes, Encoding.UTF8); } internal static void ClearNative(IntPtr pNative) @@ -203,8 +205,9 @@ namespace System.StubHelpers if (pNative == IntPtr.Zero) return; - int nbBytes = StubHelpers.strlen((sbyte*)pNative); - sb.ReplaceBufferUtf8Internal(new Span((byte*)pNative, nbBytes)); + byte* pBytes = (byte*)pNative; + int nbBytes = string.strlen(pBytes); + sb.ReplaceBufferUtf8Internal(new Span(pBytes, nbBytes)); } } @@ -1188,7 +1191,17 @@ namespace System.StubHelpers case BackPropAction.StringBuilderAnsi: { sbyte* ptr = (sbyte*)pNativeHome.ToPointer(); - ((StringBuilder)pManagedHome).ReplaceBufferAnsiInternal(ptr, Win32Native.lstrlenA(pNativeHome)); + int length; + if (pNativeHome == IntPtr.Zero) + { + length = 0; + } + else + { + length = string.strlen((byte*)pNativeHome); + } + + ((StringBuilder)pManagedHome).ReplaceBufferAnsiInternal(ptr, length); break; } @@ -1722,9 +1735,6 @@ namespace System.StubHelpers } [MethodImplAttribute(MethodImplOptions.InternalCall)] - internal static extern unsafe int strlen(sbyte* ptr); - - [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern unsafe void FmtClassUpdateNativeInternal(object obj, byte* pNative, ref CleanupWorkListElement pCleanupWorkList); [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern unsafe void FmtClassUpdateCLRInternal(object obj, byte* pNative); diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h index 82e7e97..145c949 100644 --- a/src/vm/ecalllist.h +++ b/src/vm/ecalllist.h @@ -1094,7 +1094,6 @@ FCFuncStart(gStubHelperFuncs) FCFuncElement("FmtClassUpdateCLRInternal", StubHelpers::FmtClassUpdateCLRInternal) FCFuncElement("LayoutDestroyNativeInternal", StubHelpers::LayoutDestroyNativeInternal) FCFuncElement("AllocateInternal", StubHelpers::AllocateInternal) - FCFuncElement("strlen", StubHelpers::AnsiStrlen) FCFuncElement("MarshalToUnmanagedVaListInternal", StubHelpers::MarshalToUnmanagedVaListInternal) FCFuncElement("MarshalToManagedVaListInternal", StubHelpers::MarshalToManagedVaListInternal) FCFuncElement("CalcVaListSize", StubHelpers::CalcVaListSize) diff --git a/src/vm/ilmarshalers.cpp b/src/vm/ilmarshalers.cpp index 71fd1e7..ba341f1 100644 --- a/src/vm/ilmarshalers.cpp +++ b/src/vm/ilmarshalers.cpp @@ -629,8 +629,8 @@ void ILUTF8BufferMarshaler::EmitConvertSpaceNativeToCLR(ILCodeStream* pslILEmit) if (IsIn(m_dwMarshalFlags) || IsCLRToNative(m_dwMarshalFlags)) { EmitLoadNativeValue(pslILEmit); - // static int System.StubHelpers.StubHelpers.strlen(sbyte* ptr) - pslILEmit->EmitCALL(METHOD__STUBHELPERS__STRLEN, 1, 1); + // static int System.String.strlen(byte* ptr) + pslILEmit->EmitCALL(METHOD__STRING__STRLEN, 1, 1); } else { @@ -1034,8 +1034,8 @@ void ILCSTRBufferMarshaler::EmitConvertSpaceNativeToCLR(ILCodeStream* pslILEmit) if (IsIn(m_dwMarshalFlags) || IsCLRToNative(m_dwMarshalFlags)) { EmitLoadNativeValue(pslILEmit); - // static int System.StubHelpers.StubHelpers.strlen(sbyte* ptr) - pslILEmit->EmitCALL(METHOD__STUBHELPERS__STRLEN, 1, 1); + // static int System.String.strlen(byte* ptr) + pslILEmit->EmitCALL(METHOD__STRING__STRLEN, 1, 1); } else { @@ -1063,8 +1063,8 @@ void ILCSTRBufferMarshaler::EmitConvertContentsNativeToCLR(ILCodeStream* pslILEm EmitLoadNativeValue(pslILEmit); pslILEmit->EmitDUP(); - // static int System.StubHelpers.StubHelpers.strlen(sbyte* ptr) - pslILEmit->EmitCALL(METHOD__STUBHELPERS__STRLEN, 1, 1); + // static int System.String.strlen(byte* ptr) + pslILEmit->EmitCALL(METHOD__STRING__STRLEN, 1, 1); // void System.Text.StringBuilder.ReplaceBuffer(sbyte* newBuffer, int newLength); pslILEmit->EmitCALL(METHOD__STRING_BUILDER__REPLACE_BUFFER_ANSI_INTERNAL, 3, 0); diff --git a/src/vm/metasig.h b/src/vm/metasig.h index 0897afc..e701cf3 100644 --- a/src/vm/metasig.h +++ b/src/vm/metasig.h @@ -231,6 +231,7 @@ DEFINE_METASIG(SM(PtrByte_IntPtr_RetVoid, P(b) I, v)) DEFINE_METASIG(SM(Str_Bool_Bool_RefInt_RetArrByte, s F F r(i), a(b) )) DEFINE_METASIG(SM(ArrByte_Int_PtrByte_Int_Int_RetVoid, a(b) i P(b) i i, v)) DEFINE_METASIG(SM(PtrByte_Int_ArrByte_Int_Int_RetVoid, P(b) i a(b) i i, v)) +DEFINE_METASIG(SM(PtrByte_RetInt, P(b), i)) DEFINE_METASIG(SM(PtrSByt_RetInt, P(B), i)) DEFINE_METASIG(SM(IntPtr_RetIntPtr, I, I)) DEFINE_METASIG(SM(UIntPtr_RetIntPtr, U, I)) diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h index fd57541..5d81162 100644 --- a/src/vm/mscorlib.h +++ b/src/vm/mscorlib.h @@ -791,6 +791,7 @@ DEFINE_METHOD(STRING, CTORF_SBYTEPTR_START_LEN, Ctor, DEFINE_METHOD(STRING, CTORF_SBYTEPTR_START_LEN_ENCODING, Ctor, IM_PtrSByt_Int_Int_Encoding_RetStr) DEFINE_METHOD(STRING, INTERNAL_COPY, InternalCopy, SM_Str_IntPtr_Int_RetVoid) DEFINE_METHOD(STRING, WCSLEN, wcslen, SM_PtrChar_RetInt) +DEFINE_METHOD(STRING, STRLEN, strlen, SM_PtrByte_RetInt) DEFINE_PROPERTY(STRING, LENGTH, Length, Int) DEFINE_CLASS(STRING_BUILDER, Text, StringBuilder) @@ -954,7 +955,6 @@ DEFINE_METHOD(STUBHELPERS, FMT_CLASS_UPDATE_NATIVE_INTERNAL, FmtClass DEFINE_METHOD(STUBHELPERS, FMT_CLASS_UPDATE_CLR_INTERNAL, FmtClassUpdateCLRInternal, SM_Obj_PtrByte_RetVoid) DEFINE_METHOD(STUBHELPERS, LAYOUT_DESTROY_NATIVE_INTERNAL, LayoutDestroyNativeInternal, SM_PtrByte_IntPtr_RetVoid) DEFINE_METHOD(STUBHELPERS, ALLOCATE_INTERNAL, AllocateInternal, SM_IntPtr_RetObj) -DEFINE_METHOD(STUBHELPERS, STRLEN, strlen, SM_PtrSByt_RetInt) DEFINE_METHOD(STUBHELPERS, MARSHAL_TO_MANAGED_VA_LIST_INTERNAL,MarshalToManagedVaListInternal, SM_IntPtr_IntPtr_RetVoid) DEFINE_METHOD(STUBHELPERS, MARSHAL_TO_UNMANAGED_VA_LIST_INTERNAL,MarshalToUnmanagedVaListInternal,SM_IntPtr_UInt_IntPtr_RetVoid) DEFINE_METHOD(STUBHELPERS, CALC_VA_LIST_SIZE, CalcVaListSize, SM_IntPtr_RetUInt) diff --git a/src/vm/stubhelpers.cpp b/src/vm/stubhelpers.cpp index 42e90cd..456a367 100644 --- a/src/vm/stubhelpers.cpp +++ b/src/vm/stubhelpers.cpp @@ -1695,19 +1695,6 @@ FCIMPL1(Object*, StubHelpers::AllocateInternal, EnregisteredTypeHandle pRegister } FCIMPLEND -FCIMPL1(int, StubHelpers::AnsiStrlen, __in_z char* pszStr) -{ - FCALL_CONTRACT; - - size_t len = strlen(pszStr); - - // the length should have been checked earlier (see StubHelpers.CheckStringLength) - _ASSERTE(FitsInI4(len)); - - return (int)len; -} -FCIMPLEND - FCIMPL3(void, StubHelpers::MarshalToUnmanagedVaListInternal, va_list va, DWORD cbVaListSize, const VARARGS* pArgIterator) { FCALL_CONTRACT; diff --git a/src/vm/stubhelpers.h b/src/vm/stubhelpers.h index e5376f7..50ea8d3 100644 --- a/src/vm/stubhelpers.h +++ b/src/vm/stubhelpers.h @@ -103,7 +103,6 @@ public: static FCDECL2(void, FmtClassUpdateCLRInternal, Object* pObjUNSAFE, BYTE* pbNative); static FCDECL2(void, LayoutDestroyNativeInternal, BYTE* pbNative, MethodTable* pMT); static FCDECL1(Object*, AllocateInternal, EnregisteredTypeHandle typeHnd); - static FCDECL1(int, AnsiStrlen, __in_z char* pszStr); static FCDECL3(void, MarshalToUnmanagedVaListInternal, va_list va, DWORD cbVaListSize, const VARARGS* pArgIterator); static FCDECL2(void, MarshalToManagedVaListInternal, va_list va, VARARGS* pArgIterator); static FCDECL0(void*, GetStubContext); -- 2.7.4