Fix hidden overflow issue in encoding code

[tarekgh]

When enabling overflow/underflow compilation flag globally we found one of the System.Text.Encoding.CodePages tests is failing.
The reason is we have some method that can adjust a byte pointer by some int value, when passing -1 to this method the calculation of adjusted pointer value becomes wrong when the overflow/underflow flag is wrong.
the operation is carried with 8 bytes registers so the overflow and underflow is not detected and no exception is thrown. here is some example of the generated code with and without the overflow check:

with overflow/underflow check is on
_bytes += count;
00007FFF46E67D76 mov rax,qword ptr [rbp+50h]
00007FFF46E67D7A mov edx,dword ptr [rbp+58h] <<< edx = -1
00007FFF46E67D7D mov edx,edx
00007FFF46E67D7F add rdx,qword ptr [rax+48h] <<< wrong unexpected value
00007FFF46E67D83 jae 00007FFF46E67D8A
00007FFF46E67D85 call 00007FFFA65AFBD0
00007FFF46E67D8A mov rax,qword ptr [rbp+50h]
00007FFF46E67D8E mov qword ptr [rax+48h],rdx

with overflow/underflow check is off

00007FFF46E877D6 mov eax,dword ptr [rbp+58h] <<< eax = -1
00007FFF46E877D9 movsxd rax,eax
00007FFF46E877DC mov rdx,qword ptr [rbp+50h] <<< right expected value
00007FFF46E877E0 add qword ptr [rdx+48h],rax

[tarekgh]

@dennisdietrich this should fix the issue you faced when enabled overflow/underflow checks.

[tarekgh]

CC @danmosemsft

[danmoseley]

I don't think I understand why the -1 was causing a failure when checked. Was this if _bytes is 0, so we expect it to wrap to max-int ?

@stephentoub

[ghost]

Thanks @danmosemsft, I was doubting (am still, actually) my sanity. I thought that overflow checks simply throw an exception when the overflow flag is set. I mean, clearly it does, but shouldn't it be impossible for overflow checks to change the result of a calculation?

[ghost]

Also, I can confirm that this fixes the issue I encountered. Thanks @tarekgh!

[danmoseley]

Well, we're dealing with native pointers here. I don't know what unchecked/checked are defined to mean around pointer arithmetic.

[tarekgh]

Sorry I didn't clarify in the beginning:

This is a JIT bug and I already talked to JIT guys and I am going to open issue for them in coreclr repo. this PR is just work around to unblock @dennisdietrich. here is more details about the issue

when enabling the overflow flag the jit compiler produce different code for the method:

        internal unsafe void AdjustBytes(int count)
        {
            _bytes += count;
        }

The generated code when the flag on is

00007FFF46E67D76 mov rax,qword ptr [rbp+50h]
00007FFF46E67D7A mov edx,dword ptr [rbp+58h] <<< edx = -1
00007FFF46E67D7D mov edx,edx
00007FFF46E67D7F add rdx,qword ptr [rax+48h] <<< wrong unexpected value

It is getting the value -1 which is 0xFFFFFFFF and move it to edx register. then it adds the pointer address qword ptr [rax+48h] to the rdx register so the operation is performed as unsigned operation. there is no overflow occur here because the operation performed as 8 bytes operation which will not overflow but it used 0xFFFFFFFF as positive value.

when the overflow flag is turned off, the JIT will generate the code

00007FFF46E877D6 mov eax,dword ptr [rbp+58h] <<< eax = -1
00007FFF46E877D9 movsxd rax,eax
00007FFF46E877DC mov rdx,qword ptr [rbp+50h] <<< right expected value
00007FFF46E877E0 add qword ptr [rdx+48h],rax

and the code does consider the sign (when used movsxd) and the addition will be signed operation. and we'll get the desired results.

I'll link the jit issue when I open it.

[danmoseley]

Thanks for explanaation. Looks good to me. It might be worth a comment above that line