Skip to content

Commit cc20c75

Browse files
committed
Partially revert 70d2ba0.
This fixes a regression in framework path discovery. It turns out that either ClangImporter or other Swift module loaders can discover additional framework search paths and add them to an existing CompilerInvocation on the fly. When 70d2ba0 switched to directly deserializing the compiler invocation it lost those additional late-discovered framework paths from the module contexts. rdar://89836973
1 parent d821797 commit cc20c75

File tree

11 files changed

+142
-2
lines changed

11 files changed

+142
-2
lines changed

lldb/include/lldb/Target/Target.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ class TargetProperties : public Properties {
169169

170170
llvm::StringRef GetSwiftExtraClangFlags() const;
171171

172+
bool GetSwiftCreateModuleContextsInParallel() const;
173+
172174
bool GetSwiftReadMetadataFromFileCache() const;
173175

174176
bool GetSwiftUseReflectionSymbols() const;

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,14 +2077,45 @@ ProcessModule(ModuleSP module_sp, std::string m_description,
20772077
module_search_paths.insert(module_search_paths.end(),
20782078
opts.getImportSearchPaths().begin(),
20792079
opts.getImportSearchPaths().end());
2080-
for (const auto &fwsp : opts.getFrameworkSearchPaths())
2081-
framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
20822080
auto &clang_opts = invocation.getClangImporterOptions().ExtraArgs;
20832081
for (const std::string &arg : clang_opts) {
20842082
extra_clang_args.push_back(arg);
20852083
LOG_VERBOSE_PRINTF(GetLog(LLDBLog::Types), "adding Clang argument \"%s\".",
20862084
arg.c_str());
20872085
}
2086+
// FIXME: Unfortunately this:
2087+
//
2088+
// for (const auto &fwsp : opts.getFrameworkSearchPaths())
2089+
// framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
2090+
//
2091+
// is insufficient, as ClangImporter can discover more framework
2092+
// search paths on the fly. Once a better solution is found,
2093+
// warmup_contexts can be retired (again).
2094+
{
2095+
SymbolFile *sym_file = module_sp->GetSymbolFile();
2096+
if (!sym_file)
2097+
return;
2098+
Status sym_file_error;
2099+
auto type_system_or_err =
2100+
sym_file->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
2101+
if (!type_system_or_err) {
2102+
llvm::consumeError(type_system_or_err.takeError());
2103+
return;
2104+
}
2105+
auto ts = llvm::dyn_cast_or_null<TypeSystemSwift>(&*type_system_or_err);
2106+
if (!ts)
2107+
return;
2108+
2109+
SwiftASTContext *ast_context = ts->GetSwiftASTContext();
2110+
if (ast_context && !ast_context->HasErrors()) {
2111+
if (use_all_compiler_flags ||
2112+
target.GetExecutableModulePointer() == module_sp.get()) {
2113+
const auto &opts = ast_context->GetSearchPathOptions();
2114+
for (const auto &fwsp : opts.getFrameworkSearchPaths())
2115+
framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
2116+
}
2117+
}
2118+
}
20882119
}
20892120

20902121
lldb::TypeSystemSP SwiftASTContext::CreateInstance(
@@ -2144,7 +2175,26 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
21442175
handled_sdk_path = true;
21452176
}
21462177

2178+
auto warmup_astcontexts = [&]() {
2179+
if (target.GetSwiftCreateModuleContextsInParallel()) {
2180+
// The first call to GetTypeSystemForLanguage() on a module will
2181+
// trigger the import (and thus most likely the rebuild) of all
2182+
// the Clang modules that were imported in this module. This can
2183+
// be a lot of work (potentially ten seconds per module), but it
2184+
// can be performed in parallel.
2185+
llvm::ThreadPool pool(llvm::hardware_concurrency());
2186+
for (size_t mi = 0; mi != num_images; ++mi) {
2187+
auto module_sp = target.GetImages().GetModuleAtIndex(mi);
2188+
pool.async([=] {
2189+
GetModuleSwiftASTContext(*module_sp);
2190+
});
2191+
}
2192+
pool.wait();
2193+
}
2194+
};
2195+
21472196
if (!handled_sdk_path) {
2197+
warmup_astcontexts();
21482198
for (size_t mi = 0; mi != num_images; ++mi) {
21492199
ModuleSP module_sp = target.GetImages().GetModuleAtIndex(mi);
21502200
if (!HasSwiftModules(*module_sp))

lldb/source/Target/Target.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4164,6 +4164,18 @@ void TargetProperties::SetInjectLocalVariables(ExecutionContext *exe_ctx,
41644164
true);
41654165
}
41664166

4167+
bool TargetProperties::GetSwiftCreateModuleContextsInParallel() const {
4168+
const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
4169+
nullptr, false, ePropertyExperimental);
4170+
OptionValueProperties *exp_values =
4171+
exp_property->GetValue()->GetAsProperties();
4172+
if (exp_values)
4173+
return exp_values->GetPropertyAtIndexAsBoolean(
4174+
nullptr, ePropertySwiftCreateModuleContextsInParallel, true);
4175+
else
4176+
return true;
4177+
}
4178+
41674179
bool TargetProperties::GetSwiftReadMetadataFromFileCache() const {
41684180
const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
41694181
nullptr, false, ePropertyExperimental);

lldb/source/Target/TargetProperties.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ let Definition = "target_experimental" in {
44
def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
55
Global, DefaultTrue,
66
Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
7+
def SwiftCreateModuleContextsInParallel : Property<"swift-create-module-contexts-in-parallel", "Boolean">,
8+
DefaultTrue,
9+
Desc<"Create the per-module Swift AST contexts in parallel.">;
710
def SwiftReadMetadataFromFileCache: Property<"swift-read-metadata-from-file-cache", "Boolean">,
811
DefaultTrue,
912
Desc<"Read Swift reflection metadata from the file cache instead of the process when possible">;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@_implementationOnly import Discovered
2+
public class D {
3+
public init() { member = Invisible() }
4+
private let member : Invisible
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
struct Invisible {};
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#include "Discovered.h"
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
SWIFT_SOURCES := main.swift
2+
3+
SWIFTFLAGS_EXTRAS = -F $(BUILDDIR) -framework Direct -Xlinker -rpath -Xlinker $(BUILDDIR)
4+
5+
all: Direct.framework $(EXE)
6+
7+
include Makefile.rules
8+
9+
Discovered.framework: Discovered.h
10+
$(MAKE) -f $(MAKEFILE_RULES) \
11+
DYLIB_ONLY=YES \
12+
DYLIB_NAME=Discovered \
13+
DYLIB_OBJC_SOURCES=Discovered.m \
14+
FRAMEWORK_HEADERS=$(SRCDIR)/Discovered.h \
15+
FRAMEWORK_MODULES=$(SRCDIR)/module.modulemap \
16+
FRAMEWORK=Discovered
17+
18+
Direct.framework: $(SRCDIR)/Direct.swift.in Discovered.framework
19+
mkdir -p $(BUILDDIR)/secret_path
20+
cp $< $(BUILDDIR)/Direct.swift
21+
mv Discovered.framework $(BUILDDIR)/secret_path
22+
$(MAKE) -f $(MAKEFILE_RULES) \
23+
DYLIB_NAME=Direct \
24+
DYLIB_SWIFT_SOURCES=Direct.swift \
25+
DYLIB_MODULENAME=Direct \
26+
FRAMEWORK=Direct \
27+
SWIFTFLAGS_EXTRAS=-F$(BUILDDIR)/secret_path
28+
rm -f $(BUILDDIR)/Direct.swiftmodule $(BUILDDIR)/Direct.swiftinterface $(BUILDDIR)/Direct.swift
29+
ln -s $(BUILDDIR)/Direct.framework/Direct $(BUILDDIR)/Direct # FIXME
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import lldb
2+
from lldbsuite.test.decorators import *
3+
import lldbsuite.test.lldbtest as lldbtest
4+
import lldbsuite.test.lldbutil as lldbutil
5+
import os
6+
import unittest2
7+
8+
9+
class TestSwiftSystemFramework(lldbtest.TestBase):
10+
11+
NO_DEBUG_INFO_TESTCASE = True
12+
mydir = lldbtest.TestBase.compute_mydir(__file__)
13+
14+
@swiftTest
15+
@skipIf(oslist=no_match(["macosx"]))
16+
def test_system_framework(self):
17+
"""Test the discovery of framework search paths from framework dependencies."""
18+
self.build()
19+
target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
20+
self, 'break here', lldb.SBFileSpec('main.swift'))
21+
22+
log = self.getBuildArtifact("types.log")
23+
self.runCmd('log enable lldb types -f "%s"' % log)
24+
self.expect("expression -- 0")
25+
pos = 0
26+
import io
27+
with open(log, "r", encoding='utf-8') as logfile:
28+
for line in logfile:
29+
if "SwiftASTContextForExpressions::LogConfiguration()" in line and \
30+
"/secret_path" in line:
31+
pos += 1
32+
self.assertEqual(pos, 1, "framework search path discovery is broken")
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import Direct
2+
let d = D()
3+
print("break here")
4+
print(d)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
framework module Discovered { header "Discovered.h" }

0 commit comments

Comments
 (0)