Skip to content

Commit

Permalink
[libc] fix strtointeger behavior on max values
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michaelrj-google committed Mar 29, 2023
1 parent f5e63f8 commit 5ce83ca
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
2 changes: 1 addition & 1 deletion libc/src/__support/str_to_integer.h
Expand Up @@ -135,7 +135,7 @@ LIBC_INLINE StrToNumResult<T> strtointeger(const char *__restrict src,

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
Expand Down
69 changes: 69 additions & 0 deletions libc/test/src/stdlib/StrtolTest.h
Expand Up @@ -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, 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) {
Expand Down

0 comments on commit 5ce83ca

Please sign in to comment.