Add test for StringBuilder null terminator implementation detail (#21800)

* Remove some commented out code.

* Add test verifying that we put a null terminator 2-past the end of the native buffer allocated for a StringBuilder.

diff --git a/src/vm/ilmarshalers.cpp b/src/vm/ilmarshalers.cpp
index 8b6b15e..d3869c2 100644 (file)
--- a/src/vm/ilmarshalers.cpp
+++ b/src/vm/ilmarshalers.cpp
@@ -772,17 +772,9 @@ void ILWSTRBufferMarshaler::EmitConvertContentsCLRToNative(ILCodeStream* pslILEm
     pslILEmit->EmitCALL(METHOD__STRING_BUILDER__GET_LENGTH, 1, 1);
 
     // stack: StringBuilder length
-
-    // if (!fConvertSpaceJustCalled)
-    {
-        // we don't need to double-check the length because the length
-        // must be smaller than the capacity and the capacity was already
-        // checked by EmitConvertSpaceCLRToNative
-        
-        pslILEmit->EmitDUP();
-        // static void StubHelpers.CheckStringLength(int length)
-        pslILEmit->EmitCALL(METHOD__STUBHELPERS__CHECK_STRING_LENGTH, 1, 0);
-    }
+    pslILEmit->EmitDUP();
+    // static void StubHelpers.CheckStringLength(int length)
+    pslILEmit->EmitCALL(METHOD__STUBHELPERS__CHECK_STRING_LENGTH, 1, 0);
 
     // stack: StringBuilder length 
 

diff --git a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTest.cs b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTest.cs
index cc7369f..f7684a3 100644 (file)
Binary files a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTest.cs and b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTest.cs differ

diff --git a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestNative.cpp b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestNative.cpp
index df9a6d3..833c316 100644 (file)
--- a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestNative.cpp
+++ b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestNative.cpp
@@ -153,3 +153,11 @@ extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE ReverseP_MarshalStrB_Out(Test_Del_M
     return TRUE;
 
 }
+
+// Verify that we append extra null terminators to our StringBuilder native buffers.
+// Although this is a hidden implementation detail, it would be breaking behavior to stop doing this
+// so we have a test for it. In particular, this detail prevents us from optimizing marshalling StringBuilders by pinning.
+extern "C" DLL_EXPORT BOOL STDMETHODCALLTYPE Verify_NullTerminators_PastEnd(LPCWSTR buffer, int length)
+{
+    return buffer[length+1] == W('\0');
+}

diff --git a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestPInvokeDef.cs b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestPInvokeDef.cs
index f54397b..d8648df 100644 (file)
--- a/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestPInvokeDef.cs
+++ b/tests/src/Interop/StringMarshalling/LPTSTR/LPTSTRTestPInvokeDef.cs
@@ -63,5 +63,10 @@ namespace NativeDefs
         [DllImport(NativeBinaryName)]
         public static extern bool ReverseP_MarshalStrB_InOut(Del_MarshalStrB_InOut d, [MarshalAs(UnmanagedType.LPTStr)]string s);
 
+        [DllImport(NativeBinaryName)]
+        public static extern bool Verify_NullTerminators_PastEnd(StringBuilder builder, int length);
+
+        [DllImport(NativeBinaryName, EntryPoint = "Verify_NullTerminators_PastEnd")]
+        public static extern bool Verify_NullTerminators_PastEnd_Out([Out] StringBuilder builder, int length);
     }
-}
\ No newline at end of file
+}