ICU-13634 Fixing some clang sanitizer issues, including one potentially serious one...

X-SVN-Rev: 41245

diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp
index 77597022883b4c6be21b4e005b3cbd1390e2f96d..4ad2564c6f78e66a6999332663036644a23fae93 100644 (file)
--- 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 d862d068cd2ca2980b07d48e1dda29ae89e66e40..45d0d8de40bc6d566fc522aaa37c6f888a645a20 100644 (file)
--- 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<int32_t *>(intPtr) = value;
+    int32_t signedValue = neg ? -value : static_cast<int32_t>(value);
+    *static_cast<int32_t *>(intPtr) = signedValue;
 }
 
 static void intToStr(
         const void *intPtr, UnicodeString &appendTo) {
     UChar buffer[20];
-    int32_t x = *static_cast<const int32_t *>(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<const int32_t *>(intPtr);
+    uint32_t x;
+    if (xSigned < 0) {
         appendTo.append((UChar)0x2D);
+        x = static_cast<uint32_t>(-xSigned);
+    } else {
+        x = static_cast<uint32_t>(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 0b22e57c4a27e908ee6fa31d2df98178e9af8ffd..33b0610a5acaa8e6cf6378014ebcd0e98dedfb47 100644 (file)
--- 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 f298d3f6053950355307e7e964296839cc89f502..42cc2c48ecf07a82af7c3522236ae325eab728ff 100644 (file)
--- 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 0ad9fe3ce8e92a77e1cc7de13ed777bceafe3373..d27de4be1ee62352ad0ba27adb7ac87cb4c86970 100644 (file)
--- 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.