Mark SString's single argument ctors as explicit (#91822)

* Mark SString's single ctor's as explicit

Add move ctor to SString and SBuffer
Address all places that were implicitly allocating

src/coreclr/binder/bindertracing.cpp

@@ -119,8 +119,8 @@ namespace BinderTracing
 {
     static thread_local bool t_AssemblyLoadStartInProgress = false;
 
-    AssemblyBindOperation::AssemblyBindOperation(AssemblySpec *assemblySpec, const SString& assemblyPath)
-        : m_bindRequest { assemblySpec, SString::Empty(), assemblyPath }
+    AssemblyBindOperation::AssemblyBindOperation(AssemblySpec *assemblySpec, const WCHAR* assemblyPath)
+        : m_bindRequest { assemblySpec, SString{ SString::Empty() }, SString{ assemblyPath } }
         , m_populatedBindRequest { false }
         , m_checkedIgnoreBind { false }
         , m_ignoreBind { false }

src/coreclr/binder/inc/bindertracing.h

@@ -27,7 +27,7 @@ namespace BinderTracing
     {
     public:
         // This class assumes the assembly spec will have a longer lifetime than itself
-        AssemblyBindOperation(AssemblySpec *assemblySpec, const SString& assemblyPath = SString::Empty());
+        AssemblyBindOperation(AssemblySpec *assemblySpec, const WCHAR* assemblyPath = NULL);
         ~AssemblyBindOperation();
 
         void SetResult(PEAssembly *assembly, bool cached = false);

src/coreclr/debug/di/symbolinfo.cpp

@@ -178,7 +178,7 @@ HRESULT SymbolInfo::SetMethodProps(mdToken method, mdToken cls, LPCWSTR wszName)
     {
         m_LastMethod.method=method;
         m_LastMethod.cls=cls;
-        m_LastMethod.wszName=wszName;
+        m_LastMethod.wszName.Set(wszName);
         m_LastMethod.wszName.Normalize();
     }
     EX_CATCH_HRESULT(hr)

src/coreclr/inc/sarray.h

@@ -206,6 +206,8 @@ class EMPTY_BASES_DECL InlineSArray : public SArray<ELEMENT, BITWISE_COPY>
 // StackSArray : SArray with relatively large preallocated buffer for stack use
 // ================================================================================
 
+#define STACK_ALLOC 256
+
 template <typename ELEMENT, BOOL BITWISE_COPY = TRUE>
 class EMPTY_BASES_DECL StackSArray : public InlineSArray<ELEMENT, STACK_ALLOC/sizeof(ELEMENT), BITWISE_COPY>
 {

src/coreclr/inc/sbuffer.h

@@ -97,6 +97,7 @@ class SBuffer
     SBuffer(COUNT_T size);
     SBuffer(const BYTE *buffer, COUNT_T size);
     explicit SBuffer(const SBuffer &buffer);
+    SBuffer(SBuffer &&buffer);
 
     // Immutable constructor should ONLY be used if buffer will
     // NEVER BE FREED OR MODIFIED. PERIOD. .
@@ -108,6 +109,10 @@ class SBuffer
 
     ~SBuffer();
 
+  private:
+    void InitializeInstance();
+
+  public:
     void Clear();
 
     void Set(const SBuffer &buffer);
@@ -550,13 +555,6 @@ class EMPTY_BASES_DECL InlineSBuffer : public SBuffer
 #define GARBAGE_FILL_DWORD  0x24242424 // $$$$
 #define GARBAGE_FILL_BUFFER_ITEMS 16
 #define GARBAGE_FILL_BUFFER_SIZE GARBAGE_FILL_BUFFER_ITEMS*sizeof(DWORD)
-// ================================================================================
-// StackSBuffer : SBuffer with relatively large preallocated buffer for stack use
-// ================================================================================
-
-#define STACK_ALLOC 256
-
-typedef InlineSBuffer<STACK_ALLOC> StackSBuffer;
 
 // ================================================================================
 // Inline definitions

src/coreclr/inc/sbuffer.inl

@@ -114,6 +114,32 @@ inline SBuffer::SBuffer(const SBuffer &buffer)
     RETURN;
 }
 
+inline SBuffer::SBuffer(SBuffer &&buffer)
+{
+    CONTRACT_VOID
+    {
+        CONSTRUCTOR_CHECK;
+        PRECONDITION(buffer.Check());
+        POSTCONDITION(Check());
+        THROWS;
+        GC_NOTRIGGER;
+    }
+    CONTRACT_END;
+
+    m_size = buffer.m_size;
+    m_allocation = buffer.m_allocation;
+    m_flags = buffer.m_flags;
+    m_buffer = buffer.m_buffer;
+
+#ifdef _DEBUG
+    m_revision = buffer.m_revision;
+#endif
+
+    buffer.InitializeInstance();
+
+    RETURN;
+}
+
 inline SBuffer::SBuffer(const BYTE *buffer, COUNT_T size)
   : m_size(0),
     m_allocation(0),
@@ -189,6 +215,18 @@ inline SBuffer::~SBuffer()
     RETURN;
 }
 
+inline void SBuffer::InitializeInstance()
+{
+    m_size = 0;
+    m_allocation = 0;
+    m_flags = 0;
+    m_buffer = NULL;
+
+#ifdef _DEBUG
+    m_revision = 0;
+#endif
+}
+
 inline void SBuffer::Set(const SBuffer &buffer)
 {
     CONTRACT_VOID

src/coreclr/inc/sstring.h

@@ -127,19 +127,20 @@ class EMPTY_BASES_DECL SString : private SBuffer
     SString();
 
     explicit SString(const SString &s);
+    SString(SString&& string) = default;
 
     SString(const SString &s1, const SString &s2);
     SString(const SString &s1, const SString &s2, const SString &s3);
     SString(const SString &s1, const SString &s2, const SString &s3, const SString &s4);
     SString(const SString &s, const CIterator &i, COUNT_T length);
     SString(const SString &s, const CIterator &start, const CIterator &end);
-    SString(const WCHAR *string);
+    explicit SString(const WCHAR *string);
     SString(const WCHAR *string, COUNT_T count);
     SString(enum tagASCII dummyTag, const ASCII *string);
     SString(enum tagASCII dummyTag, const ASCII *string, COUNT_T count);
     SString(enum tagUTF8 dummytag, const UTF8 *string);
     SString(enum tagUTF8 dummytag, const UTF8 *string, COUNT_T count);
-    SString(WCHAR character);
+    explicit SString(WCHAR character);
 
     // NOTE: Literals MUST be read-only never-freed strings.
     SString(enum tagLiteral dummytag, const CHAR *literal);
@@ -784,6 +785,13 @@ class EMPTY_BASES_DECL InlineSString : public SString
         Set(string, count);
     }
 
+    FORCEINLINE InlineSString(enum tagLiteral, const WCHAR *string)
+      : SString(m_inline, SBUFFER_PADDED_SIZE(MEMSIZE))
+    {
+        WRAPPER_NO_CONTRACT;
+        Set(string);
+    }
+
     FORCEINLINE InlineSString(enum tagASCII, const CHAR *string)
       : SString(m_inline, SBUFFER_PADDED_SIZE(MEMSIZE))
     {
@@ -869,7 +877,7 @@ typedef InlineSString<2 * 260> LongPathString;
 //        s = SL("My literal String");
 // ================================================================================
 
-#define SL(_literal) SString(SString::Literal, _literal)
+#define SL(_literal) SString{ SString::Literal, _literal }
 
 // ================================================================================
 // Special contract definition - THROWS_UNLESS_NORMALIZED

src/coreclr/scripts/genEventPipe.py

@@ -178,9 +178,9 @@ def generateClrEventPipeWriteEventsImpl(
     WriteEventImpl.append(
         "    EventPipeProvider" +
         providerPrettyName +
-        " = " + createProviderFunc + "(SL(" +
+        " = " + createProviderFunc + "(" +
         providerPrettyName +
-        "Name), " + eventPipeCallbackCastExpr + "(" + callbackName + "));\n")
+        "Name, " + eventPipeCallbackCastExpr + "(" + callbackName + "));\n")
     for eventNode in eventNodes:
         eventName = eventNode.getAttribute('symbol')
         templateName = eventNode.getAttribute('template')

src/coreclr/utilcode/ccomprc.cpp

@@ -618,7 +618,7 @@ HRESULT CCompRC::LoadLibraryHelper(HRESOURCEDLL *pHInst,
 
             PathString rcPathName(rcPath);
 
-            if (!rcPathName.EndsWith(W("\\")))
+            if (!rcPathName.EndsWith(SL(W("\\"))))
             {
                 rcPathName.Append(W("\\"));
             }

src/coreclr/utilcode/ex.cpp

@@ -1154,7 +1154,7 @@ void GetHRMsg(HRESULT hr, SString &result, BOOL bNoGeekStuff/* = FALSE*/)
     }
     CONTRACTL_END;
 
-    result = W("");     // Make sure this routine isn't an inadvertent data-leak exploit!
+    result.Set(W(""));     // Make sure this routine isn't an inadvertent data-leak exploit!
 
     SString strDescr;
     BOOL    fHaveDescr = FALSE;
@@ -1220,7 +1220,7 @@ void GenerateTopLevelHRExceptionMessage(HRESULT hresult, SString &result)
     }
     CONTRACTL_END;
 
-    result = W("");     // Make sure this routine isn't an inadvertent data-leak exploit!
+    result.Set(W(""));     // Make sure this routine isn't an inadvertent data-leak exploit!
 
     GetHRMsg(hresult, result);
 }

src/coreclr/utilcode/longfilepathwrappers.cpp

@@ -618,7 +618,7 @@ HRESULT LongFile::NormalizePath(SString & path)
     if (fullpath.BeginsWith(SL(UNCPathPrefix)) && prefixLen != prefix.GetCount() - (COUNT_T)u16_strlen(UNCPATHPREFIX))
     {
         //Remove the leading '\\' from the UNC path to be replaced with UNCExtendedPathPrefix
-        fullpath.Replace(fullpath.Begin(), (COUNT_T)u16_strlen(UNCPATHPREFIX), UNCExtendedPathPrefix);
+        fullpath.Replace(fullpath.Begin(), (COUNT_T)u16_strlen(UNCPATHPREFIX), SL(UNCExtendedPathPrefix));
         path.CloseBuffer();
         path.Set(fullpath);
     }

src/coreclr/vm/appdomain.cpp

@@ -1139,7 +1139,7 @@ void SystemDomain::Init()
 
     // At this point m_SystemDirectory should already be canonicalized
     m_BaseLibrary.Append(m_SystemDirectory);
-    if (!m_BaseLibrary.EndsWith(DIRECTORY_SEPARATOR_CHAR_W))
+    if (!m_BaseLibrary.EndsWith(SString{ DIRECTORY_SEPARATOR_CHAR_W }))
     {
         m_BaseLibrary.Append(DIRECTORY_SEPARATOR_CHAR_W);
     }

src/coreclr/vm/assemblyspec.cpp

@@ -505,7 +505,7 @@ Assembly *AssemblySpec::LoadAssembly(LPCWSTR pFilePath)
 
     pILImage = PEImage::OpenImage(pFilePath,
         MDInternalImport_Default,
-        Bundle::ProbeAppBundle(pFilePath));
+        Bundle::ProbeAppBundle(SString{ SString::Literal, pFilePath }));
 
     // Need to verify that this is a valid CLR assembly.
     if (!pILImage->CheckILFormat())

src/coreclr/vm/ceemain.cpp

@@ -1986,12 +1986,10 @@ static HRESULT GetThreadUICultureNames(__inout StringArrayList* pCultureNames)
             sParentCulture = sCulture;
 #endif // !TARGET_UNIX
         }
-        // (LPCWSTR) to restrict the size to null terminated size
-        pCultureNames->AppendIfNotThere((LPCWSTR)sCulture);
-        // Disabling for Dev10 for consistency with managed resource lookup (see AppCompat bug notes in ResourceFallbackManager.cs)
-        // Also, this is in the wrong order - put after the parent culture chain.
-        //AddThreadPreferredUILanguages(pCultureNames);
-        pCultureNames->AppendIfNotThere((LPCWSTR)sParentCulture);
+        sCulture.Normalize();
+        sParentCulture.Normalize();
+        pCultureNames->AppendIfNotThere(sCulture);
+        pCultureNames->AppendIfNotThere(sParentCulture);
         pCultureNames->Append(SString::Empty());
     }
     EX_CATCH

src/coreclr/vm/clrex.h

@@ -353,7 +353,7 @@ class EEResourceException : public EEException
     InlineSString<32>        m_resourceName;
 
  public:
-    EEResourceException(RuntimeExceptionKind kind, const SString &resourceName);
+    EEResourceException(RuntimeExceptionKind kind, const WCHAR* resourceName);
 
     // Unmanaged message text containing only the resource name (GC safe)
     void GetMessage(SString &result);
@@ -1029,7 +1029,7 @@ inline EEMessageException::EEMessageException(RuntimeExceptionKind kind, HRESULT
 }
 
 
-inline EEResourceException::EEResourceException(RuntimeExceptionKind kind, const SString &resourceName)
+inline EEResourceException::EEResourceException(RuntimeExceptionKind kind, const WCHAR* resourceName)
   : EEException(kind),
     m_resourceName(resourceName)
 {

src/coreclr/vm/commodule.cpp

@@ -612,17 +612,6 @@ extern "C" void QCALLTYPE RuntimeModule_GetScopeName(QCall::ModuleHandle pModule
     END_QCALL;
 }
 
-static void ReplaceNiExtension(SString& fileName, PCWSTR pwzOldSuffix, PCWSTR pwzNewSuffix)
-{
-    STANDARD_VM_CONTRACT;
-
-    if (fileName.EndsWithCaseInsensitive(pwzOldSuffix))
-    {
-        COUNT_T oldSuffixLen = (COUNT_T)u16_strlen(pwzOldSuffix);
-        fileName.Replace(fileName.End() - oldSuffixLen, oldSuffixLen, pwzNewSuffix);
-    }
-}
-
 /*============================GetFullyQualifiedName=============================
 **Action:
 **Returns:
@@ -640,9 +629,12 @@ extern "C" void QCALLTYPE RuntimeModule_GetFullyQualifiedName(QCall::ModuleHandl
     if (pModule->IsPEFile())
     {
         LPCWSTR fileName = pModule->GetPath();
-        if (*fileName != 0) {
-                retString.Set(fileName);
-        } else {
+        if (*fileName != W('\0'))
+        {
+            retString.Set(fileName);
+        }
+        else
+        {
             retString.Set(W("<Unknown>"));
         }
     }

src/coreclr/vm/debugdebugger.cpp

@@ -785,10 +785,10 @@ FCIMPL4(void, DebugStackTrace::GetStackFramesInternal,
                     else
                     {
                         // Set the pdb path (assembly file name)
-                        SString assemblyPath = pPEAssembly->GetIdentityPath();
+                        const SString& assemblyPath = pPEAssembly->GetIdentityPath();
                         if (!assemblyPath.IsEmpty())
                         {
-                            OBJECTREF obj = (OBJECTREF)StringObject::NewString(assemblyPath);
+                            OBJECTREF obj = (OBJECTREF)StringObject::NewString(assemblyPath.GetUnicode());
                             pStackFrameHelper->rgAssemblyPath->SetAt(iNumValidFrames, obj);
                         }
                     }

src/coreclr/vm/eepolicy.cpp

@@ -298,7 +298,7 @@ inline void LogCallstackForLogWorker(Thread* pThread)
     {
         WordAt.Insert(WordAt.Begin(), W("   "));
     }
-    WordAt += W(" ");
+    WordAt.Append(W(" "));
 
     CallStackLogger logger;
 
@@ -484,8 +484,10 @@ void EEPolicy::LogFatalError(UINT exitCode, UINT_PTR address, LPCWSTR pszMessage
 
                 // Format the string
                 InlineSString<80> ssMessage;
-                ssMessage.FormatMessage(FORMAT_MESSAGE_FROM_STRING, W("at IP 0x%1 (0x%2) with exit code 0x%3."), 0, 0, addressString, runtimeBaseAddressString,
-                    exitCodeString);
+                ssMessage.FormatMessage(FORMAT_MESSAGE_FROM_STRING, W("at IP 0x%1 (0x%2) with exit code 0x%3."), 0, 0,
+                    SString{ SString::Literal, addressString },
+                    SString{ SString::Literal, runtimeBaseAddressString },
+                    SString{ SString::Literal, exitCodeString });
                 reporter.AddDescription(ssMessage);
             }
 
@@ -624,7 +626,7 @@ void DECLSPEC_NORETURN EEPolicy::HandleFatalStackOverflow(EXCEPTION_POINTERS *pE
         // 2. In native code with no explicit frame above the topmost managed frame
         // 3. In native code with a explicit frame(s) above the topmost managed frame
         // The FaultingExceptionFrame's context needs to point to the topmost managed code frame except for the case 3.
-        // In that case, it needs to point to the actual frame where the stack overflow happened, otherwise the stack 
+        // In that case, it needs to point to the actual frame where the stack overflow happened, otherwise the stack
         // walker would skip the explicit frame(s) and misbehave.
         Thread *pThread = GetThreadNULLOk();
         if (pThread)

src/coreclr/vm/eventpipeadapter.h

@@ -318,7 +318,7 @@ class EventPipeAdapter final
 		ep_add_provider_to_session (provider, session);
 	}
 
-	static inline EventPipeProvider * CreateProvider(const SString &providerName, EventPipeCallback callback, void* callbackContext = nullptr)
+	static inline EventPipeProvider * CreateProvider(const WCHAR* providerName, EventPipeCallback callback, void* callbackContext = nullptr)
 	{
 		CONTRACTL
 		{
@@ -328,7 +328,7 @@ class EventPipeAdapter final
 		}
 		CONTRACTL_END;
 
-		ep_char8_t *providerNameUTF8 = ep_rt_utf16_to_utf8_string(reinterpret_cast<const ep_char16_t *>(providerName.GetUnicode ()));
+		ep_char8_t *providerNameUTF8 = ep_rt_utf16_to_utf8_string(reinterpret_cast<const ep_char16_t *>(providerName));
 		EventPipeProvider * provider = ep_create_provider (providerNameUTF8, callback, callbackContext);
 		ep_rt_utf8_string_free (providerNameUTF8);
 		return provider;

src/coreclr/vm/eventreporter.cpp

@@ -557,7 +557,7 @@ void LogCallstackForEventReporterWorker(EventReporter& reporter)
     {
         WordAt.Insert(WordAt.Begin(), W("   "));
     }
-    WordAt += W(" ");
+    WordAt.Append(W(" "));
 
     LogCallstackData data = {
         &reporter, &WordAt
@@ -679,7 +679,9 @@ void DoReportForUnhandledNativeException(PEXCEPTION_POINTERS pExceptionInfo)
             FormatInteger(addressString, ARRAY_SIZE(addressString), "%p", (SIZE_T)pExceptionInfo->ExceptionRecord->ExceptionAddress);
 
             StackSString s;
-            s.FormatMessage(FORMAT_MESSAGE_FROM_STRING, W("exception code %1, exception address %2"), 0, 0, exceptionCodeString, addressString);
+            s.FormatMessage(FORMAT_MESSAGE_FROM_STRING, W("exception code %1, exception address %2"), 0, 0,
+                SString{ SString::Literal, exceptionCodeString },
+                SString{ SString::Literal, addressString });
             reporter.AddDescription(s);
             if (pThread)
             {

src/coreclr/vm/eventtrace.cpp

@@ -4359,7 +4359,7 @@ VOID ETW::LoaderLog::SendModuleEvent(Module *pModule, DWORD dwEventOptions, BOOL
     }
 
     LPCWSTR pEmptyString = W("");
-    SString moduleName = SString::Empty();
+    SString moduleName{ SString::Empty() };
 
     if(!bIsDynamicAssembly)
     {