From 666ca6bac4a1d878fc821ed8265ff546758c2809 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 11 Jul 2025 20:03:19 +0200 Subject: [PATCH] Change RhNewString length argument to nint/intptr_t The RhNewString length argument is declared as int, but the asm implementation was using the full 64 bits of the related argument register. That doesn't match the calling convention where there is no guarantee that the upper 32 bits are zeroed. That has caused problem in the interpreter that always loads full registers from the interpreter stack and so sometimes, the upper 32 bits were a garbage and the function failed because the length was seemingly too large. This change fixes it by changing signature of the RhNewString so that the length is nint in managed code / intptr_t in native code. --- src/coreclr/nativeaot/Runtime/portable.cpp | 2 +- .../src/System/Runtime/RuntimeImports.cs | 2 +- .../System.Private.CoreLib/src/System/String.NativeAot.cs | 2 +- .../Test.CoreLib/src/System/Runtime/RuntimeImports.cs | 2 +- src/coreclr/runtime/amd64/AllocFast.S | 2 +- src/coreclr/runtime/amd64/AllocFast.asm | 2 +- src/coreclr/vm/jitinterface.h | 7 ++----- 7 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/portable.cpp b/src/coreclr/nativeaot/Runtime/portable.cpp index 2f30e42253d876..c6f83d09c1ff0d 100644 --- a/src/coreclr/nativeaot/Runtime/portable.cpp +++ b/src/coreclr/nativeaot/Runtime/portable.cpp @@ -127,7 +127,7 @@ FCIMPL2(Array *, RhpNewArrayFast, MethodTable * pArrayEEType, int numElements) } FCIMPLEND -FCIMPL2(String *, RhNewString, MethodTable * pArrayEEType, int numElements) +FCIMPL2(String *, RhNewString, MethodTable * pArrayEEType, intptr_t numElements) { // TODO: Implement. We tail call to RhpNewArrayFast for now since there's a bunch of TODOs in the places // that matter anyway. diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs index 7adbb86c7fcd2d..e1886df7006f31 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -396,7 +396,7 @@ internal static IntPtr RhHandleAllocDependent(object primary, object secondary) [MethodImpl(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhNewString")] - internal static extern unsafe string RhNewString(MethodTable* pEEType, int length); + internal static extern unsafe string RhNewString(MethodTable* pEEType, nint length); [MethodImpl(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhUnbox")] diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/String.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/String.NativeAot.cs index a8efc76018a30d..2e951b445befe9 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/String.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/String.NativeAot.cs @@ -18,7 +18,7 @@ public partial class String [Intrinsic] public static readonly string Empty = ""; - internal static unsafe string FastAllocateString(int length) + internal static unsafe string FastAllocateString(nint length) { // We allocate one extra char as an interop convenience so that our strings are null- // terminated, however, we don't pass the extra +1 to the string allocation because the base diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs index 026f0b8d109cc6..7703d63dee84a0 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -82,7 +82,7 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe [MethodImpl(MethodImplOptions.InternalCall)] [RuntimeImport(RuntimeLibrary, "RhNewString")] - internal static extern unsafe string RhNewString(MethodTable* pEEType, int length); + internal static extern unsafe string RhNewString(MethodTable* pEEType, nint length); [DllImport(RuntimeLibrary)] internal static extern unsafe void RhAllocateNewArray(MethodTable* pArrayEEType, uint numElements, uint flags, void* pResult); diff --git a/src/coreclr/runtime/amd64/AllocFast.S b/src/coreclr/runtime/amd64/AllocFast.S index d5b366b876dad6..a8b6fcd5bf5053 100644 --- a/src/coreclr/runtime/amd64/AllocFast.S +++ b/src/coreclr/runtime/amd64/AllocFast.S @@ -173,7 +173,7 @@ NESTED_END RhpNewObject, _TEXT // Allocate a string. // RDI == MethodTable -// ESI == character/element count +// RSI == character/element count LEAF_ENTRY RhNewString, _TEXT // we want to limit the element count to the non-negative 32-bit int range diff --git a/src/coreclr/runtime/amd64/AllocFast.asm b/src/coreclr/runtime/amd64/AllocFast.asm index c099cb829b2c83..cda88f3bf8f1ec 100644 --- a/src/coreclr/runtime/amd64/AllocFast.asm +++ b/src/coreclr/runtime/amd64/AllocFast.asm @@ -128,7 +128,7 @@ ENDM ; NEW_ARRAY_FAST ;; Allocate a string. ;; RCX == MethodTable -;; EDX == character/element count +;; RDX == character/element count LEAF_ENTRY RhNewString, _TEXT ; we want to limit the element count to the non-negative 32-bit int range diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 7116f37690d1dd..d715ab80939ac5 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -149,7 +149,7 @@ EXTERN_C FCDECL1(void*, JIT_GetDynamicNonGCStaticBaseNoCtor_Portable, DynamicSta EXTERN_C FCDECL1(Object*, RhpNewFast, CORINFO_CLASS_HANDLE typeHnd_); EXTERN_C FCDECL2(Object*, RhpNewArrayFast, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR size); EXTERN_C FCDECL2(Object*, RhpNewPtrArrayFast, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR size); -EXTERN_C FCDECL2(Object*, RhNewString, CORINFO_CLASS_HANDLE typeHnd_, DWORD stringLength); +EXTERN_C FCDECL2(Object*, RhNewString, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR stringLength); #if defined(FEATURE_64BIT_ALIGNMENT) EXTERN_C FCDECL1(Object*, RhpNewFastAlign8, CORINFO_CLASS_HANDLE typeHnd_); @@ -161,7 +161,7 @@ EXTERN_C FCDECL2(Object*, RhpNewArrayFastAlign8, CORINFO_CLASS_HANDLE typeHnd_, EXTERN_C FCDECL1(Object*, RhpNewFast_UP, CORINFO_CLASS_HANDLE typeHnd_); EXTERN_C FCDECL2(Object*, RhpNewArrayFast_UP, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR size); EXTERN_C FCDECL2(Object*, RhpNewPtrArrayFast_UP, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR size); -EXTERN_C FCDECL2(Object*, RhNewString_UP, CORINFO_CLASS_HANDLE typeHnd_, DWORD stringLength); +EXTERN_C FCDECL2(Object*, RhNewString_UP, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR stringLength); #endif EXTERN_C FCDECL1(Object*, RhpNew, CORINFO_CLASS_HANDLE typeHnd_); @@ -169,9 +169,6 @@ EXTERN_C FCDECL2(Object*, RhpNewVariableSizeObject, CORINFO_CLASS_HANDLE typeHnd EXTERN_C FCDECL1(Object*, RhpNewMaybeFrozen, CORINFO_CLASS_HANDLE typeHnd_); EXTERN_C FCDECL2(Object*, RhpNewArrayMaybeFrozen, CORINFO_CLASS_HANDLE typeHnd_, INT_PTR size); -EXTERN_C FCDECL1(Object*, AllocateStringFast, DWORD stringLength); -EXTERN_C FCDECL1(Object*, AllocateStringSlow, DWORD stringLength); - EXTERN_C FCDECL2(void, JITutil_MonReliableEnter, Object* obj, BYTE* pbLockTaken); EXTERN_C FCDECL3(void, JITutil_MonTryEnter, Object* obj, INT32 timeOut, BYTE* pbLockTaken); EXTERN_C FCDECL2(void, JITutil_MonReliableContention, AwareLock* awarelock, BYTE* pbLockTaken);