From 598800d3309d554dcf75aba5903c7ef72aded3f2 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 11:09:28 -0800 Subject: [PATCH 01/10] Remove PAL_strtok and the one usage of it --- src/coreclr/md/compiler/emit.cpp | 13 +++- src/coreclr/pal/inc/pal.h | 2 - src/coreclr/pal/src/CMakeLists.txt | 1 - src/coreclr/pal/src/cruntime/stringtls.cpp | 77 ------------------- src/coreclr/pal/src/include/pal/palinternal.h | 1 - src/coreclr/pal/src/include/pal/thread.hpp | 2 - 6 files changed, 11 insertions(+), 85 deletions(-) delete mode 100644 src/coreclr/pal/src/cruntime/stringtls.cpp diff --git a/src/coreclr/md/compiler/emit.cpp b/src/coreclr/md/compiler/emit.cpp index f2df925feaa8b0..6252deae8ff341 100644 --- a/src/coreclr/md/compiler/emit.cpp +++ b/src/coreclr/md/compiler/emit.cpp @@ -1951,11 +1951,20 @@ STDMETHODIMP RegMeta::DefineDocument( // S_OK or error. *partsIndexesPtr++ = 0; partsIndexesCount++; } - stringToken = strtok(docName, (const char*)delim); + char* context; +#ifdef HOST_WINDOWS + stringToken = strtok_s(docName, (const char*)delim, &context); +#else + stringToken = strtok_r(docName, (const char*)delim, &context); +#endif while (stringToken != NULL) { IfFailGo(m_pStgdb->m_MiniMd.m_BlobHeap.AddBlob(MetaData::DataBlob((BYTE*)stringToken, (ULONG)strlen(stringToken)), partsIndexesPtr++)); - stringToken = strtok(NULL, (const char*)delim); +#ifdef HOST_WINDOWS + stringToken = strtok_s(NULL, (const char*)delim, &context); +#else + stringToken = strtok_r(NULL, (const char*)delim, &context); +#endif partsIndexesCount++; } diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 7cfdf4d949968e..18534271dc269a 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3982,7 +3982,6 @@ PAL_GetCurrentThreadAffinitySet(SIZE_T size, UINT_PTR* data); #define exit PAL_exit #define realloc PAL_realloc #define fopen PAL_fopen -#define strtok PAL_strtok #define strtoul PAL_strtoul #define strtoull PAL_strtoull #define fprintf PAL_fprintf @@ -4083,7 +4082,6 @@ PALIMPORT char * __cdecl strchr(const char *, int); PALIMPORT char * __cdecl strrchr(const char *, int); PALIMPORT char * __cdecl strpbrk(const char *, const char *); PALIMPORT char * __cdecl strstr(const char *, const char *); -PALIMPORT char * __cdecl strtok(char *, const char *); PALIMPORT int __cdecl atoi(const char *); PALIMPORT ULONG __cdecl strtoul(const char *, char **, int); PALIMPORT ULONGLONG __cdecl strtoull(const char *, char **, int); diff --git a/src/coreclr/pal/src/CMakeLists.txt b/src/coreclr/pal/src/CMakeLists.txt index aa7e8d9dd023dc..bfbc067c893151 100644 --- a/src/coreclr/pal/src/CMakeLists.txt +++ b/src/coreclr/pal/src/CMakeLists.txt @@ -136,7 +136,6 @@ set(SOURCES cruntime/misc.cpp cruntime/printfcpp.cpp cruntime/string.cpp - cruntime/stringtls.cpp cruntime/thread.cpp cruntime/wchar.cpp debug/debug.cpp diff --git a/src/coreclr/pal/src/cruntime/stringtls.cpp b/src/coreclr/pal/src/cruntime/stringtls.cpp deleted file mode 100644 index a080e1dd84fdd6..00000000000000 --- a/src/coreclr/pal/src/cruntime/stringtls.cpp +++ /dev/null @@ -1,77 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -/*++ - - - -Module Name: - - stringtls.cpp - -Abstract: - - Implementation of the string functions in the C runtime library that - are Windows specific and depend on per-thread data - - - ---*/ - -#include "pal/thread.hpp" -#include "pal/dbgmsg.h" - -#include -#include -#include -#include -#include - -using namespace CorUnix; - -SET_DEFAULT_DEBUG_CHANNEL(CRT); - -/*++ -Function: - PAL_strtok - -Finds the next token in a string. - -Return value: - -A pointer to the next token found in strToken. Returns NULL when no more -tokens are found. Each call modifies strToken by substituting a NULL -character for each delimiter that is encountered. - -Parameters: -strToken String cotaining token(s) -strDelimit Set of delimiter characters - -Remarks: -In FreeBSD, strtok is not re-entrant, strtok_r is. It manages re-entrancy -by using a passed-in context pointer (which will be stored in thread local -storage) According to the strtok MSDN documentation, "Calling these functions -simultaneously from multiple threads does not have undesirable effects", so -we need to use strtok_r. ---*/ -char * -__cdecl -PAL_strtok(char *strToken, const char *strDelimit) -{ - CPalThread *pThread = NULL; - char *retval=NULL; - - PERF_ENTRY(strtok); - ENTRY("strtok (strToken=%p (%s), strDelimit=%p (%s))\n", - strToken?strToken:"NULL", - strToken?strToken:"NULL", strDelimit?strDelimit:"NULL", strDelimit?strDelimit:"NULL"); - - pThread = InternalGetCurrentThread(); - - retval = strtok_r(strToken, strDelimit, &pThread->crtInfo.strtokContext); - - LOGEXIT("strtok returns %p (%s)\n", retval?retval:"NULL", retval?retval:"NULL"); - PERF_EXIT(strtok); - - return retval; -} diff --git a/src/coreclr/pal/src/include/pal/palinternal.h b/src/coreclr/pal/src/include/pal/palinternal.h index 63e6d305d9f6e7..d9a3ec212848f2 100644 --- a/src/coreclr/pal/src/include/pal/palinternal.h +++ b/src/coreclr/pal/src/include/pal/palinternal.h @@ -375,7 +375,6 @@ function_name() to call the system's implementation #undef strtoul #undef strtoull #undef strtod -#undef strtok #undef strdup #undef tolower #undef toupper diff --git a/src/coreclr/pal/src/include/pal/thread.hpp b/src/coreclr/pal/src/include/pal/thread.hpp index 86850b260c9301..b6f5b265f39cd9 100644 --- a/src/coreclr/pal/src/include/pal/thread.hpp +++ b/src/coreclr/pal/src/include/pal/thread.hpp @@ -162,11 +162,9 @@ namespace CorUnix class CThreadCRTInfo : public CThreadInfoInitializer { public: - CHAR * strtokContext; // Context for strtok function WCHAR * wcstokContext; // Context for wcstok function CThreadCRTInfo() : - strtokContext(NULL), wcstokContext(NULL) { }; From 9cf7d8410476770e5f1eb39eb52480dae2317c59 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 19:18:34 +0000 Subject: [PATCH 02/10] Use strtok_s PAL API xplat --- src/coreclr/md/compiler/emit.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/coreclr/md/compiler/emit.cpp b/src/coreclr/md/compiler/emit.cpp index 6252deae8ff341..40fd1f68bf2aa1 100644 --- a/src/coreclr/md/compiler/emit.cpp +++ b/src/coreclr/md/compiler/emit.cpp @@ -1952,19 +1952,11 @@ STDMETHODIMP RegMeta::DefineDocument( // S_OK or error. partsIndexesCount++; } char* context; -#ifdef HOST_WINDOWS stringToken = strtok_s(docName, (const char*)delim, &context); -#else - stringToken = strtok_r(docName, (const char*)delim, &context); -#endif while (stringToken != NULL) { IfFailGo(m_pStgdb->m_MiniMd.m_BlobHeap.AddBlob(MetaData::DataBlob((BYTE*)stringToken, (ULONG)strlen(stringToken)), partsIndexesPtr++)); -#ifdef HOST_WINDOWS stringToken = strtok_s(NULL, (const char*)delim, &context); -#else - stringToken = strtok_r(NULL, (const char*)delim, &context); -#endif partsIndexesCount++; } From 8c4bab6e650cc3aa76de8b7da6bde2c2984ca216 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 19:33:45 +0000 Subject: [PATCH 03/10] Remove strtok usages in PAL tests --- src/coreclr/pal/inc/pal.h | 1 + src/coreclr/pal/src/include/pal/palinternal.h | 2 + src/coreclr/pal/tests/palsuite/CMakeLists.txt | 1 - .../palsuite/c_runtime/strtok/test1/test1.cpp | 77 ------------------- .../pal/tests/palsuite/compilableTests.txt | 1 - .../test1/GetModuleFileNameA.cpp | 5 +- .../test1/GetModuleFileNameW.cpp | 5 +- .../pal/tests/palsuite/paltestlist.txt | 1 - 8 files changed, 9 insertions(+), 84 deletions(-) delete mode 100644 src/coreclr/pal/tests/palsuite/c_runtime/strtok/test1/test1.cpp diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 18534271dc269a..1a67e6203501f0 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -4082,6 +4082,7 @@ PALIMPORT char * __cdecl strchr(const char *, int); PALIMPORT char * __cdecl strrchr(const char *, int); PALIMPORT char * __cdecl strpbrk(const char *, const char *); PALIMPORT char * __cdecl strstr(const char *, const char *); +PALIMPORT char * __cdecl strtok_r(char *, const char *, char **); PALIMPORT int __cdecl atoi(const char *); PALIMPORT ULONG __cdecl strtoul(const char *, char **, int); PALIMPORT ULONGLONG __cdecl strtoull(const char *, char **, int); diff --git a/src/coreclr/pal/src/include/pal/palinternal.h b/src/coreclr/pal/src/include/pal/palinternal.h index d9a3ec212848f2..2193307d214527 100644 --- a/src/coreclr/pal/src/include/pal/palinternal.h +++ b/src/coreclr/pal/src/include/pal/palinternal.h @@ -182,6 +182,7 @@ function_name() to call the system's implementation #define strrchr DUMMY_strrchr #define strpbrk DUMMY_strpbrk #define strtod DUMMY_strtod +#define strtok_r DUMMY_strtok_r #define tolower DUMMY_tolower #define toupper DUMMY_toupper #define isprint DUMMY_isprint @@ -375,6 +376,7 @@ function_name() to call the system's implementation #undef strtoul #undef strtoull #undef strtod +#undef strtok_r #undef strdup #undef tolower #undef toupper diff --git a/src/coreclr/pal/tests/palsuite/CMakeLists.txt b/src/coreclr/pal/tests/palsuite/CMakeLists.txt index ee5a58f608ec88..2acc3f31b413d4 100644 --- a/src/coreclr/pal/tests/palsuite/CMakeLists.txt +++ b/src/coreclr/pal/tests/palsuite/CMakeLists.txt @@ -198,7 +198,6 @@ add_executable_clr(paltests c_runtime/strstr/test1/test1.cpp c_runtime/strtod/test1/test1.cpp c_runtime/strtod/test2/test2.cpp - c_runtime/strtok/test1/test1.cpp c_runtime/strtoul/test1/test1.cpp c_runtime/tan/test1/test1.cpp c_runtime/tanf/test1/test1.cpp diff --git a/src/coreclr/pal/tests/palsuite/c_runtime/strtok/test1/test1.cpp b/src/coreclr/pal/tests/palsuite/c_runtime/strtok/test1/test1.cpp deleted file mode 100644 index 6eca5728abb4bf..00000000000000 --- a/src/coreclr/pal/tests/palsuite/c_runtime/strtok/test1/test1.cpp +++ /dev/null @@ -1,77 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -/*============================================================================ -** -** Source: test1.c -** -** Purpose: -** Search for a number of tokens within strings. Check that the return values -** are what is expect, and also that the strings match up with our expected -** results. -** -** -**==========================================================================*/ - -#include - - -PALTEST(c_runtime_strtok_test1_paltest_strtok_test1, "c_runtime/strtok/test1/paltest_strtok_test1") -{ - char str[] = "foo bar baz"; - char *result1= "foo \0ar baz"; - char *result2= "foo \0a\0 baz"; - int len = strlen(str) + 1; - char *ptr; - - - if (PAL_Initialize(argc, argv)) - { - return FAIL; - } - - - ptr = strtok(str, "bz"); - if (ptr != str) - { - Fail("Expected strtok() to return %p, got %p!\n", str, ptr); - } - if (memcmp(str, result1, len) != 0) - { - Fail("strtok altered the string in an unexpected way!\n"); - } - - ptr = strtok(NULL, "r "); - if (ptr != str + 5) - { - Fail("Expected strtok() to return %p, got %p!\n", str+5, ptr); - } - if (memcmp(str, result2, len) != 0) - { - Fail("strtok altered the string in an unexpected way!\n"); - } - - - ptr = strtok(NULL, "X"); - if (ptr != str + 7) - { - Fail("Expected strtok() to return %p, got %p!\n", str + 7, ptr); - } - if (memcmp(str, result2, len) != 0) - { - Fail("strtok altered the string in an unexpected way!\n"); - } - - ptr = strtok(NULL, "X"); - if (ptr != NULL) - { - Fail("Expected strtok() to return %p, got %p!\n", NULL, ptr); - } - if (memcmp(str, result2, len) != 0) - { - Fail("strtok altered the string in an unexpected way!\n"); - } - - PAL_Terminate(); - return PASS; -} diff --git a/src/coreclr/pal/tests/palsuite/compilableTests.txt b/src/coreclr/pal/tests/palsuite/compilableTests.txt index b96903dbc44d12..6b803fd1d4a832 100644 --- a/src/coreclr/pal/tests/palsuite/compilableTests.txt +++ b/src/coreclr/pal/tests/palsuite/compilableTests.txt @@ -132,7 +132,6 @@ c_runtime/strrchr/test1/paltest_strrchr_test1 c_runtime/strstr/test1/paltest_strstr_test1 c_runtime/strtod/test1/paltest_strtod_test1 c_runtime/strtod/test2/paltest_strtod_test2 -c_runtime/strtok/test1/paltest_strtok_test1 c_runtime/strtoul/test1/paltest_strtoul_test1 c_runtime/tan/test1/paltest_tan_test1 c_runtime/tanf/test1/paltest_tanf_test1 diff --git a/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameA/test1/GetModuleFileNameA.cpp b/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameA/test1/GetModuleFileNameA.cpp index c962a468396e5b..ad32b1014b0fc5 100644 --- a/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameA/test1/GetModuleFileNameA.cpp +++ b/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameA/test1/GetModuleFileNameA.cpp @@ -58,12 +58,13 @@ PALTEST(filemapping_memmgt_GetModuleFileNameA_test1_paltest_getmodulefilenamea_t MODULENAMEBUFFERSIZE); //strip out all full path - TempBuf = strtok(ModuleFileNameBuf,Delimiter); + char* context; + TempBuf = strtok_r(ModuleFileNameBuf,Delimiter, &context); LastBuf = TempBuf; while(NULL != TempBuf) { LastBuf = TempBuf; - TempBuf = strtok(NULL,Delimiter); + TempBuf = strtok_r(NULL,Delimiter, &context); } diff --git a/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameW/test1/GetModuleFileNameW.cpp b/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameW/test1/GetModuleFileNameW.cpp index cb2077b84c1e69..3e6085854b01ed 100644 --- a/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameW/test1/GetModuleFileNameW.cpp +++ b/src/coreclr/pal/tests/palsuite/filemapping_memmgt/GetModuleFileNameW/test1/GetModuleFileNameW.cpp @@ -73,12 +73,13 @@ PALTEST(filemapping_memmgt_GetModuleFileNameW_test1_paltest_getmodulefilenamew_t strcpy(NewModuleFileNameBuf,convertC(ModuleFileNameBuf)); //strip out all full path - TempBuf = strtok(NewModuleFileNameBuf,Delimiter); + char* context; + TempBuf = strtok_r(NewModuleFileNameBuf,Delimiter, &context); LastBuf = TempBuf; while(NULL != TempBuf) { LastBuf = TempBuf; - TempBuf = strtok(NULL,Delimiter); + TempBuf = strtok_r(NULL,Delimiter, &context); } diff --git a/src/coreclr/pal/tests/palsuite/paltestlist.txt b/src/coreclr/pal/tests/palsuite/paltestlist.txt index 7491006cc7e607..716b647caec26a 100644 --- a/src/coreclr/pal/tests/palsuite/paltestlist.txt +++ b/src/coreclr/pal/tests/palsuite/paltestlist.txt @@ -124,7 +124,6 @@ c_runtime/strrchr/test1/paltest_strrchr_test1 c_runtime/strstr/test1/paltest_strstr_test1 c_runtime/strtod/test1/paltest_strtod_test1 c_runtime/strtod/test2/paltest_strtod_test2 -c_runtime/strtok/test1/paltest_strtok_test1 c_runtime/strtoul/test1/paltest_strtoul_test1 c_runtime/tan/test1/paltest_tan_test1 c_runtime/tanf/test1/paltest_tanf_test1 From b16e13dd72b30709b7e418b0663d6e08975ef52b Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 19:35:45 +0000 Subject: [PATCH 04/10] Revert "Use strtok_s PAL API xplat" This reverts commit 9cf7d8410476770e5f1eb39eb52480dae2317c59. --- src/coreclr/md/compiler/emit.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/md/compiler/emit.cpp b/src/coreclr/md/compiler/emit.cpp index 40fd1f68bf2aa1..6252deae8ff341 100644 --- a/src/coreclr/md/compiler/emit.cpp +++ b/src/coreclr/md/compiler/emit.cpp @@ -1952,11 +1952,19 @@ STDMETHODIMP RegMeta::DefineDocument( // S_OK or error. partsIndexesCount++; } char* context; +#ifdef HOST_WINDOWS stringToken = strtok_s(docName, (const char*)delim, &context); +#else + stringToken = strtok_r(docName, (const char*)delim, &context); +#endif while (stringToken != NULL) { IfFailGo(m_pStgdb->m_MiniMd.m_BlobHeap.AddBlob(MetaData::DataBlob((BYTE*)stringToken, (ULONG)strlen(stringToken)), partsIndexesPtr++)); +#ifdef HOST_WINDOWS stringToken = strtok_s(NULL, (const char*)delim, &context); +#else + stringToken = strtok_r(NULL, (const char*)delim, &context); +#endif partsIndexesCount++; } From 52353221d970533256b6fa728ab7a320f4204728 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 19:45:39 +0000 Subject: [PATCH 05/10] Remove strtok_s PAL implementation, just use system-provided strtok_r --- src/coreclr/pal/inc/mbusafecrt.h | 1 - src/coreclr/pal/inc/rt/safecrt.h | 81 +------------------ src/coreclr/pal/src/CMakeLists.txt | 1 - src/coreclr/pal/src/misc/cgroup.cpp | 12 +-- src/coreclr/pal/src/safecrt/strtok_s.cpp | 26 ------ src/coreclr/pal/src/safecrt/tcstok_s.inl | 2 +- src/coreclr/pal/src/safecrt/xtoa_s.cpp | 5 +- src/coreclr/pal/src/safecrt/xtow_s.cpp | 5 +- .../vm/eventing/eventpipe/ep-rt-coreclr.h | 4 + 9 files changed, 17 insertions(+), 120 deletions(-) delete mode 100644 src/coreclr/pal/src/safecrt/strtok_s.cpp diff --git a/src/coreclr/pal/inc/mbusafecrt.h b/src/coreclr/pal/inc/mbusafecrt.h index 3ea804f09ad8af..150f17ce5c971c 100644 --- a/src/coreclr/pal/inc/mbusafecrt.h +++ b/src/coreclr/pal/inc/mbusafecrt.h @@ -57,7 +57,6 @@ extern errno_t wcscpy_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* i extern errno_t strncpy_s( char* outDest, size_t inDestBufferSize, const char* inSrc, size_t inCount ); extern errno_t wcsncpy_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* inSrc, size_t inCount ); -extern char* strtok_s( char* inString, const char* inControl, char** ioContext ); extern WCHAR* wcstok_s( WCHAR* inString, const WCHAR* inControl, WCHAR** ioContext ); // strnlen is not required unless the source string is completely untrusted (e.g. anonymous input on a website) diff --git a/src/coreclr/pal/inc/rt/safecrt.h b/src/coreclr/pal/inc/rt/safecrt.h index 007b3deae7e1d6..a53d34f49b700b 100644 --- a/src/coreclr/pal/inc/rt/safecrt.h +++ b/src/coreclr/pal/inc/rt/safecrt.h @@ -370,7 +370,7 @@ void __cdecl _invalid_parameter(const WCHAR *_Message, const WCHAR *_FunctionNam #define _tcsncat_s strncat_s #define _tcsset_s _strset_s #define _tcsnset_s _strnset_s -#define _tcstok_s strtok_s +#define _tcstok_s strtok_r #define _vsntprintf_s _vsnprintf_s #elif defined(_UNICODE) || defined(UNICODE) @@ -1118,87 +1118,10 @@ errno_t __cdecl _wcsnset_s(WCHAR *_Dst, size_t _SizeInWords, WCHAR _Value, size_ #endif -/* strtok_s */ +/* wcstok_s */ /* - * strtok_s, wcstok_s ; * uses _Context to keep track of the position in the string. */ -_SAFECRT__EXTERN_C -char * __cdecl strtok_s(char *_String, const char *_Control, char **_Context); - -#if _SAFECRT_USE_INLINES || _SAFECRT_IMPL - -_SAFECRT__INLINE -char * __cdecl strtok_s(char *_String, const char *_Control, char **_Context) -{ - unsigned char *str; - const unsigned char *ctl = (const unsigned char *)_Control; - unsigned char map[32]; - int count; - - /* validation section */ - _SAFECRT__VALIDATE_POINTER_ERROR_RETURN(_Context, EINVAL, nullptr); - _SAFECRT__VALIDATE_POINTER_ERROR_RETURN(_Control, EINVAL, nullptr); - _SAFECRT__VALIDATE_CONDITION_ERROR_RETURN(_String != nullptr || *_Context != nullptr, EINVAL, nullptr); - - /* Clear control map */ - for (count = 0; count < 32; count++) - { - map[count] = 0; - } - - /* Set bits in delimiter table */ - do { - map[*ctl >> 3] |= (1 << (*ctl & 7)); - } while (*ctl++); - - /* If string is nullptr, set str to the saved - * pointer (i.e., continue breaking tokens out of the string - * from the last strtok call) */ - if (_String != nullptr) - { - str = (unsigned char *)_String; - } - else - { - str = (unsigned char *)*_Context; - } - - /* Find beginning of token (skip over leading delimiters). Note that - * there is no token iff this loop sets str to point to the terminal - * null (*str == 0) */ - while ((map[*str >> 3] & (1 << (*str & 7))) && *str != 0) - { - str++; - } - - _String = (char *)str; - - /* Find the end of the token. If it is not the end of the string, - * put a null there. */ - for ( ; *str != 0 ; str++ ) - { - if (map[*str >> 3] & (1 << (*str & 7))) - { - *str++ = 0; - break; - } - } - - /* Update context */ - *_Context = (char *)str; - - /* Determine if a token has been found. */ - if (_String == (char *)str) - { - return nullptr; - } - else - { - return _String; - } -} -#endif /* wcstok_s */ _SAFECRT__EXTERN_C diff --git a/src/coreclr/pal/src/CMakeLists.txt b/src/coreclr/pal/src/CMakeLists.txt index bfbc067c893151..d0fbf18f9daa29 100644 --- a/src/coreclr/pal/src/CMakeLists.txt +++ b/src/coreclr/pal/src/CMakeLists.txt @@ -186,7 +186,6 @@ set(SOURCES safecrt/strlen_s.cpp safecrt/strncat_s.cpp safecrt/strncpy_s.cpp - safecrt/strtok_s.cpp safecrt/vsprintf.cpp safecrt/wcscat_s.cpp safecrt/wcscpy_s.cpp diff --git a/src/coreclr/pal/src/misc/cgroup.cpp b/src/coreclr/pal/src/misc/cgroup.cpp index d80cfe0cf64614..ee3c0ae5843929 100644 --- a/src/coreclr/pal/src/misc/cgroup.cpp +++ b/src/coreclr/pal/src/misc/cgroup.cpp @@ -218,11 +218,11 @@ class CGroup if (!isSubsystemMatch) { char* context = nullptr; - char* strTok = strtok_s(options, ",", &context); + char* strTok = strtok_r(options, ",", &context); while (!isSubsystemMatch && strTok != nullptr) { isSubsystemMatch = is_subsystem(strTok); - strTok = strtok_s(nullptr, ",", &context); + strTok = strtok_r(nullptr, ",", &context); } } if (isSubsystemMatch) @@ -302,7 +302,7 @@ class CGroup } char* context = nullptr; - char* strTok = strtok_s(subsystem_list, ",", &context); + char* strTok = strtok_r(subsystem_list, ",", &context); while (strTok != nullptr) { if (is_subsystem(strTok)) @@ -310,7 +310,7 @@ class CGroup result = true; break; } - strTok = strtok_s(nullptr, ",", &context); + strTok = strtok_r(nullptr, ",", &context); } } else if (s_cgroup_version == 2) @@ -395,13 +395,13 @@ class CGroup // $MAX $PERIOD // Where "$MAX" may be the string literal "max" - max_quota_string = strtok_s(line, " ", &context); + max_quota_string = strtok_r(line, " ", &context); if (max_quota_string == nullptr) { _ASSERTE(!"Unable to parse " CGROUP2_CPU_MAX_FILENAME " file contents."); goto done; } - period_string = strtok_s(nullptr, " ", &context); + period_string = strtok_r(nullptr, " ", &context); if (period_string == nullptr) { _ASSERTE(!"Unable to parse " CGROUP2_CPU_MAX_FILENAME " file contents."); diff --git a/src/coreclr/pal/src/safecrt/strtok_s.cpp b/src/coreclr/pal/src/safecrt/strtok_s.cpp deleted file mode 100644 index c183ad5ab3433c..00000000000000 --- a/src/coreclr/pal/src/safecrt/strtok_s.cpp +++ /dev/null @@ -1,26 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -/*** -*strtok_s.c - tokenize a string with given delimiters -* - -* -*Purpose: -* defines strtok_s() - breaks string into series of token -* via repeated calls. -* -*******************************************************************************/ - -#include -#include -#include -#include "internal_securecrt.h" - -#include "mbusafecrt_internal.h" - -#define _FUNC_PROLOGUE -#define _FUNC_NAME strtok_s -#define _CHAR char - -#include "tcstok_s.inl" diff --git a/src/coreclr/pal/src/safecrt/tcstok_s.inl b/src/coreclr/pal/src/safecrt/tcstok_s.inl index f3ed934c19b294..9314212a9fc9f3 100644 --- a/src/coreclr/pal/src/safecrt/tcstok_s.inl +++ b/src/coreclr/pal/src/safecrt/tcstok_s.inl @@ -7,7 +7,7 @@ * *Purpose: -* This file contains the general algorithm for strtok_s and its variants. +* This file contains the general algorithm for wcstok_s and its variants. * ****/ diff --git a/src/coreclr/pal/src/safecrt/xtoa_s.cpp b/src/coreclr/pal/src/safecrt/xtoa_s.cpp index e535b1b388c5e6..d930adb15644af 100644 --- a/src/coreclr/pal/src/safecrt/xtoa_s.cpp +++ b/src/coreclr/pal/src/safecrt/xtoa_s.cpp @@ -2,13 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. /*** -*strtok_s.c - tokenize a string with given delimiters +*xtoa_s.c - convert integers to ASCII strings * * *Purpose: -* defines strtok_s() - breaks string into series of token -* via repeated calls. +* defines _*tox_s() functions - convert integers to ASCII strings * *******************************************************************************/ #include "pal/palinternal.h" diff --git a/src/coreclr/pal/src/safecrt/xtow_s.cpp b/src/coreclr/pal/src/safecrt/xtow_s.cpp index 4a5d077532bba0..127a1e766fb712 100644 --- a/src/coreclr/pal/src/safecrt/xtow_s.cpp +++ b/src/coreclr/pal/src/safecrt/xtow_s.cpp @@ -2,13 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. /*** -*strtok_s.c - tokenize a string with given delimiters +*xtow_s.c - convert integers to UTF-16 strings * * *Purpose: -* defines strtok_s() - breaks string into series of token -* via repeated calls. +* defines _*tox_s() functions - convert integers to UTF-16 strings * *******************************************************************************/ diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h index f5a07a3f1adaa2..95c568f50acbfe 100644 --- a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h @@ -1370,7 +1370,11 @@ ep_rt_utf8_string_strtok ( ep_char8_t **context) { STATIC_CONTRACT_NOTHROW; +#ifdef HOST_WINDOWS return strtok_s (str, delimiter, context); +#else + return strtok_r (str, delimiter, context); +#endif } // STATIC_CONTRACT_NOTHROW From e6344bcc592f8cbff6d1d99fd4d282db0fde9d09 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 22:05:07 +0000 Subject: [PATCH 06/10] Convert ProfilingAPIUtility::AttemptLoadProfilerList to use SString instead of wcstok_s. --- src/coreclr/vm/profilinghelper.cpp | 35 +++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/coreclr/vm/profilinghelper.cpp b/src/coreclr/vm/profilinghelper.cpp index 7d615f092e868c..6a01e773510770 100644 --- a/src/coreclr/vm/profilinghelper.cpp +++ b/src/coreclr/vm/profilinghelper.cpp @@ -773,7 +773,7 @@ HRESULT ProfilingAPIUtility::AttemptLoadDelayedStartupProfilers() HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() { HRESULT hr = S_OK; - NewArrayHolder wszProfilerList(NULL); + CLRConfigStringHolder wszProfilerList(NULL); #if defined(TARGET_ARM64) CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_CORECLR_NOTIFICATION_PROFILERS_ARM64, &wszProfilerList); @@ -806,23 +806,28 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() return S_OK; } - WCHAR *pOuter = NULL; - WCHAR *pInner = NULL; - WCHAR *currentSection = NULL; - WCHAR *currentPath = NULL; - WCHAR *currentGuid = NULL; + SString profilerList{wszProfilerList}; HRESULT storedHr = S_OK; - // Get each semicolon delimited config - currentSection = wcstok_s(wszProfilerList, W(";"), &pOuter); - for (;currentSection != NULL; currentSection = wcstok_s(NULL, W(";"), &pOuter)) + for (SString::Iterator sectionStart = profilerList.Begin(), sectionEnd = profilerList.Begin(); + profilerList.Find(sectionEnd, W(';')); + sectionStart = ++sectionEnd) { - // Parse this config "path={guid}" - currentPath = wcstok_s(currentSection, W("="), &pInner); - currentGuid = wcstok_s(NULL, W("="), &pInner); - + SString::Iterator pathEnd = sectionStart; + if (!profilerList.Find(pathEnd, W('=')) || pathEnd > sectionEnd) + { + ProfilingAPIUtility::LogProfError(IDS_E_PROF_BAD_PATH); + storedHr = E_FAIL; + continue; + } + SString::Iterator clsidStart = pathEnd + 1; + profilerList.Replace(pathEnd, W('\0')); + profilerList.Replace(sectionEnd, W('\0')); + + SString clsidString{profilerList.GetUnicode(clsidStart)}; + NewArrayHolder clsidStringRaw = clsidString.GetCopyOfUnicodeString(); CLSID clsid; - hr = ProfilingAPIUtility::ProfilerCLSIDFromString(currentGuid, &clsid); + hr = ProfilingAPIUtility::ProfilerCLSIDFromString(clsidStringRaw, &clsid); if (FAILED(hr)) { // ProfilerCLSIDFromString already logged an event if there was a failure @@ -836,7 +841,7 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() kStartupLoad, &clsid, (LPCSTR)clsidUtf8, - currentPath, + profilerList.GetUnicode(sectionStart), NULL, // No client data for startup load 0); // No client data for startup load if (FAILED(hr)) From 10c71b6e16eb4ef2e762b694d3fbd596638d3898 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 22:11:29 +0000 Subject: [PATCH 07/10] Remove wcstok_s from the PAL --- src/coreclr/pal/inc/mbusafecrt.h | 3 +- src/coreclr/pal/inc/rt/safecrt.h | 72 ---------------------- src/coreclr/pal/src/CMakeLists.txt | 1 - src/coreclr/pal/src/include/pal/thread.hpp | 12 ---- src/coreclr/pal/src/safecrt/tcstok_s.inl | 70 --------------------- src/coreclr/pal/src/safecrt/wcstok_s.cpp | 26 -------- src/coreclr/pal/src/thread/thread.cpp | 12 ---- 7 files changed, 1 insertion(+), 195 deletions(-) delete mode 100644 src/coreclr/pal/src/safecrt/tcstok_s.inl delete mode 100644 src/coreclr/pal/src/safecrt/wcstok_s.cpp diff --git a/src/coreclr/pal/inc/mbusafecrt.h b/src/coreclr/pal/inc/mbusafecrt.h index 150f17ce5c971c..2d72863299f036 100644 --- a/src/coreclr/pal/inc/mbusafecrt.h +++ b/src/coreclr/pal/inc/mbusafecrt.h @@ -56,8 +56,7 @@ extern errno_t wcscpy_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* i extern errno_t strncpy_s( char* outDest, size_t inDestBufferSize, const char* inSrc, size_t inCount ); extern errno_t wcsncpy_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* inSrc, size_t inCount ); - -extern WCHAR* wcstok_s( WCHAR* inString, const WCHAR* inControl, WCHAR** ioContext ); +extern errno_t wcsncpy_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* inSrc, size_t inCount ); // strnlen is not required unless the source string is completely untrusted (e.g. anonymous input on a website) #ifndef SUPPRESS_STRNLEN diff --git a/src/coreclr/pal/inc/rt/safecrt.h b/src/coreclr/pal/inc/rt/safecrt.h index a53d34f49b700b..8579b30eefa7e6 100644 --- a/src/coreclr/pal/inc/rt/safecrt.h +++ b/src/coreclr/pal/inc/rt/safecrt.h @@ -370,7 +370,6 @@ void __cdecl _invalid_parameter(const WCHAR *_Message, const WCHAR *_FunctionNam #define _tcsncat_s strncat_s #define _tcsset_s _strset_s #define _tcsnset_s _strnset_s -#define _tcstok_s strtok_r #define _vsntprintf_s _vsnprintf_s #elif defined(_UNICODE) || defined(UNICODE) @@ -381,7 +380,6 @@ void __cdecl _invalid_parameter(const WCHAR *_Message, const WCHAR *_FunctionNam #define _tcsncat_s wcsncat_s #define _tcsset_s _wcsset_s #define _tcsnset_s _wcsnset_s -#define _tcstok_s wcstok_s #define _tmakepath_s _wmakepath_s #define _stprintf_s swprintf_s #define _tscanf_s wscanf_s @@ -1118,76 +1116,6 @@ errno_t __cdecl _wcsnset_s(WCHAR *_Dst, size_t _SizeInWords, WCHAR _Value, size_ #endif -/* wcstok_s */ -/* - * uses _Context to keep track of the position in the string. - */ - -/* wcstok_s */ -_SAFECRT__EXTERN_C -WCHAR * __cdecl wcstok_s(WCHAR *_String, const WCHAR *_Control, WCHAR **_Context); - -#if _SAFECRT_USE_INLINES || _SAFECRT_IMPL - -_SAFECRT__INLINE -WCHAR * __cdecl wcstok_s(WCHAR *_String, const WCHAR *_Control, WCHAR **_Context) -{ - WCHAR *token; - const WCHAR *ctl; - - /* validation section */ - _SAFECRT__VALIDATE_POINTER_ERROR_RETURN(_Context, EINVAL, nullptr); - _SAFECRT__VALIDATE_POINTER_ERROR_RETURN(_Control, EINVAL, nullptr); - _SAFECRT__VALIDATE_CONDITION_ERROR_RETURN(_String != nullptr || *_Context != nullptr, EINVAL, nullptr); - - /* If string==nullptr, continue with previous string */ - if (!_String) - { - _String = *_Context; - } - - /* Find beginning of token (skip over leading delimiters). Note that - * there is no token iff this loop sets string to point to the terminal null. */ - for ( ; *_String != 0 ; _String++) - { - for (ctl = _Control; *ctl != 0 && *ctl != *_String; ctl++) - ; - if (*ctl == 0) - { - break; - } - } - - token = _String; - - /* Find the end of the token. If it is not the end of the string, - * put a null there. */ - for ( ; *_String != 0 ; _String++) - { - for (ctl = _Control; *ctl != 0 && *ctl != *_String; ctl++) - ; - if (*ctl != 0) - { - *_String++ = 0; - break; - } - } - - /* Update the context */ - *_Context = _String; - - /* Determine if a token has been found. */ - if (token == _String) - { - return nullptr; - } - else - { - return token; - } -} -#endif - #ifndef PAL_STDCPP_COMPAT /* strnlen */ /* diff --git a/src/coreclr/pal/src/CMakeLists.txt b/src/coreclr/pal/src/CMakeLists.txt index d0fbf18f9daa29..291f7ee2b031bf 100644 --- a/src/coreclr/pal/src/CMakeLists.txt +++ b/src/coreclr/pal/src/CMakeLists.txt @@ -193,7 +193,6 @@ set(SOURCES safecrt/wcslwr_s.cpp safecrt/wcsncat_s.cpp safecrt/wcsncpy_s.cpp - safecrt/wcstok_s.cpp safecrt/wmakepath_s.cpp safecrt/xtoa_s.cpp safecrt/xtow_s.cpp diff --git a/src/coreclr/pal/src/include/pal/thread.hpp b/src/coreclr/pal/src/include/pal/thread.hpp index b6f5b265f39cd9..411040f39b21a5 100644 --- a/src/coreclr/pal/src/include/pal/thread.hpp +++ b/src/coreclr/pal/src/include/pal/thread.hpp @@ -159,17 +159,6 @@ namespace CorUnix }; #endif // HAVE_MACH_EXCEPTIONS - class CThreadCRTInfo : public CThreadInfoInitializer - { - public: - WCHAR * wcstokContext; // Context for wcstok function - - CThreadCRTInfo() : - wcstokContext(NULL) - { - }; - }; - class CPalThread { friend @@ -328,7 +317,6 @@ namespace CorUnix CThreadSynchronizationInfo synchronizationInfo; CThreadSuspensionInfo suspensionInfo; CThreadApcInfo apcInfo; - CThreadCRTInfo crtInfo; CPalThread() : diff --git a/src/coreclr/pal/src/safecrt/tcstok_s.inl b/src/coreclr/pal/src/safecrt/tcstok_s.inl deleted file mode 100644 index 9314212a9fc9f3..00000000000000 --- a/src/coreclr/pal/src/safecrt/tcstok_s.inl +++ /dev/null @@ -1,70 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -/*** -*tcstok_s.inl - general implementation of _tcstok_s -* - -* -*Purpose: -* This file contains the general algorithm for wcstok_s and its variants. -* -****/ - -_FUNC_PROLOGUE -_CHAR * __cdecl _FUNC_NAME(_CHAR *_String, const _CHAR *_Control, _CHAR **_Context) -{ - _CHAR *token; - const _CHAR *ctl; - - /* validation section */ - _VALIDATE_POINTER_ERROR_RETURN(_Context, EINVAL, NULL); - _VALIDATE_POINTER_ERROR_RETURN(_Control, EINVAL, NULL); - _VALIDATE_CONDITION_ERROR_RETURN(_String != NULL || *_Context != NULL, EINVAL, NULL); - - /* If string==NULL, continue with previous string */ - if (!_String) - { - _String = *_Context; - } - - /* Find beginning of token (skip over leading delimiters). Note that - * there is no token iff this loop sets string to point to the terminal null. */ - for ( ; *_String != 0 ; _String++) - { - for (ctl = _Control; *ctl != 0 && *ctl != *_String; ctl++) - ; - if (*ctl == 0) - { - break; - } - } - - token = _String; - - /* Find the end of the token. If it is not the end of the string, - * put a null there. */ - for ( ; *_String != 0 ; _String++) - { - for (ctl = _Control; *ctl != 0 && *ctl != *_String; ctl++) - ; - if (*ctl != 0) - { - *_String++ = 0; - break; - } - } - - /* Update the context */ - *_Context = _String; - - /* Determine if a token has been found. */ - if (token == _String) - { - return NULL; - } - else - { - return token; - } -} diff --git a/src/coreclr/pal/src/safecrt/wcstok_s.cpp b/src/coreclr/pal/src/safecrt/wcstok_s.cpp deleted file mode 100644 index 83751d6565587c..00000000000000 --- a/src/coreclr/pal/src/safecrt/wcstok_s.cpp +++ /dev/null @@ -1,26 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -/*** -*wcstok_s.c - tokenize a wide-character string with given delimiters -* - -* -*Purpose: -* defines wcstok_s() - breaks wide-character string into series of token -* via repeated calls. -* -*******************************************************************************/ - -#include -#include -#include -#include "internal_securecrt.h" - -#include "mbusafecrt_internal.h" - -#define _FUNC_PROLOGUE -#define _FUNC_NAME wcstok_s -#define _CHAR char16_t - -#include "tcstok_s.inl" diff --git a/src/coreclr/pal/src/thread/thread.cpp b/src/coreclr/pal/src/thread/thread.cpp index 90501bbafbde56..9420a442c1f6ae 100644 --- a/src/coreclr/pal/src/thread/thread.cpp +++ b/src/coreclr/pal/src/thread/thread.cpp @@ -2220,12 +2220,6 @@ CPalThread::RunPreCreateInitializers( goto RunPreCreateInitializersExit; } - palError = crtInfo.InitializePreCreate(); - if (NO_ERROR != palError) - { - goto RunPreCreateInitializersExit; - } - RunPreCreateInitializersExit: return palError; @@ -2318,12 +2312,6 @@ CPalThread::RunPostCreateInitializers( goto RunPostCreateInitializersExit; } - palError = crtInfo.InitializePostCreate(this, m_threadId, m_dwLwpId); - if (NO_ERROR != palError) - { - goto RunPostCreateInitializersExit; - } - palError = SEHEnable(this); if (NO_ERROR != palError) { From d98bb30e056ec6902d64eb65667f11b52c88c0d1 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Feb 2024 22:53:39 +0000 Subject: [PATCH 08/10] Remove one string copy --- src/coreclr/vm/profilinghelper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/profilinghelper.cpp b/src/coreclr/vm/profilinghelper.cpp index 6a01e773510770..3e874b942d5b74 100644 --- a/src/coreclr/vm/profilinghelper.cpp +++ b/src/coreclr/vm/profilinghelper.cpp @@ -824,7 +824,7 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() profilerList.Replace(pathEnd, W('\0')); profilerList.Replace(sectionEnd, W('\0')); - SString clsidString{profilerList.GetUnicode(clsidStart)}; + SString clsidString{SString::Literal, profilerList.GetUnicode(clsidStart)}; NewArrayHolder clsidStringRaw = clsidString.GetCopyOfUnicodeString(); CLSID clsid; hr = ProfilingAPIUtility::ProfilerCLSIDFromString(clsidStringRaw, &clsid); From f5f1cd64fee7843405788c152e05bd439f473167 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 6 Feb 2024 09:28:45 -0800 Subject: [PATCH 09/10] Create substrings with SString constructor, use inline-storage types to avoid heap allocations --- src/coreclr/vm/profilinghelper.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/profilinghelper.cpp b/src/coreclr/vm/profilinghelper.cpp index 3e874b942d5b74..2593811e1cd9fa 100644 --- a/src/coreclr/vm/profilinghelper.cpp +++ b/src/coreclr/vm/profilinghelper.cpp @@ -821,10 +821,9 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() continue; } SString::Iterator clsidStart = pathEnd + 1; - profilerList.Replace(pathEnd, W('\0')); - profilerList.Replace(sectionEnd, W('\0')); - - SString clsidString{SString::Literal, profilerList.GetUnicode(clsidStart)}; + + PathString path{profilerList, sectionStart, pathEnd}; + StackSString clsidString{profilerList, clsidStart, sectionEnd}; NewArrayHolder clsidStringRaw = clsidString.GetCopyOfUnicodeString(); CLSID clsid; hr = ProfilingAPIUtility::ProfilerCLSIDFromString(clsidStringRaw, &clsid); @@ -841,7 +840,7 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() kStartupLoad, &clsid, (LPCSTR)clsidUtf8, - profilerList.GetUnicode(sectionStart), + path.GetUnicode(), NULL, // No client data for startup load 0); // No client data for startup load if (FAILED(hr)) From 04ff39b91094f070d0dd1076ed005b56f7092512 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 6 Feb 2024 12:24:03 -0800 Subject: [PATCH 10/10] Change ProfilerCLSIDFromString to not modify the passed-in string --- src/coreclr/vm/profilinghelper.cpp | 32 +++++++++--------------------- src/coreclr/vm/profilinghelper.h | 2 +- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/coreclr/vm/profilinghelper.cpp b/src/coreclr/vm/profilinghelper.cpp index 2593811e1cd9fa..49d95f3b9dedd4 100644 --- a/src/coreclr/vm/profilinghelper.cpp +++ b/src/coreclr/vm/profilinghelper.cpp @@ -507,11 +507,10 @@ HRESULT ProfilingAPIUtility::InitializeProfiling() // corresponding CLSID structure. // // Arguments: -// * wszClsid - [in / out] CLSID string to convert. This may also be a progid. This +// * wszClsid - [in] CLSID string to convert. This may also be a progid. This // ensures our behavior is backward-compatible with previous CLR versions. I don't // know why previous versions allowed the user to set a progid in the environment, -// but well whatever. On [out], this string is normalized in-place (e.g., -// double-quotes around progid are removed). +// but well whatever. For back-compatibility, we also strip all double-quotes from the passed in ProgId. // * pClsid - [out] CLSID structure corresponding to wszClsid // // Return Value: @@ -523,7 +522,7 @@ HRESULT ProfilingAPIUtility::InitializeProfiling() // static HRESULT ProfilingAPIUtility::ProfilerCLSIDFromString( - __inout_z LPWSTR wszClsid, + __in_z LPCWSTR wszClsid, CLSID * pClsid) { CONTRACTL @@ -552,27 +551,15 @@ HRESULT ProfilingAPIUtility::ProfilerCLSIDFromString( else { #ifndef TARGET_UNIX - WCHAR *szFrom, *szTo; - -#ifdef _PREFAST_ -#pragma warning(push) -#pragma warning(disable:26000) // "espX thinks there is an overflow here, but there isn't any" -#endif - for (szFrom=szTo=wszClsid; *szFrom; ) + SString progId{wszClsid}; + for (auto it = progId.Begin(); it != progId.End(); ++it) { - if (*szFrom == W('"')) + while (it != progId.End() && *it == W('"')) { - ++szFrom; - continue; + progId.Delete(it, 1); } - *szTo++ = *szFrom++; } - *szTo = 0; - hr = CLSIDFromProgID(wszClsid, pClsid); -#ifdef _PREFAST_ -#pragma warning(pop) -#endif /*_PREFAST_*/ - + hr = CLSIDFromProgID(progId.GetUnicode(), pClsid); #else // !TARGET_UNIX // ProgID not supported on TARGET_UNIX hr = E_INVALIDARG; @@ -824,9 +811,8 @@ HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() PathString path{profilerList, sectionStart, pathEnd}; StackSString clsidString{profilerList, clsidStart, sectionEnd}; - NewArrayHolder clsidStringRaw = clsidString.GetCopyOfUnicodeString(); CLSID clsid; - hr = ProfilingAPIUtility::ProfilerCLSIDFromString(clsidStringRaw, &clsid); + hr = ProfilingAPIUtility::ProfilerCLSIDFromString(clsidString.GetUnicode(), &clsid); if (FAILED(hr)) { // ProfilerCLSIDFromString already logged an event if there was a failure diff --git a/src/coreclr/vm/profilinghelper.h b/src/coreclr/vm/profilinghelper.h index 51b0242db84e52..e33071062426a0 100644 --- a/src/coreclr/vm/profilinghelper.h +++ b/src/coreclr/vm/profilinghelper.h @@ -106,7 +106,7 @@ class ProfilingAPIUtility LPVOID pvClientData, UINT cbClientData, DWORD dwConcurrentGCWaitTimeoutInMs = INFINITE); - static HRESULT ProfilerCLSIDFromString(__inout_z LPWSTR wszClsid, CLSID * pClsid); + static HRESULT ProfilerCLSIDFromString(__in_z LPCWSTR wszClsid, CLSID * pClsid); static HRESULT AttemptLoadProfilerForStartup(); static HRESULT AttemptLoadDelayedStartupProfilers(); static HRESULT AttemptLoadProfilerList();