From 4b67f1a0c0752ccb732a6ed3cd5af68578598407 Mon Sep 17 00:00:00 2001 From: Mateusz Klatecki Date: Wed, 12 Jun 2019 15:47:06 +0200 Subject: [PATCH 1/3] Convert function throws exceptions if value in input string isn't correct number or is out of range (cherry picked from commit ff0975b0c3f352ac3bea71aa16eca2f3635c72f5) --- .../CorLib/corlib_native_System_Convert.cpp | 70 ++++++++++++++----- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/src/CLR/CorLib/corlib_native_System_Convert.cpp b/src/CLR/CorLib/corlib_native_System_Convert.cpp index 5592612274..a75c7fff36 100644 --- a/src/CLR/CorLib/corlib_native_System_Convert.cpp +++ b/src/CLR/CorLib/corlib_native_System_Convert.cpp @@ -19,36 +19,72 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING #if (SUPPORT_ANY_BASE_CONVERSION == TRUE) // suport for conversion from any base + char* endptr = NULL; - int64_t zero = 0; + bool UInt64 = false; bool isSigned = (bool)stack.Arg1().NumericByRef().u1; long long minValue = stack.Arg2().NumericByRef().s8; long long maxValue = stack.Arg3().NumericByRef().s8; - - // normally we check if the result is in the given range - bool check = true; - // Int64? => use also strtoull the result will be casted to Int64 - if (isSigned && maxValue == 0x7FFFFFFFFFFFFFFF) isSigned = false; - - // UInt64? => use also strtoull the result will be casted to Int64 and bypass the range check - if (minValue == 0 && maxValue == 0) - { + // UInt64? => use also strtoull the result will be casted to Int64 + if (minValue == 0 && maxValue == 0) { + UInt64 = true; isSigned = false; - check = false; + + //allow spaces before digits + while (*str == ' ') { + str++; + } + + //UInt64 can't begin with minus + if (*str == '-' ) { + NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_RANGE); + } } // convert via strtoll / strtoull - result = isSigned ? strtoll(str, nullptr, radix) : (long long) strtoull(str, nullptr, radix); + result = isSigned ? strtoll(str, &endptr, radix) : (long long) strtoull(str, &endptr, radix); + + // TODO: + // If the value in input string is out of the range of representable values + // by a long long int / unsigned long int, the function returns + // LLONG_MAX or LLONG_MIN for signed conversion + // and ULONG_MAX for unsigned conversion + // It is necessary to add a check + + // if no valid conversion endptr is equal str + if (str == endptr) { + NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_PARAMETER); + } + + // allow spaces after digits + while (*endptr == ' ') { + endptr++; + } + // should reach end of string no aditional chars + if (*endptr != 0) { + NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_PARAMETER); + } + // the signed values for SByte, Int16 and Int32 are always positive for base 2, 8 or 16 conversions // because the 64-bit function strtoll is used; need the post process the value // if the result is greater max and smaller (max + 1) * 2 this value should be subtracted - if (isSigned && result > maxValue && result < (maxValue + 1) * 2) result -= (maxValue + 1) * 2; - - // for UInt64 the check will be bypassed - stack.SetResult_I8 ((check && (result > maxValue || result < minValue)) ? zero : result); + if (radix == 2 || radix == 8 || radix == 16) { + if (isSigned && result > maxValue && result < (maxValue + 1) * 2) result -= (maxValue + 1) * 2; + } + + if (UInt64) { + // for UInt64 the check will be bypassed + stack.SetResult_I8(result); + } else if (!isSigned && maxValue != 0x7FFFFFFFFFFFFFFF && (uint64_t)result > (uint64_t)maxValue) { + NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_RANGE); + } else if (result > maxValue || result < minValue) { + NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_RANGE); + } else { + stack.SetResult_I8(result); + } #else // support for conversion from base 10 and 16 (partial) @@ -94,7 +130,7 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING #endif // defined(SUPPORT_ANY_BASE_CONVERSION) } - NANOCLR_NOCLEANUP_NOLABEL(); + NANOCLR_NOCLEANUP(); } HRESULT Library_corlib_native_System_Convert::NativeToDouble___STATIC__R8__STRING( CLR_RT_StackFrame& stack ) From edcced2f6c7d75aae89c29d971c44e4dbce7be69 Mon Sep 17 00:00:00 2001 From: Mateusz Klatecki Date: Wed, 12 Jun 2019 16:22:12 +0200 Subject: [PATCH 2/3] Fixed compilation for targets that didn't support any base convertion --- src/CLR/CorLib/corlib_native_System_Convert.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/CLR/CorLib/corlib_native_System_Convert.cpp b/src/CLR/CorLib/corlib_native_System_Convert.cpp index a75c7fff36..162a9549a7 100644 --- a/src/CLR/CorLib/corlib_native_System_Convert.cpp +++ b/src/CLR/CorLib/corlib_native_System_Convert.cpp @@ -17,7 +17,7 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING char* str = (char*)stack.Arg0().RecoverString(); signed int radix = stack.Arg4().NumericByRef().s4; - #if (SUPPORT_ANY_BASE_CONVERSION == TRUE) +#if (SUPPORT_ANY_BASE_CONVERSION == TRUE) // suport for conversion from any base char* endptr = NULL; @@ -85,8 +85,9 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING } else { stack.SetResult_I8(result); } - - #else + } + NANOCLR_NOCLEANUP(); +#else // support for conversion from base 10 and 16 (partial) if(radix == 10) @@ -127,10 +128,10 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING } stack.SetResult_I8(result); - - #endif // defined(SUPPORT_ANY_BASE_CONVERSION) - } - NANOCLR_NOCLEANUP(); + } + NANOCLR_NOCLEANUP_NOLABEL(); + #endif // defined(SUPPORT_ANY_BASE_CONVERSION) + } HRESULT Library_corlib_native_System_Convert::NativeToDouble___STATIC__R8__STRING( CLR_RT_StackFrame& stack ) From 2232712f4e5ab9c7de400bf7aa391b91ec75fa51 Mon Sep 17 00:00:00 2001 From: Mateusz Klatecki Date: Thu, 13 Jun 2019 09:18:00 +0200 Subject: [PATCH 3/3] Rename variable and refactor return conditions --- src/CLR/CorLib/corlib_native_System_Convert.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/CLR/CorLib/corlib_native_System_Convert.cpp b/src/CLR/CorLib/corlib_native_System_Convert.cpp index 162a9549a7..d1379da90b 100644 --- a/src/CLR/CorLib/corlib_native_System_Convert.cpp +++ b/src/CLR/CorLib/corlib_native_System_Convert.cpp @@ -21,7 +21,7 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING // suport for conversion from any base char* endptr = NULL; - bool UInt64 = false; + bool isUInt64 = false; bool isSigned = (bool)stack.Arg1().NumericByRef().u1; long long minValue = stack.Arg2().NumericByRef().s8; @@ -29,7 +29,7 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING // UInt64? => use also strtoull the result will be casted to Int64 if (minValue == 0 && maxValue == 0) { - UInt64 = true; + isUInt64 = true; isSigned = false; //allow spaces before digits @@ -75,12 +75,9 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING if (isSigned && result > maxValue && result < (maxValue + 1) * 2) result -= (maxValue + 1) * 2; } - if (UInt64) { - // for UInt64 the check will be bypassed - stack.SetResult_I8(result); - } else if (!isSigned && maxValue != 0x7FFFFFFFFFFFFFFF && (uint64_t)result > (uint64_t)maxValue) { + if (!isUInt64 && !isSigned && (uint64_t)result > (uint64_t)maxValue) { NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_RANGE); - } else if (result > maxValue || result < minValue) { + } else if (!isUInt64 && (result > maxValue || result < minValue)) { NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_RANGE); } else { stack.SetResult_I8(result); @@ -130,7 +127,7 @@ HRESULT Library_corlib_native_System_Convert::NativeToInt64___STATIC__I8__STRING stack.SetResult_I8(result); } NANOCLR_NOCLEANUP_NOLABEL(); - #endif // defined(SUPPORT_ANY_BASE_CONVERSION) +#endif // defined(SUPPORT_ANY_BASE_CONVERSION) }