[libc] add malloc funcs as external entrypoints

[michaelrj]

Summary

malloc, calloc, realloc, and free are all functions that other libc
functions depend on, but are pulled from external sources, instead of
having an internal implementation. This patch adds a way to include
functions like that as entrypoints in the list of external entrypoints,
and includes the malloc functions using this new path.

[sivachandra]

While this is OK logically, I would like to take a step back and design the external entrypoint concept in a more cleaner fashion. With this change as is, there will be an external entrypoint rule and there is also an EXT_DEPS option to add_entrypoint_library. We should unify these. What I mean is the following steps:

1. Make add_external_entrypoint have a DEPENDS option which captures the external deps. We should move the SCUDO related logic from lib/CMakeLists.txt to src/stdlib/CMakeLists.txt and list the SCUDO entrypoints there with the appropriate DEPENDS list..
2. Remove EXT_DEPS option to add_entrypoint_library and list the external entrypoint deps as part of the normal DEPENDS option.
3. add_entrypoint_library should recognize that a DEPENDS entry is an external entrypoint and get the objects appropriately.
4. We will not need any special handling in libc/CMakeLists.txt as in being done in this change. Also, the external entrypoints can be listed like any other entrypoint in the entrypoints.txt files.

libc/src/stdlib/CMakeLists.txt

184

I think these targets should be added conditionally like this:

if(LLVM_LIBC_INCLUDE_SCUDO)
  ...
endif()

[abrachet]

I think I remember the original plan in this case was to use redirectors. We have add_redirector_object, but I don't think it has ever been used.

[sivachandra]

In D112104#3077996, @abrachet wrote:

I think I remember the original plan in this case was to use redirectors. We have add_redirector_object, but I don't think it has ever been used.

Redirectors were for a different use case: When LLVM libc does not provide a function, a redirector still makes it appear as if it provides it by using a "redirector" to the system libc function [1]. The use case relevant for this patch is different. With allocators, it is further more nuanced [2]. LLVM libc provides an allocator, which is the SCUDO standalone allocator. So, there is no need of a redirector. But, we can't use the normal add_entrypoint_object rule for them because LLVM libc doesn't "own" those sources.

[1] For all practical purposes, we have given up on the idea of redirectors. We might use them still, but not sure for what exactly.
[2] Allocators are a little nuanced because when calling the allocator functions from libc code, we want to call them using the public name and not an internal name. This is because we want the users to be able to override the libc allocators.

[michaelrj]

change the way external entrypoints are handled to be cleaner

[sivachandra]

This is almost what I had in mind. It is still "almost" because of a question I left inline.

libc/cmake/modules/LLVMLibCLibraryRules.cmake

42

Wouldn't this silently ignore all external objects?

libc/cmake/modules/LLVMLibCObjectRules.cmake

287

External entrypoints will not have an object file produced by the libc build. So, we don't need OBJECT_FILE and OBJECT_FILE_RAW properties.

libc/src/stdlib/CMakeLists.txt

190

Indent this?

[michaelrj]

fix the external entrypoints not actually being included

[sivachandra]

libc/cmake/modules/LLVMLibCLibraryRules.cmake

42

Add a comment here saying something like,

It is not possible to recursively extract deps of external dependencies.
So, we just accumulate the direct dep and return.

[michaelrj]

add an explaining comment

libc/cmake/modules/LLVMLibCLibraryRules.cmake

42

You're right it does, I'm not sure why this ever worked. It's fixed now (I checked the resulting .a file to be sure).

libc/cmake/modules/LLVMLibCLibraryRules.cmake

if(${target_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE})
     get_target_property(deps ${target} "DEPS")
     foreach(dep IN LISTS deps)
       collect_object_file_deps(${dep} dep_targets)
       list(APPEND all_deps ${dep_targets})
     endforeach(dep)
     set(${result} ${all_deps} PARENT_SCOPE)
     return()
   endif()
+
+  if(${target_type} STREQUAL ${ENTRYPOINT_EXT_TARGET_TYPE})
+    return()
+  endif()
 endfunction(collect_object_file_deps)
 
 # A rule to build a library from a collection of entrypoint objects.
 # Usage:
 #     add_entrypoint_library(
 #       DEPENDS <list of add_entrypoint_object targets>
-#       EXT_DEPS <list of external object targets, no type checking is done>
 #     )
 #
-# NOTE: If one wants an entrypoint to be availabe in a library, then they will
+# NOTE: If one wants an entrypoint to be available in a library, then they will
 # have to list the entrypoint target explicitly in the DEPENDS list. Implicit
 # entrypoint dependencies will not be added to the library.
 function(add_entrypoint_library target_name)
   cmake_parse_arguments(
     "ENTRYPOINT_LIBRARY"
     "" # No optional arguments
     "" # No single value arguments
-    "DEPENDS;EXT_DEPS" # Multi-value arguments
+    "DEPENDS" # Multi-value arguments
     ${ARGN}
   )
   if(NOT ENTRYPOINT_LIBRARY_DEPENDS)
     message(FATAL_ERROR "'add_entrypoint_library' target requires a DEPENDS list "
                         "of 'add_entrypoint_object' targets.")
   endif()
 
   get_fq_deps_list(fq_deps_list ${ENTRYPOINT_LIBRARY_DEPENDS})
   set(all_deps "")
   foreach(dep IN LISTS fq_deps_list)
     get_target_property(dep_type ${dep} "TARGET_TYPE")
-    if(NOT (${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE}))
+    if(NOT ((${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE}) OR (${dep_type} STREQUAL ${ENTRYPOINT_EXT_TARGET_TYPE})))
       message(FATAL_ERROR "Dependency '${dep}' of 'add_entrypoint_collection' is "
-                          "not an 'add_entrypoint_object' target.")
+                          "not an 'add_entrypoint_object' or 'add_entrypoint_external' target.")
     endif()
     collect_object_file_deps(${dep} recursive_deps)
     list(APPEND all_deps ${recursive_deps})
   endforeach(dep)
   list(REMOVE_DUPLICATES all_deps)
   set(objects "")
   foreach(dep IN LISTS all_deps)
     list(APPEND objects $<TARGET_OBJECTS:${dep}>)
   endforeach(dep)
 
-  foreach(dep IN LISTS ENTRYPOINT_LIBRARY_EXT_DEPS)
-    list(APPEND objects $<TARGET_OBJECTS:${dep}>)
-  endforeach(dep)
-
   add_library(
     ${target_name}
     STATIC
     ${objects}
   )
   set_target_properties(${target_name}  PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 endfunction(add_entrypoint_library)
 

libc/cmake/modules/LLVMLibCObjectRules.cmake

if(LLVM_LIBC_ENABLE_LINTING)
     add_custom_target(${fq_target_name}.__lint__
       DEPENDS ${lint_timestamp})
     add_dependencies(lint-libc ${fq_target_name}.__lint__)
     add_dependencies(${fq_target_name} ${fq_target_name}.__lint__)
   endif()
 
 endfunction(add_entrypoint_object)
 
+set(ENTRYPOINT_EXT_TARGET_TYPE "ENTRYPOINT_EXT")
+
+# A rule for external entrypoint targets.
+# Usage:
+#     add_entrypoint_external(
+#       <target_name>
+#       DEPENDS <list of dependencies>
+#     )
+function(add_entrypoint_external target_name)
+  cmake_parse_arguments(
+    "ADD_ENTRYPOINT_EXT"
+    "" # No optional arguments
+    "" # No single value arguments
+    "DEPENDS"  # Multi value arguments
+    ${ARGN}
+  )
+  get_fq_target_name(${target_name} fq_target_name)
+  set(entrypoint_name ${target_name})
+
+  add_custom_target(${fq_target_name})
+  set_target_properties(
+    ${fq_target_name}
+    PROPERTIES
+      "ENTRYPOINT_NAME" ${entrypoint_name}
+      "TARGET_TYPE" ${ENTRYPOINT_EXT_TARGET_TYPE}
+      "OBJECT_FILE" ""
+      "OBJECT_FILE_RAW" ""
+      "DEPS" "${ADD_ENTRYPOINT_EXT_DEPENDS}"
+  )
+
+endfunction(add_entrypoint_external)
+
 # Rule build a redirector object file.
 function(add_redirector_object target_name)
   cmake_parse_arguments(
     "REDIRECTOR_OBJECT"
     "" # No optional arguments
     "SRC" # The cpp file in which the redirector is defined.
     "" # No multivalue arguments
     ${ARGN}

libc/config/linux/x86_64/entrypoints.txt

list(APPEND TARGET_LIBC_ENTRYPOINTS
     libc.src.time.gmtime_r
     libc.src.time.mktime
 
     # unistd.h entrypoints
     libc.src.unistd.write
   )
 endif()
 
+if(LLVM_LIBC_INCLUDE_SCUDO)
+  list(APPEND TARGET_LIBC_ENTRYPOINTS
+
+    # stdlib.h external entrypoints
+    libc.src.stdlib.malloc
+    libc.src.stdlib.calloc
+    libc.src.stdlib.realloc
+    libc.src.stdlib.free
+
+
+  )
+endif()
+
 set(TARGET_LLVMLIBC_ENTRYPOINTS
   ${TARGET_LIBC_ENTRYPOINTS}
   ${TARGET_LIBM_ENTRYPOINTS}
 )

libc/lib/CMakeLists.txt

-set(SCUDO_DEPS "")
-
-if(LLVM_LIBC_INCLUDE_SCUDO)
-  include(../../compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake)
-  if(NOT (LIBC_TARGET_ARCHITECTURE IN_LIST ALL_SCUDO_STANDALONE_SUPPORTED_ARCH))
-    message(FATAL_ERROR "Architecture ${LIBC_TARGET_ARCHITECTURE} is not supported by SCUDO. 
-Either disable LLVM_LIBC_INCLUDE_SCUDO or change your target architecture.")
-  endif()
-  list(APPEND SCUDO_DEPS RTScudoStandalone.${LIBC_TARGET_ARCHITECTURE} RTScudoStandaloneCWrappers.${LIBC_TARGET_ARCHITECTURE})
-  if((LIBC_TARGET_ARCHITECTURE IN_LIST ALL_GWP_ASAN_SUPPORTED_ARCH) AND COMPILER_RT_BUILD_GWP_ASAN)
-    list(APPEND SCUDO_DEPS RTGwpAsan.${LIBC_TARGET_ARCHITECTURE} 
-                            RTGwpAsanBacktraceLibc.${LIBC_TARGET_ARCHITECTURE} 
-                            RTGwpAsanSegvHandler.${LIBC_TARGET_ARCHITECTURE})
-  elseif(COMPILER_RT_BUILD_GWP_ASAN)
-    message(WARNING "Architecture ${LIBC_TARGET_ARCHITECTURE} is not supported by GWP-ASan. Skipping.")
-  endif()
-endif()
-
 add_entrypoint_library(
   llvmlibc
   DEPENDS
   ${TARGET_LLVMLIBC_ENTRYPOINTS}
-  EXT_DEPS
-  ${SCUDO_DEPS}
 )
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
   set(LIBC_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
 else()
   set(LIBC_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX})
 endif()
 

libc/spec/stdc.td

HeaderSpec StdLib = HeaderSpec<
 
           FunctionSpec<"strtod", RetValSpec<DoubleType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>]>,
           FunctionSpec<"strtof", RetValSpec<FloatType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>]>,
           FunctionSpec<"strtol", RetValSpec<LongType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>, ArgSpec<IntType>]>,
           FunctionSpec<"strtoll", RetValSpec<LongLongType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>, ArgSpec<IntType>]>,
           FunctionSpec<"strtoul", RetValSpec<UnsignedLongType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>, ArgSpec<IntType>]>,
           FunctionSpec<"strtoull", RetValSpec<UnsignedLongLongType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>, ArgSpec<IntType>]>,
 
+          FunctionSpec<"malloc", RetValSpec<VoidPtr>, [ArgSpec<SizeTType>]>,
+          FunctionSpec<"calloc", RetValSpec<VoidPtr>, [ArgSpec<SizeTType>, ArgSpec<SizeTType>]>,
+          FunctionSpec<"realloc", RetValSpec<VoidPtr>, [ArgSpec<VoidPtr>, ArgSpec<SizeTType>]>,
+          FunctionSpec<"free", RetValSpec<VoidType>, [ArgSpec<VoidPtr>]>,
+
           FunctionSpec<"_Exit", RetValSpec<NoReturn>, [ArgSpec<IntType>]>,
       ]
   >;
 
   NamedType IMaxDivTType = NamedType<"imaxdiv_t">;
 
   HeaderSpec IntTypes = HeaderSpec<
       "inttypes.h",

libc/src/stdlib/CMakeLists.txt

add_entrypoint_object(
   SRCS
     qsort.cpp
   HDRS
     qsort.h
   DEPENDS
     libc.include.stdlib
 )
 
+if(LLVM_LIBC_INCLUDE_SCUDO)
+  set(SCUDO_DEPS "")
+
+  include(${LIBC_SOURCE_DIR}/../compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake)
+  if(NOT (LIBC_TARGET_ARCHITECTURE IN_LIST ALL_SCUDO_STANDALONE_SUPPORTED_ARCH))
+    message(FATAL_ERROR "Architecture ${LIBC_TARGET_ARCHITECTURE} is not supported by SCUDO. 
+Either disable LLVM_LIBC_INCLUDE_SCUDO or change your target architecture.")
+  endif()
+  list(APPEND SCUDO_DEPS RTScudoStandalone.${LIBC_TARGET_ARCHITECTURE} RTScudoStandaloneCWrappers.${LIBC_TARGET_ARCHITECTURE})
+  if((LIBC_TARGET_ARCHITECTURE IN_LIST ALL_GWP_ASAN_SUPPORTED_ARCH) AND COMPILER_RT_BUILD_GWP_ASAN)
+    list(APPEND SCUDO_DEPS RTGwpAsan.${LIBC_TARGET_ARCHITECTURE} 
+                            RTGwpAsanBacktraceLibc.${LIBC_TARGET_ARCHITECTURE} 
+                            RTGwpAsanSegvHandler.${LIBC_TARGET_ARCHITECTURE})
+  elseif(COMPILER_RT_BUILD_GWP_ASAN)
+    message(WARNING "Architecture ${LIBC_TARGET_ARCHITECTURE} is not supported by GWP-ASan. Skipping.")
+  endif()
+
+  add_entrypoint_external(
+    malloc
+    DEPENDS
+      ${SCUDO_DEPS}
+  )
+  add_entrypoint_external(
+    calloc
+    DEPENDS
+      ${SCUDO_DEPS}
+  )
+  add_entrypoint_external(
+    realloc
+    DEPENDS
+      ${SCUDO_DEPS}
+  )
+  add_entrypoint_external(
+    free
+    DEPENDS
+      ${SCUDO_DEPS}
+  )
+
+endif()
+
 if(NOT LLVM_LIBC_FULL_BUILD)
   return()
 endif()
 
 if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
   add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
 endif()