Skip to content
This repository was archived by the owner on Jan 26, 2024. It is now read-only.

Commit 53804d4

Browse files
[libc] fix strtol returning the wrong length
Previously, strtol/ll/ul/ull would return a pointer to the end of its parsing, regardless of if it detected a number. Now it will return a length of 0 when it doesn't find a number. Reviewed By: sivachandra Differential Revision: https://reviews.llvm.org/D112176
1 parent 2feafa2 commit 53804d4

File tree

5 files changed

+140
-9
lines changed

5 files changed

+140
-9
lines changed

libc/src/__support/str_conv_utils.h

+16-5
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,23 @@ static inline bool is_hex_start(const char *__restrict src) {
4242
}
4343

4444
// Takes the address of the string pointer and parses the base from the start of
45-
// it. This will advance the string pointer.
45+
// it. This function will advance |src| to the first valid digit in the inferred
46+
// base.
4647
static inline int infer_base(const char *__restrict *__restrict src) {
48+
// A hexadecimal number is defined as "the prefix 0x or 0X followed by a
49+
// sequence of the deimal digits and the letters a (or A) through f (or F)
50+
// with values 10 through 15 respectively." (C standard 6.4.4.1)
4751
if (is_hex_start(*src)) {
4852
(*src) += 2;
4953
return 16;
50-
} else if (**src == '0') {
51-
++(*src);
54+
} // An octal number is defined as "the prefix 0 optionally followed by a
55+
// sequence of the digits 0 through 7 only" (C standard 6.4.4.1) and so any
56+
// number that starts with 0, including just 0, is an octal number.
57+
else if (**src == '0') {
5258
return 8;
53-
} else {
59+
} // A decimal number is defined as beginning "with a nonzero digit and
60+
// consist[ing] of a sequence of decimal digits." (C standard 6.4.4.1)
61+
else {
5462
return 10;
5563
}
5664
}
@@ -62,6 +70,8 @@ template <class T>
6270
static inline T strtointeger(const char *__restrict src,
6371
char **__restrict str_end, int base) {
6472
unsigned long long result = 0;
73+
bool is_number = false;
74+
const char *original_src = src;
6575

6676
if (base < 0 || base == 1 || base > 36) {
6777
errno = EINVAL; // NOLINT
@@ -97,6 +107,7 @@ static inline T strtointeger(const char *__restrict src,
97107
if (cur_digit >= base)
98108
break;
99109

110+
is_number = true;
100111
++src;
101112

102113
// If the number has already hit the maximum value for the current type then
@@ -122,7 +133,7 @@ static inline T strtointeger(const char *__restrict src,
122133
}
123134

124135
if (str_end != nullptr)
125-
*str_end = const_cast<char *>(src);
136+
*str_end = const_cast<char *>(is_number ? src : original_src);
126137

127138
if (result == ABS_MAX) {
128139
if (is_positive || is_unsigned)

libc/test/src/stdlib/strtol_test.cpp

+31-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ TEST(LlvmLibcStrToLTest, MessyBaseTenDecode) {
118118
errno = 0;
119119
ASSERT_EQ(__llvm_libc::strtol(two_signs, &str_end, 10), 0l);
120120
ASSERT_EQ(errno, 0);
121-
EXPECT_EQ(str_end - two_signs, ptrdiff_t(1));
121+
EXPECT_EQ(str_end - two_signs, ptrdiff_t(0));
122122

123123
const char *sign_before = "+2=4";
124124
errno = 0;
@@ -143,6 +143,18 @@ TEST(LlvmLibcStrToLTest, MessyBaseTenDecode) {
143143
ASSERT_EQ(__llvm_libc::strtol(all_together, &str_end, 10), -12345l);
144144
ASSERT_EQ(errno, 0);
145145
EXPECT_EQ(str_end - all_together, ptrdiff_t(9));
146+
147+
const char *just_spaces = " ";
148+
errno = 0;
149+
ASSERT_EQ(__llvm_libc::strtol(just_spaces, &str_end, 10), 0l);
150+
ASSERT_EQ(errno, 0);
151+
EXPECT_EQ(str_end - just_spaces, ptrdiff_t(0));
152+
153+
const char *just_space_and_sign = " +";
154+
errno = 0;
155+
ASSERT_EQ(__llvm_libc::strtol(just_space_and_sign, &str_end, 10), 0l);
156+
ASSERT_EQ(errno, 0);
157+
EXPECT_EQ(str_end - just_space_and_sign, ptrdiff_t(0));
146158
}
147159

148160
static char int_to_b36_char(int input) {
@@ -322,4 +334,22 @@ TEST(LlvmLibcStrToLTest, AutomaticBaseSelection) {
322334
ASSERT_EQ(__llvm_libc::strtol(base_eight_with_prefix, &str_end, 0), 012345l);
323335
ASSERT_EQ(errno, 0);
324336
EXPECT_EQ(str_end - base_eight_with_prefix, ptrdiff_t(6));
337+
338+
const char *just_zero = "0";
339+
errno = 0;
340+
ASSERT_EQ(__llvm_libc::strtol(just_zero, &str_end, 0), 0l);
341+
ASSERT_EQ(errno, 0);
342+
EXPECT_EQ(str_end - just_zero, ptrdiff_t(1));
343+
344+
const char *just_zero_x = "0x";
345+
errno = 0;
346+
ASSERT_EQ(__llvm_libc::strtol(just_zero_x, &str_end, 0), 0l);
347+
ASSERT_EQ(errno, 0);
348+
EXPECT_EQ(str_end - just_zero_x, ptrdiff_t(1));
349+
350+
const char *just_zero_eight = "08";
351+
errno = 0;
352+
ASSERT_EQ(__llvm_libc::strtol(just_zero_eight, &str_end, 0), 0l);
353+
ASSERT_EQ(errno, 0);
354+
EXPECT_EQ(str_end - just_zero_eight, ptrdiff_t(1));
325355
}

libc/test/src/stdlib/strtoll_test.cpp

+31-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ TEST(LlvmLibcStrToLLTest, MessyBaseTenDecode) {
142142
errno = 0;
143143
ASSERT_EQ(__llvm_libc::strtoll(two_signs, &str_end, 10), 0ll);
144144
ASSERT_EQ(errno, 0);
145-
EXPECT_EQ(str_end - two_signs, ptrdiff_t(1));
145+
EXPECT_EQ(str_end - two_signs, ptrdiff_t(0));
146146

147147
const char *sign_before = "+2=4";
148148
errno = 0;
@@ -167,6 +167,18 @@ TEST(LlvmLibcStrToLLTest, MessyBaseTenDecode) {
167167
ASSERT_EQ(__llvm_libc::strtoll(all_together, &str_end, 10), -12345ll);
168168
ASSERT_EQ(errno, 0);
169169
EXPECT_EQ(str_end - all_together, ptrdiff_t(9));
170+
171+
const char *just_spaces = " ";
172+
errno = 0;
173+
ASSERT_EQ(__llvm_libc::strtoll(just_spaces, &str_end, 10), 0ll);
174+
ASSERT_EQ(errno, 0);
175+
EXPECT_EQ(str_end - just_spaces, ptrdiff_t(0));
176+
177+
const char *just_space_and_sign = " +";
178+
errno = 0;
179+
ASSERT_EQ(__llvm_libc::strtoll(just_space_and_sign, &str_end, 10), 0ll);
180+
ASSERT_EQ(errno, 0);
181+
EXPECT_EQ(str_end - just_space_and_sign, ptrdiff_t(0));
170182
}
171183

172184
static char int_to_b36_char(int input) {
@@ -350,4 +362,22 @@ TEST(LlvmLibcStrToLLTest, AutomaticBaseSelection) {
350362
012345ll);
351363
ASSERT_EQ(errno, 0);
352364
EXPECT_EQ(str_end - base_eight_with_prefix, ptrdiff_t(6));
365+
366+
const char *just_zero = "0";
367+
errno = 0;
368+
ASSERT_EQ(__llvm_libc::strtoll(just_zero, &str_end, 0), 0ll);
369+
ASSERT_EQ(errno, 0);
370+
EXPECT_EQ(str_end - just_zero, ptrdiff_t(1));
371+
372+
const char *just_zero_x = "0x";
373+
errno = 0;
374+
ASSERT_EQ(__llvm_libc::strtoll(just_zero_x, &str_end, 0), 0ll);
375+
ASSERT_EQ(errno, 0);
376+
EXPECT_EQ(str_end - just_zero_x, ptrdiff_t(1));
377+
378+
const char *just_zero_eight = "08";
379+
errno = 0;
380+
ASSERT_EQ(__llvm_libc::strtoll(just_zero_eight, &str_end, 0), 0ll);
381+
ASSERT_EQ(errno, 0);
382+
EXPECT_EQ(str_end - just_zero_eight, ptrdiff_t(1));
353383
}

libc/test/src/stdlib/strtoul_test.cpp

+31-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ TEST(LlvmLibcStrToULTest, MessyBaseTenDecode) {
110110
errno = 0;
111111
ASSERT_EQ(__llvm_libc::strtoul(two_signs, &str_end, 10), 0ul);
112112
ASSERT_EQ(errno, 0);
113-
EXPECT_EQ(str_end - two_signs, ptrdiff_t(1));
113+
EXPECT_EQ(str_end - two_signs, ptrdiff_t(0));
114114

115115
const char *sign_before = "+2=4";
116116
errno = 0;
@@ -135,6 +135,18 @@ TEST(LlvmLibcStrToULTest, MessyBaseTenDecode) {
135135
ASSERT_EQ(__llvm_libc::strtoul(all_together, &str_end, 10), -(12345ul));
136136
ASSERT_EQ(errno, 0);
137137
EXPECT_EQ(str_end - all_together, ptrdiff_t(9));
138+
139+
const char *just_spaces = " ";
140+
errno = 0;
141+
ASSERT_EQ(__llvm_libc::strtoul(just_spaces, &str_end, 10), 0ul);
142+
ASSERT_EQ(errno, 0);
143+
EXPECT_EQ(str_end - just_spaces, ptrdiff_t(0));
144+
145+
const char *just_space_and_sign = " +";
146+
errno = 0;
147+
ASSERT_EQ(__llvm_libc::strtoul(just_space_and_sign, &str_end, 10), 0ul);
148+
ASSERT_EQ(errno, 0);
149+
EXPECT_EQ(str_end - just_space_and_sign, ptrdiff_t(0));
138150
}
139151

140152
static char int_to_b36_char(int input) {
@@ -318,4 +330,22 @@ TEST(LlvmLibcStrToULTest, AutomaticBaseSelection) {
318330
012345ul);
319331
ASSERT_EQ(errno, 0);
320332
EXPECT_EQ(str_end - base_eight_with_prefix, ptrdiff_t(6));
333+
334+
const char *just_zero = "0";
335+
errno = 0;
336+
ASSERT_EQ(__llvm_libc::strtoul(just_zero, &str_end, 0), 0ul);
337+
ASSERT_EQ(errno, 0);
338+
EXPECT_EQ(str_end - just_zero, ptrdiff_t(1));
339+
340+
const char *just_zero_x = "0x";
341+
errno = 0;
342+
ASSERT_EQ(__llvm_libc::strtoul(just_zero_x, &str_end, 0), 0ul);
343+
ASSERT_EQ(errno, 0);
344+
EXPECT_EQ(str_end - just_zero_x, ptrdiff_t(1));
345+
346+
const char *just_zero_eight = "08";
347+
errno = 0;
348+
ASSERT_EQ(__llvm_libc::strtoul(just_zero_eight, &str_end, 0), 0ul);
349+
ASSERT_EQ(errno, 0);
350+
EXPECT_EQ(str_end - just_zero_eight, ptrdiff_t(1));
321351
}

libc/test/src/stdlib/strtoull_test.cpp

+31-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ TEST(LlvmLibcStrToULLTest, MessyBaseTenDecode) {
118118
errno = 0;
119119
ASSERT_EQ(__llvm_libc::strtoull(two_signs, &str_end, 10), 0ull);
120120
ASSERT_EQ(errno, 0);
121-
EXPECT_EQ(str_end - two_signs, ptrdiff_t(1));
121+
EXPECT_EQ(str_end - two_signs, ptrdiff_t(0));
122122

123123
const char *sign_before = "+2=4";
124124
errno = 0;
@@ -143,6 +143,18 @@ TEST(LlvmLibcStrToULLTest, MessyBaseTenDecode) {
143143
ASSERT_EQ(__llvm_libc::strtoull(all_together, &str_end, 10), -(12345ull));
144144
ASSERT_EQ(errno, 0);
145145
EXPECT_EQ(str_end - all_together, ptrdiff_t(9));
146+
147+
const char *just_spaces = " ";
148+
errno = 0;
149+
ASSERT_EQ(__llvm_libc::strtoull(just_spaces, &str_end, 10), 0ull);
150+
ASSERT_EQ(errno, 0);
151+
EXPECT_EQ(str_end - just_spaces, ptrdiff_t(0));
152+
153+
const char *just_space_and_sign = " +";
154+
errno = 0;
155+
ASSERT_EQ(__llvm_libc::strtoull(just_space_and_sign, &str_end, 10), 0ull);
156+
ASSERT_EQ(errno, 0);
157+
EXPECT_EQ(str_end - just_space_and_sign, ptrdiff_t(0));
146158
}
147159

148160
static char int_to_b36_char(int input) {
@@ -326,4 +338,22 @@ TEST(LlvmLibcStrToULLTest, AutomaticBaseSelection) {
326338
012345ull);
327339
ASSERT_EQ(errno, 0);
328340
EXPECT_EQ(str_end - base_eight_with_prefix, ptrdiff_t(6));
341+
342+
const char *just_zero = "0";
343+
errno = 0;
344+
ASSERT_EQ(__llvm_libc::strtoull(just_zero, &str_end, 0), 0ull);
345+
ASSERT_EQ(errno, 0);
346+
EXPECT_EQ(str_end - just_zero, ptrdiff_t(1));
347+
348+
const char *just_zero_x = "0x";
349+
errno = 0;
350+
ASSERT_EQ(__llvm_libc::strtoull(just_zero_x, &str_end, 0), 0ull);
351+
ASSERT_EQ(errno, 0);
352+
EXPECT_EQ(str_end - just_zero_x, ptrdiff_t(1));
353+
354+
const char *just_zero_eight = "08";
355+
errno = 0;
356+
ASSERT_EQ(__llvm_libc::strtoull(just_zero_eight, &str_end, 0), 0ull);
357+
ASSERT_EQ(errno, 0);
358+
EXPECT_EQ(str_end - just_zero_eight, ptrdiff_t(1));
329359
}

0 commit comments

Comments
 (0)