[R2R] Debug assert failing for condition that is definitely true

[adamsitnik]

In #73055 I've encountered a very strange bug: an assert that should definitely not be failing, was failing.

The tests were failing with Encountered infinite recursion while looking up resource 'Word_At' in System.Private.CoreLib.:

The callstack was quite long, but it showed that failure was starting from WidenAsciiToUtf16_Vector256 method:

   at System.Diagnostics.Debug.Fail(System.String, System.String)
   at System.Text.ASCIIUtility.WidenAsciiToUtf16_Vector256(Byte*, Char*, UIntPtr)
   at System.Text.ASCIIUtility.WidenAsciiToUtf16(Byte*, Char*, UIntPtr)

This method had 3 debug asserts:

runtime/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs

Lines 1753 to 1757 in 20a2f23

	private static unsafe nuint WidenAsciiToUtf16_Vector256(byte* pAsciiBuffer, char* pUtf16Buffer, nuint elementCount)
	{
	Debug.Assert(Vector256.IsHardwareAccelerated);
	Debug.Assert(BitConverter.IsLittleEndian);
	Debug.Assert(elementCount >= 2 * (uint)Vector256<byte>.Count);

And the only place where it was called from had exactly the same guards:

runtime/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs

Lines 1581 to 1584 in 20a2f23

	if (BitConverter.IsLittleEndian && Vector256.IsHardwareAccelerated && elementCount >= 2 * (uint)Vector256<byte>.Count)
	{
	currentOffset = WidenAsciiToUtf16_Vector256(pAsciiBuffer, pUtf16Buffer, elementCount);
	}

Based on suggestions from @stephentoub I've narrowed it down to Debug.Assert(Vector256.IsHardwareAccelerated); assert and R2R (when I disable R2R, the failures are gone).

To repro the bug please checkout specific commit from my fork, as I want to remove this particular assert to make progress with the PR itself:

git clone https://github.com/adamsitnik/runtime.git --branch WidenAsciiToUtf16 repro
cd repro
git reset --hard 20a2f2313c34d2c48052336167895f66916e9cb5
.\build.cmd -c Checked -subset clr+libs+libs.tests
.\dotnet.cmd build .\src\libraries\System.Text.Encoding\tests\System.Text.Encoding.Tests.csproj /p:Configuration=Checked /t:Test

[danmoseley]

Is this relevant? #69578

[adamsitnik]

Is this relevant? #69578

It seems that @tannergooding has hit the same or a very similar issue in the referenced PR:
#69578 (comment)

looks like ReadyToRun for Vector256.IsHardwareAccelerated is returning false for the indirect invocation

[mangod9]

Hi @adamsitnik, is there a standalone repro for this, or does it occur under very specific conditions? Assume it works as expected when R2R is disabled?

[davidwrighton]

Alas, this is by design. The rules for using intrinsics in CoreLib are different than in any other assembly, and extremely easy to get wrong (and extremely unforgiving to folks trying to follow good software practices and write helper functions.)

See https://github.com/dotnet/runtime/blob/d80d71fa6d805bc9129982fbfc26df951d88cce3/docs/design/coreclr/botr/vectors-and-intrinsics.md#code-review-rules-for-code-written-in-systemprivatecorelibdll

The end result is that you either need to have this function have an implementation that behaves exactly the same if the generated code think that the various hardware intrinsics are not present, or you need to disable this function from being compiled to R2R.

To do that, either mark the function with the AggressiveOptimization flag on the WidenAsciiToUtf16_Vector256 method, which will disable R2R for at least this release, or define and put a System.Runtime.BypassReadyToRunAttribute on the WidenAsciiToUtf16_Vector256 method.