From 2ed45c66d1dc6557b0ed4523f265ccb131704268 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Fri, 31 Mar 2023 10:52:36 -0400 Subject: [PATCH 1/6] Explicitly specify the calling convention for PINIT_ONCE_FN --- tests/generic_win_port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic_win_port.c b/tests/generic_win_port.c index ba685a18b..3a4fb2034 100644 --- a/tests/generic_win_port.c +++ b/tests/generic_win_port.c @@ -188,7 +188,7 @@ gettimeofday(struct timeval *tp, void *tzp) typedef void (WINAPI *QueryUnbiasedInterruptTimePreciseT)(PULONGLONG); static QueryUnbiasedInterruptTimePreciseT QueryUnbiasedInterruptTimePrecisePtr; -static BOOL +static BOOL WINAPI mach_absolute_time_init(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContext) { // QueryUnbiasedInterruptTimePrecise() is declared in the Windows headers From 0e7d14981d02a1877b1186a7cd337afbeeff3ef8 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 31 Mar 2023 09:10:46 -0700 Subject: [PATCH 2/6] build: slightly improve the build for libdispatch Avoid polluting the build directory a small amount given that we can use `-fmodule-map-file=` for the C/C++ build of libdispatch. Unfortunately, for the Swift build, we need to have the file copied over due to the umbrella header resolution. Hopefully this reduces some of the race conditions that we have seen in the build. Thanks to @dgregor for reminding me of the flag! --- .gitignore | 1 - CMakeLists.txt | 22 ++++------------------ src/swift/CMakeLists.txt | 14 +++++++++++++- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index aa26514c9..e0873a153 100644 --- a/.gitignore +++ b/.gitignore @@ -30,4 +30,3 @@ configure libtool .dirstamp /dispatch/module.modulemap -/private/module.modulemap diff --git a/CMakeLists.txt b/CMakeLists.txt index 2bef26395..57a37d3b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -261,26 +261,12 @@ endif() if(CMAKE_SYSTEM_NAME STREQUAL Darwin) - add_custom_command(OUTPUT - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap" "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/private/darwin/module.modulemap" "${PROJECT_SOURCE_DIR}/private/module.modulemap") + add_compile_options($<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap> + $<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/private/darwin/module.modulemap>) else() - add_custom_command(OUTPUT - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap" "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - COMMAND - ${CMAKE_COMMAND} -E copy_if_different "${PROJECT_SOURCE_DIR}/private/generic/module.modulemap" "${PROJECT_SOURCE_DIR}/private/module.modulemap") + add_compile_options($<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap> + $<$,$>:-fmodule-map-file=${PROJECT_SOURCE_DIR}/private/generic/module.modulemap>) endif() -add_custom_target(module-maps ALL - DEPENDS - "${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" - "${PROJECT_SOURCE_DIR}/private/module.modulemap") configure_file("${PROJECT_SOURCE_DIR}/cmake/config.h.in" "${PROJECT_BINARY_DIR}/config/config_ac.h") diff --git a/src/swift/CMakeLists.txt b/src/swift/CMakeLists.txt index c26d00d02..317faed2a 100644 --- a/src/swift/CMakeLists.txt +++ b/src/swift/CMakeLists.txt @@ -18,6 +18,18 @@ target_include_directories(DispatchStubs PRIVATE set_target_properties(DispatchStubs PROPERTIES POSITION_INDEPENDENT_CODE YES) + +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) +else() + add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) +endif() +add_custom_target(module-map ALL + DEPENDS ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) + + add_library(swiftDispatch Block.swift Data.swift @@ -42,7 +54,7 @@ target_link_libraries(swiftDispatch PRIVATE BlocksRuntime::BlocksRuntime) target_link_libraries(swiftDispatch PUBLIC dispatch) -add_dependencies(swiftDispatch module-maps) +add_dependencies(swiftDispatch module-map) get_swift_host_arch(swift_arch) install(FILES From 7f894dd45cca0a80f532adb734aedcd8b5746c95 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 31 Mar 2023 10:57:37 -0700 Subject: [PATCH 3/6] dispatch: attempt to use a VFS overlay Use a VFS overlay to avoid polluting the source tree. --- .gitignore | 1 - dispatch/CMakeLists.txt | 22 ++++++++++------------ dispatch/dispatch-vfs.yaml.in | 11 +++++++++++ src/swift/CMakeLists.txt | 15 ++------------- 4 files changed, 23 insertions(+), 26 deletions(-) create mode 100644 dispatch/dispatch-vfs.yaml.in diff --git a/.gitignore b/.gitignore index e0873a153..1bf54ca69 100644 --- a/.gitignore +++ b/.gitignore @@ -29,4 +29,3 @@ config configure libtool .dirstamp -/dispatch/module.modulemap diff --git a/dispatch/CMakeLists.txt b/dispatch/CMakeLists.txt index 7f68ed381..352915d91 100644 --- a/dispatch/CMakeLists.txt +++ b/dispatch/CMakeLists.txt @@ -1,4 +1,13 @@ +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap) +else() + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap) +endif() +configure_file(dispatch-vfs.yaml.in + ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml + @ONLY) + install(FILES base.h block.h @@ -16,19 +25,8 @@ install(FILES DESTINATION "${INSTALL_DISPATCH_HEADERS_DIR}") if(ENABLE_SWIFT) - set(base_dir "${CMAKE_CURRENT_SOURCE_DIR}") - if(NOT BUILD_SHARED_LIBS) - set(base_dir "${CMAKE_CURRENT_SOURCE_DIR}/generic_static") - endif() - - get_filename_component( - MODULE_MAP - module.modulemap - REALPATH - BASE_DIR "${base_dir}") - install(FILES - ${MODULE_MAP} + ${DISPATCH_MODULE_MAP} DESTINATION "${INSTALL_DISPATCH_HEADERS_DIR}") endif() diff --git a/dispatch/dispatch-vfs.yaml.in b/dispatch/dispatch-vfs.yaml.in new file mode 100644 index 000000000..9416dda81 --- /dev/null +++ b/dispatch/dispatch-vfs.yaml.in @@ -0,0 +1,11 @@ +--- +version: 0 +case-sensitive: false +use-external-names: false +roots: + - name: "@CMAKE_CURRENT_SOURCE_DIR@" + type: directory + contents: + - name: module.modulemap + type: file + external-contents: "@DISPATCH_MODULE_MAP@" diff --git a/src/swift/CMakeLists.txt b/src/swift/CMakeLists.txt index 317faed2a..c073e136d 100644 --- a/src/swift/CMakeLists.txt +++ b/src/swift/CMakeLists.txt @@ -18,18 +18,6 @@ target_include_directories(DispatchStubs PRIVATE set_target_properties(DispatchStubs PROPERTIES POSITION_INDEPENDENT_CODE YES) - -if(CMAKE_SYSTEM_NAME STREQUAL Darwin) - add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) -else() - add_custom_command(OUTPUT ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) -endif() -add_custom_target(module-map ALL - DEPENDS ${PROJECT_SOURCE_DIR}/dispatch/module.modulemap) - - add_library(swiftDispatch Block.swift Data.swift @@ -45,6 +33,8 @@ target_compile_options(swiftDispatch PRIVATE "SHELL:-Xcc -fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/module.modulemap" "SHELL:-Xcc -I${PROJECT_SOURCE_DIR}" "SHELL:-Xcc -I${PROJECT_SOURCE_DIR}/src/swift/shims") +target_compile_options(swiftDispatch PUBLIC + "SHELL:-vfsoverlay ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml") set_target_properties(swiftDispatch PROPERTIES Swift_MODULE_NAME Dispatch Swift_MODULE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/swift @@ -54,7 +44,6 @@ target_link_libraries(swiftDispatch PRIVATE BlocksRuntime::BlocksRuntime) target_link_libraries(swiftDispatch PUBLIC dispatch) -add_dependencies(swiftDispatch module-map) get_swift_host_arch(swift_arch) install(FILES From 9b13c2a73b5777d34d869df7bc060a6b5c6754e4 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Fri, 2 Jun 2023 08:34:54 -0700 Subject: [PATCH 4/6] dispatch: install the correct modulemap when building static There is a subtle difference between libdispatch built dynamically and statically: DispatchStubs. We erroneously emit ObjC runtime calls into the build and the stubs provides a shim to provide a definition on non-ObjC runtime enabled targets. When built dynamically, this is internalised and distributed as part of dispatch.dll/libdispatch.so, however when built statically, the consumer is responsible for linking against DispatchStubs. The modulemap reflects this and we need to ensure that we correctly provide that modulemap. Thanks to @MaxDesiatov for the help with debugging and testing a fix for this! Fixes: rdar://23335318 --- dispatch/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dispatch/CMakeLists.txt b/dispatch/CMakeLists.txt index 352915d91..a7f5fc306 100644 --- a/dispatch/CMakeLists.txt +++ b/dispatch/CMakeLists.txt @@ -1,8 +1,10 @@ if(CMAKE_SYSTEM_NAME STREQUAL Darwin) set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/darwin/module.modulemap) -else() +elseif(BUILD_SHARED_LIBS) set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic/module.modulemap) +else() + set(DISPATCH_MODULE_MAP ${PROJECT_SOURCE_DIR}/dispatch/generic_static/module.modulemap) endif() configure_file(dispatch-vfs.yaml.in ${CMAKE_BINARY_DIR}/dispatch-vfs-overlay.yaml From b2f50e53871f895ca9437a6a0a77a8c281f96586 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 23 May 2023 09:56:41 -0700 Subject: [PATCH 5/6] build: add a link against AdvAPI32 on Windows The `WaitChain*` functions require linking against AdvAPI32. --- src/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ac6a9aa09..811e131e0 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -141,6 +141,7 @@ target_link_libraries(dispatch PUBLIC BlocksRuntime::BlocksRuntime) if(CMAKE_SYSTEM_NAME STREQUAL Windows) target_link_libraries(dispatch PRIVATE + AdvAPI32 ShLwApi WS2_32 WinMM From ce166c2d5def11a265964b6c7ac01816873fb62a Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 23 May 2023 14:07:43 -0700 Subject: [PATCH 6/6] shims: fix a subtle bug in semaphore initialisation on Windows This function is the initializer for the semaphore. The seamphore storage itself may be stack allocated (or heap allocated) but without guarantee of 0-initialisation. As a result, the subsequent CAS for the atomic replacement will fail silently, leaving the previously non-zero value in place, indicating that the value is a valid handle. This would fail randomly and would ultimately result in a crash in the `CloseHandle` call associated with the clean up. This issue was identified by SwiftLint on Windows. --- src/shims/lock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shims/lock.c b/src/shims/lock.c index 62fa6f385..88fb8f8b6 100644 --- a/src/shims/lock.c +++ b/src/shims/lock.c @@ -266,6 +266,7 @@ void _dispatch_sema4_init(_dispatch_sema4_t *sema, int policy DISPATCH_UNUSED) // lazily allocate the semaphore port + os_atomic_cmpxchg(sema, *sema, 0, relaxed); while (!dispatch_assume(tmp = CreateSemaphore(NULL, 0, LONG_MAX, NULL))) { _dispatch_temporary_resource_shortage(); }