From: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
Date: Fri, 4 Jan 2019 05:35:53 +0000 (-0800)
Subject: Add test for StringBuilder null terminator implementation detail (#21800)
X-Git-Tag: accepted/tizen/unified/20190422.045933~207
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=08019ac94eb8cd57abad4d03c803371290cfe210;p=platform%2Fupstream%2Fcoreclr.git

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
--- 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
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
--- 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
--- 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
+}