Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jun 10, 2024

Instead of hardcoding a loop for small strings, always call char_traits::compare which ends up desugaring to __builtin_memcmp.

Note that the original code dates back 11 years, when we didn't lower to intrinsics in char_traits::compare.

Fixes #94222

Instead of hardcoding a loop for small strings, always call
char_traits::compare which ends up desugaring to __builtin_memcmp.

Note that the original code dates back 11 years, when we didn't
lower to intrinsics in `char_traits::compare`.

Fixes llvm#94222
@ldionne ldionne requested a review from a team as a code owner June 10, 2024 15:21
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 10, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

Instead of hardcoding a loop for small strings, always call char_traits::compare which ends up desugaring to __builtin_memcmp.

Note that the original code dates back 11 years, when we didn't lower to intrinsics in char_traits::compare.

Fixes #94222


Full diff: https://github.com/llvm/llvm-project/pull/95000.diff

1 Files Affected:

  • (modified) libcxx/include/string (+3-10)
diff --git a/libcxx/include/string b/libcxx/include/string
index 1db803e822d72..5301f8a87d9bb 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3746,17 +3746,10 @@ template <class _Allocator>
 inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
 operator==(const basic_string<char, char_traits<char>, _Allocator>& __lhs,
            const basic_string<char, char_traits<char>, _Allocator>& __rhs) _NOEXCEPT {
-  size_t __lhs_sz = __lhs.size();
-  if (__lhs_sz != __rhs.size())
+  size_t __sz = __lhs.size();
+  if (__sz != __rhs.size())
     return false;
-  const char* __lp = __lhs.data();
-  const char* __rp = __rhs.data();
-  if (__lhs.__is_long())
-    return char_traits<char>::compare(__lp, __rp, __lhs_sz) == 0;
-  for (; __lhs_sz != 0; --__lhs_sz, ++__lp, ++__rp)
-    if (*__lp != *__rp)
-      return false;
-  return true;
+  return char_traits<char>::compare(__lhs.data(), __rhs.data(), __sz) == 0;
 }
 
 #if _LIBCPP_STD_VER <= 17

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@ldionne ldionne merged commit 6faae13 into llvm:main Jun 11, 2024
@ldionne ldionne deleted the review/fix-string-comparison branch June 11, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libc++'s std::string::operator== could be more libfuzzer friendly
3 participants