Enable ILLinkClearInitLocals across corefx

[stephentoub]

Enabled for all projects except tests and VB (where we can't use spans anyway).

There's a chance there's still something uninitialized that needs to be that's slipped through, but I've done a bunch of outerloop test runs on both Windows and Linux and so hopefully have caught the vast majority (there were just a few, which I knew about as well from when looking at this previously).

cc: @ViktorHofer, @jkotas, @danmosemsft, @bartonjs, @safern
Fixes https://github.com/dotnet/corefx/issues/26939

[jkotas]

Is this actually needed for CryptDecodeObjectPointer here, or is this just a "defense in depth"?

[stephentoub]

Strangely it was actually needed and I didn't invest heavily in figuring out why.

[jkotas]

Which case was it needed? (I am worried that there is a bug that this is masking.)

[stephentoub]

Which case was it needed? (I am worried that there is a bug that this is masking.)

This one, L88. Without this Clear call, this test fails:

      System.Security.Cryptography.X509Certificates.Tests.ExtensionsTests.KeyUsageExtension_BER [FAIL]
        Assert.Equal() Failure
        Expected: DigitalSignature
        Actual:   69402752
        Stack Trace:
          d:\repos\corefx\src\System.Security.Cryptography.X509Certificates\tests\ExtensionsTests.cs(199,0): at System.Security.Cryptography.X509Certificates.Tests.ExtensionsTests.KeyUsageExtension_BER()

(Of course, the other two Clear calls I added might also be working around some issue, but removing them doesn't cause any test failures.)

[jkotas]

@bartonjs Is it intentional that the X509_KEY_USAGE is set as 16-bit int, but read as 32-bit int?

corefx/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/X509Pal.CustomExtensions.cs

Line 30 in a10890f

ushort keyUsagesAsShort = (ushort)keyUsages;

vs.

corefx/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/X509Pal.CustomExtensions.cs

Line 54 in a10890f

keyUsagesAsUint = *(uint*)(pBlob->pbData);

We are reading a memory that is not getting set by the decode API. It just happens when we clear the memory ourselves upfront, everything works fine.

[bartonjs]

Probably not. Desktop introduced the asymmetry (https://referencesource.microsoft.com/#System/security/system/security/cryptography/x509/x509extension.cs,124), probably back in net20; but it copied into a local buffer. During the port to core the code must have gotten "simplified" and since it worked it has been running since.

I don't know if the Windows implementation will always report cbData as 2 (making reading as a ushort* fine), or if they report 0/1 when the data is smaller.

Since DigitalSignature is in the low byte this could be easily validated with reading via ushort* and adding in a stopgap of span.Fill(0xFF) instead of span.Clear()

[stephentoub]

Since DigitalSignature is in the low byte this could be easily validated with reading via ushort* and adding in a stopgap of span.Fill(0xFF) instead of span.Clear()

Since this is pre-existing and since you're in the best position Jeremy to figure out the right fix here, I'm going to open an issue and go ahead with this change including the Clears, which can then hopefully be removed subsequently.

[stephentoub]

@dotnet-bot test Windows x64 Debug Build please

[stephentoub]

@dotnet-bot test Outerloop Linux x64 Release Build please

[stephentoub]

@dotnet-bot test Outerloop Linux x64 Release Build please