[libc-commits] [PATCH] D147171: [libc] fix strtointeger behavior on max values

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added reviewers: sivachandra, lntue.
Herald added subscribers: libc-commits, ecnelises, tschuett.
Herald added projects: libc-project, All.
michaelrj requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147171

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


Index: libc/test/src/stdlib/StrtolTest.h
===================================================================
--- libc/test/src/stdlib/StrtolTest.h
+++ libc/test/src/stdlib/StrtolTest.h
@@ -313,6 +313,75 @@
     ASSERT_EQ(func(letter_after_prefix, &str_end, 16), ReturnT(0xabc123));
     ASSERT_EQ(libc_errno, 0);
     EXPECT_EQ(str_end - letter_after_prefix, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_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, ptrdiff_t(19));
   }
 
   void MessyBaseSixteenDecode(FunctionT func) {
Index: libc/src/__support/str_to_integer.h
===================================================================
--- libc/src/__support/str_to_integer.h
+++ libc/src/__support/str_to_integer.h
@@ -135,7 +135,7 @@
 
   ptrdiff_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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147171.509418.patch
Type: text/x-patch
Size: 3725 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230329/e029662f/attachment.bin>