[CMake] Streamline code signing for debugserver

[sgraenitz]

Summary

Use llvm_codesign to sign debugserver with entitlements. Handle reconfigurations correctly. We have a lot of cases, make them explicit:

(1) build and sign debugserver, if all conditions apply:

• LLDB_NO_DEBUGSERVER=OFF (default)
• On Darwin: LLDB_USE_SYSTEM_DEBUGSERVER=OFF (default)
• On Darwin: LLVM_CODESIGNING_IDENTITY == lldb_codesign

(2) use system debugserver, if on Darwin and any of:

• LLDB_USE_SYSTEM_DEBUGSERVER=ON and debugserver found on system (explicit case)
• LLVM_CODESIGNING_IDENTITY != lldb_codesign and debugserver found on system (fallback case)

(3) debugserver will not be available, in case of:

• LLDB_NO_DEBUGSERVER=ON
• On Darwin: LLVM_CODESIGNING_IDENTITY != lldb_codesign and debugserver not found on system

(4) error state, in case of:

• LLDB_USE_SYSTEM_DEBUGSERVER=ON and debugserver not found on system
• LLDB_USE_SYSTEM_DEBUGSERVER=ON and LLDB_NO_DEBUGSERVER=ON

[JDevlieghere]

tools/debugserver/source/CMakeLists.txt

101 ↗

(On Diff #173854)

Should we emit and error if these variables are in an inconsistent state, e.g. when both are set?

118 ↗

(On Diff #173854)

Why did you make this variable name lowercase? Does that have any semantic meaning?

[xiaobai]

tools/debugserver/source/CMakeLists.txt

128–133 ↗

(On Diff #173854)

I'm pretty sure that if you do not copy debugserver it will still find the one from your ${lldb_framework_dir} when you run lldb anyway. Do you know why we copy it over?

[sgraenitz]

tools/debugserver/source/CMakeLists.txt

101 ↗

(On Diff #173854)

Yes, could do that.

118 ↗

(On Diff #173854)

LLVM tends to use lowercase names for local/non-cached variables, e.g. update_src_props in:
https://github.com/llvm-mirror/llvm/blob/master/cmake/modules/AddLLVM.cmake

I didn't find any guidelines on conventions for CMake, but I thought it's a good idea. It's a little surprising to mix in such changes to patches that also have semantic changes. Maybe I should continue to use uppercase names for now and add a subsequent patch (with a proper title) that does the change for the whole file. What do you think?

128–133 ↗

(On Diff #173854)

Yes this is what I mean with "old behaviour" here. DEBUGSERVER_PATH still points to the original system binary and gets passed to tests, e.g. in test/CMakeLists.txt above.

[JDevlieghere]

tools/debugserver/source/CMakeLists.txt

118 ↗

(On Diff #173854)

Cool, I wasn't aware of the convention. I think either is fine, whatever you prefer.

[sgraenitz]

debugserver cannot be ad-hoc code signed; handle entitlements&identity also in standalone debugserver, add feasibility checks, polishing

[sgraenitz]

tools/debugserver/source/CMakeLists.txt

128–133 ↗

(On Diff #173854)

Do you know why we copy it over?

It was introduced with https://reviews.llvm.org/rL325068 and the commit message points out, that this makes the built debugger functional on Darwin when compiling without code signing. In order to use debugserver on Darwin, it cannot be ad-hoc code signed, but needs a "real" certificate (as documented in docs/code-signing.txt).

Got in touch with Vedant and it seems, that DEBUGSERVER_PATH could also point to the copied version. Did that.

[labath]

tools/debugserver/source/CMakeLists.txt

101 ↗

(On Diff #173854)

Alternatively, we could make this a single tri-state variable. LLDB_DEBUGSERVER_MODE=OFF|SYSTEM|TREE or something ?

[sgraenitz]

Handle reconfigurations correctly; fix configuration messages; add note for generator expressions

[sgraenitz]

Improve description for options LLDB_NO_DEBUGSERVER and LLDB_USE_SYSTEM_DEBUGSERVER

[sgraenitz]

Last revision changed a lot, but I think support for reconfigurations is worth it.
The current state would be acceptable from my side now -- if I didn't miss anything! :-)

tools/debugserver/source/CMakeLists.txt

159 ↗

(On Diff #174254)

This is not a regression. Just added a verbose comment to remember once we look at this.

190 ↗

(On Diff #174254)

This is resetting DEBUGSERVER_PATH and SKIP_TEST_DEBUGSERVER cached variables for the 3 non-error cases (see updated summary). It should be reconfiguration-safe. All manual testing on Darwin passed.

101 ↗

(On Diff #173854)

Yes possible, but I hope it's not necessary. It would be great to keep existing names for user-definable options where possible. Otherwise it breaks bots, etc.

[JDevlieghere]

Thanks for working on this, Stefan. This LGTM.

[sgraenitz]

Thanks. Waiting for https://reviews.llvm.org/D54443

[sgraenitz]

Set global LLVM_CODESIGNING_IDENTITY from LLDB_CODESIGN_IDENTITY (if given).
Pass through ENTITLEMENTS from add_lldb_executable to add_llvm_executable.

[sgraenitz]

My change for lldb-server (https://reviews.llvm.org/D54444) became obsolete, but two small pieces of it are required in general/here.
Added them post-approval with the update above. It's the two additions to cmake/modules/AddLLDB.cmake and CMakeLists.txt. Will leave this open for a little longer and commit next week.

lldb/trunk/CMakeLists.txt

 cmake_minimum_required(VERSION 3.4.3)
 
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
   )
 
 include(LLDBStandalone)
 include(LLDBConfig)
 include(AddLLDB)
 
+option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
+if(LLDB_CODESIGN_IDENTITY)
+  # In the future we may use LLVM_CODESIGNING_IDENTITY directly.
+  set(LLVM_CODESIGNING_IDENTITY ${LLDB_CODESIGN_IDENTITY})
+endif()
+
 # Define the LLDB_CONFIGURATION_xxx matching the build type
 if( uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
   add_definitions( -DLLDB_CONFIGURATION_DEBUG )
 else()
   add_definitions( -DLLDB_CONFIGURATION_RELEASE )
 endif()
 
 if (CMAKE_SYSTEM_NAME MATCHES "Windows|Android")

lldb/trunk/cmake/modules/AddLLDB.cmake

function(add_lldb_library name)
   target_compile_options(${name} PRIVATE ${PARAM_EXTRA_CXXFLAGS})
 
   set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
 endfunction(add_lldb_library)
 
 function(add_lldb_executable name)
   cmake_parse_arguments(ARG
     "INCLUDE_IN_SUITE;GENERATE_INSTALL"
-    ""
+    "ENTITLEMENTS"
     "LINK_LIBS;LINK_COMPONENTS"
     ${ARGN}
     )
 
   list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
-  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS})
+  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS} ENTITLEMENTS ${ARG_ENTITLEMENTS})
 
   target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
   set_target_properties(${name} PROPERTIES
     FOLDER "lldb executables")
 
   if(ARG_INCLUDE_IN_SUITE)
     add_dependencies(lldb-suite ${name})
     if(LLDB_BUILD_FRAMEWORK)

lldb/trunk/test/CMakeLists.txt

 endif()
 
 if (NOT "${LLDB_LIT_TOOLS_DIR}" STREQUAL "")
   if (NOT EXISTS "${LLDB_LIT_TOOLS_DIR}")
     message(WARNING "LLDB_LIT_TOOLS_DIR ${LLDB_LIT_TOOLS_DIR} does not exist.")
   endif()
 endif()
 
-if(CMAKE_HOST_APPLE)
+if(CMAKE_HOST_APPLE AND DEBUGSERVER_PATH)
   list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
 endif()
 
-if(SKIP_DEBUGSERVER)
+if(SKIP_TEST_DEBUGSERVER)
   list(APPEND LLDB_TEST_COMMON_ARGS --out-of-tree-debugserver)
 endif()
 
 set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS})
 set_property(GLOBAL PROPERTY LLDB_DOTEST_ARGS_PROPERTY ${LLDB_DOTEST_ARGS})
 
 add_python_test_target(check-lldb-single
   ${LLDB_SOURCE_DIR}/test/dotest.py

lldb/trunk/tools/debugserver/CMakeLists.txt

 cmake_minimum_required(VERSION 3.4.3)
 
 project(Debugserver LANGUAGES C CXX ASM-ATT)
 
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   set(CMAKE_MODULE_PATH
     ${CMAKE_MODULE_PATH}
     "${CMAKE_SOURCE_DIR}/../../cmake"
     "${CMAKE_SOURCE_DIR}/../../cmake/modules"
     )
-  
   include(LLDBStandalone)
   include(AddLLDB)
 
   set(LLDB_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../../")
   include_directories(${LLDB_SOURCE_DIR}/include)
+
+  option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
+  if(LLDB_CODESIGN_IDENTITY)
+    # In the future we may use LLVM_CODESIGNING_IDENTITY directly.
+    set(LLVM_CODESIGNING_IDENTITY ${LLDB_CODESIGN_IDENTITY})
+  endif()
 endif()
 
 add_subdirectory(source)

lldb/trunk/tools/debugserver/source/CMakeLists.txt

set(lldbDebugserverCommonSources
   MacOSX/MachVMMemory.cpp
   MacOSX/MachVMRegion.cpp
   MacOSX/OsLogger.cpp
   ${generated_mach_interfaces}
   ${DEBUGSERVER_VERS_GENERATED_FILE})
 
 add_library(lldbDebugserverCommon ${lldbDebugserverCommonSources})
 
+option(LLDB_NO_DEBUGSERVER "Disable the debugserver target" OFF)
+option(LLDB_USE_SYSTEM_DEBUGSERVER "Use the system's debugserver instead of building it from source (Darwin only)." OFF)
 
-set(LLDB_CODESIGN_IDENTITY "lldb_codesign"
-  CACHE STRING "Identity used for code signing. Set to empty string to skip the signing step.")
+# Incompatible options
+if(LLDB_NO_DEBUGSERVER AND LLDB_USE_SYSTEM_DEBUGSERVER)
+  message(FATAL_ERROR "Inconsistent options: LLDB_NO_DEBUGSERVER and LLDB_USE_SYSTEM_DEBUGSERVER")
+endif()
 
-if(NOT LLDB_CODESIGN_IDENTITY STREQUAL "")
-  set(DEBUGSERVER_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX} CACHE PATH "Path to debugserver.")
-  set(SKIP_DEBUGSERVER OFF CACHE BOOL "Skip building the in-tree debug server")
-else()
+# Try to locate the system debugserver.
+# Subsequent feasibility checks depend on it.
+if(APPLE AND CMAKE_HOST_APPLE)
   execute_process(
     COMMAND xcode-select -p
-    OUTPUT_VARIABLE XCODE_DEV_DIR)
-  string(STRIP ${XCODE_DEV_DIR} XCODE_DEV_DIR)
-  if(EXISTS "${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/")
-    set(DEBUGSERVER_PATH
-      "${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/Resources/debugserver" CACHE PATH "Path to debugserver.")
-  elseif(EXISTS "${XCODE_DEV_DIR}/Library/PrivateFrameworks/LLDB.framework/")
-    set(DEBUGSERVER_PATH
-      "${XCODE_DEV_DIR}/Library/PrivateFrameworks/LLDB.framework/Resources/debugserver" CACHE PATH "Path to debugserver.")
+    OUTPUT_VARIABLE xcode_dev_dir)
+  string(STRIP ${xcode_dev_dir} xcode_dev_dir)
+
+  set(debugserver_rel_path "LLDB.framework/Resources/debugserver")
+  set(debugserver_shared "${xcode_dev_dir}/../SharedFrameworks/${debugserver_rel_path}")
+  set(debugserver_private "${xcode_dev_dir}/Library/PrivateFrameworks/${debugserver_rel_path}")
+
+  if(EXISTS ${debugserver_shared})
+    set(system_debugserver ${debugserver_shared})
+  elseif(EXISTS ${debugserver_private})
+    set(system_debugserver ${debugserver_private})
+  endif()
+endif()
+
+# Handle unavailability
+if(LLDB_USE_SYSTEM_DEBUGSERVER)
+  if(system_debugserver)
+    set(use_system_debugserver ON)
+  elseif(APPLE AND CMAKE_HOST_APPLE)
+    # Binary not found on system. Keep cached variable, to try again on reconfigure.
+    message(SEND_ERROR
+      "LLDB_USE_SYSTEM_DEBUGSERVER option set, but no debugserver found in:\
+        ${debugserver_shared}\
+        ${debugserver_private}")
   else()
-    message(SEND_ERROR "Cannot find debugserver on system.")
+    # Non-Apple target platform or non-Darwin host. Reset invalid cached variable.
+    message(WARNING "Reverting invalid option LLDB_USE_SYSTEM_DEBUGSERVER (Darwin only)")
+    set(LLDB_USE_SYSTEM_DEBUGSERVER OFF CACHE BOOL "" FORCE)
+  endif()
+elseif(NOT LLDB_NO_DEBUGSERVER)
+  # Default case: on Darwin we need the right code signing ID.
+  # See lldb/docs/code-signing.txt for details.
+  if(CMAKE_HOST_APPLE AND NOT LLVM_CODESIGNING_IDENTITY STREQUAL "lldb_codesign")
+    set(msg "Cannot code sign debugserver with identity '${LLVM_CODESIGNING_IDENTITY}'.")
+    if(system_debugserver)
+      message(WARNING "${msg} Will fall back to system's debugserver.")
+      set(use_system_debugserver ON)
+    else()
+      message(WARNING "${msg} debugserver will not be available.")
+    endif()
+  else()
+    set(build_and_sign_debugserver ON)
   endif()
-  set(SKIP_DEBUGSERVER ON CACHE BOOL "Skip building the in-tree debug server")
 endif()
-message(STATUS "Path to the lldb debugserver: ${DEBUGSERVER_PATH}")
+
+# TODO: We don't use the $<TARGET_FILE:debugserver> generator expression here,
+# because the value of DEBUGSERVER_PATH is used to build LLDB_DOTEST_ARGS,
+# which is used for configuring lldb-dotest.in, which does not have a generator
+# step at the moment.
+set(default_debugserver_path "${LLVM_TOOLS_BINARY_DIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX}")
+
+# Remember where debugserver binary goes and whether or not we have to test it.
+set(DEBUGSERVER_PATH "" CACHE FILEPATH "Path to debugserver")
+set(SKIP_TEST_DEBUGSERVER OFF CACHE BOOL "Building the in-tree debugserver was skipped")
+
+# Reset values in all cases in order to correctly support reconfigurations.
+if(use_system_debugserver)
+  add_custom_target(debugserver
+    COMMAND ${CMAKE_COMMAND} -E copy_if_different
+            ${system_debugserver} ${LLVM_TOOLS_BINARY_DIR}
+    COMMENT "Copying the system debugserver to LLDB's binaries directory.")
+
+  # Don't test debugserver itself.
+  # Tests that require debugserver will use the copy.
+  set(DEBUGSERVER_PATH ${default_debugserver_path} CACHE FILEPATH "" FORCE)
+  set(SKIP_TEST_DEBUGSERVER ON CACHE BOOL "" FORCE)
+
+  message(STATUS "Copy system debugserver from: ${system_debugserver}")
+elseif(build_and_sign_debugserver)
+  # Build, sign and test debugserver (below)
+  set(DEBUGSERVER_PATH ${default_debugserver_path} CACHE FILEPATH "" FORCE)
+  set(SKIP_TEST_DEBUGSERVER OFF CACHE BOOL "" FORCE)
+
+  message(STATUS "lldb debugserver: ${DEBUGSERVER_PATH}")
+else()
+  # No tests for debugserver, no tests that require it.
+  set(DEBUGSERVER_PATH "" CACHE FILEPATH "" FORCE)
+  set(SKIP_TEST_DEBUGSERVER ON CACHE BOOL "" FORCE)
+
+  message(STATUS "lldb debugserver will not be available.")
+endif()
 
-if (APPLE)
+if(APPLE)
   if(IOS)
     find_library(BACKBOARD_LIBRARY BackBoardServices
       PATHS ${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks)
     find_library(FRONTBOARD_LIBRARY FrontBoardServices
       PATHS ${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks)
     find_library(SPRINGBOARD_LIBRARY SpringBoardServices
       PATHS ${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks)
     find_library(MOBILESERVICES_LIBRARY MobileCoreServices
       PATHS ${CMAKE_OSX_SYSROOT}/System/Library/PrivateFrameworks)
     find_library(LOCKDOWN_LIBRARY lockdown)
 
     if(NOT BACKBOARD_LIBRARY)
-      set(SKIP_DEBUGSERVER ON CACHE BOOL "Skip building the in-tree debug server" FORCE)
+      set(SKIP_TEST_DEBUGSERVER ON CACHE BOOL "" FORCE)
     endif()
   else()
     find_library(COCOA_LIBRARY Cocoa)
   endif()
 endif()
 
 if(HAVE_LIBCOMPRESSION)
   set(LIBCOMPRESSION compression)
 endif()
 
-if(NOT SKIP_DEBUGSERVER)
+if(LLDB_USE_ENTITLEMENTS)
+  if(IOS)
+    set(entitlements ${CMAKE_CURRENT_SOURCE_DIR}/debugserver-entitlements.plist)
+  else()
+    # Same entitlements file as used for lldb-server
+    set(entitlements ${LLDB_SOURCE_DIR}/resources/debugserver-macosx-entitlements.plist)
+  endif()
+endif()
+
+if(build_and_sign_debugserver)
   target_link_libraries(lldbDebugserverCommon
                         INTERFACE ${COCOA_LIBRARY}
                         ${CORE_FOUNDATION_LIBRARY}
                         ${FOUNDATION_LIBRARY}
                         ${BACKBOARD_LIBRARY}
                         ${FRONTBOARD_LIBRARY}
                         ${SPRINGBOARD_LIBRARY}
                         ${MOBILESERVICES_LIBRARY}
                         ${LOCKDOWN_LIBRARY}
                         lldbDebugserverArchSupport
                         lldbDebugserverDarwin_DarwinLog
                         ${LIBCOMPRESSION})
   if(HAVE_LIBCOMPRESSION)
     set_property(TARGET lldbDebugserverCommon APPEND PROPERTY
                  COMPILE_DEFINITIONS HAVE_LIBCOMPRESSION)
   endif()
   set(LLVM_OPTIONAL_SOURCES ${lldbDebugserverCommonSources})
   add_lldb_tool(debugserver INCLUDE_IN_SUITE
     debugserver.cpp
 
     LINK_LIBS
       lldbDebugserverCommon
+
+    ENTITLEMENTS
+      ${entitlements}
     )
   if(IOS)
     set_property(TARGET lldbDebugserverCommon APPEND PROPERTY COMPILE_DEFINITIONS
       WITH_LOCKDOWN
       WITH_FBS
       WITH_BKS
       )
     set_property(TARGET debugserver APPEND PROPERTY COMPILE_DEFINITIONS
set_property(TARGET lldbDebugserverCommon_NonUI APPEND PROPERTY
                  COMPILE_DEFINITIONS HAVE_LIBCOMPRESSION)
   endif()
 
   add_lldb_tool(debugserver-nonui
     debugserver.cpp
 
     LINK_LIBS
       lldbDebugserverCommon_NonUI
-    )
-endif()
-
-set(entitlements_xml ${CMAKE_CURRENT_SOURCE_DIR}/debugserver-macosx-entitlements.plist)
-if(IOS)
-  set(entitlements_xml ${CMAKE_CURRENT_SOURCE_DIR}/debugserver-entitlements.plist)
-else()
-  set(entitlements_xml ${CMAKE_CURRENT_SOURCE_DIR}/../../../resources/debugserver-macosx-entitlements.plist)
-endif()
-
-set(LLDB_USE_ENTITLEMENTS_Default On)
-option(LLDB_USE_ENTITLEMENTS "Use entitlements when codesigning (Defaults Off when using lldb_codesign identity, otherwise On)" ${LLDB_USE_ENTITLEMENTS_Default})
 
-if (SKIP_DEBUGSERVER)
-  if (CMAKE_HOST_APPLE)
-    # If we haven't built a signed debugserver, copy the one from the system.
-    add_custom_target(debugserver
-      COMMAND ${CMAKE_COMMAND} -E copy_if_different ${DEBUGSERVER_PATH} ${CMAKE_BINARY_DIR}/bin
-      VERBATIM
-      COMMENT "Copying the system debugserver to LLDB's binaries directory.")
-  endif()
-else()
-  if(LLDB_USE_ENTITLEMENTS)
-    set(entitlements_flags --entitlements ${entitlements_xml})
-  endif()
-  execute_process(
-    COMMAND xcrun -f codesign_allocate
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-    OUTPUT_VARIABLE CODESIGN_ALLOCATE
-    )
-  add_custom_command(TARGET debugserver
-    POST_BUILD
-    COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE}
-            codesign --force --sign ${LLDB_CODESIGN_IDENTITY}
-            ${entitlements_flags}
-            $<TARGET_FILE:debugserver>
-  )
-  if(IOS)
-    add_custom_command(TARGET debugserver-nonui
-      POST_BUILD
-      COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE}
-              codesign --force --sign ${LLDB_CODESIGN_IDENTITY}
-              ${entitlements_flags}
-              $<TARGET_FILE:debugserver>
+    ENTITLEMENTS
+      ${entitlements}
     )
-  endif()
-endif()
-
-
-
-

lldb/trunk/unittests/tools/CMakeLists.txt

 if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
-  if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD))
+  if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_TEST_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD))
     # These tests are meant to test lldb-server/debugserver in isolation, and
     # don't provide any value if run against a server copied from somewhere.
   else()
     add_subdirectory(lldb-server)
   endif()
 endif()