[libc] Move strdup implementation to a new header

[jhuber6]

Summary

The strdup family of functions rely on malloc to be implemented.
Its presence in the string_utils.h header meant that compiling many of
the string functions relied on malloc being implementated as well.
This patch simply moves the implementation into a new file to avoid
including stdlib.h from the other string functions. This was a barrier
for compiling string functions for the GPU where there is no malloc
currently.

[sivachandra]

I would think that GPU is one example of a platform where malloc isn't (yet) available. So, the mechanical aspects of this patch LGTM. I think we should do a few things more:

1. At the least, add comments warning developers that allocators should not be called from string_utils.h. Likewise, add comments explaining what exactly is string_utils_malloc.h (or whatever it will be called) for.
2. Does not have to be done in this patch, but would be nice if a developer mistake/mixup can be caught during build + test cycle. Worst case, the GPU builder will catch it I believe, but would be nice to be able to do it during a normal host build + test development cycle.

libc/src/string/string_utils_malloc.h

1 ↗

(On Diff #477594)

malloc is one among a bunch of standard allocator functions. So, may be call this allocating_string_utils.h?

[jhuber6]

Addressing comments

[sivachandra]

You should add another header library for the new header file like this: https://github.com/llvm/llvm-project/blob/main/libc/src/string/CMakeLists.txt#L3. Otherwise LGTM.

libc/src/string/CMakeLists.txt

 add_subdirectory(memory_utils)
 
 add_header_library(
   string_utils
   HDRS
     string_utils.h
   DEPENDS
     libc.include.stdlib
     libc.src.__support.CPP.bitset
     .memory_utils.memcpy_implementation
     .memory_utils.bzero_implementation
 )
 
+add_header_library(
+  allocating_string_utils
+  HDRS
+    allocating_string_utils.h
+  DEPENDS
+    libc.include.stdlib
+    .memory_utils.memcpy_implementation
+)
+
 add_entrypoint_object(
   memccpy
   SRCS
     memccpy.cpp
   HDRS
     memccpy.h
 )
 

libc/src/string/allocating_string_utils.h

+//===-- Allocating string utils ---------------------------------*- 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 LIBC_SRC_STRING_ALLOCATING_STRING_UTILS_H
+#define LIBC_SRC_STRING_ALLOCATING_STRING_UTILS_H
+
+#include "src/__support/CPP/bitset.h"
+#include "src/__support/common.h"
+#include "src/string/memory_utils/bzero_implementations.h"
+#include "src/string/memory_utils/memcpy_implementations.h"
+#include "src/string/string_utils.h"
+#include <stddef.h> // For size_t
+#include <stdlib.h> // For malloc
+
+namespace __llvm_libc {
+namespace internal {
+
+inline char *strdup(const char *src) {
+  if (src == nullptr)
+    return nullptr;
+  size_t len = string_length(src) + 1;
+  char *newstr = reinterpret_cast<char *>(::malloc(len));
+  if (newstr == nullptr)
+    return nullptr;
+  inline_memcpy(newstr, src, len);
+  return newstr;
+}
+
+} // namespace internal
+} // namespace __llvm_libc
+
+#endif

libc/src/string/strdup.cpp

 //===-- Implementation of strdup ------------------------------------------===//
 //
 // 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 "src/string/strdup.h"
+#include "src/string/allocating_string_utils.h"
 #include "src/string/memory_utils/memcpy_implementations.h"
-#include "src/string/string_utils.h"
 
 #include "src/__support/common.h"
 
 #include <stdlib.h>
 
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(char *, strdup, (const char *src)) {
   return internal::strdup(src);
 }
 
 } // namespace __llvm_libc

libc/src/string/string_utils.h

 //===-- String utils --------------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
+//
+// Standalone string utility functions. Utilities requiring memory allocations
+// should be placed in allocating_string_utils.h intead.
+//
+//===----------------------------------------------------------------------===//
 
 #ifndef LIBC_SRC_STRING_STRING_UTILS_H
 #define LIBC_SRC_STRING_STRING_UTILS_H
 
 #include "src/__support/CPP/bitset.h"
 #include "src/__support/common.h"
 #include "src/string/memory_utils/bzero_implementations.h"
 #include "src/string/memory_utils/memcpy_implementations.h"
 #include <stddef.h> // For size_t
-#include <stdlib.h> // For malloc and free
 
 namespace __llvm_libc {
 namespace internal {
 
 // Returns the length of a string, denoted by the first occurrence
 // of a null terminator.
 static inline size_t string_length(const char *src) {
   size_t length;
static inline size_t strlcpy(char *__restrict dst, const char *__restrict src,
   if (!size)
     return len;
   size_t n = len < size - 1 ? len : size - 1;
   inline_memcpy(dst, src, n);
   inline_bzero(dst + n, size - n);
   return len;
 }
 
-inline char *strdup(const char *src) {
-  if (src == nullptr)
-    return nullptr;
-  size_t len = string_length(src) + 1;
-  char *newstr = reinterpret_cast<char *>(::malloc(len));
-  if (newstr == nullptr)
-    return nullptr;
-  inline_memcpy(newstr, src, len);
-  return newstr;
-}
-
 } // namespace internal
 } // namespace __llvm_libc
 
 #endif //  LIBC_SRC_STRING_STRING_UTILS_H

libc/src/unistd/linux/getcwd.cpp

 //===-- Linux implementation of getcwd ------------------------------------===//
 //
 // 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 "src/unistd/getcwd.h"
 
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
-#include "src/string/string_utils.h" // For strdup.
+#include "src/string/allocating_string_utils.h" // For strdup.
 
 #include <errno.h>
 #include <linux/limits.h> // This is safe to include without any name pollution.
 #include <stdlib.h>
 #include <sys/syscall.h> // For syscall numbers.
 
 namespace __llvm_libc {