[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 ↗

(On Diff #382118)

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 ↗

(On Diff #382118)

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 ↗

(On Diff #382118)

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/CMakeLists.txt

if(${last_dot_loc} EQUAL -1)
     message(FATAL "Invalid entrypoint target name ${entrypoint}; Expected a '.' "
                   "(dot) in the name.")
   endif()
   math(EXPR name_loc "${last_dot_loc} + 1")
   string(SUBSTRING ${entrypoint} ${name_loc} -1 entrypoint_name)
   list(APPEND TARGET_ENTRYPOINT_NAME_LIST ${entrypoint_name})
 endforeach()
 
+# External entrypoints are only included in TARGET_ENTRYPOINT_NAME_LIST, 
+# not TARGET_LLVMLIBC_ENTRYPOINTS.
+# TARGET_ENTRYPOINT_NAME_LIST just defines which function prototypes are
+# included in the generated headers, while TARGET_LLVMLIBC_ENTRYPOINTS is used
+# for determining which functions will be built.
+if(TARGET_LIBC_EXTERNAL_ENTRYPOINTS)
+  foreach(entrypoint IN LISTS TARGET_LIBC_EXTERNAL_ENTRYPOINTS)
+    string(FIND ${entrypoint} "." last_dot_loc REVERSE)
+    if(${last_dot_loc} EQUAL -1)
+      message(FATAL "Invalid entrypoint target name ${entrypoint}; Expected a '.' "
+                    "(dot) in the name.")
+    endif()
+    math(EXPR name_loc "${last_dot_loc} + 1")
+    string(SUBSTRING ${entrypoint} ${name_loc} -1 entrypoint_name)
+    list(APPEND TARGET_ENTRYPOINT_NAME_LIST ${entrypoint_name})
+  endforeach()
+endif()
+
 if(LLVM_LIBC_FULL_BUILD)
   # We need to set up hdrgen first since other targets depend on it.
   add_subdirectory(utils/LibcTableGenUtil)
   add_subdirectory(utils/HdrGen)
 endif()
 
 add_subdirectory(include)
 add_subdirectory(config)

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)
 
+# A rule for external entrypoint targets.
+# Usage:
+#     add_entrypoint_external(
+#       <target_name>
+#     )
+function(add_entrypoint_external target_name)
+
+  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_OBJ_TARGET_TYPE}
+      "OBJECT_FILE" ""
+      "OBJECT_FILE_RAW" ""
+      "DEPS" ""
+  )
+
+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}
   )
   if(NOT REDIRECTOR_OBJECT_SRC)
     message(FATAL_ERROR "'add_redirector_object' rule requires SRC option listing one source file.")
   endif()

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()
 
+set(TARGET_LIBC_EXTERNAL_ENTRYPOINTS
+    # stdlib.h external entrypoints
+    libc.src.stdlib.malloc
+    libc.src.stdlib.calloc
+    libc.src.stdlib.realloc
+    libc.src.stdlib.free
+)
+
 set(TARGET_LLVMLIBC_ENTRYPOINTS
   ${TARGET_LIBC_ENTRYPOINTS}
   ${TARGET_LIBM_ENTRYPOINTS}
 )

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
 )
 
+add_entrypoint_external(
+  malloc
+)
+add_entrypoint_external(
+  calloc
+)
+add_entrypoint_external(
+  realloc
+)
+add_entrypoint_external(
+  free
+)
+
 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()