Skip to content

Commit

Permalink
Merge pull request #66246 from apple/maxd/5.8-cmake-dispatch-fix
Browse files Browse the repository at this point in the history
[5.8] 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 PRs: #65795 and #64312 for `main`, #65824 and #64633 for `release/5.9`
Reviewed by: @al45tair @drexin @etcwilde 
Resolves: some of the issues reported in #65097, also resolves #58380
Tests: Added in apple/swift-integration-tests#118

`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 our trivial lit tests previously didn't link Dispatch statically, they 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.
  • Loading branch information
MaxDesiatov committed Jun 1, 2023
2 parents 85f8090 + 3fd9fdf commit 3f5029f
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 34 deletions.
11 changes: 2 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -619,13 +619,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 @@ -634,9 +627,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
59 changes: 48 additions & 11 deletions lib/DriverTool/autolink_extract_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,50 @@ int autolink_extract_main(ArrayRef<const char *> Args, const char *Argv0,
std::vector<std::string> LinkerFlags;

// Keep track of whether we've already added the common
// Swift libraries that ususally have autolink directives
// in most object fiels
std::unordered_map<std::string, bool> SwiftRuntimeLibraries = {
{"-lswiftSwiftOnoneSupport", false},
{"-lswiftCore", false},
{"-lswift_Concurrency", false},
{"-lswift_StringProcessing", false},
{"-lswift_RegexParser", false}
// Swift libraries that usually have autolink directives
// in most object files

std::vector<std::string> SwiftRuntimeLibsOrdered = {
// Common Swift runtime libs
"-lswiftSwiftOnoneSupport",
"-lswiftCore",
"-lswift_Concurrency",
"-lswift_StringProcessing",
"-lswift_RegexBuilder",
"-lswift_RegexParser",
"-lswift_Backtracing",
"-lswiftGlibc",
"-lBlocksRuntime",
// Dispatch-specific Swift runtime libs
"-ldispatch",
"-lDispatchStubs",
"-lswiftDispatch",
// CoreFoundation and Foundation Swift runtime libs
"-lCoreFoundation",
"-lFoundation",
"-lFoundationNetworking",
"-lFoundationXML",
// Foundation support libs
"-lcurl",
"-lxml2",
"-luuid",
// XCTest runtime libs (must be first due to http://github.com/apple/swift-corelibs-xctest/issues/432)
"-lXCTest",
// ICU Swift runtime libs
"-licui18nswift",
"-licuucswift",
"-licudataswift",
// Common-use ordering-agnostic Linux system libs
"-lm",
"-lpthread",
"-lutil",
"-ldl",
"-lz",
};
std::unordered_map<std::string, bool> SwiftRuntimeLibraries;
for (const auto &RuntimeLib : SwiftRuntimeLibsOrdered) {
SwiftRuntimeLibraries[RuntimeLib] = false;
}

// Extract the linker flags from the objects.
for (const auto &BinaryFileName : Invocation.getInputFilenames()) {
Expand Down Expand Up @@ -288,9 +323,11 @@ int autolink_extract_main(ArrayRef<const char *> Args, const char *Argv0,
OutOS << Flag << '\n';
}

for (const auto &RuntimeLib : SwiftRuntimeLibraries) {
if (RuntimeLib.second)
OutOS << RuntimeLib.first << '\n';
for (const auto &RuntimeLib : SwiftRuntimeLibsOrdered) {
auto entry = SwiftRuntimeLibraries.find(RuntimeLib);
if (entry != SwiftRuntimeLibraries.end() && entry->second) {
OutOS << entry->first << '\n';
}
}


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 @@ -228,3 +228,16 @@ option(SWIFT_STDLIB_CONCURRENCY_TRACING
"Enable concurrency tracing in the runtime; assumes the presence of os_log(3)
and the os_signpost(3) API."
"${SWIFT_STDLIB_CONCURRENCY_TRACING_default}")

# 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 requires 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 @@ -1483,19 +1483,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 @@ -2519,6 +2513,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 @@ -2011,7 +2011,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 3f5029f

Please sign in to comment.