Skip to content

[ClangImporter] Swift needs to pass -Xclang -fbuiltin-headers-in-system-modules for its module maps that group cstd headers #69707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ bool isCxxConstReferenceType(const clang::Type *type);
struct ClangInvocationFileMapping {
SmallVector<std::pair<std::string, std::string>, 2> redirectedFiles;
SmallVector<std::pair<std::string, std::string>, 1> overridenFiles;
bool requiresBuiltinHeadersInSystemModules;
};

/// On Linux, some platform libraries (glibc, libstdc++) are not modularized.
Expand Down
4 changes: 4 additions & 0 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1205,6 +1205,10 @@ ClangImporter::create(ASTContext &ctx,
// Create a new Clang compiler invocation.
{
importer->Impl.ClangArgs = getClangArguments(ctx);
if (fileMapping.requiresBuiltinHeadersInSystemModules) {
importer->Impl.ClangArgs.push_back("-Xclang");
importer->Impl.ClangArgs.push_back("-fbuiltin-headers-in-system-modules");
}
ArrayRef<std::string> invocationArgStrs = importer->Impl.ClangArgs;
if (importerOpts.DumpClangDiagnostics) {
llvm::errs() << "'";
Expand Down
44 changes: 37 additions & 7 deletions lib/ClangImporter/ClangIncludePaths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,8 @@ GetWindowsAuxiliaryFile(StringRef modulemap, const SearchPathOptions &Options) {

SmallVector<std::pair<std::string, std::string>, 2> GetWindowsFileMappings(
ASTContext &Context,
const llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> &driverVFS) {
const llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> &driverVFS,
bool &requiresBuiltinHeadersInSystemModules) {
const llvm::Triple &Triple = Context.LangOpts.Target;
const SearchPathOptions &SearchPathOpts = Context.SearchPathOpts;
SmallVector<std::pair<std::string, std::string>, 2> Mappings;
Expand Down Expand Up @@ -468,8 +469,21 @@ SmallVector<std::pair<std::string, std::string>, 2> GetWindowsFileMappings(
llvm::sys::path::append(UCRTInjection, "module.modulemap");

AuxiliaryFile = GetWindowsAuxiliaryFile("ucrt.modulemap", SearchPathOpts);
if (!AuxiliaryFile.empty())
if (!AuxiliaryFile.empty()) {
// The ucrt module map has the C standard library headers all together.
// That leads to module cycles with the clang _Builtin_ modules. e.g.
// <fenv.h> on ucrt includes <float.h>. The clang builtin <float.h>
// include-nexts <float.h>. When both of those UCRT headers are in the
// ucrt module, there's a module cycle ucrt -> _Builtin_float -> ucrt
// (i.e. fenv.h (ucrt) -> float.h (builtin) -> float.h (ucrt)). Until the
// ucrt module map is updated, the builtin headers need to join the system
// modules. i.e. when the builtin float.h is in the ucrt module too, the
// cycle goes away. Note that -fbuiltin-headers-in-system-modules does
// nothing to fix the same problem with C++ headers, and is generally
// fragile.
Mappings.emplace_back(std::string(UCRTInjection), AuxiliaryFile);
requiresBuiltinHeadersInSystemModules = true;
}
}

struct {
Expand Down Expand Up @@ -515,20 +529,36 @@ ClangInvocationFileMapping swift::getClangInvocationFileMapping(

const llvm::Triple &triple = ctx.LangOpts.Target;

SmallVector<std::pair<std::string, std::string>, 2> libcFileMapping;
if (triple.isOSWASI()) {
// WASI Mappings
result.redirectedFiles.append(
getLibcFileMapping(ctx, "wasi-libc.modulemap", std::nullopt, vfs));
libcFileMapping =
getLibcFileMapping(ctx, "wasi-libc.modulemap", std::nullopt, vfs);
} else {
// Android/BSD/Linux Mappings
result.redirectedFiles.append(getLibcFileMapping(
ctx, "glibc.modulemap", StringRef("SwiftGlibc.h"), vfs));
libcFileMapping = getLibcFileMapping(ctx, "glibc.modulemap",
StringRef("SwiftGlibc.h"), vfs);
}
result.redirectedFiles.append(libcFileMapping);
// Both libc module maps have the C standard library headers all together in a
// SwiftLibc module. That leads to module cycles with the clang _Builtin_
// modules. e.g. <inttypes.h> includes <stdint.h> on these platforms. The
// clang builtin <stdint.h> include-nexts <stdint.h>. When both of those
// platform headers are in the SwiftLibc module, there's a module cycle
// SwiftLibc -> _Builtin_stdint -> SwiftLibc (i.e. inttypes.h (platform) ->
// stdint.h (builtin) -> stdint.h (platform)). Until this can be fixed in
// these module maps, the clang builtin headers need to join the "system"
// modules (SwiftLibc). i.e. when the clang builtin stdint.h is in the
// SwiftLibc module too, the cycle goes away. Note that
// -fbuiltin-headers-in-system-modules does nothing to fix the same problem
// with C++ headers, and is generally fragile.
result.requiresBuiltinHeadersInSystemModules = !libcFileMapping.empty();

if (ctx.LangOpts.EnableCXXInterop)
getLibStdCxxFileMapping(result, ctx, vfs);

result.redirectedFiles.append(GetWindowsFileMappings(ctx, vfs));
result.redirectedFiles.append(GetWindowsFileMappings(
ctx, vfs, result.requiresBuiltinHeadersInSystemModules));

return result;
}
17 changes: 17 additions & 0 deletions lib/DriverTool/sil_opt_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ struct SILOptOptions {
"interfaces for all public declarations by "
"default"));

llvm::cl::opt<bool>
StrictImplicitModuleContext = llvm::cl::opt<bool>("strict-implicit-module-context",
llvm::cl::desc("Enable the strict forwarding of compilation "
"context to downstream implicit module dependencies"));

llvm::cl::opt<bool>
DisableSILOwnershipVerifier = llvm::cl::opt<bool>(
"disable = llvm::cl::opt<bool> DisableSILOwnershipVerifier(-sil-ownership-verifier",
Expand Down Expand Up @@ -518,6 +523,10 @@ struct SILOptOptions {
swift::UnavailableDeclOptimization::Complete, "complete",
"Eliminate unavailable decls from lowered SIL/IR")),
llvm::cl::init(swift::UnavailableDeclOptimization::None));

llvm::cl::list<std::string> ClangXCC = llvm::cl::list<std::string>(
"Xcc",
llvm::cl::desc("option to pass to clang"));
};

/// Regular expression corresponding to the value given in one of the
Expand Down Expand Up @@ -608,6 +617,8 @@ int sil_opt_main(ArrayRef<const char *> argv, void *MainAddr) {
Invocation.setRuntimeResourcePath(options.ResourceDir);
Invocation.getFrontendOptions().EnableLibraryEvolution
= options.EnableLibraryEvolution;
Invocation.getFrontendOptions().StrictImplicitModuleContext
= options.StrictImplicitModuleContext;
// Set the module cache path. If not passed in we use the default swift module
// cache.
Invocation.getClangImporterOptions().ModuleCachePath = options.ModuleCachePath;
Expand Down Expand Up @@ -671,6 +682,12 @@ int sil_opt_main(ArrayRef<const char *> argv, void *MainAddr) {
Invocation.getDiagnosticOptions().VerifyMode =
options.VerifyMode ? DiagnosticOptions::Verify : DiagnosticOptions::NoVerify;

ClangImporterOptions &clangImporterOptions =
Invocation.getClangImporterOptions();
for (const auto &xcc : options.ClangXCC) {
clangImporterOptions.ExtraArgs.push_back(xcc);
}

// Setup the SIL Options.
SILOptions &SILOpts = Invocation.getSILOptions();
SILOpts.InlineThreshold = options.SILInlineThreshold;
Expand Down
4 changes: 2 additions & 2 deletions stdlib/cmake/modules/AddSwiftStdlib.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ function(add_swift_target_library_single target name)
-Xcc;-Xclang;-Xcc;-ivfsoverlay;-Xcc;-Xclang;-Xcc;${SWIFTLIB_SINGLE_VFS_OVERLAY})
endif()
list(APPEND SWIFTLIB_SINGLE_SWIFT_COMPILE_FLAGS
-vfsoverlay;"${SWIFT_WINDOWS_VFS_OVERLAY}")
-vfsoverlay;"${SWIFT_WINDOWS_VFS_OVERLAY}";-strict-implicit-module-context;-Xcc;-Xclang;-Xcc;-fbuiltin-headers-in-system-modules)
if(NOT CMAKE_HOST_SYSTEM MATCHES Windows)
swift_windows_include_for_arch(${SWIFTLIB_SINGLE_ARCHITECTURE} SWIFTLIB_INCLUDE)
foreach(directory ${SWIFTLIB_INCLUDE})
Expand Down Expand Up @@ -2659,7 +2659,7 @@ function(_add_swift_target_executable_single name)

if(SWIFTEXE_SINGLE_SDK STREQUAL "WINDOWS")
list(APPEND SWIFTEXE_SINGLE_COMPILE_FLAGS
-vfsoverlay;"${SWIFT_WINDOWS_VFS_OVERLAY}")
-vfsoverlay;"${SWIFT_WINDOWS_VFS_OVERLAY}";-strict-implicit-module-context;-Xcc;-Xclang;-Xcc;-fbuiltin-headers-in-system-modules)
endif()

if ("${SWIFTEXE_SINGLE_SDK}" STREQUAL "LINUX")
Expand Down
5 changes: 5 additions & 0 deletions stdlib/public/Backtracing/SymbolicatedBacktrace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ import Swift

@_implementationOnly import OS.Libc
@_implementationOnly import Runtime
// Because we've turned off the OS/SDK modules, and we don't have a module for
// stddef.h, and we sometimes build with -fbuiltin-headers-in-system-modules for
// vfs reasons, stddef.h can be absorbed into a random module. Sometimes it's
// SwiftOverlayShims.
@_implementationOnly import SwiftOverlayShims

/// A symbolicated backtrace
public struct SymbolicatedBacktrace: CustomStringConvertible {
Expand Down
3 changes: 3 additions & 0 deletions test/ClangImporter/pcm-emit-direct-cc1-mode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// RUN: %empty-directory(%t)
// RUN: %swift-frontend -emit-pcm -direct-clang-cc1-module-build -only-use-extra-clang-opts -module-name script -o %t/script.pcm %S/Inputs/custom-modules/module.modulemap -Xcc %S/Inputs/custom-modules/module.modulemap -Xcc -o -Xcc %t/script.pcm -Xcc -fmodules -Xcc -triple -Xcc %target-triple -Xcc -x -Xcc objective-c -dump-clang-diagnostics 2> %t.diags.txt

// Sometimes returns a 1 exit code with no stdout or stderr or even an indication of which command failed.
// XFAIL: OS=linux-gnu

// Verify some of the output of the -dump-pcm flag.
// RUN: %swift-dump-pcm %t/script.pcm | %FileCheck %s --check-prefix=CHECK-DUMP
// CHECK-DUMP: Information for module file '{{.*}}/script.pcm':
Expand Down
11 changes: 10 additions & 1 deletion test/DebugInfo/C-typedef-Darwin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,13 @@ let blah = size_t(1024)
use(blah)
// CHECK: !DIDerivedType(tag: DW_TAG_typedef,
// CHECK-SAME: scope: ![[DARWIN_MODULE:[0-9]+]],
// CHECK: ![[DARWIN_MODULE]] = !DIModule({{.*}}, name: "stddef"

// size_t is defined in clang, originally by <stddef.h>, and later split out to
// <__stddef_size_t.h>. Depending on the state of clang's builtin headers and
// modules, size_t can be in either Darwin.C.stddef or _Builtin_stddef.size_t.
// size_t is also defined in macOS by <sys/_types/_size_t.h> which is in the
// Darwin.POSIX._types._size_t module. Ideally Apple will remove the duplicate
// declaration and clang will settle to _Builtin_stddef.size_t, but while it's
// all in flux allow all three of those modules.
// Darwin.C.stddef|_Builtin_stddef.size_t|Darwin.POSIX._types._size_t
// CHECK: ![[DARWIN_MODULE]] = !DIModule({{.*}}, name: "{{stddef|size_t|_size_t}}"
2 changes: 2 additions & 0 deletions test/Inputs/clang-importer-sdk/usr/include/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ module ctypes {
header "ctypes.h"
explicit module bits {
header "ctypes/bits.h"
export *
}
export *
}
module stdio { header "stdio.h" }
module cvars { header "cvars.h" }
Expand Down
9 changes: 6 additions & 3 deletions test/Interop/lit.local.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ if get_target_os() in ['windows-msvc']:
# Clang should build object files with link settings equivalent to -libc MD
# when building for the MSVC target.
clang_opt = clang_compile_opt + '-D_MT -D_DLL -Xclang --dependent-lib=msvcrt -Xclang --dependent-lib=oldnames '
config.substitutions.insert(0, ('%target-swift-flags', '-vfsoverlay {}'.format(os.path.join(config.swift_obj_root,
'stdlib',
'windows-vfs-overlay.yaml'))))
# ucrt.modulemap currently requires -fbuiltin-headers-in-system-modules. -strict-implicit-module-context
# is necessary for -Xcc arguments to be passed through ModuleInterfaceLoader.
config.substitutions.insert(0, ('%target-swift-flags', '-vfsoverlay {} -strict-implicit-module-context -Xcc -Xclang -Xcc -fbuiltin-headers-in-system-modules'.format(
os.path.join(config.swift_obj_root,
'stdlib',
'windows-vfs-overlay.yaml'))))
else:
# FIXME(compnerd) do all the targets we currently support use SysV ABI?
config.substitutions.insert(0, ('%target-abi', 'SYSV'))
Expand Down
4 changes: 4 additions & 0 deletions test/ModuleInterface/no-implicit-extra-clang-opts.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t/ModuleCache)

// https://github.com/apple/swift/issues/70330
// We can't yet call member functions correctly on Windows.
// XFAIL: OS=windows-msvc

import SIMod

// Step 0: Copy relevant files into the temp dir which will serve as the search path
Expand Down
9 changes: 6 additions & 3 deletions test/lit.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,12 @@ else:
config.swift_system_overlay_opt = ""
config.clang_system_overlay_opt = ""
if kIsWindows:
config.swift_system_overlay_opt = "-vfsoverlay {}".format(
# ucrt.modulemap currently requires -fbuiltin-headers-in-system-modules. -strict-implicit-module-context
# is necessary for -Xcc arguments to be passed through ModuleInterfaceLoader.
config.swift_system_overlay_opt = "-vfsoverlay {} -strict-implicit-module-context -Xcc -Xclang -Xcc -fbuiltin-headers-in-system-modules".format(
os.path.join(config.swift_obj_root, "stdlib", "windows-vfs-overlay.yaml")
)
config.clang_system_overlay_opt = "-Xcc -ivfsoverlay -Xcc {}".format(
config.clang_system_overlay_opt = "-Xcc -ivfsoverlay -Xcc {} -Xcc -Xclang -Xcc -fbuiltin-headers-in-system-modules".format(
os.path.join(config.swift_obj_root, "stdlib", "windows-vfs-overlay.yaml")
)
stdlib_resource_dir_opt = config.resource_dir_opt
Expand Down Expand Up @@ -1592,7 +1594,8 @@ elif run_os in ['windows-msvc']:
config.target_swift_modulewrap = \
('%r -modulewrap -target %s' % (config.swiftc, config.variant_triple))
config.target_swift_emit_pcm = \
('%r -emit-pcm -target %s' % (config.swiftc, config.variant_triple))
('%r -emit-pcm -target %s %s' % (config.swiftc, config.variant_triple, \
config.swift_system_overlay_opt))


elif (run_os in ['linux-gnu', 'linux-gnueabihf', 'freebsd', 'openbsd', 'windows-cygnus', 'windows-gnu'] or
Expand Down
2 changes: 1 addition & 1 deletion utils/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ function Build-CMakeProject {

$SwiftArgs += @("-resource-dir", "$SwiftResourceDir")
$SwiftArgs += @("-L", "$SwiftResourceDir\windows")
$SwiftArgs += @("-vfsoverlay", "$RuntimeBinaryCache\stdlib\windows-vfs-overlay.yaml")
$SwiftArgs += @("-vfsoverlay", "$RuntimeBinaryCache\stdlib\windows-vfs-overlay.yaml", "-strict-implicit-module-context", "-Xcc", "-Xclang", "-Xcc", "-fbuiltin-headers-in-system-modules")
}
} else {
$SwiftArgs += @("-sdk", (Get-PinnedToolchainSDK))
Expand Down