From 55080e2804c089620fa4ba5c0c575d6819846c3c Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Wed, 18 Apr 2018 10:52:36 +0000 Subject: [PATCH] ICU-13634 Fixing some clang sanitizer issues, including one potentially serious one deep inside DecimalQuantity. X-SVN-Rev: 41245 --- icu4c/source/i18n/number_decimalquantity.cpp | 2 +- .../test/intltest/numberformattesttuple.cpp | 24 +++++++++---------- icu4c/source/test/intltest/numbertest_api.cpp | 18 ++++++++++++-- .../DecimalQuantity_DualStorageBCD.java | 2 +- .../test/number/NumberFormatterApiTest.java | 23 ++++++++++++++++++ 5 files changed, 52 insertions(+), 17 deletions(-) diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index 77597022883..4ad2564c6f7 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -837,7 +837,7 @@ UnicodeString DecimalQuantity::toScientificString() const { int8_t DecimalQuantity::getDigitPos(int32_t position) const { if (usingBytes) { - if (position < 0 || position > precision) { return 0; } + if (position < 0 || position >= precision) { return 0; } return fBCD.bcdBytes.ptr[position]; } else { if (position < 0 || position >= 16) { return 0; } diff --git a/icu4c/source/test/intltest/numberformattesttuple.cpp b/icu4c/source/test/intltest/numberformattesttuple.cpp index d862d068cd2..45d0d8de40b 100644 --- a/icu4c/source/test/intltest/numberformattesttuple.cpp +++ b/icu4c/source/test/intltest/numberformattesttuple.cpp @@ -143,7 +143,7 @@ static void strToInt( status = U_ILLEGAL_ARGUMENT_ERROR; return; } - int32_t value = 0; + uint32_t value = 0; for (int32_t i = start; i < len; ++i) { UChar ch = str[i]; if (ch < 0x30 || ch > 0x39) { @@ -152,25 +152,23 @@ static void strToInt( } value = value * 10 - 0x30 + (int32_t) ch; } - if (neg) { - value = -value; - } - *static_cast(intPtr) = value; + int32_t signedValue = neg ? -value : static_cast(value); + *static_cast(intPtr) = signedValue; } static void intToStr( const void *intPtr, UnicodeString &appendTo) { UChar buffer[20]; - int32_t x = *static_cast(intPtr); - UBool neg = FALSE; - if (x < 0) { - neg = TRUE; - x = -x; - } - if (neg) { + // int64_t such that all int32_t values can be negated + int64_t xSigned = *static_cast(intPtr); + uint32_t x; + if (xSigned < 0) { appendTo.append((UChar)0x2D); + x = static_cast(-xSigned); + } else { + x = static_cast(xSigned); } - int32_t len = uprv_itou(buffer, UPRV_LENGTHOF(buffer), (uint32_t) x, 10, 1); + int32_t len = uprv_itou(buffer, UPRV_LENGTHOF(buffer), x, 10, 1); appendTo.append(buffer, 0, len); } diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index 0b22e57c4a2..33b0610a5ac 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -2047,9 +2047,23 @@ void NumberFormatterApiTest::locale() { void NumberFormatterApiTest::formatTypes() { UErrorCode status = U_ZERO_ERROR; LocalizedNumberFormatter formatter = NumberFormatter::withLocale(Locale::getEnglish()); - const char* str1 = "98765432123456789E1"; - UnicodeString actual = formatter.formatDecimal(str1, status).toString(); + + // Double + assertEquals("Format double", "514.23", formatter.formatDouble(514.23, status).toString()); + + // Int64 + assertEquals("Format int64", "51,423", formatter.formatDouble(51423L, status).toString()); + + // decNumber + UnicodeString actual = formatter.formatDecimal("98765432123456789E1", status).toString(); assertEquals("Format decNumber", u"987,654,321,234,567,890", actual); + + // Also test proper DecimalQuantity bytes storage when all digits are in the fraction. + // The number needs to have exactly 40 digits, which is the size of the default buffer. + // (issue discovered by the address sanitizer in C++) + static const char* str = "0.009876543210987654321098765432109876543211"; + actual = formatter.rounding(Rounder::unlimited()).formatDecimal(str, status).toString(); + assertEquals("Format decNumber to 40 digits", str, actual); } void NumberFormatterApiTest::errors() { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java index f298d3f6053..42cc2c48ecf 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java @@ -90,7 +90,7 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra @Override protected byte getDigitPos(int position) { if (usingBytes) { - if (position < 0 || position > precision) + if (position < 0 || position >= precision) return 0; return bcdBytes[position]; } else { diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java index 0ad9fe3ce8e..d27de4be1ee 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java @@ -2017,6 +2017,29 @@ public class NumberFormatterApiTest { assertNotEquals(NumberFormatter.with().locale(ULocale.ENGLISH), NumberFormatter.with().locale(Locale.FRENCH)); } + @Test + public void formatTypes() { + LocalizedNumberFormatter formatter = NumberFormatter.withLocale(ULocale.ENGLISH); + + // Double + assertEquals("514.23", formatter.format(514.23).toString()); + + // Int64 + assertEquals("51,423", formatter.format(51423L).toString()); + + // BigDecimal + assertEquals("987,654,321,234,567,890", + formatter.format(new BigDecimal("98765432123456789E1")).toString()); + + // Also test proper DecimalQuantity bytes storage when all digits are in the fraction. + // The number needs to have exactly 40 digits, which is the size of the default buffer. + // (issue discovered by the address sanitizer in C++) + assertEquals("0.009876543210987654321098765432109876543211", + formatter.rounding(Rounder.unlimited()) + .format(new BigDecimal("0.009876543210987654321098765432109876543211")) + .toString()); + } + @Test public void plurals() { // TODO: Expand this test.