From e025466e3ac94bcf4a8062b98b68f4c6b7e00990 Mon Sep 17 00:00:00 2001 From: Frank Tang Date: Wed, 27 Nov 2024 13:40:54 -0800 Subject: [PATCH] ICU-22983 Fix DecimalQuantity::shiftLeft ubsan issue ICU-22983 Add java tests and comments --- icu4c/source/i18n/number_decimalquantity.cpp | 2 +- icu4c/source/test/intltest/numfmtst.cpp | 5 +++++ icu4c/source/test/intltest/numfmtst.h | 1 + .../ibm/icu/dev/test/format/NumberRegressionTests.java | 5 +++++ .../icu/impl/number/DecimalQuantity_DualStorageBCD.java | 9 ++++++++- 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index f9350d5d5cc8..ca1dacd3579d 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -1133,7 +1133,7 @@ void DecimalQuantity::setDigitPos(int32_t position, int8_t value) { } void DecimalQuantity::shiftLeft(int32_t numDigits) { - if (!usingBytes && precision + numDigits > 16) { + if (!usingBytes && precision + numDigits >= 16) { switchStorage(); } if (usingBytes) { diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index a37111a8540d..ede924722952 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -253,6 +253,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n TESTCASE_AUTO(Test10997_FormatCurrency); TESTCASE_AUTO(Test21556_CurrencyAsDecimal); TESTCASE_AUTO(Test22088_Ethiopic); + TESTCASE_AUTO(Test22983_LongFraction); TESTCASE_AUTO_END; } @@ -10161,5 +10162,9 @@ void NumberFormatTest::Test22088_Ethiopic() { assertEquals("Wrong result with UNUM_NUMBERING_SYSTEM and English", u"123", nf3->format(123, result)); } } +void NumberFormatTest::Test22983_LongFraction() { + IcuTestErrorCode status(*this, "Test22983_LongFraction"); + DecimalFormat df(u"0.0000000000000001", status); +} #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h index 5bdbc131cda3..10dd3386d4c9 100644 --- a/icu4c/source/test/intltest/numfmtst.h +++ b/icu4c/source/test/intltest/numfmtst.h @@ -309,6 +309,7 @@ class NumberFormatTest: public CalendarTimeZoneTest { void Test10997_FormatCurrency(); void Test21556_CurrencyAsDecimal(); void Test22088_Ethiopic(); + void Test22983_LongFraction(); private: UBool testFormattableAsUFormattable(const char *file, int line, Formattable &f); diff --git a/icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberRegressionTests.java b/icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberRegressionTests.java index 08da26d398d2..3cd021fd45bc 100644 --- a/icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberRegressionTests.java +++ b/icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberRegressionTests.java @@ -1835,6 +1835,11 @@ public void test4241880() { } Locale.setDefault(savedLocale); } + + @Test + public void test22983LongFraction() { + DecimalFormat format = new DecimalFormat("0.0000000000000001"); + } } class myformat implements Serializable diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/core/src/main/java/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java index 414f62938eb5..947d134ddd59 100644 --- a/icu4j/main/core/src/main/java/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java +++ b/icu4j/main/core/src/main/java/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java @@ -167,7 +167,14 @@ protected void setDigitPos(int position, byte value) { @Override protected void shiftLeft(int numDigits) { - if (!usingBytes && precision + numDigits > 16) { + // https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.19 + // If the promoted type of the left-hand operand is long, then only the + // six lowest-order bits of the right-hand operand are used as the shift + // distance. It is as if the right-hand operand were subjected to a bitwise + // logical AND operator & (ยง15.22.1) with the mask value 0x3f (0b111111). + // The shift distance actually used is therefore always in the range 0 to + // 63, inclusive. + if (!usingBytes && precision + numDigits >= 16) { switchStorage(); } if (usingBytes) {