Skip to content

Commit

Permalink
Merge pull request #65824 from apple/maxd/5.9-toolchain-dispatch
Browse files Browse the repository at this point in the history
[5.9] CMake: fix missing `SWIFT_CONCURRENCY_GLOBAL_EXECUTOR`

Explanation: Resolves issues with static linking on Linux
Risk: Medium, affects Linux builds and top-level CMake declarations.
Original PR: #65795.
Reviewed by: @al45tair @drexin @etcwilde 
Resolves: some of the issues reported in #65097
Tests: Added in swiftlang/swift-integration-tests#115

`SWIFT_CONCURRENCY_GLOBAL_EXECUTOR` is defined in `stdlib/cmake/modules/StdlibOptions.cmake`, which is not included during the first pass of evaluation of the root `CMakeLists.txt`. It is available on subsequent evaluations after the value is stored in CMake cache. This led to subtle bugs, where `usr/lib/swift_static/linux/static-stdlib-args.lnk` didn't contain certain flags on clean toolchain builds, but did contain them in incremental builds.

Not having these autolinking flags in toolchain builds leads to errors when statically linking executables on Linux.

Additionally, since are trivial tests previously didn't link Dispatch statically, the didn't expose a bug where `%import-static-libdispatch` substitution had a missing value. To fix that I had to update `lit.cfg` and clean up some of the related path computations to infer a correct substitution value.

Resolves some of the errors reported in #65097.
  • Loading branch information
MaxDesiatov committed May 26, 2023
2 parents 0763971 + be35d46 commit f3a3eef
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 23 deletions.
11 changes: 2 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -696,13 +696,6 @@ if(NOT EXISTS "${SWIFT_PATH_TO_SWIFT_SYNTAX_SOURCE}")
message(SEND_ERROR "swift-syntax is required to build the Swift compiler. Please run update-checkout or specify SWIFT_PATH_TO_SWIFT_SYNTAX_SOURCE")
endif()

# Use dispatch as the system scheduler by default.
# For convenience, we set this to false when concurrency is disabled.
set(SWIFT_CONCURRENCY_USES_DISPATCH FALSE)
if(SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY AND "${SWIFT_CONCURRENCY_GLOBAL_EXECUTOR}" STREQUAL "dispatch")
set(SWIFT_CONCURRENCY_USES_DISPATCH TRUE)
endif()

set(SWIFT_BUILD_HOST_DISPATCH FALSE)
if(SWIFT_ENABLE_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin)
# Only build libdispatch for the host if the host tools are being built and
Expand All @@ -711,9 +704,9 @@ if(SWIFT_ENABLE_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin)
set(SWIFT_BUILD_HOST_DISPATCH TRUE)
endif()

if(SWIFT_BUILD_HOST_DISPATCH OR SWIFT_CONCURRENCY_USES_DISPATCH)
if(SWIFT_BUILD_HOST_DISPATCH)
if(NOT EXISTS "${SWIFT_PATH_TO_LIBDISPATCH_SOURCE}")
message(SEND_ERROR "SourceKit and concurrency require libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE")
message(SEND_ERROR "SourceKit requires libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE")
endif()
endif()
endif()
Expand Down
13 changes: 13 additions & 0 deletions stdlib/cmake/modules/StdlibOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,16 @@ set(SWIFT_RUNTIME_FIXED_BACKTRACER_PATH "" CACHE STRING
"If set, provides a fixed path to the swift-backtrace binary. This
will disable dynamic determination of the path and will also disable
the setting in SWIFT_BACKTRACE.")

# Use dispatch as the system scheduler by default.
# For convenience, we set this to false when concurrency is disabled.
set(SWIFT_CONCURRENCY_USES_DISPATCH FALSE)
if(SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY AND "${SWIFT_CONCURRENCY_GLOBAL_EXECUTOR}" STREQUAL "dispatch")
set(SWIFT_CONCURRENCY_USES_DISPATCH TRUE)
endif()

if(SWIFT_CONCURRENCY_USES_DISPATCH AND NOT CMAKE_SYSTEM_NAME STREQUAL Darwin)
if(NOT EXISTS "${SWIFT_PATH_TO_LIBDISPATCH_SOURCE}")
message(SEND_ERROR "Concurrency require libdispatch on non-Darwin hosts. Please specify SWIFT_PATH_TO_LIBDISPATCH_SOURCE")
endif()
endif()
3 changes: 1 addition & 2 deletions test/Driver/static-stdlib-autolink-linux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
// RUN: echo 'public func asyncFunc() async { print("Hello") }' > %t/asyncModule.swift

// RUN: %target-swiftc_driver -emit-library -emit-module -module-name asyncModule -module-link-name asyncModule %t/asyncModule.swift -static -static-stdlib -o %t/libasyncModule.a
// TODO: "-ldispatch -lBlocksRuntime" should be told by asyncModule.swiftmodule transitively
// RUN: %target-swiftc_driver -parse-as-library -static -static-stdlib -module-name main %s %import-static-libdispatch -I%t -L%t -ldispatch -lBlocksRuntime -o %t/main
// RUN: %target-swiftc_driver -parse-as-library -static -static-stdlib -module-name main %s %import-static-libdispatch -I%t -L%t -o %t/main

// RUN: %t/main | %FileCheck %s
// CHECK: Hello
Expand Down
2 changes: 1 addition & 1 deletion test/Driver/static-stdlib-linux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// REQUIRES: static_stdlib
print("hello world!")
// RUN: %empty-directory(%t)
// RUN: %target-swiftc_driver -static-stdlib -o %t/static-stdlib %s
// RUN: %target-swiftc_driver %import-static-libdispatch -static-stdlib -o %t/static-stdlib %s
// RUN: %t/static-stdlib | %FileCheck %s
// RUN: ldd %t/static-stdlib | %FileCheck %s --check-prefix=LDD
// CHECK: hello world!
Expand Down
18 changes: 8 additions & 10 deletions test/lit.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -1615,19 +1615,13 @@ elif (run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'openbsd', 'windows-
config.import_libdispatch = ('-I %s -I %s -L %s'
% (libdispatch_source_dir, libdispatch_swift_module_dir, libdispatch_artifact_dir))

libdispatch_static_artifact_dir = config.libdispatch_static_build_path
libdispatch_swift_static_module_dir = make_path(libdispatch_static_artifact_dir, 'src', 'swift', 'swift')
libdispatch_static_artifact_dir = os.path.join(config.libdispatch_static_build_path, 'lib')
libdispatch_static_artifacts = [
make_path(libdispatch_static_artifact_dir, 'src', 'libdispatch.a'),
make_path(libdispatch_static_artifact_dir, 'src', 'swift', 'libswiftDispatch.a'),
make_path(libdispatch_swift_static_module_dir, 'Dispatch.swiftmodule')]
make_path(libdispatch_static_artifact_dir, 'libdispatch.a'),
make_path(libdispatch_static_artifact_dir, 'libBlocksRuntime.a')]
if (all(os.path.exists(p) for p in libdispatch_static_artifacts)):
config.available_features.add('libdispatch_static')
config.import_libdispatch_static = ('-I %s -I %s -L %s -L %s -L %s'
% (libdispatch_source_dir, libdispatch_swift_static_module_dir,
make_path(libdispatch_static_artifact_dir, 'src'),
make_path(libdispatch_static_artifact_dir, 'src', 'BlocksRuntime'),
make_path(libdispatch_static_artifact_dir, 'src', 'swift')))
config.import_libdispatch_static = '-L %s' % libdispatch_static_artifact_dir

config.target_build_swift = (
'%s -target %s -toolchain-stdlib-rpath %s %s %s %s %s'
Expand Down Expand Up @@ -2668,6 +2662,10 @@ run_filecheck = '%s %s --allow-unused-prefixes --sanitize BUILD_DIR=%s --sanitiz
config.substitutions.append(('%FileCheck', run_filecheck))
config.substitutions.append(('%raw-FileCheck', shell_quote(config.filecheck)))
config.substitutions.append(('%import-libdispatch', getattr(config, 'import_libdispatch', '')))
# WARNING: the order of components in a substitution name has to be different from the previous one, as lit does
# a pure string substitution without understanding that these components are grouped together. That is, the following
# subsitution name can't be `%import-libdispatch-static`, otherwise the first two components will be substituted with
# the value of `%import-libdispatch` substitution with `-static` string appended to it.
config.substitutions.append(('%import-static-libdispatch', getattr(config, 'import_libdispatch_static', '')))

# Disable COW sanity checks in the swift runtime by default.
Expand Down
2 changes: 1 addition & 1 deletion utils/build-script-impl
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,7 @@ for host in "${ALL_HOSTS[@]}"; do
-DSWIFT_PATH_TO_CMARK_BUILD:PATH="$(build_directory ${host} cmark)"
-DSWIFT_PATH_TO_LIBDISPATCH_SOURCE:PATH="${LIBDISPATCH_SOURCE_DIR}"
-DSWIFT_PATH_TO_LIBDISPATCH_BUILD:PATH="$(build_directory ${host} libdispatch)"
-DSWIFT_PATH_TO_LIBDISPATCH_STATIC_BUILD:PATH="$(build_directory ${host} libdispatch_static)"
-DSWIFT_PATH_TO_LIBDISPATCH_STATIC_BUILD:PATH="$(build_directory ${host} swift)/$(basename $(build_directory ${host} libdispatch))-static-prefix"
)

if [[ "${SWIFT_SDKS}" ]] ; then
Expand Down

0 comments on commit f3a3eef

Please sign in to comment.