Index: libc/src/string/CMakeLists.txt =================================================================== --- libc/src/string/CMakeLists.txt +++ libc/src/string/CMakeLists.txt @@ -105,6 +105,8 @@ strchr.cpp HDRS strchr.h + DEPENDS + .memory_utils.strchr_implementation ) add_entrypoint_object( @@ -313,6 +315,8 @@ strrchr.cpp HDRS strrchr.h + DEPENDS + .memory_utils.strchr_implementation ) add_entrypoint_object( Index: libc/src/string/memory_utils/CMakeLists.txt =================================================================== --- libc/src/string/memory_utils/CMakeLists.txt +++ libc/src/string/memory_utils/CMakeLists.txt @@ -76,3 +76,9 @@ HDRS strstr_implementations.h ) + +add_header_library( + strchr_implementation + HDRS + strchr_implementations.h +) Index: libc/src/string/memory_utils/strchr_implementations.h =================================================================== --- /dev/null +++ libc/src/string/memory_utils/strchr_implementations.h @@ -0,0 +1,35 @@ +//===-- strchr implementation -----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_STRCHR_IMPLEMENTATIONS_H +#define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_STRCHR_IMPLEMENTATIONS_H + +#include + +namespace __llvm_libc { + +constexpr static char *strchr_implementation(const char *src, int c) { + char ch = static_cast(c); + for (; *src && *src != ch; ++src) + ; + return *src == ch ? const_cast(src) : nullptr; +} + +constexpr static char *strrchr_implementation(const char *src, int c) { + char ch = static_cast(c); + char *last_occurrence = nullptr; + for (; *src; ++src) { + if (*src == ch) + last_occurrence = const_cast(src); + } + return last_occurrence; +} + +} // namespace __llvm_libc + +#endif // LLVM_LIBC_SRC_STRING_MEMORY_UTILS_STRCHR_IMPLEMENTATIONS_H Index: libc/src/string/strchr.cpp =================================================================== --- libc/src/string/strchr.cpp +++ libc/src/string/strchr.cpp @@ -9,15 +9,13 @@ #include "src/string/strchr.h" #include "src/__support/common.h" +#include "src/string/memory_utils/strchr_implementations.h" namespace __llvm_libc { // TODO: Look at performance benefits of comparing words. LLVM_LIBC_FUNCTION(char *, strchr, (const char *src, int c)) { - const char ch = static_cast(c); - for (; *src && *src != ch; ++src) - ; - return *src == ch ? const_cast(src) : nullptr; + return strchr_implementation(src, c); } } // namespace __llvm_libc Index: libc/src/string/strrchr.cpp =================================================================== --- libc/src/string/strrchr.cpp +++ libc/src/string/strrchr.cpp @@ -9,17 +9,12 @@ #include "src/string/strrchr.h" #include "src/__support/common.h" +#include "src/string/memory_utils/strchr_implementations.h" namespace __llvm_libc { LLVM_LIBC_FUNCTION(char *, strrchr, (const char *src, int c)) { - const char ch = static_cast(c); - char *last_occurrence = nullptr; - for (; *src; ++src) { - if (*src == ch) - last_occurrence = const_cast(src); - } - return last_occurrence; + return strrchr_implementation(src, c); } } // namespace __llvm_libc Index: libc/test/src/string/CMakeLists.txt =================================================================== --- libc/test/src/string/CMakeLists.txt +++ libc/test/src/string/CMakeLists.txt @@ -84,6 +84,12 @@ libc.src.string.strcat ) +add_header_library( + strchr_test_support + HDRS + StrchrTest.h +) + add_libc_unittest( strchr_test SUITE @@ -92,6 +98,7 @@ strchr_test.cpp DEPENDS libc.src.string.strchr + .strchr_test_support ) add_libc_unittest( @@ -296,6 +303,7 @@ strrchr_test.cpp DEPENDS libc.src.string.strrchr + .strchr_test_support ) add_libc_unittest( Index: libc/test/src/string/StrchrTest.h =================================================================== --- /dev/null +++ libc/test/src/string/StrchrTest.h @@ -0,0 +1,201 @@ +//===-- Tests for str{,r}chr and {,r}index functions ------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "test/UnitTest/Test.h" + +template struct StrchrTest : public __llvm_libc::testing::Test { + void findsFirstCharacter() { + const char *src = "abcde"; + + // Should return original string since 'a' is the first character. + ASSERT_STREQ(Func(src, 'a'), "abcde"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsMiddleCharacter() { + const char *src = "abcde"; + + // Should return characters after (and including) 'c'. + ASSERT_STREQ(Func(src, 'c'), "cde"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsLastCharacterThatIsNotNullTerminator() { + const char *src = "abcde"; + + // Should return 'e' and null-terminator. + ASSERT_STREQ(Func(src, 'e'), "e"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsNullTerminator() { + const char *src = "abcde"; + + // Should return null terminator. + ASSERT_STREQ(Func(src, '\0'), ""); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void characterNotWithinStringShouldReturnNullptr() { + // Since 'z' is not within the string, should return nullptr. + ASSERT_STREQ(Func("123?", 'z'), nullptr); + } + + void theSourceShouldNotChange() { + const char *src = "abcde"; + // When the character is found, the source string should not change. + Func(src, 'd'); + ASSERT_STREQ(src, "abcde"); + // Same case for when the character is not found. + Func(src, 'z'); + ASSERT_STREQ(src, "abcde"); + // Same case for when looking for nullptr. + Func(src, '\0'); + ASSERT_STREQ(src, "abcde"); + } + + void shouldFindFirstOfDuplicates() { + // '1' is duplicated in the string, but it should find the first copy. + ASSERT_STREQ(Func("abc1def1ghi", '1'), "1def1ghi"); + + const char *dups = "XXXXX"; + // Should return original string since 'X' is the first character. + ASSERT_STREQ(Func(dups, 'X'), dups); + } + + void emptyStringShouldOnlyMatchNullTerminator() { + // Null terminator should match. + ASSERT_STREQ(Func("", '\0'), ""); + // All other characters should not match. + ASSERT_STREQ(Func("", 'Z'), nullptr); + ASSERT_STREQ(Func("", '3'), nullptr); + ASSERT_STREQ(Func("", '*'), nullptr); + } +}; + +template struct StrrchrTest : public __llvm_libc::testing::Test { + void findsFirstCharacter() { + const char *src = "abcde"; + + // Should return original string since 'a' is the first character. + ASSERT_STREQ(Func(src, 'a'), "abcde"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsMiddleCharacter() { + const char *src = "abcde"; + + // Should return characters after (and including) 'c'. + ASSERT_STREQ(Func(src, 'c'), "cde"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsLastCharacterThatIsNotNullTerminator() { + const char *src = "abcde"; + + // Should return 'e' and null-terminator. + ASSERT_STREQ(Func(src, 'e'), "e"); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsNullTerminator() { + const char *src = "abcde"; + + // Should return null terminator. + ASSERT_STREQ(Func(src, '\0'), ""); + // Source string should not change. + ASSERT_STREQ(src, "abcde"); + } + + void findsLastBehindFirstNullTerminator() { + const char src[6] = {'a', 'a', '\0', 'b', '\0', 'c'}; + // 'b' is behind a null terminator, so should not be found. + ASSERT_STREQ(Func(src, 'b'), nullptr); + // Same goes for 'c'. + ASSERT_STREQ(Func(src, 'c'), nullptr); + + // Should find the second of the two a's. + ASSERT_STREQ(Func(src, 'a'), "a"); + } + + void characterNotWithinStringShouldReturnNullptr() { + // Since 'z' is not within the string, should return nullptr. + ASSERT_STREQ(Func("123?", 'z'), nullptr); + } + + void shouldFindLastOfDuplicates() { + // '1' is duplicated in the string, but it should find the last copy. + ASSERT_STREQ(Func("abc1def1ghi", '1'), "1ghi"); + + const char *dups = "XXXXX"; + // Should return the last occurrence of 'X'. + ASSERT_STREQ(Func(dups, 'X'), "X"); + } + + void emptyStringShouldOnlyMatchNullTerminator() { + // Null terminator should match. + ASSERT_STREQ(Func("", '\0'), ""); + // All other characters should not match. + ASSERT_STREQ(Func("", 'A'), nullptr); + ASSERT_STREQ(Func("", '2'), nullptr); + ASSERT_STREQ(Func("", '*'), nullptr); + } +}; + +#define STRCHR_TEST(name, func) \ + using LlvmLibc##name##Test = StrchrTest; \ + TEST_F(LlvmLibc##name##Test, FindsFirstCharacter) { findsFirstCharacter(); } \ + TEST_F(LlvmLibc##name##Test, FindsMiddleCharacter) { \ + findsMiddleCharacter(); \ + } \ + TEST_F(LlvmLibc##name##Test, FindsLastCharacterThatIsNotNullTerminator) { \ + findsLastCharacterThatIsNotNullTerminator(); \ + } \ + TEST_F(LlvmLibc##name##Test, FindsNullTerminator) { findsNullTerminator(); } \ + TEST_F(LlvmLibc##name##Test, CharacterNotWithinStringShouldReturnNullptr) { \ + characterNotWithinStringShouldReturnNullptr(); \ + } \ + TEST_F(LlvmLibc##name##Test, TheSourceShouldNotChange) { \ + theSourceShouldNotChange(); \ + } \ + TEST_F(LlvmLibc##name##Test, ShouldFindFirstOfDuplicates) { \ + shouldFindFirstOfDuplicates(); \ + } \ + TEST_F(LlvmLibc##name##Test, EmptyStringShouldOnlyMatchNullTerminator) { \ + emptyStringShouldOnlyMatchNullTerminator(); \ + } + +#define STRRCHR_TEST(name, func) \ + using LlvmLibc##name##Test = StrrchrTest; \ + TEST_F(LlvmLibc##name##Test, FindsFirstCharacter) { findsFirstCharacter(); } \ + TEST_F(LlvmLibc##name##Test, FindsMiddleCharacter) { \ + findsMiddleCharacter(); \ + } \ + TEST_F(LlvmLibc##name##Test, FindsLastCharacterThatIsNotNullTerminator) { \ + findsLastCharacterThatIsNotNullTerminator(); \ + } \ + TEST_F(LlvmLibc##name##Test, FindsNullTerminator) { findsNullTerminator(); } \ + TEST_F(LlvmLibc##name##Test, FindsLastBehindFirstNullTerminator) { \ + findsLastBehindFirstNullTerminator(); \ + } \ + TEST_F(LlvmLibc##name##Test, CharacterNotWithinStringShouldReturnNullptr) { \ + characterNotWithinStringShouldReturnNullptr(); \ + } \ + TEST_F(LlvmLibc##name##Test, ShouldFindLastOfDuplicates) { \ + shouldFindLastOfDuplicates(); \ + } \ + TEST_F(LlvmLibc##name##Test, EmptyStringShouldOnlyMatchNullTerminator) { \ + emptyStringShouldOnlyMatchNullTerminator(); \ + } Index: libc/test/src/string/strchr_test.cpp =================================================================== --- libc/test/src/string/strchr_test.cpp +++ libc/test/src/string/strchr_test.cpp @@ -6,77 +6,9 @@ // //===----------------------------------------------------------------------===// +#include "StrchrTest.h" + #include "src/string/strchr.h" #include "test/UnitTest/Test.h" -TEST(LlvmLibcStrChrTest, FindsFirstCharacter) { - const char *src = "abcde"; - - // Should return original string since 'a' is the first character. - ASSERT_STREQ(__llvm_libc::strchr(src, 'a'), "abcde"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrChrTest, FindsMiddleCharacter) { - const char *src = "abcde"; - - // Should return characters after (and including) 'c'. - ASSERT_STREQ(__llvm_libc::strchr(src, 'c'), "cde"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrChrTest, FindsLastCharacterThatIsNotNullTerminator) { - const char *src = "abcde"; - - // Should return 'e' and null-terminator. - ASSERT_STREQ(__llvm_libc::strchr(src, 'e'), "e"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrChrTest, FindsNullTerminator) { - const char *src = "abcde"; - - // Should return null terminator. - ASSERT_STREQ(__llvm_libc::strchr(src, '\0'), ""); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrChrTest, CharacterNotWithinStringShouldReturnNullptr) { - // Since 'z' is not within the string, should return nullptr. - ASSERT_STREQ(__llvm_libc::strchr("123?", 'z'), nullptr); -} - -TEST(LlvmLibcStrChrTest, TheSourceShouldNotChange) { - const char *src = "abcde"; - // When the character is found, the source string should not change. - __llvm_libc::strchr(src, 'd'); - ASSERT_STREQ(src, "abcde"); - // Same case for when the character is not found. - __llvm_libc::strchr(src, 'z'); - ASSERT_STREQ(src, "abcde"); - // Same case for when looking for nullptr. - __llvm_libc::strchr(src, '\0'); - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrChrTest, ShouldFindFirstOfDuplicates) { - // '1' is duplicated in the string, but it should find the first copy. - ASSERT_STREQ(__llvm_libc::strchr("abc1def1ghi", '1'), "1def1ghi"); - - const char *dups = "XXXXX"; - // Should return original string since 'X' is the first character. - ASSERT_STREQ(__llvm_libc::strchr(dups, 'X'), dups); -} - -TEST(LlvmLibcStrChrTest, EmptyStringShouldOnlyMatchNullTerminator) { - // Null terminator should match. - ASSERT_STREQ(__llvm_libc::strchr("", '\0'), ""); - // All other characters should not match. - ASSERT_STREQ(__llvm_libc::strchr("", 'Z'), nullptr); - ASSERT_STREQ(__llvm_libc::strchr("", '3'), nullptr); - ASSERT_STREQ(__llvm_libc::strchr("", '*'), nullptr); -} +STRCHR_TEST(Strchr, __llvm_libc::strchr) Index: libc/test/src/string/strrchr_test.cpp =================================================================== --- libc/test/src/string/strrchr_test.cpp +++ libc/test/src/string/strrchr_test.cpp @@ -6,75 +6,9 @@ // //===----------------------------------------------------------------------===// +#include "StrchrTest.h" + #include "src/string/strrchr.h" #include "test/UnitTest/Test.h" -TEST(LlvmLibcStrRChrTest, FindsFirstCharacter) { - const char *src = "abcde"; - - // Should return original string since 'a' is the first character. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'a'), "abcde"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrRChrTest, FindsMiddleCharacter) { - const char *src = "abcde"; - - // Should return characters after (and including) 'c'. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'c'), "cde"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrRChrTest, FindsLastCharacterThatIsNotNullTerminator) { - const char *src = "abcde"; - - // Should return 'e' and null-terminator. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'e'), "e"); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrRChrTest, FindsNullTerminator) { - const char *src = "abcde"; - - // Should return null terminator. - ASSERT_STREQ(__llvm_libc::strrchr(src, '\0'), ""); - // Source string should not change. - ASSERT_STREQ(src, "abcde"); -} - -TEST(LlvmLibcStrRChrTest, FindsLastBehindFirstNullTerminator) { - const char src[6] = {'a', 'a', '\0', 'b', '\0', 'c'}; - // 'b' is behind a null terminator, so should not be found. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'b'), nullptr); - // Same goes for 'c'. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'c'), nullptr); - - // Should find the second of the two a's. - ASSERT_STREQ(__llvm_libc::strrchr(src, 'a'), "a"); -} - -TEST(LlvmLibcStrRChrTest, CharacterNotWithinStringShouldReturnNullptr) { - // Since 'z' is not within the string, should return nullptr. - ASSERT_STREQ(__llvm_libc::strrchr("123?", 'z'), nullptr); -} - -TEST(LlvmLibcStrRChrTest, ShouldFindLastOfDuplicates) { - // '1' is duplicated in the string, but it should find the last copy. - ASSERT_STREQ(__llvm_libc::strrchr("abc1def1ghi", '1'), "1ghi"); - - const char *dups = "XXXXX"; - // Should return the last occurrence of 'X'. - ASSERT_STREQ(__llvm_libc::strrchr(dups, 'X'), "X"); -} - -TEST(LlvmLibcStrRChrTest, EmptyStringShouldOnlyMatchNullTerminator) { - // Null terminator should match. - ASSERT_STREQ(__llvm_libc::strrchr("", '\0'), ""); - // All other characters should not match. - ASSERT_STREQ(__llvm_libc::strrchr("", 'A'), nullptr); - ASSERT_STREQ(__llvm_libc::strrchr("", '2'), nullptr); - ASSERT_STREQ(__llvm_libc::strrchr("", '*'), nullptr); -} +STRRCHR_TEST(Strrchr, __llvm_libc::strrchr) Index: utils/bazel/llvm-project-overlay/libc/BUILD.bazel =================================================================== --- utils/bazel/llvm-project-overlay/libc/BUILD.bazel +++ utils/bazel/llvm-project-overlay/libc/BUILD.bazel @@ -1589,6 +1589,7 @@ "src/string/memory_utils/memcpy_implementations.h", "src/string/memory_utils/memmove_implementations.h", "src/string/memory_utils/memset_implementations.h", + "src/string/memory_utils/strchr_implementations.h", "src/string/memory_utils/strcmp_implementations.h", "src/string/memory_utils/strstr_implementations.h", "src/string/memory_utils/x86_64/memcmp_implementations.h", @@ -1635,7 +1636,7 @@ name = "memcpy", srcs = ["src/string/memcpy.cpp"], hdrs = ["src/string/memcpy.h"], - copts = ["-mllvm --tail-merge-threshold=0"], + #copts = ["-mllvm --tail-merge-threshold=0"], features = no_sanitize_features, weak = True, deps = [ Index: utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel =================================================================== --- utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel +++ utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel @@ -42,12 +42,19 @@ ], ) +cc_library( + name = "strchr_test_helper", + hdrs = ["StrchrTest.h"], + deps = ["//libc/test/UnitTest:LibcUnitTest"], +) + libc_test( name = "strchr_test", srcs = ["strchr_test.cpp"], libc_function_deps = [ "//libc:strchr", ], + deps = [":strchr_test_helper"], ) libc_test( @@ -80,6 +87,7 @@ libc_function_deps = [ "//libc:strrchr", ], + deps = [":strchr_test_helper"], ) libc_test(