[libc] warn when depending on public entrypoints

[michaelrj-google]

Add a cmake warning when an entrypoint or object library depends on a
public entrypoint.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Add a cmake warning when an entrypoint or object library depends on a
public entrypoint.

---

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

1 Files Affected:

• (modified) libc/cmake/modules/LLVMLibCObjectRules.cmake (+52-4)

diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 805da91284ce8..2587de56e0cac 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -1,4 +1,32 @@
 set(OBJECT_LIBRARY_TARGET_TYPE "OBJECT_LIBRARY")
+set(ENTRYPOINT_OBJ_TARGET_TYPE "ENTRYPOINT_OBJ")
+set(ENTRYPOINT_EXT_TARGET_TYPE "ENTRYPOINT_EXT")
+
+# Rule to check if a list of dependencies contains any entrypoint objects. Returns a list in entrypoint_deps.
+function(check_entrypoint_deps fq_deps_list)
+  set(PUBLIC_DEPS "")
+  foreach(dep IN LISTS fq_deps_list)
+    if(NOT TARGET ${dep})
+      continue()
+    endif()
+    #don't warn for deps that are allowed, such as errno
+    set(ALLOWED_DEPS
+      "libc.src.errno.errno"
+      "libc.src.setjmp.longjmp"
+    )
+    LIST(FIND ALLOWED_DEPS ${dep} dep_index)
+    if(NOT ${dep_index} EQUAL -1)
+      continue()
+    endif()
+
+    get_target_property(target_type ${dep} "TARGET_TYPE")
+    if(${target_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE})
+      list(APPEND PUBLIC_DEPS ${dep})
+    endif()
+  endforeach()
+  set(entrypoint_deps ${PUBLIC_DEPS} PARENT_SCOPE)
+endfunction()
+
 
 # Rule which is essentially a wrapper over add_library to compile a set of
 # sources to object files.
@@ -65,6 +93,18 @@ function(create_object_library fq_target_name)
   target_include_directories(${fq_target_name} PRIVATE ${LIBC_SOURCE_DIR})
   target_compile_options(${fq_target_name} PRIVATE ${compile_options})
 
+  #TODO: combine this with the identical check in create_entrypoint_object
+  #loop through the deps, check if any have the TARGET_TYPE of ENTRYPOINT_OBJ_TARGET_TYPE, and print a warning if they do.
+  if(LIBC_CMAKE_VERBOSE_LOGGING)
+    set(entrypoint_deps "")
+    if(NOT "${fq_deps_list}" STREQUAL "")
+      check_entrypoint_deps("${fq_deps_list}")
+    endif()
+    if(NOT "${entrypoint_deps}" STREQUAL "")
+      message(WARNING "Object ${fq_target_name} depends on public entrypoint(s) ${entrypoint_deps}.\nDepending on public entrypoints is not allowed in internal code.")
+    endif()
+  endif()
+
   if(SHOW_INTERMEDIATE_OBJECTS)
     message(STATUS "Adding object library ${fq_target_name}")
     if(${SHOW_INTERMEDIATE_OBJECTS} STREQUAL "DEPS")
@@ -110,7 +150,6 @@ function(add_object_library target_name)
     ${ARGN})
 endfunction(add_object_library)
 
-set(ENTRYPOINT_OBJ_TARGET_TYPE "ENTRYPOINT_OBJ")
 
 # A rule for entrypoint object targets.
 # Usage:
@@ -179,7 +218,6 @@ function(create_entrypoint_object fq_target_name)
 
     get_target_property(obj_type ${fq_dep_name} "TARGET_TYPE")
     if((NOT obj_type) OR (NOT ${obj_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE}))
-                              
       message(FATAL_ERROR "The aliasee of an entrypoint alias should be an entrypoint.")
     endif()
 
@@ -230,6 +268,18 @@ function(create_entrypoint_object fq_target_name)
   _get_common_compile_options(common_compile_options "${ADD_ENTRYPOINT_OBJ_FLAGS}")
   list(APPEND common_compile_options ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS})
   get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
+
+  #loop through the deps, check if any have the TARGET_TYPE of entrypoint_target_type, and print a warning if they do.
+  if(LIBC_CMAKE_VERBOSE_LOGGING)
+    set(entrypoint_deps "")
+    if(NOT "${fq_deps_list}" STREQUAL "")
+      check_entrypoint_deps("${fq_deps_list}")
+    endif()
+    if(NOT "${entrypoint_deps}" STREQUAL "")
+      message(WARNING "Entrypoint ${fq_target_name} depends on public entrypoint(s) ${entrypoint_deps}.\nDepending on public entrypoints is not allowed in internal code.")
+    endif()
+  endif()
+
   set(full_deps_list ${fq_deps_list} libc.src.__support.common)
 
   if(SHOW_INTERMEDIATE_OBJECTS)
@@ -390,8 +440,6 @@ function(add_entrypoint_object target_name)
   )
 endfunction(add_entrypoint_object)
 
-set(ENTRYPOINT_EXT_TARGET_TYPE "ENTRYPOINT_EXT")
-
 # A rule for external entrypoint targets.
 # Usage:
 #     add_entrypoint_external(

[michaelrj-google]

Before this lands I'm planning to also file cleanup bugs for each of the functions that currently has a warning.

[lntue]

You shouldn't have implicit out-parameter entrypoint_deps, make it a required first argument of this call.
fq_dep_list can be implicitly collected in ARGN is another option:

function(check_entrypoint_deps entrypoint_deps)

then all the strings provided in the calls check_entrypoint_deps(entrypoint_deps ...) will be collected in ARGN list. So inside the loop, you just need

foreach(dep IN LISTS ARGN)

and this function can be called as:

check_entrypoint_deps(entrypoint_deps ${fq_dep_list})

[lntue]

before the foreach loop, you can remove the ALLOWED_DEPS list from the input list:

list(REMOVE_ITEM fq_dep_list ${ALLOWED_DEPS})

[lntue]

nit: break the string into multiple lines