[libc] Fix mixed-sign comparison warnings and an out-of-bound access in high_precision_decimal.h

[lntue]

Summary

Fix mixed-sign comparison warnings and an out-of-bound access in high_precision_decimal.h

[michaelrj]

libc/src/__support/high_precision_decimal.h

131

I think that instead of having this, you could change the condition at the start of the function from roundToDigit < 0 to roundToDigit <= 0, and it would have a more useful effect. If we're requesting 0 digits, then it should always round down, even if it's truncated.

[lntue]

libc/src/__support/high_precision_decimal.h

131

Then it will return false for something that needs roundup like ".51".

[michaelrj]

LGTM

libc/src/__support/high_precision_decimal.h

131

ah, yes, I see what you mean, nevermind then.

[michaelrj]

as an additional part of this patch, could you add
`simple_decimal_conversion_test<float>(

"7.00649232162408535461864791644958065640130970938257885878534141944895541342930300743319094181060791015625e-46", 0x0, 0,
ERANGE);`

to line 235 of str_to_float_test.cpp?

[lntue]

Add test to str_to_float_test.cpp.

libc/src/__support/high_precision_decimal.h

class HighPrecisionDecimal {
 
   uint32_t num_digits = 0;
   int32_t decimal_point = 0;
   bool truncated = false;
   uint8_t digits[MAX_NUM_DIGITS];
 
 private:
   bool should_round_up(int32_t roundToDigit) {
-    if (roundToDigit < 0 || roundToDigit >= this->num_digits) {
+    if (roundToDigit < 0 ||
+        static_cast<uint32_t>(roundToDigit) >= this->num_digits) {
       return false;
     }
 
     // If we're right in the middle and there are no extra digits
     if (this->digits[roundToDigit] == 5 &&
-        roundToDigit + 1 == this->num_digits) {
+        static_cast<uint32_t>(roundToDigit + 1) == this->num_digits) {
 
       // Round up if we've truncated (since that means the result is slightly
       // higher than what's represented.)
       if (this->truncated) {
         return true;
       }
 
       // If this exactly halfway, round to even.
+      if (roundToDigit == 0)
+        // When the input is ".5".
+        return false;
       return this->digits[roundToDigit - 1] % 2 != 0;
     }
     // If there are digits after roundToDigit, they must be non-zero since we
     // trim trailing zeroes after all operations that change digits.
     return this->digits[roundToDigit] >= 5;
   }
 
   // Takes an amount to left shift and returns the number of new digits needed

libc/test/src/__support/high_precision_decimal_test.cpp

TEST(LlvmLibcHighPrecisionDecimalTest, BasicShift) {
 
   hpd.shift(1); // shift left 1, equal to multiplying by 2.
 
   EXPECT_EQ(digits[0], uint8_t(2));
   EXPECT_EQ(hpd.get_num_digits(), 1u);
   EXPECT_EQ(hpd.get_decimal_point(), 1);
 }
 
+TEST(LlvmLibcHighPrecisionDecimalTest, ShouldRoundup) {
+  __llvm_libc::internal::HighPrecisionDecimal hpd =
+      __llvm_libc::internal::HighPrecisionDecimal(".5");
+  uint8_t *digits = hpd.get_digits();
+
+  EXPECT_EQ(digits[0], uint8_t(5));
+  EXPECT_EQ(hpd.get_num_digits(), 1u);
+  EXPECT_EQ(hpd.get_decimal_point(), 0);
+  EXPECT_EQ(hpd.round_to_integer_type<int>(), 0);
+}
+
 TEST(LlvmLibcHighPrecisionDecimalTest, SmallShift) {
   __llvm_libc::internal::HighPrecisionDecimal hpd =
       __llvm_libc::internal::HighPrecisionDecimal("1.2345");
   uint8_t *digits = hpd.get_digits();
 
   hpd.shift(-1); // shift right one, equal to dividing by 2
   // result should be 0.61725
 

libc/test/src/__support/str_to_float_test.cpp

TEST_F(LlvmLibcStrToFloatTest, SimpleDecimalConversion64SubnormalRounding) {
   simple_decimal_conversion_test<double>("2.2250738585072012e-308",
                                          0x10000000000000, 1);
 }
 
 TEST_F(LlvmLibcStrToFloatTest, SimpleDecimalConversion32SpecificFailures) {
   simple_decimal_conversion_test<float>(
       "1.4012984643248170709237295832899161312802619418765e-45", 0x1, 0,
       ERANGE);
+  simple_decimal_conversion_test<float>(
+      "7."
+      "006492321624085354618647916449580656401309709382578858785341419448955413"
+      "42930300743319094181060791015625e-46",
+      0x0, 0, ERANGE);
 }
 
 TEST(LlvmLibcStrToFloatTest, SimpleDecimalConversionExtraTypes) {
   uint32_t float_output_mantissa = 0;
   uint32_t output_exp2 = 0;
 
   errno = 0;
   __llvm_libc::internal::simple_decimal_conversion<float>(