Finish (mostly) erradicating StringBuilder marshaling from corefx

[stephentoub]

This should finish my quest of removing StringBuilder marshaling from coreclr and corefx, which I've strived to do because it often adds unnecessary overhead and often is done incorrectly because of intricacies in how the marshaling works; while not using it often leads to unsafe code at call sites, there's much less magic, and it affords the ability to optimize more if desired.

There are still three remaining [Out] StringBuilder's in Interop.Odbc.cs, which I've not removed because tests are limited and the call graph to these is non-trivial with StringBuilders passed through. There may also be a few straggling DllImports I missed while scouring interop that's not following our guidelines of being in src\Common.

Other than those, the remaining StringBuilder usage I found in DllImports was for [In] only, and in those cases it's reasonable, as the code is building up StringBuilders and then passing them off to the call sites; converting those to use char* or similar could actually make them more expensive. I did, however, ensure they were properly annotated as [In], in order to make the intent clear and avoid potential marshaling costs for the unnecessary [Out] direction.

Where I was touching a function in one of these stray interop files that was duplicated elsewhere, I also consolidated it to a centralized location, but I've not in this PR done the cleanup work for the rest of the files.

cc: @jkotas, @bartonjs, @JeremyKuhne, @tarekgh

[jkotas]

Nice!

[bartonjs]

GetUnixVersion asserted on macOS:

https://mc.dot.net/#/user/stephentoub/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/1ef34f0231392dc64e915ca0c8ba410d9103dfbd/workItem/System.Security.Cryptography.Algorithms.Tests/wilogs

2018-11-30 21:11:47,333: INFO: proc(55): run_and_log_output: Output: �[mAssertion Failed
2018-11-30 21:11:47,333: INFO: proc(55): run_and_log_output: Output:
2018-11-30 21:11:47,333: INFO: proc(55): run_and_log_output: Output: at Interop.Sys.GetUnixVersion() in /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/Common/src/Interop/Unix/System.Native/Interop.GetUnixVersion.cs:line 40
2018-11-30 21:11:47,334: INFO: proc(55): run_and_log_output: Output: at System.Runtime.InteropServices.RuntimeInformation.get_OSDescription() in /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Runtime.InteropServices.RuntimeInformation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Unix.cs:line 31
2018-11-30 21:11:47,334: INFO: proc(55): run_and_log_output: Output: at System.Security.Cryptography.Rsa.Tests.DefaultRSAProvider.get_Supports384PrivateKey() in /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Security.Cryptography.Algorithms/tests/DefaultRSAProvider.cs:line 40

[stephentoub]

Probably not a coincidence. I'll take a look tomorrow. Thanks.

[JeremyKuhne]

Nice!

[stephentoub]

GetUnixVersion asserted on macOS

@bartonjs, the issue was the assert itself. I had code something like:

byte[] arr = new byte[1] { 0 };
Debug.Assert(Array.IndexOf(arr, 0) != -1);

which you'd expect to always succeed, except it always fails, because overload resolution ends up mapping to the non-generic Array.IndexOf, with the 0 getting boxed as an Int32, and thus comparing each element of the byte[] doesn't match. The fix is either to do Array.IndexOf<byte>(arr, 0) or Array.IndexOf(arr, (byte)0), both of which lead to using the generic overload; I did the former.

[bartonjs]

Yep, consts and overload resolution make for things that look funny sometimes. I'm also self-satisfied that I diagnosed that as the problem based on the diff 😄

[stephentoub]

@dotnet-bot test this please