From e604724b975dfc9654436aa5b687bd62d68a6fd6 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 14 Feb 2017 13:13:40 -0800 Subject: [PATCH] Fix hidden overflow issue in encoding code 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 --- .../src/System/Text/EncodingCharBuffer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Text.Encoding.CodePages/src/System/Text/EncodingCharBuffer.cs b/src/System.Text.Encoding.CodePages/src/System/Text/EncodingCharBuffer.cs index 16c8afe94aeb..bdb19d52059f 100644 --- a/src/System.Text.Encoding.CodePages/src/System/Text/EncodingCharBuffer.cs +++ b/src/System.Text.Encoding.CodePages/src/System/Text/EncodingCharBuffer.cs @@ -90,7 +90,7 @@ internal unsafe bool AddChar(char ch1, char ch2, int numBytes) [System.Security.SecurityCritical] // auto-generated internal unsafe void AdjustBytes(int count) { - _bytes += count; + _bytes = unchecked(_bytes + count); } internal unsafe bool MoreData