Skip to content

Commit ded5f43

Browse files
authored
[libc] fixed signed char issues in strsep()/strtok()/strtok_r(). (#156705)
Also add the missing tests for all the related functions (even the ones that were already right), and add the missing bazel build rules.
1 parent 878fa7b commit ded5f43

File tree

9 files changed

+68
-7
lines changed

9 files changed

+68
-7
lines changed

libc/src/string/string_utils.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,28 +212,28 @@ LIBC_INLINE char *string_token(char *__restrict src,
212212
static_assert(CHAR_BIT == 8, "bitset of 256 assumes char is 8 bits");
213213
cpp::bitset<256> delims;
214214
for (; *delimiter_string != '\0'; ++delimiter_string)
215-
delims.set(static_cast<size_t>(*delimiter_string));
215+
delims.set(*reinterpret_cast<const unsigned char *>(delimiter_string));
216216

217-
char *tok_start = src;
217+
unsigned char *tok_start = reinterpret_cast<unsigned char *>(src);
218218
if constexpr (SkipDelim)
219-
while (*tok_start != '\0' && delims.test(static_cast<size_t>(*tok_start)))
219+
while (*tok_start != '\0' && delims.test(*tok_start))
220220
++tok_start;
221221
if (*tok_start == '\0' && SkipDelim) {
222222
*context = nullptr;
223223
return nullptr;
224224
}
225225

226-
char *tok_end = tok_start;
227-
while (*tok_end != '\0' && !delims.test(static_cast<size_t>(*tok_end)))
226+
unsigned char *tok_end = tok_start;
227+
while (*tok_end != '\0' && !delims.test(*tok_end))
228228
++tok_end;
229229

230230
if (*tok_end == '\0') {
231231
*context = nullptr;
232232
} else {
233233
*tok_end = '\0';
234-
*context = tok_end + 1;
234+
*context = reinterpret_cast<char *>(tok_end + 1);
235235
}
236-
return tok_start;
236+
return reinterpret_cast<char *>(tok_start);
237237
}
238238

239239
LIBC_INLINE size_t strlcpy(char *__restrict dst, const char *__restrict src,

libc/test/src/string/strcspn_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,7 @@ TEST(LlvmLibcStrCSpnTest, DuplicatedCharactersNotPartOfComplementarySpan) {
4848
EXPECT_EQ(LIBC_NAMESPACE::strcspn("aaaa", "aa"), size_t{0});
4949
EXPECT_EQ(LIBC_NAMESPACE::strcspn("aaaa", "baa"), size_t{0});
5050
}
51+
52+
TEST(LlvmLibcStrCSpnTest, TopBitSet) {
53+
EXPECT_EQ(LIBC_NAMESPACE::strcspn("hello\x80world", "\x80"), size_t{5});
54+
}

libc/test/src/string/strpbrk_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,7 @@ TEST(LlvmLibcStrPBrkTest, FindsFirstOfRepeated) {
6060
TEST(LlvmLibcStrPBrkTest, FindsFirstInBreakset) {
6161
EXPECT_STREQ(LIBC_NAMESPACE::strpbrk("12345", "34"), "345");
6262
}
63+
64+
TEST(LlvmLibcStrPBrkTest, TopBitSet) {
65+
EXPECT_STREQ(LIBC_NAMESPACE::strpbrk("hello\x80world", "\x80 "), "\x80world");
66+
}

libc/test/src/string/strsep_test.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ TEST(LlvmLibcStrsepTest, SubsequentSearchesReturnNull) {
6161
ASSERT_EQ(LIBC_NAMESPACE::strsep(&string, ":"), nullptr);
6262
}
6363

64+
TEST(LlvmLibcStrsepTest, TopBitSet) {
65+
char top_bit_set_str[] = "hello\x80world";
66+
char *p = top_bit_set_str;
67+
ASSERT_STREQ(LIBC_NAMESPACE::strsep(&p, "\x80"), "hello");
68+
ASSERT_STREQ(LIBC_NAMESPACE::strsep(&p, "\x80"), "world");
69+
ASSERT_EQ(LIBC_NAMESPACE::strsep(&p, "\x80"), nullptr);
70+
}
71+
6472
#if defined(LIBC_ADD_NULL_CHECKS)
6573

6674
TEST(LlvmLibcStrsepTest, CrashOnNullPtr) {

libc/test/src/string/strspn_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ TEST(LlvmLibcStrSpnTest, DuplicatedCharactersToBeSearchedForShouldStillMatch) {
8585
EXPECT_EQ(LIBC_NAMESPACE::strspn("aaaa", "aa"), size_t{4});
8686
}
8787

88+
TEST(LlvmLibcStrSpnTest, TopBitSet) {
89+
EXPECT_EQ(LIBC_NAMESPACE::strspn("hello\x80world", "helo\x80rld"), size_t{6});
90+
}
91+
8892
#if defined(LIBC_ADD_NULL_CHECKS)
8993

9094
TEST(LlvmLibcStrSpnTest, CrashOnNullPtr) {

libc/test/src/string/strtok_r_test.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,3 +131,11 @@ TEST(LlvmLibcStrTokReentrantTest, SubsequentSearchesReturnNull) {
131131
ASSERT_EQ(LIBC_NAMESPACE::strtok_r(nullptr, ":", &reserve), nullptr);
132132
ASSERT_EQ(LIBC_NAMESPACE::strtok_r(nullptr, ":", &reserve), nullptr);
133133
}
134+
135+
TEST(LlvmLibcStrTokReentrantTest, TopBitSet) {
136+
char top_bit_set_str[] = "hello\x80world";
137+
char *p;
138+
ASSERT_STREQ(LIBC_NAMESPACE::strtok_r(top_bit_set_str, "\x80", &p), "hello");
139+
ASSERT_STREQ(LIBC_NAMESPACE::strtok_r(nullptr, "\x80", &p), "world");
140+
ASSERT_EQ(LIBC_NAMESPACE::strtok_r(nullptr, "\x80", &p), nullptr);
141+
}

libc/test/src/string/strtok_test.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,10 @@ TEST(LlvmLibcStrTokTest, SubsequentSearchesReturnNull) {
8383
ASSERT_EQ(LIBC_NAMESPACE::strtok(nullptr, ":"), nullptr);
8484
ASSERT_EQ(LIBC_NAMESPACE::strtok(nullptr, ":"), nullptr);
8585
}
86+
87+
TEST(LlvmLibcStrTokTest, TopBitSet) {
88+
char top_bit_set_str[] = "hello\x80world";
89+
ASSERT_STREQ(LIBC_NAMESPACE::strtok(top_bit_set_str, "\x80"), "hello");
90+
ASSERT_STREQ(LIBC_NAMESPACE::strtok(nullptr, "\x80"), "world");
91+
ASSERT_EQ(LIBC_NAMESPACE::strtok(nullptr, "\x80"), nullptr);
92+
}

utils/bazel/llvm-project-overlay/libc/BUILD.bazel

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5251,6 +5251,16 @@ libc_function(
52515251
],
52525252
)
52535253

5254+
libc_function(
5255+
name = "strtok_r",
5256+
srcs = ["src/string/strtok_r.cpp"],
5257+
hdrs = ["src/string/strtok_r.h"],
5258+
deps = [
5259+
":__support_common",
5260+
":string_utils",
5261+
],
5262+
)
5263+
52545264
################################ fcntl targets #################################
52555265

52565266
libc_function(

utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ libc_test(
5959
],
6060
)
6161

62+
libc_test(
63+
name = "strpbrk_test",
64+
srcs = ["strpbrk_test.cpp"],
65+
deps = [
66+
"//libc:strpbrk",
67+
],
68+
)
69+
6270
libc_test(
6371
name = "strsep_test",
6472
srcs = ["strsep_test.cpp"],
@@ -127,6 +135,14 @@ libc_test(
127135
],
128136
)
129137

138+
libc_test(
139+
name = "strtok_r_test",
140+
srcs = ["strtok_r_test.cpp"],
141+
deps = [
142+
"//libc:strtok_r",
143+
],
144+
)
145+
130146
libc_test_library(
131147
name = "memory_check_utils",
132148
hdrs = ["memory_utils/memory_check_utils.h"],

0 commit comments

Comments
 (0)