[libc] Independent strcat/strncat/stpcpy

[michaelrj-google]

The previous implementations called other entrypoints. This patch fixes
strcat, strncat, and stpcpy to be properly independent.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

The previous implementations called other entrypoints. This patch fixes
strcat and strncat to be properly independent.

---

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

3 Files Affected:

• (modified) libc/src/string/CMakeLists.txt (-2)
• (modified) libc/src/string/strcat.cpp (+5-4)
• (modified) libc/src/string/strncat.cpp (+5-4)

diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index 2c607bf8ea895..0e9fc33418515 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -110,7 +110,6 @@ add_entrypoint_object(
   HDRS
     strcat.h
   DEPENDS
-    .strcpy
     .string_utils
     libc.include.llvm-libc-types.size_t
 )
@@ -267,7 +266,6 @@ add_entrypoint_object(
   HDRS
     strncat.h
   DEPENDS
-    .strncpy
     .string_utils
     libc.include.llvm-libc-types.size_t
 )
diff --git a/libc/src/string/strcat.cpp b/libc/src/string/strcat.cpp
index 0eb189ce204f0..4b6717c3c98e5 100644
--- a/libc/src/string/strcat.cpp
+++ b/libc/src/string/strcat.cpp
@@ -8,7 +8,6 @@
 
 #include "src/string/strcat.h"
 #include "src/__support/macros/config.h"
-#include "src/string/strcpy.h"
 #include "src/string/string_utils.h"
 
 #include "src/__support/common.h"
@@ -18,9 +17,11 @@ namespace LIBC_NAMESPACE_DECL {
 LLVM_LIBC_FUNCTION(char *, strcat,
                    (char *__restrict dest, const char *__restrict src)) {
   size_t dest_length = internal::string_length(dest);
-  size_t src_length = internal::string_length(src);
-  LIBC_NAMESPACE::strcpy(dest + dest_length, src);
-  dest[dest_length + src_length] = '\0';
+  size_t i;
+  for (i = 0; src[i] != '\0'; ++i)
+    dest[dest_length + i] = src[i];
+
+  dest[dest_length + i] = '\0';
   return dest;
 }
 
diff --git a/libc/src/string/strncat.cpp b/libc/src/string/strncat.cpp
index 221881f93c47a..abd8d001f779c 100644
--- a/libc/src/string/strncat.cpp
+++ b/libc/src/string/strncat.cpp
@@ -18,11 +18,12 @@ namespace LIBC_NAMESPACE_DECL {
 LLVM_LIBC_FUNCTION(char *, strncat,
                    (char *__restrict dest, const char *__restrict src,
                     size_t count)) {
-  size_t src_length = internal::string_length(src);
-  size_t copy_amount = src_length > count ? count : src_length;
   size_t dest_length = internal::string_length(dest);
-  LIBC_NAMESPACE::strncpy(dest + dest_length, src, copy_amount);
-  dest[dest_length + copy_amount] = '\0';
+  size_t i;
+  for (i = 0; i < count && src[i] != '\0'; ++i)
+    dest[dest_length + i] = src[i];
+
+  dest[dest_length + i] = '\0';
   return dest;
 }
 

[michaelrj-google]

fixed merge conflict and switched to __builtin_memcpy

[jhuber6]

We have a definition for this in libc/src/string/memory_utils/generic/builtin.h.

[michaelrj-google]

is there a reason to call that version instead of calling the builtin directly?

[jhuber6]

Mostly was wondering if there would be an easier way to select which flavor of memcpy we wanted. Might've been better as a template function than this macro dispatch but that's a big change.

[michaelrj-google]

I'd be interested in making memcpy specialization template controlled, though to keep the includes for different architectures separate we would probably still need some amount of macro control. I think that's probably a topic for a separate PR though.

[michaelrj-google]

rebased, planning to merge once presubmits finish