[libc-commits] [libc] 5ce83ca - [libc] fix strtointeger behavior on max values

[Michael Jones via libc-commits]

Author: Michael Jones
Date: 2023-03-29T13:02:40-07:00
New Revision: 5ce83caba59d7c470e1a12a8d6ef63c33d98e177

URL: https://github.com/llvm/llvm-project/commit/5ce83caba59d7c470e1a12a8d6ef63c33d98e177
DIFF: https://github.com/llvm/llvm-project/commit/5ce83caba59d7c470e1a12a8d6ef63c33d98e177.diff

LOG: [libc] fix strtointeger behavior on max values

Previously the check to just return MAX or MIN used the caclulated
number being the maximum absolute value. This was right in every case
except for an unsigned conversion being passed its maximum value with a
negative sign on the front. This should return -MAX, but was returning
just MAX.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D147171

Added: 
    

Modified: 
    libc/src/__support/str_to_integer.h
    libc/test/src/stdlib/StrtolTest.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/str_to_integer.h b/libc/src/__support/str_to_integer.h
index 39ed9406663b7..965ac11cb2173 100644
--- a/libc/src/__support/str_to_integer.h
+++ b/libc/src/__support/str_to_integer.h
@@ -135,7 +135,7 @@ LIBC_INLINE StrToNumResult<T> strtointeger(const char *__restrict src,
 
   ptr
diff _t str_len = is_number ? (src - original_src) : 0;
 
-  if (result == abs_max) {
+  if (error_val == ERANGE) {
     if (is_positive || IS_UNSIGNED)
       return {cpp::numeric_limits<T>::max(), str_len, error_val};
     else // T is signed and there is a negative overflow

diff  --git a/libc/test/src/stdlib/StrtolTest.h b/libc/test/src/stdlib/StrtolTest.h
index 09d8da1279bbc..4b8e33e0563b9 100644
--- a/libc/test/src/stdlib/StrtolTest.h
+++ b/libc/test/src/stdlib/StrtolTest.h
@@ -313,6 +313,75 @@ struct StrtoTest : public __llvm_libc::testing::Test {
     ASSERT_EQ(func(letter_after_prefix, &str_end, 16), ReturnT(0xabc123));
     ASSERT_EQ(libc_errno, 0);
     EXPECT_EQ(str_end - letter_after_prefix, ptr
diff _t(8));
+
+    // These tests check what happens when the number passed is exactly the max
+    // value for the conversion.
+
+    // Max size for unsigned 32 bit numbers
+
+    const char *max_32_bit_value = "0xFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(max_32_bit_value, &str_end, 0),
+              ((is_signed_v<ReturnT> && sizeof(ReturnT) == 4)
+                   ? T_MAX
+                   : ReturnT(0xFFFFFFFF)));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - max_32_bit_value, ptr
diff _t(10));
+
+    const char *negative_max_32_bit_value = "-0xFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(negative_max_32_bit_value, &str_end, 0),
+              ((is_signed_v<ReturnT> && sizeof(ReturnT) == 4)
+                   ? T_MIN
+                   : -ReturnT(0xFFFFFFFF)));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - negative_max_32_bit_value, ptr
diff _t(11));
+
+    // Max size for signed 32 bit numbers
+
+    const char *max_31_bit_value = "0x7FFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(max_31_bit_value, &str_end, 0), ReturnT(0x7FFFFFFF));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - max_31_bit_value, ptr
diff _t(10));
+
+    const char *negative_max_31_bit_value = "-0x7FFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(negative_max_31_bit_value, &str_end, 0),
+              -ReturnT(0x7FFFFFFF));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - negative_max_31_bit_value, ptr
diff _t(11));
+
+    // Max size for unsigned 64 bit numbers
+
+    const char *max_64_bit_value = "0xFFFFFFFFFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(max_64_bit_value, &str_end, 0),
+              (is_signed_v<ReturnT> ? T_MAX : ReturnT(0xFFFFFFFFFFFFFFFF)));
+    ASSERT_EQ(libc_errno, (is_signed_v<ReturnT> ? ERANGE : 0));
+    EXPECT_EQ(str_end - max_64_bit_value, ptr
diff _t(18));
+
+    const char *negative_max_64_bit_value = "-0xFFFFFFFFFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(negative_max_64_bit_value, &str_end, 0),
+              (is_signed_v<ReturnT> ? T_MIN : -ReturnT(0xFFFFFFFFFFFFFFFF)));
+    ASSERT_EQ(libc_errno, (is_signed_v<ReturnT> ? ERANGE : 0));
+    EXPECT_EQ(str_end - negative_max_64_bit_value, ptr
diff _t(19));
+
+    // Max size for signed 64 bit numbers
+
+    const char *max_63_bit_value = "0x7FFFFFFFFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(max_63_bit_value, &str_end, 0), ReturnT(0x7FFFFFFFFFFFFFFF));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - max_63_bit_value, ptr
diff _t(18));
+
+    const char *negative_max_63_bit_value = "-0x7FFFFFFFFFFFFFFF";
+    libc_errno = 0;
+    ASSERT_EQ(func(negative_max_63_bit_value, &str_end, 0),
+              -ReturnT(0x7FFFFFFFFFFFFFFF));
+    ASSERT_EQ(libc_errno, 0);
+    EXPECT_EQ(str_end - negative_max_63_bit_value, ptr
diff _t(19));
   }
 
   void MessyBaseSixteenDecode(FunctionT func) {